fix: detect and decode UTF-16 BOM in read_file (#610) - #621
Conversation
PowerShell 5.1 on Windows writes files as UTF-16 LE when using redirection (e.g., echo "hello" > file.txt). The read_file tool previously read these as UTF-8, producing garbled text with embedded NUL characters. Added BOM detection (0xFF 0xFE for UTF-16 LE, 0xFE 0xFF for UTF-16 BE) in readFileFromDisk and readFileInternal. When a BOM is detected, the file is decoded using TextDecoder with the correct encoding and the BOM character is stripped from the output.
📝 WalkthroughWalkthroughThe filesystem now detects UTF-16 little-endian and big-endian BOMs for disk and internal reads. It decodes matching files, removes BOMs, preserves line-based selection, and falls back to existing handling when detection or decoding fails. ChangesUTF-16 read support
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tools/filesystem.ts`:
- Around line 525-526: Update the BOM-detection flow around
fs.readFile(validPath) to read only the first two bytes using
runWithAbortableTimeout with the existing three-minute cancellation behavior.
Only perform a full-file read when the BOM confirms UTF-16, and ensure that read
also runs through runWithAbortableTimeout; preserve the normal handler path
without rereading non-UTF-16 files.
- Around line 529-530: Update both TextDecoder constructions in
src/tools/filesystem.ts at lines 529-530 and 671-672 to use fatal UTF-16
decoding, so malformed input throws and is handled explicitly before internal
edit operations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 508ce956-17a5-4005-b9e3-eeff4d38bd84
📒 Files selected for processing (1)
src/tools/filesystem.ts
| try { | ||
| const rawBuffer = await fs.readFile(validPath); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Keep BOM probing bounded and cancellable.
fs.readFile(validPath) at Line [526] reads the entire file before the existing timeout starts. It does this for every file without a BOM, and the normal handler reads non-UTF-16 files again. A large or stalled file can consume excessive memory and keep the request pending past the three-minute timeout, even when the caller requests a small line range.
Read only the first two bytes under runWithAbortableTimeout. Read the full file only after a BOM is confirmed, and keep that full read cancellable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/tools/filesystem.ts` around lines 525 - 526, Update the BOM-detection
flow around fs.readFile(validPath) to read only the first two bytes using
runWithAbortableTimeout with the existing three-minute cancellation behavior.
Only perform a full-file read when the BOM confirms UTF-16, and ensure that read
also runs through runWithAbortableTimeout; preserve the normal handler path
without rereading non-UTF-16 files.
| const decoder = new TextDecoder(utf16Encoding); | ||
| let decoded = decoder.decode(rawBuffer); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline =="
ast-grep outline src/tools/filesystem.ts --match TextDecoder --view expanded || true
echo "== relevant lines =="
sed -n '500,545p' src/tools/filesystem.ts
printf '\n---\n'
sed -n '650,685p' src/tools/filesystem.ts
echo "== TextDecoder construction sites =="
rg -n "new TextDecoder|decoder.decode|utf16Encoding|replace\(" src/tools/filesystem.ts
echo "== deterministic JS probe for default TextDecoder behavior =="
node - <<'JS'
for (const inputHex of [
'd800',
'dfff',
'fffd',
'd8000061',
]) {
const buffer = Uint8Array.from(inputHex.split(/(..)/).filter(Boolean).map(h => parseInt(h, 16)));
console.log(inputHex, new TextDecoder().decode(buffer));
}
JSRepository: wonderwhy-er/DesktopCommanderMCP
Length of output: 4371
Use fatal UTF-16 decoders when decoding errors must be handled explicitly.
Both decoder calls use replacement mode by default, so malformed UTF-16 decoding produces U+FFFD instead of failing and being handled explicitly. Use { fatal: true } at these sites, or reject malformed content before internal edit operations.
src/tools/filesystem.ts#L529-L530src/tools/filesystem.ts#L671-L672
📍 Affects 1 file
src/tools/filesystem.ts#L529-L530(this comment)src/tools/filesystem.ts#L671-L672
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/tools/filesystem.ts` around lines 529 - 530, Update both TextDecoder
constructions in src/tools/filesystem.ts at lines 529-530 and 671-672 to use
fatal UTF-16 decoding, so malformed input throws and is handled explicitly
before internal edit operations.
1 participant
Problem
When PowerShell 5.1 writes files via redirection (
echo "hello" > file.txt), it uses UTF-16 LE encoding. Theread_filetool reads these files as UTF-8, producing text with embedded NUL characters that breaks downstream processing.Solution
Detect UTF-16 BOM (Byte Order Mark) at the start of file content and decode accordingly:
0xFF 0xFE→ UTF-16 LE0xFE 0xFF→ UTF-16 BEChanges
src/tools/filesystem.ts: Added BOM detection and UTF-16 decoding in the file reading pathTesting
Fixes #610
Summary by CodeRabbit