Skip to content

fix(sidebar): read a linked SQL file's header in the encoding the whole file uses - #3083

Merged
datlechin merged 1 commit into
mainfrom
fix/linked-header-utf8-boundary
Sep 23, 2026
Merged

datlechin merged 1 commit into
mainfrom
fix/linked-header-utf8-boundary

Conversation

@datlechin

Copy link
Copy Markdown
Member

Summary

The sidebar builds each linked SQL file's row from its first 4 KB. When a UTF-8 file had a multibyte character straddling byte 4096, TablePro labelled it ISO-8859-1 and garbled its frontmatter: -- @name: Báo cáo doanh thu showed as Báo cáo doanh thu. Edit Metadata then seeded that garbled name and wrote it back into the file. BOM'd UTF-16 and UTF-32 files got the same label, and a big-endian UTF-32 file was garbled even when opened. Found while investigating #2505.

Root cause

  • FileTextLoader.loadHeader read exactly 4096 bytes and called String(data:encoding: .utf8), falling back to .isoLatin1. Foundation has no notion of a partial character, so a character cut at the limit fails the whole prefix.
  • The header never looked for a byte order mark. FileTextLoader.load relied on String(contentsOf:usedEncoding:), which rejects a big-endian UTF-32 BOM (measured) and falls back to Latin-1. So the header and the full load disagreed, and the index kept the header's answer.

Fix

  • ByteOrderMark is the one place that recognises UTF-16 and UTF-32 marks. SQLChunkDecoder now uses it instead of its own copy of the tables; its own trimming at chunk boundaries is unchanged.
  • TextPrefixDecoder decodes a byte prefix: a mark's encoding first, then UTF-8, then ISO Latin-1. When the file is longer than the limit, it reads up to 3 bytes past it and completes a character the limit cut in half. Completing is stricter than trimming: a Latin-1 0xE9 followed by ASCII still reads as Latin-1.
  • FileTextLoader.load decodes a file with a mark through the same decoder, so the header and the full load agree for every mark, UTF-32 big-endian included. A mark now wins over the com.apple.TextEncoding attribute, as it already did for the header. Files without a mark load exactly as before.

Measured against the old load() over 12,240 handmade and fuzzed files: 259 UTF-32 BE files now load as UTF-32 instead of Latin-1 (the fix); about 600 files starting FF FE 00 00 whose length is not a multiple of 4 now load as Latin-1 keeping every byte, where Foundation used to drop the trailing bytes silently; files whose mark and attribute disagree follow the mark. No other file changed. SQLChunkDecoder gave identical output to the old code over 2,588,124 chunked decodes.

Tests

  • TextPrefixDecoderTests: 2-, 3- and 4-byte characters cut at every inner position, genuine Latin-1, a Latin-1 lookalike at the limit, UTF-16 and UTF-32 marks, a surrogate pair across the limit, cut code units, short and empty input.
  • FileTextLoaderTests: real files at the 4096 limit, including the reported Vietnamese name; big-endian UTF-32; a cut marked file keeping every byte; a mark outranking the encoding attribute; and the header agreeing with the full load for every fixture.
  • ByteOrderMarkTests, plus new SQLChunkDecoderTests cases that pass on both the old and new code and guard the refactor. Also run: SQLFileParserTests, SQLFileParserPLSQLTests, FileTabBaselineTests.

Risks

  • Edit Metadata on a UTF-32 big-endian file writes it back little-endian with a mark, as it already did for UTF-16 big-endian. Before, it rewrote such a file as Latin-1.
  • The header is still a 4 KB prefix: a file whose first 4 KB is valid UTF-8 and whose invalid bytes come later is still labelled UTF-8.

@datlechin
datlechin force-pushed the fix/linked-header-utf8-boundary branch from 1bdf5d0 to d7ed434 Compare September 23, 2026 15:48
@datlechin
datlechin merged commit 1dda5ca into main Sep 23, 2026
3 checks passed
@datlechin
datlechin deleted the fix/linked-header-utf8-boundary branch September 23, 2026 19:19
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.

1 participant