[
https://issues.apache.org/jira/browse/PDFBOX-6250?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18110320#comment-18110320
]
ASF subversion and git services commented on PDFBOX-6250:
---------------------------------------------------------
Commit 1937727 from Tilman Hausherr in branch 'pdfbox/branches/3.0'
[ https://svn.apache.org/r1937727 ]
PDFBOX-6250: add test file by Claude
> A CMap's own cid mappings are overridden by the ones it inherits via usecmap,
> sovertical CJK draws horizontal glyph forms
> -------------------------------------------------------------------------------------------------------------------------
>
> Key: PDFBOX-6250
> URL: https://issues.apache.org/jira/browse/PDFBOX-6250
> Project: PDFBox
> Issue Type: Bug
> Components: FontBox
> Affects Versions: 3.0.8 PDFBox
> Reporter: Patrick Corless
> Priority: Major
>
> *Description*
> A CMap that starts with `usecmap` is meant to be able to redefine any code it
> inherits, but in `org.apache.fontbox.cmap.CMap` the inherited
> `cidchar`/`cidrange` mappings are merged into the same collections as the
> importing CMap's own. `usecmap` is processed while the header is read, so the
> parent's mappings land in `codeToCid` / `codeToCidRanges` _before_ the CMap's
> own are parsed and appended. `toCID` consults the map and then scans
> `codeToCidRanges` first-match, so an inherited range that covers a code beats
> the mapping the importing CMap declared for it, and the override is silently
> lost.
> {code:java}
> CMap cmap = new CMapParser().parsePredefined("ETenms-B5-H");
> System.out.println(cmap.toCID(0x41, 1)); // prints 13681, expected 34{code}
> `ETenms-B5-H` exists purely to do this override: it uses `ETen-B5-H` and then
> remaps 0x20-0x7E to the proportional latin CIDs 1-95, where the parent maps
> them to the fullwidth forms at 13648+. So 0x41 ('A') must be CID 34, not
> 13681 (the fullwidth 'A').
>
> Dumping the parsed ranges of `ETenms-B5-H` shows both present, inherited
> first:
> {code:java}
> [0] 32..126 -> 13648 (inherited from ETen-B5-H, always matches first)
> ...
> [226] 32..126 -> 1 (ETenms-B5-H's own, never reached){code}
> *Scope*
> 31 of the 92 bundled predefined CMaps use `usecmap` *and* declare mappings of
> their own, and all 31 are measurably affected — the sweep below reaches every
> one of them and nothing else. Horizontal: `ETenms-B5-H`, `UniJIS-UCS2-HW-H`.
> The remaining 29 are the `-V` variants, whose own mappings select the
> vertical glyph forms — so vertical CJK is affected too. `UniJIS-UCS2-HW-H` is
> the same shape as ETenms: its parent `UniJIS-UCS2-H` maps 0x0041 to the
> proportional CID 34, and the `-HW` CMap overrides it to the halfwidth CID 264.
> *Downstream symptom*
> Wrong CID means the wrong glyph is drawn, and because `/W` is indexed by CID,
> the wrong CID usually falls outside the font's `/W` array, so the advance
> silently falls back to `/DW` = 1000 — every latin glyph becomes a full em
> wide (rendering as `J a v a S e r v e r`). Documents using `ETenms-B5-H` with
> a `/W` array that starts at CID 1 are proof the producer expected the
> proportional CIDs.
> *Proposed fix*
> Keep inherited CID mappings in their own map and range list
> (`inheritedCodeToCid` `inheritedCodeToCidRanges`) and consult them only
> after the CMap's own map and ranges: own map -> own ranges -> inherited map
> -> inherited ranges.
> A `usecmap` chain keeps nearest-wins ordering, which needs opposite insertion
> orders for the two structures: the maps are looked up by key so `putAll` is
> last-wins (deepest applied first, the
> nearer CMap's own on top), while the range list is scanned first-match so the
> nearer CMap's ranges are added ahead of the ones it inherited.
> Three details worth calling out:
> - `hasCIDMappings` has to account for the inherited mappings.* `Identity-V`
> declares no cid mappings of its own at all, it only uses `Identity-H`, so
> once the inherited ones live in separate collections `hasCIDMappings` would
> report false and `toCID` would return 0 for every code. Covered by
> `testUseCmapOnlyInheritedMappings`.
> - The maps are copied, not shared.* This also removes a latent aliasing bug:
> the previous
> `codeToCid.putIfAbsent(length, mappings)` stored the used CMap's inner map
> _by reference_ whenever the importing CMap had no mapping of that code
> length yet, so a later `addCIDMapping` on the importing CMap wrote straight
> into the cached predefined parent, corrupting it for every later user.
> Covered by `testUseCmapDoesNotShareMappingsWithTheUsedCMap`.
> - CID 0 is a valid mapping target, not a "not found" marker.* The private
> range scan returned 0 for both, which with two range lists would make a
> CMap's own `cidrange` to CID 0 fall through to the inherited mappings. It
> now returns -1 for "no range covers this code"; the public `toCID` still
> returns 0. Covered by `testUseCmapOwnMappingToCidZeroIsNotAFallthrough`
> *Testing*
> Seven tests added to `TestCMapParser`:
> ||test||covers||
> |testUseCmapOwnMappingsWin|the reproducer above, both `toCID` overloads|
> |testUseCmapChainKeepsNearestMapping|a two level `usecmap` chain stays
> nearest-wins |
> |testUseCmapOwnMappingsBeatInheritedRanges| own cidchar and own cidrange both
> beat an inherited range|
> |testUseCmapOnlyInheritedMappings|`Identity-V`, where every mapping is
> inherited |
> |testUseCmapDoesNotShareMappingsWithTheUsedCMap|the used CMap is not mutated|
> |testUseCmapOwnRangeBeatsInheritedChar|own cidrange beats an inherited
> cidchar|
> |testUseCmapOwnMappingToCidZeroIsNotAFallthrough|CID 0 is a mapping, not a
> miss|
> `mvn -pl fontbox -am test` is green (214 tests)
> *toCID sweep over the bundled predefined CMaps*
> All 92 bundled predefined CMaps were swept with both `toCID` overloads and
> the length guessing one: the 1 and 2 byte code spaces exhaustively (0x00-0xFF
> and 0x0000-0xFFFF), the 3 byte boundaries, and for 4 byte codes every code
> covered by a declared `cidrange`/`cidchar` plus a margin either side of each
> bound, plus a strided sample of the whole 32 bit space (99,434 probes). That
> is 2,889,588 non-zero results per build.
> Each result was checked against an oracle built independently of the patched
> code, by reading the CMap resource files directly and resolving each
> `usecmap` chain nearest-wins (a CMap's own `cidchar`/`cidrange` shadowing
> what it uses, `cidchar` ahead of `cidrange`, earlier range first):
> ||Build||non-zero results||disagreeing with the oracle||
> |trunk (before the fix)|2,889,588|2,714, across exactly the 31 CMaps named
> under Scope|
> | with the fix|2,889,588|0|
> So the fix does not merely change behaviour, it makes `toCID` agree with what
> the CMap files declare on every code swept, and it changes nothing on the
> other 61 CMaps. All 2,714 corrected lookups resolve to the nearest
> declaration in the CMap's own `usecmap` chain.
> The CID 0 sentinel change is behaviour preserving for the bundled set: the
> sweeps taken with and without it are byte identical. It is there because a
> CMap *may* legitimately map a code to CID 0, and the tests cover that case
> directly.
> Patch / PR attached.
>
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
