From ea10910190e9df2d47a1e7d13152b6912c18faca Mon Sep 17 00:00:00 2001 From: Kei Ishida Date: Sun, 17 May 2026 20:19:51 +0900 Subject: [PATCH] fix(filesystem): preserve UTF-8 multibyte characters across chunk boundaries in head/tail read_text_file with head or tail corrupts multi-byte UTF-8 characters that straddle the 1024-byte chunk read boundary. Each chunk was decoded independently via Buffer.toString('utf-8'), so partial byte sequences at chunk edges became U+FFFD. - headFile: use node:string_decoder StringDecoder to hold trailing partial bytes across reads - tailFile: accumulate raw Buffer chunks and decode the concatenated buffer once; count newlines at byte level (0x0A is unambiguous in UTF-8). The discarded partial first line absorbs any U+FFFD at the lowest read position. Adds encoding.test.ts with real-I/O tests placing 3-byte and 4-byte UTF-8 characters at exact chunk boundaries for both head and tail. --- src/filesystem/__tests__/encoding.test.ts | 140 ++++++++++++++++++++++ src/filesystem/lib.ts | 58 ++++----- 2 files changed, 171 insertions(+), 27 deletions(-) create mode 100644 src/filesystem/__tests__/encoding.test.ts diff --git a/src/filesystem/__tests__/encoding.test.ts b/src/filesystem/__tests__/encoding.test.ts new file mode 100644 index 0000000000..25e14850d4 --- /dev/null +++ b/src/filesystem/__tests__/encoding.test.ts @@ -0,0 +1,140 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import fs from 'fs/promises'; +import path from 'path'; +import os from 'os'; +import { headFile, tailFile } from '../lib.js'; + +// These tests use real file I/O (no fs mocking) because the bug exists in the +// interaction between byte-level reads and UTF-8 decoding. The implementation +// reads in 1024-byte chunks, so we construct files where a UTF-8 multi-byte +// character is placed to straddle a chunk boundary at exact byte offsets. + +describe('UTF-8 multi-byte character handling at chunk boundaries', () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-fs-encoding-')); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + describe('headFile', () => { + it('preserves a 3-byte UTF-8 character split across the 1024-byte boundary', async () => { + // 'あ' (U+3042) is E3 81 82 in UTF-8. + // 1023 'a' chars (bytes 0..1022) + 'あ' (bytes 1023..1025) + '\n' + 'second\n'. + // Boundary at offset 1024 falls between 'あ' byte 1 (E3 @ 1023) and byte 2 (81 @ 1024). + // Buggy version: chunk 1 ends with E3 alone -> U+FFFD; chunk 2 starts with 81 82 -> U+FFFD U+FFFD. + const file = path.join(tmpDir, 'head-3byte.txt'); + const padding = 'a'.repeat(1023); + await fs.writeFile(file, `${padding}あ\nsecond line\n`, 'utf-8'); + + const result = await headFile(file, 1); + + expect(result).toBe(`${padding}あ`); + expect(result).not.toContain('\uFFFD'); + }); + + it('preserves a 4-byte UTF-8 character split across the 1024-byte boundary', async () => { + // '😀' (U+1F600) is F0 9F 98 80 in UTF-8. + // 1022 'a' chars (bytes 0..1021) + '😀' (bytes 1022..1025) straddles the boundary at 1024. + const file = path.join(tmpDir, 'head-4byte.txt'); + const padding = 'a'.repeat(1022); + await fs.writeFile(file, `${padding}😀\nsecond line\n`, 'utf-8'); + + const result = await headFile(file, 1); + + expect(result).toBe(`${padding}😀`); + expect(result).not.toContain('\uFFFD'); + }); + + it('preserves multi-byte characters across multiple consecutive boundaries', async () => { + // Three lines, each 1023 bytes of ASCII + 'あ' = 1026 bytes per line + '\n' = 1027. + // Total ~3081 bytes; boundaries at 1024, 2048, 3072 all fall inside a multi-byte char. + const file = path.join(tmpDir, 'head-multi-boundary.txt'); + const line = 'a'.repeat(1023) + 'あ'; + await fs.writeFile(file, `${line}\n${line}\n${line}\n`, 'utf-8'); + + const result = await headFile(file, 3); + + expect(result).toBe(`${line}\n${line}\n${line}`); + expect(result).not.toContain('\uFFFD'); + }); + + it('returns plain ASCII content correctly (regression)', async () => { + const file = path.join(tmpDir, 'head-ascii.txt'); + await fs.writeFile(file, 'line1\nline2\nline3\n', 'utf-8'); + + expect(await headFile(file, 2)).toBe('line1\nline2'); + }); + + it('handles a file without a trailing newline', async () => { + const file = path.join(tmpDir, 'head-no-trailing.txt'); + await fs.writeFile(file, 'line1\nline2', 'utf-8'); + + expect(await headFile(file, 2)).toBe('line1\nline2'); + }); + }); + + describe('tailFile', () => { + it('preserves a 3-byte UTF-8 character split across the boundary measured from EOF', async () => { + // tailFile reads backwards in 1024-byte chunks from EOF. + // Construct: 'HEADER\n' + 'a'*100 + 'あ' + 'a'*1017 + '\nLAST' + // total = 7 + 100 + 3 + 1017 + 1 + 4 = 1132 bytes + // boundary at offset (1132 - 1024) = 108 + // 'あ' bytes at offsets 107 (E3), 108 (81), 109 (82) + // chunk 1 (offsets 108..1131) starts with [81, 82, ...] + // chunk 2 (offsets 0..107) ends with [..., E3] + // Buggy version: chunk 1's leading 81 82 decode to U+FFFD U+FFFD; + // chunk 2's trailing E3 decodes to U+FFFD. + const file = path.join(tmpDir, 'tail-3byte.txt'); + const middleBefore = 'a'.repeat(100); + const middleAfter = 'a'.repeat(1017); + const content = `HEADER\n${middleBefore}あ${middleAfter}\nLAST`; + await fs.writeFile(file, content, 'utf-8'); + + const result = await tailFile(file, 2); + + expect(result).toBe(`${middleBefore}あ${middleAfter}\nLAST`); + expect(result).not.toContain('\uFFFD'); + }); + + it('preserves a 4-byte UTF-8 character split across the boundary measured from EOF', async () => { + // '😀' (4 bytes). Place it so 2 bytes lie on each side of the boundary. + // total = 7 + 100 + 4 + 1016 + 1 + 4 = 1132 bytes; boundary at offset 108 + // '😀' bytes at offsets 107, 108, 109, 110 -> 2 bytes in each chunk. + const file = path.join(tmpDir, 'tail-4byte.txt'); + const middleBefore = 'a'.repeat(100); + const middleAfter = 'a'.repeat(1016); + const content = `HEADER\n${middleBefore}😀${middleAfter}\nLAST`; + await fs.writeFile(file, content, 'utf-8'); + + const result = await tailFile(file, 2); + + expect(result).toBe(`${middleBefore}😀${middleAfter}\nLAST`); + expect(result).not.toContain('\uFFFD'); + }); + + it('returns plain ASCII content correctly (regression)', async () => { + const file = path.join(tmpDir, 'tail-ascii.txt'); + await fs.writeFile(file, 'line1\nline2\nline3\n', 'utf-8'); + + expect(await tailFile(file, 2)).toBe('line3\n'); + }); + + it('handles a file without a trailing newline', async () => { + const file = path.join(tmpDir, 'tail-no-trailing.txt'); + await fs.writeFile(file, 'line1\nline2\nline3', 'utf-8'); + + expect(await tailFile(file, 2)).toBe('line2\nline3'); + }); + + it('handles an empty file', async () => { + const file = path.join(tmpDir, 'tail-empty.txt'); + await fs.writeFile(file, '', 'utf-8'); + + expect(await tailFile(file, 5)).toBe(''); + }); + }); +}); diff --git a/src/filesystem/lib.ts b/src/filesystem/lib.ts index 17e4654cd5..e0d096b147 100644 --- a/src/filesystem/lib.ts +++ b/src/filesystem/lib.ts @@ -2,6 +2,7 @@ import fs from "fs/promises"; import path from "path"; import os from 'os'; import { randomBytes } from 'crypto'; +import { StringDecoder } from 'string_decoder'; import { diffLines, createTwoFilesPatch } from 'diff'; import { minimatch } from 'minimatch'; import { normalizePath, expandHome } from './path-utils.js'; @@ -292,42 +293,37 @@ export async function tailFile(filePath: string, numLines: number): Promise 0 && linesFound < numLines) { + // Read chunks from the end of the file until we have enough newlines (or BOF). + // Counting newlines at the byte level is safe: 0x0A (LF) never appears inside + // a UTF-8 multi-byte sequence by design. + while (position > 0 && newlineCount < numLines) { const size = Math.min(CHUNK_SIZE, position); position -= size; const { bytesRead } = await fileHandle.read(chunk, 0, size, position); if (!bytesRead) break; - // Get the chunk as a string and prepend any remaining text from previous iteration - const readData = chunk.slice(0, bytesRead).toString('utf-8'); - const chunkText = readData + remainingText; - - // Split by newlines and count - const chunkLines = normalizeLineEndings(chunkText).split('\n'); - - // If this isn't the end of the file, the first line is likely incomplete - // Save it to prepend to the next chunk - if (position > 0) { - remainingText = chunkLines[0]; - chunkLines.shift(); // Remove the first (incomplete) line - } - - // Add lines to our result (up to the number we need) - for (let i = chunkLines.length - 1; i >= 0 && linesFound < numLines; i--) { - lines.unshift(chunkLines[i]); - linesFound++; + // Copy bytes because the chunk buffer is reused across iterations + const justRead = Buffer.from(chunk.subarray(0, bytesRead)); + for (let i = 0; i < bytesRead; i++) { + if (justRead[i] === 0x0A) newlineCount++; } + chunks.unshift(justRead); } - return lines.join('\n'); + // Decode all accumulated bytes at once so multi-byte characters that span + // a chunk boundary are not split mid-sequence. Any character whose leading + // bytes lie before the lowest read position becomes U+FFFD at the head of + // the decoded text, but that falls within the partial first line we discard. + const fullText = normalizeLineEndings(Buffer.concat(chunks).toString('utf-8')); + const allLines = fullText.split('\n'); + + return allLines.slice(-numLines).join('\n'); } finally { await fileHandle.close(); } @@ -341,13 +337,21 @@ export async function headFile(filePath: string, numLines: number): Promise