Skip to content

fix(filesystem): use StringDecoder for UTF-8 safe headFile/tailFile - #4670

Open
re2zero wants to merge 2 commits into
modelcontextprotocol:mainfrom
re2zero:fix/utf8-streaming-decode
Open

fix(filesystem): use StringDecoder for UTF-8 safe headFile/tailFile#4670
re2zero wants to merge 2 commits into
modelcontextprotocol:mainfrom
re2zero:fix/utf8-streaming-decode

Conversation

@re2zero

@re2zero re2zero commented Aug 19, 2026

Copy link
Copy Markdown

Closes #4666

Problem

headFile() and tailFile() read files in 1024-byte chunks and call .toString('utf-8') on each chunk independently. When a multi-byte UTF-8 character (e.g. CJK characters, which are 3 bytes) straddles a chunk boundary, the split decoding produces mojibike (U+FFFD replacement characters).

Fix

Replace per-chunk .toString('utf-8') with Node.js StringDecoder:

  • headFile: reads forward, the decoder naturally buffers incomplete trailing byte sequences across write() calls. A final decoder.end() flushes any remaining bytes.

  • tailFile: reads backwards from file end. The function now collects raw Buffer slices in reverse order, reverses them for correct chronology, then decodes the concatenated buffer as a single UTF-8 stream. Newline counting is done on raw bytes (0x0A is single-byte even in UTF-8), so we stop reading at the right point.

Testing

  • server-filesystem test suite: 152 passed (7 test files).

re2zero and others added 2 commits August 20, 2026 02:16
headFile() and tailFile() read files in 1024-byte chunks and called
.toString('utf-8') on each chunk independently. When a multi-byte UTF-8
character straddled a chunk boundary, the split decoding produced mojibake
(U+FFFD replacement characters).

Fix headFile by using StringDecoder across sequential reads so that
incomplete trailing byte sequences are buffered and completed on the
next write() call.

Fix tailFile by collecting raw byte buffers from backwards reads, then
decoding the concatenated buffer (in forward order) as a single UTF-8 stream
with StringDecoder. Newline counting is done on raw bytes (0x0A is
single-byte in UTF-8) so we stop reading at the right point.

Fixes modelcontextprotocol#4666

@tarunag10 tarunag10 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I compared this with #4667 and ran the filesystem suite plus two real-file edge cases. This PR passes the existing 46 tests, but both added edge cases fail at fc760fe3:

  1. tailFile(file, 2) on discard\\n + + 1017 ASCII bytes + \\nlast returns U+FFFD at the 1024-byte boundary. rawBuffers.push(chunk.slice(...)) stores views into the single reused chunk; later reads overwrite earlier entries. Copying each read (Buffer.from(chunk.subarray(0, bytesRead))) avoids that aliasing.
  2. tailFile(file, 0) returns the whole file because newlineCount <= numLines enters the loop and slice(-0) is slice(0).

The same two tests pass on #4667, which also has focused head/tail boundary tests (48/48 filesystem tests versus 46/46 here). I recommend either adopting the copying and zero-line behavior here with regression tests, or closing this in favor of #4667. There is also an unused relevantLines local in the current diff.

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.

headFile/tailFile in filesystem server corrupts multi-byte UTF-8 characters at 1024-byte chunk boundaries

2 participants