fix(serialize): encode built-in font text as WinAnsi, not raw UTF-8#274
Open
JohnHarrison wants to merge 1 commit into
Open
fix(serialize): encode built-in font text as WinAnsi, not raw UTF-8#274JohnHarrison wants to merge 1 commit into
JohnHarrison wants to merge 1 commit into
Conversation
Built-in font text was handed to lopdf as `Encoding::SimpleEncoding(b"WinAnsiEncoding")`. lopdf does not recognize that name and silently falls back to raw UTF-8. Because the font dictionary declares `/Encoding /WinAnsiEncoding`, every non-ASCII glyph was mojibake'd (`é` -> `C3 A9` instead of `E9`), corrupting accented names and Markdown typography (curly quotes, en/em dashes, ellipsis, bullet). ASCII was unaffected, which is why it went unnoticed. Encode built-in text via `Encoding::OneByteEncoding` with a standard WinAnsi table. All three text-showing operators now share one encoder (`encode_text_run` / `encode_builtin_text_winansi`): Tj/TJ, the `'` operator (MoveToNextLineShowText) and the `"` operator (SetSpacingMoveAndShowText) each previously emitted `text.as_bytes()` (raw UTF-8) and are fixed together; the duplicate encoder in text.rs (`encode_text_items`) is routed through the same helper. The table leaves the C0 control range (0x00..=0x1F) undefined, matching the standard encoding. Adds unit tests: accented letters, Markdown typography, ASCII pass-through, a UTF-8 regression guard, the shared no-font path, and the table shape. Fixes fschutt#273 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cd9f5e8 to
575f87a
Compare
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
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.
Fixes #273.
Problem
Text drawn with a built-in font was handed to lopdf as
Encoding::SimpleEncoding(b"WinAnsiEncoding"), but lopdf 0.39 doesn't recognize that name and falls through to raw UTF-8 bytes. Since the font dictionary declares/Encoding /WinAnsiEncoding, the declared encoding and the stream bytes disagree, so every non-ASCII character mojibakes —éis written asC3 A9instead ofE9and renders asé. This corrupts accented names ("José" → "José") and Markdown typography (curly quotes, en/em dashes, ellipsis, bullet). ASCII is unaffected, which is why it went unnoticed. See #273 for a standalone repro with byte-level evidence.Fix
SimpleEncoding(name)only handles a couple of named CJK encodings; the correct single-byte path isOneByteEncoding(&table), which reverse-maps Unicode → byte. lopdf doesn't export its WinAnsi table publicly, so this adds the standard WinAnsi table (Latin-1 identity for0x20..=0x7E/0xA0..=0xFF, Windows-1252 for0x80..=0x9F, C0 controls and the five undefined CP1252 slots leftNone).All three text-showing operators now share one encoder so they can't drift apart:
Tj/TJ(encode_text_items_to_pdf)'—Op::MoveToNextLineShowText"—Op::SetSpacingMoveAndShowTextThe
'and"operators previously emittedtext.as_bytes()(raw UTF-8) directly, reproducing the same bug; they now go through the sharedencode_text_run(embedded fonts → glyph IDs, built-in fonts → WinAnsi). The duplicate encoder intext.rs(encode_text_items) is routed through the sameencode_builtin_text_winansihelper.Tests
serialize::win_ansi_tests: ASCII pass-through; accented letters → single WinAnsi bytes (é→E9,José→4A 6F 73 E9); Markdown typography (curly quotes, en/em dash, ellipsis, bullet,×,·,€,™); a regression guard thatéis not emitted as UTF-8C3 A9; the sharedencode_text_run(_, None)built-in path; and the table shape. Verified withcargo test --no-default-features --lib win_ansi(6 pass) and the existingtests/op.rsround-trip suite (11 pass). The change is in the core serialization path and is feature-independent; locally I neutralized the[patch.crates-io]azul entries (they point at../azul), which are untouched by this PR.Known limitations / possible follow-ups
text.rs::decode_pdf_stringstill usesString::from_utf8_lossy, so re-parsing a PDF written by the fixed encoder decodes single-byte WinAnsi (e.g.0xE9) as invalid UTF-8 →U+FFFD. Existing round-trip tests are ASCII-only so they pass, but a symmetric WinAnsi-aware decoder would be a good follow-up. Happy to do it in a separate PR if you'd like.e+ U+0301) silently vanish. Callers needing full Unicode should embed a font; normalizing input to NFC avoids the combining-mark case. This matches the documented behavior of the encoding and only changes previously-corrupt output.