fix(tounicode): decode Type0 GBK-EUC-H fonts without ToUnicode via predefined CMaps - #422
fix(tounicode): decode Type0 GBK-EUC-H fonts without ToUnicode via predefined CMaps#422yzxcj797 wants to merge 4 commits into
Conversation
…edefined CMaps A Type0 font whose /Encoding names a predefined CMap like GBK-EUC-H (not Identity-H/V) was skipped during ToUnicode CMap discovery, so its text degraded to U+FFFD and could misroute the page to OCR (firecrawl#400). Recognize any predefined encoding CMap shipped in external/bcmaps (name-whitelisted - untrusted /Encoding values are never used as filesystem paths), parse the binary CMap differential operators correctly (signed CID deltas, hex increments, codespace/notdef operand consumption), and map GB1 CIDs to Unicode through a new Adobe-GB1 table generated from cmap-resources UniGB-UCS2 (plain ideographs preferred over Kangxi/compatibility forms), mirroring the existing Adobe-Korea1 module. The regression test builds a synthetic Type0/GBK-EUC-H PDF without ToUnicode and asserts end-to-end extraction of multi-byte GBK codes.
There was a problem hiding this comment.
1 issue found across 5 files
Not reviewed (too large): src/adobe_gb1.rs (~28,678 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/extractor/fonts.rs">
<violation number="1" location="src/extractor/fonts.rs:940">
P2: When a predefined Type0 font is used across many pages, this predicate rereads the entire `.bcmap` from disk during every page’s font setup. Cache the shipped-name validation or reuse the loaded CMap so extraction does not perform page-count-scaled filesystem I/O.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Fix all with cubic | Re-trigger cubic
| let encoding = font_dict.get(b"Encoding").ok()?.as_name().ok()?; | ||
| if encoding != b"Identity-H" && encoding != b"Identity-V" { | ||
| let is_identity = encoding == b"Identity-H" || encoding == b"Identity-V"; | ||
| if !is_identity && !crate::tounicode::is_predefined_encoding_cmap(encoding) { |
There was a problem hiding this comment.
P2: When a predefined Type0 font is used across many pages, this predicate rereads the entire .bcmap from disk during every page’s font setup. Cache the shipped-name validation or reuse the loaded CMap so extraction does not perform page-count-scaled filesystem I/O.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/extractor/fonts.rs, line 940:
<comment>When a predefined Type0 font is used across many pages, this predicate rereads the entire `.bcmap` from disk during every page’s font setup. Cache the shipped-name validation or reuse the loaded CMap so extraction does not perform page-count-scaled filesystem I/O.</comment>
<file context>
@@ -936,7 +936,8 @@ pub(crate) fn get_font_file2_obj_num(doc: &Document, font_dict: &lopdf::Dictiona
let encoding = font_dict.get(b"Encoding").ok()?.as_name().ok()?;
- if encoding != b"Identity-H" && encoding != b"Identity-V" {
+ let is_identity = encoding == b"Identity-H" || encoding == b"Identity-V";
+ if !is_identity && !crate::tounicode::is_predefined_encoding_cmap(encoding) {
return None;
}
</file context>
Keep binary CMap code-space ranges instead of collapsing every encoding to its maximum code width. Also reject overflowing differential codes, decode wide zigzag deltas without i32 truncation, and check bundled CMap availability without reading the full file.
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
Check the significant width of binary CMap hexadecimal numbers before truncating them to the destination size, so malformed differential operands fail cleanly.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
Summary
A Type0 font whose
/Encodingnames a predefined CMap such asGBK-EUC-H(anything other thanIdentity-H/Identity-V) was skipped during ToUnicode CMap discovery, so its text decoded to U+FFFD garble and could misroute otherwise text-based pages to OCR (#400).Three coordinated changes:
src/extractor/fonts.rs): Type0 fonts with any predefined encoding CMap shipped inexternal/bcmapsnow participate in CMap collection. Untrusted/Encodingvalues are validated against a whitelisted character set and resolved only to files that actually ship — never interpreted as filesystem paths.src/tounicode.rs): correct consumption ofcodespace/notdefoperand pairs,cidchar/cidrangewith hex code increments and signed CID deltas (read_signed), 16-bit range checks on both code and CID, and a bounded-expansion guard on the resulting map.src/adobe_gb1.rs, 28,660 entries): generated from adobe-type-tools/cmap-resourcesAdobe-GB1-6/cid2code.txt(UniGB-UCS2 column, last mapping per CID — plain ideographs preferred over Kangxi radicals/compatibility forms), mirroring the existingadobe_korea1module convention. GB1-ordered CIDSystemInfo now resolves through this table instead of failing.Testing
test_type0_gbk_euc_h_without_tounicode_decodes_predefined_cmap: builds a synthetic Type0/GBK-EUC-H PDF without ToUnicode in memory and asserts both the composed CMap (0xB6AB 0xB7BD → "东方") and fullextract_text_with_positions_memoutput.gbk_euc_h_binary_cmap_maps_issue_code_points(>20k mappings, spot-checks the issue's code points) andadobe_gb1_prefers_plain_ideographs_for_reverse_mapping.cargo test --lib— 955/955 passcargo test --test integration_tests— 163/163 passcargo fmt --checkclean;cargo clippy --lib --tests— 0 errorsFixes #400
Summary by cubic
Decode Type0 fonts whose
/Encodingnames a predefined CMap (e.g., GBK‑EUC‑H) even without ToUnicode, avoiding U+FFFD output and OCR fallback. Previously these were skipped; now we compose the predefined encoding code→CID map with the collection CID→Unicode map.external/bcmapswith a new Adobe‑GB1 CID→Unicode table (adobe_gb1.rs); GB1 fonts now resolve >28k mappings.codespace/notdefoperands, handle hex increments and signed CID deltas, reject differential code overflow, enforce 16‑bit bounds, reject oversized hexadecimal operands before packing, and guard bounded expansion.FontCMaps: Identity‑H/V still try embedded TrueType/OpenType then CIDSystemInfo; for other predefined encodings (e.g., GBK‑EUC‑H), build a composed ToUnicode directly./Encodingagainst a safe charset, only load bundled CMaps by name, never interpret names as filesystem paths, and check bundled availability without reading full files.Written for commit 7116f19. Summary will update on new commits.