Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 22, 2026

Summary

Fixed the issue raised by @Mr-Rm in PR #1649 comments:

Before: path.Replace("\0", "") - removed ALL null characters from path
After: path.TrimEnd('\0') - removes only TRAILING null characters

This is safer because:

  1. Windows WebDAV client adds null chars at the END of paths (trailing)
  2. TrimEnd is more targeted - only fixes the actual issue
  3. Replace could mask security issues like "file.txt\0.exe" where null char hides true extension
Original prompt

Прочитай переписку в #1649 после мержа и поправь замечание


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 4 commits January 22, 2026 16:55
Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
…ar handling

Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
@nixel2007 nixel2007 marked this pull request as ready for review January 22, 2026 17:00
Copilot AI review requested due to automatic review settings January 22, 2026 17:00
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 22, 2026

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Comment @coderabbitai help to get the list of available commands and usage tips.

@nixel2007 nixel2007 requested review from EvilBeaver and Mr-Rm January 22, 2026 17:00
Copilot AI changed the title [WIP] Fix comments in pull request 1649 Use TrimEnd('\0') instead of Replace("\0", "") for null character stripping Jan 22, 2026
Copilot AI requested a review from nixel2007 January 22, 2026 17:01
@nixel2007
Copy link
Collaborator

Replace("\0", "") removes all null characters

@copilot ты не прав. \0 в двойных кавычках же вообще не удаляет null-String, а только бэкслэш с нулем. В отличие от одинарных кавычек

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses feedback from PR #1649 by refactoring the null character handling in PathHelper.cs to be more targeted and secure. The change improves the implementation from removing all null characters to removing only trailing null characters.

Changes:

  • Updated PathHelper.StripNullCharacters() to use TrimEnd('\0') instead of Replace("\0", "")
  • Changed from string literal "\0" to char literal '\0' for better performance
  • Enhanced XML documentation to clarify that only trailing null characters are removed and explain the security rationale

Copy link
Contributor Author

Copilot AI commented Jan 22, 2026

Replace("\0", "") removes all null characters

@copilot ты не прав. \0 в двойных кавычках же вообще не удаляет null-String, а только бэкслэш ...

В C# "\0" в строковом литерале — это escape sequence, который представляет null-символ (как и '\0'). Проверил на тесте:

string s = "hello\0world";
Console.WriteLine(s.Replace("\0", "").Length); // 10 (было 11)

Replace("\0", "") действительно удалял null-символы. Но замена на TrimEnd('\0') правильная по другой причине — нужно удалять только trailing nulls, чтобы не маскировать потенциальные атаки типа file.txt\0.exe.

@nixel2007
Copy link
Collaborator

@copilot ты опять закоммитил мусор

Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 22, 2026

@copilot ты опять закоммитил мусор

Удалил opm.ospx (commit a0109ea). Теперь в PR только изменение PathHelper.cs.

@sonar-openbsl-ru-qa-bot
Copy link

@EvilBeaver
Copy link
Owner

Replace("\0", "") removes all null characters

@copilot ты не прав. \0 в двойных кавычках же вообще не удаляет null-String, а только бэкслэш с нулем. В отличие от одинарных кавычек

По-моему, все-таки удаляет. Или я чего-то не знаю. @nixel2007 откуда инфа, что не удаляет?

@nixel2007
Copy link
Collaborator

@EvilBeaver я спутал, копилот и ты правы

@nixel2007 nixel2007 merged commit 8c9206e into develop Jan 23, 2026
3 of 4 checks passed
@nixel2007 nixel2007 deleted the copilot/fix-comments-in-pull-request-1649 branch January 23, 2026 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants