Skip to content

PDFBOX-6251 usecmap cidrange precedence #515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pcorless wants to merge 2 commits into
base: trunk
Choose a base branch
from
+198 −24
Open

PDFBOX-6251 usecmap cidrange precedence #515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
72522a5
PDFBOX-6251: Keep a CMap's own cid mappings ahead of the inherited ones
pcorless Sep 1, 2026
f9ead3a
PDFBOX-6251: Cover the remaining usecmap cid lookup paths
pcorless Sep 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 55 additions & 24 deletions fontbox/src/main/java/org/apache/fontbox/cmap/CMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ public class CMap
private final Map<Integer, Map<Integer, Integer>> codeToCid = new HashMap<>();
private final List<CIDRange> codeToCidRanges = new ArrayList<>();

// CID mappings inherited through usecmap, kept apart from this CMap's own mappings because they
// rank below them: a CMap that uses another may redefine any of the codes it inherits, and those
// redefinitions have to win. Merging the two would make the outcome depend on parse order, and
// usecmap is read from the header before the CMap's own mappings, so the inherited ones would
// always be found first.
private final Map<Integer, Map<Integer, Integer>> inheritedCodeToCid = new HashMap<>();
private final List<CIDRange> inheritedCodeToCidRanges = new ArrayList<>();

// inverted map
private final Map <String, byte[]> unicodeToByteCodes = new HashMap<>();

Expand All @@ -85,7 +93,8 @@ public class CMap
*/
public boolean hasCIDMappings()
{
return !codeToCid.isEmpty() || !codeToCidRanges.isEmpty();
return !codeToCid.isEmpty() || !codeToCidRanges.isEmpty()
|| !inheritedCodeToCid.isEmpty() || !inheritedCodeToCidRanges.isEmpty();
}

/**
Expand Down Expand Up @@ -241,13 +250,19 @@ public int toCID(byte[] code)
{
return 0;
}
Integer cid = null;
Map<Integer, Integer> codeToCidMap = codeToCid.get(code.length);
if (codeToCidMap != null)
Integer cid = lookup(codeToCid, code.length, toInt(code));
if (cid != null)
{
return cid;
}
int fromRanges = toCIDFromRanges(codeToCidRanges, code);
if (fromRanges != 0)
{
cid = codeToCidMap.get(toInt(code));
return fromRanges;
}
return cid != null ? cid : toCIDFromRanges(code);
// nothing of this CMap's own matched, fall back to whatever it inherited
cid = lookup(inheritedCodeToCid, code.length, toInt(code));
return cid != null ? cid : toCIDFromRanges(inheritedCodeToCidRanges, code);
}

/**
Expand Down Expand Up @@ -290,13 +305,19 @@ public int toCID(int code, int length)
{
return 0;
}
Integer cid = null;
Map<Integer, Integer> codeToCidMap = codeToCid.get(length);
if (codeToCidMap != null)
Integer cid = lookup(codeToCid, length, code);
if (cid != null)
{
return cid;
}
int fromRanges = toCIDFromRanges(codeToCidRanges, code, length);
if (fromRanges != 0)
{
cid = codeToCidMap.get(code);
return fromRanges;
}
return cid != null ? cid : toCIDFromRanges(code, length);
// nothing of this CMap's own matched, fall back to whatever it inherited
cid = lookup(inheritedCodeToCid, length, code);
return cid != null ? cid : toCIDFromRanges(inheritedCodeToCidRanges, code, length);
}

/**
Expand All @@ -306,9 +327,15 @@ public int toCID(int code, int length)
* @return CID
*/

private int toCIDFromRanges(int code, int length)
private static Integer lookup(Map<Integer, Map<Integer, Integer>> mappings, int length, int code)
{
for (CIDRange range : codeToCidRanges)
Map<Integer, Integer> codeToCidMap = mappings.get(length);
return codeToCidMap != null ? codeToCidMap.get(code) : null;
}

private static int toCIDFromRanges(List<CIDRange> cidRanges, int code, int length)
{
for (CIDRange range : cidRanges)
{
int ch = range.map(code, length);
if (ch != -1)
Expand All @@ -326,9 +353,9 @@ private int toCIDFromRanges(int code, int length)
* @return CID
*/

private int toCIDFromRanges(byte[] code)
private static int toCIDFromRanges(List<CIDRange> cidRanges, byte[] code)
{
for (CIDRange range : codeToCidRanges)
for (CIDRange range : cidRanges)
{
int ch = range.map(code);
if (ch != -1)
Expand Down Expand Up @@ -476,15 +503,19 @@ void useCmap(CMap cmap)
}
unicodeToByteCodes.put(v, bar);
});
cmap.codeToCid.forEach((key, value) ->
{
Map<Integer, Integer> existingMapping = codeToCid.putIfAbsent(key, value);
if (existingMapping!=null)
{
existingMapping.putAll(value);
}
});
codeToCidRanges.addAll(cmap.codeToCidRanges);
// CID mappings are inherited, not merged: they rank below anything this CMap defines itself
// (see inheritedCodeToCid). The copy is deliberate - sharing the other CMap's collections
// would let a later addCIDMapping here mutate a cached predefined CMap.
// A usecmap chain stays nearest-first, which needs opposite orderings for the two structures:
// the maps are consulted by key so putAll is last-wins, and the other CMap's own mappings go
// in after the ones it inherited; the range list is scanned first-match, so the other CMap's
// own ranges go in ahead of the ones it inherited.
cmap.inheritedCodeToCid.forEach((length, mappings) ->
inheritedCodeToCid.computeIfAbsent(length, k -> new HashMap<>()).putAll(mappings));
cmap.codeToCid.forEach((length, mappings) ->
inheritedCodeToCid.computeIfAbsent(length, k -> new HashMap<>()).putAll(mappings));
inheritedCodeToCidRanges.addAll(cmap.codeToCidRanges);
inheritedCodeToCidRanges.addAll(cmap.inheritedCodeToCidRanges);
maxCodeLength = Math.max(maxCodeLength, cmap.maxCodeLength);
minCodeLength = Math.min(minCodeLength, cmap.minCodeLength);
maxCidLength = Math.max(maxCidLength, cmap.maxCidLength);
Expand Down
143 changes: 143 additions & 0 deletions fontbox/src/test/java/org/apache/fontbox/cmap/TestCMapParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,4 +261,147 @@ void testBadIncrement() throws IOException
CMap cmap = parser.parse(new RandomAccessReadBuffer(cmapData));
assertNotNull(cmap);
}

/**
* A CMap that redefines a code it inherits through usecmap must win over the CMap it uses.
*
* ETenms-B5-H exists only to do that: it uses ETen-B5-H and then remaps 0x20-0x7E to the
* proportional latin CIDs 1-95, where the parent maps them to the full width forms at 13648+.
*/
@Test
void testUseCmapOwnMappingsWin() throws IOException
{
CMap parent = new CMapParser().parsePredefined("ETen-B5-H");
assertEquals(13681, parent.toCID(0x41, 1), "ETen-B5-H maps 0x41 to the full width form");

CMap cMap = new CMapParser().parsePredefined("ETenms-B5-H");
assertEquals(34, cMap.toCID(0x41, 1), "ETenms-B5-H overrides 0x41 to the proportional form");
assertEquals(1, cMap.toCID(0x20, 1), "ETenms-B5-H overrides 0x20 to the proportional form");

// codes the CMap does not redefine still come from the one it uses
assertEquals(parent.toCID(new byte[] { (byte) 0xA1, 0x40 }),
cMap.toCID(new byte[] { (byte) 0xA1, 0x40 }),
"an inherited code is unaffected");

// the byte[] overload repeats the lookup order of the int one, so check the override there too
assertEquals(13681, parent.toCID(new byte[] { 0x41 }));
assertEquals(34, cMap.toCID(new byte[] { 0x41 }),
"the byte[] overload has to prefer the CMap's own mapping as well");

// UniJIS-UCS2-HW-H likewise overrides its parent's proportional latin with the half width forms
CMap halfWidth = new CMapParser().parsePredefined("UniJIS-UCS2-HW-H");
assertEquals(34, new CMapParser().parsePredefined("UniJIS-UCS2-H").toCID(0x41, 2),
"UniJIS-UCS2-H maps 0x0041 to the proportional form");
assertEquals(264, halfWidth.toCID(0x41, 2),
"UniJIS-UCS2-HW-H overrides 0x0041 to the half width form");
}

/**
* The override has to survive a chain of usecmap: ETenms-B5-V uses ETenms-B5-H, which in turn
* uses ETen-B5-H. A code that only the middle CMap redefines has to keep that redefinition.
*/
@Test
void testUseCmapChainKeepsNearestMapping() throws IOException
{
CMap cMap = new CMapParser().parsePredefined("ETenms-B5-V");

assertEquals(1, cMap.getWMode(), "ETenms-B5-V is vertical");
assertEquals(34, cMap.toCID(0x41, 1),
"ETenms-B5-V inherits the proportional override from ETenms-B5-H, not ETen-B5-H");
}

/**
* Both kinds of mapping a CMap declares have to beat the ranges it inherits. ETenms-B5-V
* declares six cidchars and twelve cidranges for the punctuation whose vertical form differs,
* on top of the horizontal forms it inherits from ETenms-B5-H and ETen-B5-H.
*
* The cidchars were already resolved correctly before the inherited mappings were separated out,
* a cidchar being consulted ahead of any range either way, so they are here as a guard rather
* than as a second reproducer.
*/
@Test
void testUseCmapOwnMappingsBeatInheritedRanges() throws IOException
{
CMap horizontal = new CMapParser().parsePredefined("ETenms-B5-H");
CMap vertical = new CMapParser().parsePredefined("ETenms-B5-V");

// the horizontal forms come from an inherited range in both CMaps
assertEquals(110, horizontal.toCID(0xA14B, 2));
assertEquals(111, horizontal.toCID(0xA14C, 2));
assertEquals(121, horizontal.toCID(0xA156, 2));

// ETenms-B5-V's own cidchars replace them with the vertical forms
assertEquals(13646, vertical.toCID(0xA14B, 2), "own cidchar has to beat the inherited range");
assertEquals(109, vertical.toCID(0xA14C, 2), "own cidchar has to beat the inherited range");
assertEquals(312, vertical.toCID(0xA156, 2), "own cidchar has to beat the inherited range");

// and its own cidranges likewise, two usecmap levels down
assertEquals(128, horizontal.toCID(0xA15D, 2));
assertEquals(130, vertical.toCID(0xA15D, 2), "own cidrange has to beat the inherited range");
}

/**
* Identity-V is the one predefined CMap that declares no cid mappings at all, it only uses
* Identity-H. Every lookup it answers is therefore an inherited one, which also makes it the
* case that proves hasCIDMappings has to account for what a CMap inherited.
*/
@Test
void testUseCmapOnlyInheritedMappings() throws IOException
{
CMap cMap = new CMapParser().parsePredefined("Identity-V");

assertEquals(1, cMap.getWMode(), "Identity-V is vertical");
assertTrue(cMap.hasCIDMappings(), "Identity-V has cid mappings, all of them inherited");

assertEquals(65, cMap.toCID(new byte[] { 0, 65 }), "Identity-V CID 65");
assertEquals(12345, cMap.toCID(new byte[] { 0x30, 0x39 }), "Identity-V CID 12345");
assertEquals(0xFFFF, cMap.toCID(new byte[] { (byte) 0xFF, (byte) 0xFF }),
"Identity-V CID 0xFFFF");
assertEquals(12345, cMap.toCID(0x3039, 2), "Identity-V CID 12345");
}

/**
* Predefined CMaps are cached and handed out repeatedly, so what a CMap inherits has to be a
* copy: adding a mapping to the importing CMap must not reach back into the one it used.
*/
@Test
void testUseCmapDoesNotShareMappingsWithTheUsedCMap() throws IOException
{
CMap used = new CMap();
used.addCIDMapping(new byte[] { 0x41 }, 100);
used.addCIDRange(new byte[] { 0x50 }, new byte[] { 0x5F }, 200);

CMap cMap = new CMap();
cMap.useCmap(used);
assertEquals(100, cMap.toCID(0x41, 1), "the mapping is inherited");
assertEquals(205, cMap.toCID(0x55, 1), "the range is inherited");

cMap.addCIDMapping(new byte[] { 0x41 }, 300);
cMap.addCIDRange(new byte[] { 0x50 }, new byte[] { 0x5F }, 400);

assertEquals(300, cMap.toCID(0x41, 1), "the CMap's own mapping wins");
assertEquals(405, cMap.toCID(0x55, 1), "the CMap's own range wins");
assertEquals(100, used.toCID(0x41, 1), "the used CMap must not have been modified");
assertEquals(205, used.toCID(0x55, 1), "the used CMap must not have been modified");
}

/**
* Everything a CMap declares outranks everything it inherits, so a cidrange of its own has to
* beat an inherited cidchar too, not just an inherited cidrange. No predefined CMap pairs the
* two that way round, hence the hand built pair here.
*/
@Test
void testUseCmapOwnRangeBeatsInheritedChar() throws IOException
{
CMap used = new CMap();
used.addCIDMapping(new byte[] { 0x41 }, 100);

CMap cMap = new CMap();
cMap.useCmap(used);
cMap.addCIDRange(new byte[] { 0x40 }, new byte[] { 0x4F }, 200);

assertEquals(201, cMap.toCID(0x41, 1), "the CMap's own range has to beat the inherited char");
assertEquals(200, cMap.toCID(0x40, 1), "a code the used CMap says nothing about");
assertEquals(100, used.toCID(0x41, 1), "the used CMap must not have been modified");
}
}