fix(filesystem): use StringDecoder for UTF-8 safe headFile/tailFile - #4670
Open
re2zero wants to merge 2 commits into
Open
fix(filesystem): use StringDecoder for UTF-8 safe headFile/tailFile#4670re2zero wants to merge 2 commits into
re2zero wants to merge 2 commits into
Conversation
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
reviewed
Aug 20, 2026
tarunag10
left a comment
There was a problem hiding this comment.
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:
tailFile(file, 2)ondiscard\\n+界+ 1017 ASCII bytes +\\nlastreturns U+FFFD at the 1024-byte boundary.rawBuffers.push(chunk.slice(...))stores views into the single reusedchunk; later reads overwrite earlier entries. Copying each read (Buffer.from(chunk.subarray(0, bytesRead))) avoids that aliasing.tailFile(file, 0)returns the whole file becausenewlineCount <= numLinesenters the loop andslice(-0)isslice(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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4666
Problem
headFile()andtailFile()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.jsStringDecoder:headFile: reads forward, the decoder naturally buffers incomplete trailing byte sequences across
write()calls. A finaldecoder.end()flushes any remaining bytes.tailFile: reads backwards from file end. The function now collects raw
Bufferslices 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-filesystemtest suite: 152 passed (7 test files).