[ajroetker]

Summary

This PR improves how font encodings are handled, which fixes text extraction for PDFs with complex font setups. It has two main fixes:

1. Prioritize the ToUnicode CMap. Check /ToUnicode first, and fall back to /Encoding only if it is missing or does not parse.
2. Apply BaseEncoding + Differences properly. For an Encoding dictionary, start from BaseEncoding and then overlay the Differences array.

The expanded CMap parsing is also hardened, because every font now goes through it (see "Robustness" below).

Problem

Characters were decoded incorrectly in some PDFs (for example, scanned court filings):

• Subset fonts that assign glyph codes in order of first use set /Encoding /WinAnsiEncoding as well as a /ToUnicode CMap. We ignored the CMap, so 0 came out as M, and so on.
• For Encoding dictionaries, only Differences was applied. Every other code was passed through as its raw byte, not looked up in BaseEncoding. So WinAnsi characters such as ’ (0x92) and – (0x96) came out as C1 control characters.

Solution

The code samples below match the code in this PR, trimmed for readability (DebugOn logging and some comments are left out).

Fix 1: ToUnicode priority

The PDF spec (32000-1:2008, section 9.10.2) makes ToUnicode the primary way to map character codes to Unicode. getEncoder now checks it first and falls back to /Encoding only when there is no ToUnicode stream or when readCmap rejects it:

func (f Font) getEncoder() TextEncoding {
	// Check ToUnicode first - it's the authoritative character mapping
	// per PDF spec and takes precedence over Encoding
	toUnicode := f.V.Key("ToUnicode")
	if toUnicode.Kind() == Stream {
		if m := readCmap(toUnicode); m != nil {
			return m
		}
		// ToUnicode stream exists but failed to parse - fall through to Encoding
		if DebugOn {
			println("ToUnicode stream failed to parse, falling back to Encoding")
		}
	}

	// Fall back to Encoding-based decoding
	enc := f.V.Key("Encoding")
	switch enc.Kind() {
	case Name:
		switch enc.Name() {
		case "WinAnsiEncoding":
			return &byteEncoder{&winAnsiEncoding}
		case "MacRomanEncoding":
			return &byteEncoder{&macRomanEncoding}
		case "Identity-H":
			// No usable ToUnicode: pass raw bytes through (CID decoding is out of scope).
			return &nopEncoder{}
		case "UniGB-UCS2-H":
			return &ucs2Encoder{}
		default:
			// Symbol, ZapfDingbats, Identity-V, ...: don't guess a Latin table.
			return &nopEncoder{}
		}
	case Dict:
		return newDictEncoder(enc, f.isSymbolic())
	case Null:
		return &byteEncoder{&pdfDocEncoding}
	default:
		return &nopEncoder{}
	}
}

Because ToUnicode is now checked up front, master's charmapEncoding helper is no longer needed and has been removed. The UniGB-UCS2-H case from #56 is kept.

Fix 2: BaseEncoding + Differences

The PDF spec (section 9.6.6) says an Encoding dictionary starts from BaseEncoding and has Differences applied on top. dictEncoder now builds a 256-entry table once and looks up each byte in O(1). Before, it scanned the Differences array for every byte:

func newDictEncoder(enc Value, symbolic bool) *dictEncoder {
	e := &dictEncoder{}

	// Start with base encoding
	baseEnc := enc.Key("BaseEncoding")
	var baseTable *[256]rune
	switch baseEnc.Name() {
	case "WinAnsiEncoding":
		baseTable = &winAnsiEncoding
	case "MacRomanEncoding":
		baseTable = &macRomanEncoding
	case "StandardEncoding":
		// Not allowed by Table 114, but some producers write it.
		baseTable = &standardEncoding
	case "MacExpertEncoding":
		// No MacExpertEncoding table; fall back to StandardEncoding.
		baseTable = &standardEncoding
	default:
		// Per PDF spec (9.6.6.1), if BaseEncoding is absent the base is
		// StandardEncoding for nonsymbolic fonts and the font program's
		// built-in encoding for symbolic ones. We don't parse font
		// programs, so for symbolic fonts pass codes outside Differences
		// through as their raw byte value, as this decoder did before
		// BaseEncoding support.
		if symbolic {
			baseTable = &identityEncoding
		} else {
			baseTable = &standardEncoding
		}
	}
	copy(e.table[:], baseTable[:])

	// Apply Differences array on top
	// Format: [firstCode /name1 /name2 ... nextCode /nameN ...]
	diff := enc.Key("Differences")
	if diff.Kind() == Array {
		code := -1
		for j := 0; j < diff.Len(); j++ {
			x := diff.Index(j)
			if x.Kind() == Integer {
				code = int(x.Int64())
				continue
			}
			if x.Kind() == Name {
				if code >= 0 && code < 256 {
					if glyphName := x.Name(); glyphName == ".notdef" {
						e.table[code] = noRune
					} else if r := nameToRune[glyphName]; r != 0 {
						e.table[code] = r
					}
				}
				// Advance even when code is out of range so that a bad
				// start code doesn't shift or drop later entries.
				code++
			}
		}
	}

	return e
}

A full standardEncoding table (Table D.2) is added to text.go.

Behavior change: fonts with Differences but no BaseEncoding

This change is intended. Previously, any code not listed in Differences was returned as its raw byte. Now the implicit base depends on the font type, as in spec 9.6.6.1:

• Nonsymbolic fonts use StandardEncoding, as the spec requires. This corrects 0x27/0x60 (quoteright/quoteleft). Codes that StandardEncoding leaves undefined, including 0x80–0x9F, now decode to U+FFFD. Before, they came out as C1 control characters, which were never correct text either.
• Symbolic fonts keep the old raw-byte behavior. A font is treated as symbolic if its FontDescriptor has the Symbolic flag (bit 3) set and the Nonsymbolic flag (bit 6) clear. Without a FontDescriptor, only the standard Symbol and ZapfDingbats fonts count as symbolic. The spec says these fonts use their built-in encoding, which we can't read without parsing the font program, so we keep the old behavior and they do not start returning U+FFFD for 0x80–0x9F. Differences is still applied to symbolic fonts, as before. Some readings of the spec say a symbolic font's /Encoding should be ignored entirely, but honoring Differences matches master and what common readers such as Acrobat do.

Tests: TestSymbolicFontDifferencesWithoutBaseEncoding and TestFontIsSymbolic in encoding_test.go.

Edge cases this doesn't special-case, because we don't parse embedded font programs (true before this PR as well):

• Type3 fonts usually have no FontDescriptor flags, so they count as nonsymbolic (a Type3 FontDescriptor with the Symbolic flag set is honored via isSymbolic()). Codes in Differences decode by glyph name, and the rest use StandardEncoding. Codes outside Differences normally have no glyph in a Type3 font anyway.
• TrueType fonts with no /Encoding (for example, symbolic fonts that rely on their built-in (3,0) cmap) take the Null path and decode as PDFDocEncoding, the same as on master. Symbolic detection doesn't come into play there.

Behavior change: Identity-H without a usable ToUnicode

On master, an Identity-H font with no ToUnicode decoded each byte of the 2-byte CIDs as PDFDocEncoding. A malformed ToUnicode could panic and fail the whole page's text extraction (for example, endbfchar without beginbfchar, or a codespace range wider than 4 bytes), or come out as U+FFFD (for example, a CMap truncated inside a section). Both cases now pass the raw bytes through (nopEncoder). Neither is a real CID decode, but this way two-byte codes aren't split into unrelated characters. Proper CID decoding is out of scope. In practice Identity-H fonts almost always include a ToUnicode CMap, and that path works as before.

"Raw bytes" here means the decoder's output: GetPlainText re-encodes text as UTF-8 (unchanged from master), so passed-through bytes that aren't valid UTF-8 appear there as U+FFFD.

Tests for these fallbacks: TestEncoderFallbacksWithoutToUnicode and TestIdentityHMalformedToUnicodePassesThrough in encoding_test.go.

Robustness

Every font with a /ToUnicode stream now goes through readCmap, so its handling of malformed input was hardened, with a regression test for each case:

• readCmap returns nil rather than panicking for: malformed bfchar/bfrange sections, codespace ranges wider than 4 bytes, truncated CMaps, sections that don't match (beginbfchar … endbfrange), counts larger than the operand stack (DoS), entries that are missing operands, CMaps with no codespace range or no mappings, and panics from the PostScript interpreter (recovered at the readCmap boundary). In each case extraction falls back to /Encoding.
• At decode time, odd-length UTF-16 destinations and empty bfrange destinations produce U+FFFD rather than panicking.
• The Differences parser ignores codes outside 0–255 without losing track of later entries, and maps /.notdef to U+FFFD.

Testing

go build ./..., go vet ./... and go test ./... all pass on this branch, rebased onto current master.

Regression PDF: testdata/font_encoding.pdf is 1.9 KB and uncompressed so it can be read by eye. TestFontEncodingRegressionPDF reads it. It has one page with two fonts, built to match the documents where I found the bug:

Font	Setup	master output	this PR
F1	Subset TrueType: sequential glyph codes, /Encoding /WinAnsiEncoding + /ToUnicode	!"#$%&'&()*+),-.//)012	Case 1:15-cv-07433-LAP
F2	/BaseEncoding /WinAnsiEncoding + /Differences [1 /zero … /nine]	Defendants\^R reply \^V filed 03/21/16	Defendants’ reply – filed 03/21/16

The test fails on master and passes on this branch. To regenerate the PDF, run go run testdata/font_encoding_gen.go testdata/font_encoding.pdf.

About the "104 pages → 2 pages" figure in an earlier version of this description: it was measured on a private set of court filings that can't be shared, so it can't be reproduced from this repository. The regression PDF above recreates the two font setups behind that damage, and it is the reproducible evidence for this fix.

[ledongthuc]

Thanks for this PR — the direction is correct and both fixes are spec-compliant (ToUnicode precedence per §9.10.2, and BaseEncoding + Differences per §9.6.6). I verified locally: it builds, go vet is clean, the existing tests pass, and it merges cleanly.

I'd like a few changes before merging:

1. Identity-H fallback — Identity-H is a 2-byte CMap for Type0/CID fonts, so decoding byte-by-byte through pdfDocEncoding produces garbage when ToUnicode is absent. This predates the PR, but since this line is being touched, can we leave it as nopEncoder for this case (or explicitly note it's out of scope)?

2. Unknown named encodings (default case) — this changed from nopEncoder to pdfDocEncoding, which silently alters behavior for Symbol, ZapfDingbats, StandardEncoding, Identity-V, etc. None of those equal PDFDocEncoding; Symbol/ZapfDingbats especially will decode to wrong (but plausible-looking) Latin text. Recommend keeping nopEncoder for genuinely unknown names, or adding proper tables.

3. MacExpertEncoding fallback — silently mapping it to pdfDocEncoding is inaccurate; please at least log via DebugOn or add a real table.

4. Default BaseEncoding = PDFDocEncoding — per spec the default should be the font's built-in encoding (usually StandardEncoding). The comment acknowledges the simplification, but consider adding a StandardEncoding table since the 0x80–0x9F range differs from PDFDocEncoding.

5. /.notdef in Differences — nameToRune has no .notdef, so a /.notdef entry silently keeps the BaseEncoding value instead of mapping to noRune.

6. readCmap can panic — readCmap has panic("missing beginbfchar") / panic("missing beginbfrange"). Now that ToUnicode is parsed for every font, this expands the panic surface; Content() has no recover(), so a malformed ToUnicode CMap could crash. Recommend converting those to the existing ok = false / return nil pattern.

7. Tests — the PR reports a big improvement but has no regression tests. Could you add a unit test for newDictEncoder (BaseEncoding + Differences, incl. a gap and an out-of-range code) and a ToUnicode-precedence test (e.g., a synthetic PDF like ps_test.go)?

8. Minor: the branch is ~20 commits behind master; it merges cleanly, but a rebase would be nice.

None of these are hard blockers except arguably (2) and (6). Happy to iterate.

[ledongthuc]

@ajroetker Nice work recently, The ToUnicode priority change and the BaseEncoding + Differences change are correct. The follow-up fixes for the review feedback are also good. Before we merge, please do the following:

1. Rebase the branch on the current master. GitHub shows a conflict in page.go. The conflict is with the new commits on master that change lex.go, read.go, and the encryption code. Run go build, go vet, and go test ./... after the rebase.
2. Check the symbolic font case. If a font has an Encoding dictionary with Differences but no BaseEncoding, the codes that are not in Differences now use StandardEncoding. Before, they used the raw byte. Codes from 0x80 to 0x9F now return U+FFFD. Add a test for a symbolic font, or write a note in the PR description that says this change is intended.
3. Add a regression test with a real PDF, or explain the test result. The description says that 104 pages were reduced to 2 pages. We cannot repeat that result from the repository. A small test PDF in testdata/ would show the fix.
4. Update the PR description. The code example in the "Fix 1" section is out of date. Show the code that is in the PR now.

When these items are done, we can review the PR again.

[lazen5000-code]

Another real-world case that this PR fixes: PDFs printed from Chrome ("Save as PDF") with Korean text embed the Hangul glyphs as a Type 3 font whose /Encoding is a dictionary (/Differences with custom glyph names) and a valid /ToUnicode CMap. getEncoder takes the case Dict branch, never consults /ToUnicode, and GetPlainText returns raw glyph codes (e.g. ý ¢ _) for every Hangul character, while Latin text in the same document (CID TrueType, Identity-H) is fine. poppler's pdftotext extracts the same file correctly.

Preferring /ToUnicode when it is present and parses — the first change in this PR — fixes it. We verified with a minimal Type 3 + Differences + ToUnicode fixture (the patched build yields the Unicode text, the current HEAD yields an empty string), and we're carrying that one-hunk patch on top of 5e46805f718e until this lands. Thanks for the PR — it would be great to see it merged.