Trim fixed-width NUL padding in /CIDSystemInfo strings - #5
Merged
soadzoor merged 1 commit intoSep 17, 2026
Merged
Conversation
readCidSystemInfoString rejected every byte outside printable ASCII, including the trailing NULs that producers emit when they write /Registry and /Ordering as fixed-width strings. A real-world drawing declares its ordering as 49 64 65 6e 74 69 74 79 00 00, that is "Identity" padded to ten bytes, and the padding alone made the whole document fail to render. The padding is trimmed before the character check. The rest stays strict: any other non-printable byte is still an error, and a string made only of NULs is refused with its own message rather than silently becoming empty. Trimming rather than tolerating matters, because the value is compared against collection names. A kept "Japan1\0\0" would resolve no collection and quietly degrade to the unavailable-fallback path instead of decoding the collection the font actually declares. requireCMapSystemInfoString duplicates the same rule for embedded CMaps, so it gets the same treatment; a producer that pads one can pad the other, and two validators of the same field should not disagree. Both new cases use a padded Japan1 rather than Identity, so that keeping the padding instead of trimming it fails the assertions on decoded Unicode. Refs soadzoor#2 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 8, 2026
Owner
|
Looks good, thank you! Added an additional test for this for more coverage with this commit: #d01a9d07 |
soadzoor
added a commit
that referenced
this pull request
Sep 17, 2026
- 0.1.30 - Fix: use native ESM paths in Vite config - Prefer usable PDF output with bounded raster fallback and approximation warnings - Strengthen CFF BaseFontBlend regression coverage - Merge pull request #10 from sebgoubier/fix/cff-basefontblend-is-not-mm - Clarify stitching bounds tolerance and expand regression tests - Merge pull request #9 from sebgoubier/fix/stitching-bounds-plateau - Feat(pdf): Add support for ICC color profiles (qcms, lcms, and alternate) with fallback - Fix(pdf): recover Flate EOL padding without losing decoded output - Feat(pdf): synthesize cloudy borders for Square annotations - Feat(pdf): support underline borders on Square annotations - Feat(pdf): synthesize Square annotations with validated geometry - Test: reject invalid CIDSystemInfo strings - Merge pull request #5 from sebgoubier/fix/cid-system-info-nul-padding - Fix(tests): handle file URLs correctly on Windows - Merge pull request #4 from sebgoubier/fix/ccitt-b1-strictly-right-of-a0 - Stop rejecting BaseFontBlend as a Multiple Master CFF font - Treat repeated stitching Bounds as the empty subdomain they describe - Trim fixed-width NUL padding in /CIDSystemInfo strings - Fix CCITT b1 selection so vertical modes cannot move backwards Source-Revision: 5d761c9
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
2 participants
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.
What this fixes
The CID font item of #2:
CID font /CIDSystemInfo /Ordering must contain printable ASCII.Probed on a real-world MEP drawing that opens on v0.1.23, the offending
/Orderingis:That is
Identity— the most ordinary collection there is — written as a fixed-width string and padded with two NULs. The padding is the only thing the validator objects to.readCidSystemInfoStringnow trims trailing NULs before the character check. Everything else stays strict: any other byte outside printable ASCII is still an error, and a string made only of NULs is refused with its own message instead of silently becoming empty.Trimming rather than merely tolerating is the point. The value is compared against collection names, so a kept
"Japan1\0\0"would resolve no collection at all and quietly fall through to the unavailable-fallback path — a wrong result instead of a loud one.requireCMapSystemInfoStringduplicates the same rule for embedded CMaps, so it gets the same treatment. Leaving them apart would make the two validators disagree about the same field of the same standard, and a producer that pads one can pad the other.Tests
Two cases in
testCidUnicodeFallbackAndRos, one per validator: a CID font dictionary whose ROS strings carry NUL padding, and an embedded CMap declaring/Registry <41646f626500>and/Ordering <4a6170616e310000>.Both use a padded Japan1 rather than Identity on purpose. If the padding were kept instead of trimmed, the collection would not resolve and the assertions on decoded Unicode would fail — so the tests tell "trim" apart from "merely tolerate", which asserting on Identity could not.
Both fail on
mainwith the production error and pass with the patch.Checks
npm run test:file -- scripts/test-native-font-cmap-semantics.mjs— passesnpm test—tsc --noEmitclean, 36 / 37 fast files passnpm run test:integration— 41 / 41npm run test:unit— 66 / 68The two failing files (
test-text-lod-core.mjs,test-room-segment-extractor.mjs) also fail onmainwithout this change. They are Windows-only: a POSIX-style path reaches Node as/C:/dev/...and resolves toC:\C:\dev\.... Unrelated to this patch.No existing test asserted the old rejection, so nothing here loosens a documented expectation.
Scope
Only the CID font item of #2, and independent of #4 (CCITT). The
/Squareannotation and ICCBased items are untouched — the first of those is a missing feature rather than a defect, and the policy question it raises is yours to settle.Refs #2