Skip to content

[File Locksmith] Fix IPC text-mode file I/O corrupting Unicode paths#47361

Merged
MuyuanMS merged 4 commits into
mainfrom
copilot/fix-ipc-writer-text-mode
May 25, 2026
Merged

[File Locksmith] Fix IPC text-mode file I/O corrupting Unicode paths#47361
MuyuanMS merged 4 commits into
mainfrom
copilot/fix-ipc-writer-text-mode

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 29, 2026

Summary of the Pull Request

The File Locksmith IPC layer reads and writes raw UTF-16 (WCHAR) bytes to last-run.log, but all three stream opens were using the default text mode. On Windows, the CRT translates 0x0A bytes to 0x0D 0x0A on write and collapses 0x0D 0x0A back to 0x0A on read. Because each WCHAR is 2 bytes, any code unit whose little-endian byte pair contains 0x0A in the low position (e.g. U+010A, U+0A0D) is silently corrupted. The fix opens all three streams in binary mode and adds an explicit open-failure guard.

// IPC.cpp — Writer::start()
// Before
m_stream = std::ofstream(path);
// After
m_stream = std::ofstream(path, std::ios::binary);
// + is_open() guard returning E_FAIL on failure

// NativeMethods.cpp — StartAsElevated() writer
// Before
std::ofstream stream(paths_file());
// After
std::ofstream stream(paths_file(), std::ios::binary);

// NativeMethods.cpp — ReadPathsFromFile() reader
// Before
std::ifstream stream(paths_file());
// After
std::ifstream stream(paths_file(), std::ios::binary);

PR Checklist

  • Communication: I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected
  • Tests: Added/updated and all pass
  • Localization: All end-user-facing strings can be localized
  • Dev docs: Added/updated
  • New binaries: Added on the required places
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Three targeted changes across two files:

  1. FileLocksmithLib/IPC.cppWriter::start(): switched std::ofstream from text to binary mode and added an is_open() check that returns E_FAIL immediately when the file cannot be opened (previously the try/catch did not catch a silent open failure because std::ofstream does not throw by default).

  2. FileLocksmithLibInterop/NativeMethods.cppStartAsElevated(): switched std::ofstream from text to binary mode. This is the elevated-restart writer path; without this fix, Unicode corruption persisted when File Locksmith relaunched as administrator.

  3. FileLocksmithLibInterop/NativeMethods.cppReadPathsFromFile(): switched std::ifstream from text to binary mode. This is the symmetric reader-side bug — even with both writers corrected, the CRT text-mode reader could collapse a 0x0D 0x0A byte pair (a valid UTF-16 LE code unit, e.g. U+0A0D GURMUKHI EK ONKAR) into a single byte, desynchronising the 2-bytes-at-a-time read loop and corrupting all subsequent path data.

No behaviour change for purely ASCII paths. Paths containing Unicode code points whose little-endian UTF-16 byte pair spans 0x0D 0x0A were silently corrupted in all three code paths before this fix.

Validation Steps Performed

  • Code review: no issues flagged.
  • Full diff reviewed: all three stream opens (ofstream writer in IPC.cpp, ofstream writer in NativeMethods.cpp, ifstream reader in NativeMethods.cpp) now use std::ios::binary, making write and read paths byte-exact and symmetric.
  • Mechanically correct: std::ios::binary suppresses Windows CRT \n\r\n translation; the delimiter L'\n' (LE bytes 0x0A 0x00) is unambiguous in binary mode and is handled correctly by the existing read loop.

Copilot AI changed the title [WIP] Fix IPC writer to open in binary mode for Unicode paths [File Locksmith] Fix IPC Writer opening in text mode, corrupting Unicode paths Apr 29, 2026
Copilot AI requested a review from MuyuanMS April 29, 2026 08:54
@niels9001 niels9001 added the Product-File Locksmith Refers to the File Locksmith utility label Apr 29, 2026
@MuyuanMS MuyuanMS requested a review from Copilot May 14, 2026 03:10
Copy link
Copy Markdown
Contributor

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 a File Locksmith IPC reliability issue where ipc::Writer writes raw UTF-16 bytes to last-run.log, but the file was opened in Windows text mode, allowing CRT \n translation to corrupt any UTF-16 code unit containing a 0x0A low byte. The fix switches the writer stream to binary mode so bytes are written exactly as produced.

Changes:

  • Open ipc::Writer’s std::ofstream with std::ios::binary to prevent text-mode newline translation and preserve UTF-16 byte sequences.

try
{
m_stream = std::ofstream(path);
m_stream = std::ofstream(path, std::ios::binary);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added an explicit is_open() check in ipc::Writer::start() after opening the stream in binary mode, so open failures now return E_FAIL instead of incorrectly returning S_OK.

try
{
m_stream = std::ofstream(path);
m_stream = std::ofstream(path, std::ios::binary);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Found the other last-run.log write path in src/modules/FileLocksmith/FileLocksmithLibInterop/NativeMethods.cpp (NativeMethods::StartAsElevated) and switched that std::ofstream open to std::ios::binary as well.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@MuyuanMS
Copy link
Copy Markdown
Contributor

@copilot please re-review the latest changes.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 14, 2026

@copilot please re-review the latest changes.

Re-review complete. The two previous review comments (binary mode on StartAsElevated writer and is_open() guard in IPC.cpp) were already addressed in commit 27d771b.

The re-review also surfaced a symmetric reader-side bug: ReadPathsFromFile() was still opening its ifstream in text mode (line 64). Since the file is raw UTF-16 LE, any WCHAR whose byte pair is 0x0D 0x0A (e.g. U+0A0D) would be silently collapsed to one byte by CRT text-mode translation, desynchronising the 2-byte read loop and corrupting all subsequent path data. Fixed in 511e610 by adding std::ios::binary to that ifstream constructor.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/graphql
    • Triggering command: /usr/bin/gh gh pr view --json number,baseRefName,headRefName,baseRefOid,headRefOid,changedFiles,files (http block)
    • Triggering command: /usr/bin/gh gh pr list --head copilot/fix-ipc-writer-text-mode --json number (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [File Locksmith] Fix IPC Writer opening in text mode, corrupting Unicode paths [File Locksmith] Fix IPC text-mode file I/O corrupting Unicode paths May 14, 2026
@MuyuanMS MuyuanMS marked this pull request as ready for review May 24, 2026 11:18
try
{
m_stream = std::ofstream(path);
m_stream = std::ofstream(path, std::ios::binary);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added an explicit is_open() check in ipc::Writer::start() after opening the stream in binary mode, so open failures now return E_FAIL instead of incorrectly returning S_OK.

try
{
m_stream = std::ofstream(path);
m_stream = std::ofstream(path, std::ios::binary);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Found the other last-run.log write path in src/modules/FileLocksmith/FileLocksmithLibInterop/NativeMethods.cpp (NativeMethods::StartAsElevated) and switched that std::ofstream open to std::ios::binary as well.

Copy link
Copy Markdown
Contributor

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@MuyuanMS MuyuanMS merged commit bcf0b68 into main May 25, 2026
17 checks passed
@MuyuanMS MuyuanMS deleted the copilot/fix-ipc-writer-text-mode branch May 25, 2026 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Product-File Locksmith Refers to the File Locksmith utility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[File Locksmith] IPC Writer opens in text mode, corrupts Unicode paths

5 participants