Jason Harrop created FOP-3328:
---------------------------------
Summary: [PATCH] GPOS: Anchor Table format 3 device tables are
read from the wrong offset
Key: FOP-3328
URL: https://issues.apache.org/jira/browse/FOP-3328
Project: FOP
Issue Type: Bug
Components: font/opentype
Affects Versions: 2.11
Reporter: Jason Harrop
Attachments: FOP-anchor-device-table.patch
There are two defects in {{OTFAdvancedTypographicTableReader}}, one of which
makes FOP misparse the GPOS table of any variable font.
h3. 1. Anchor device table offsets use the wrong base (root cause)
{{readPosAnchor(long anchorTableOffset)}} saves the reader's position on entry:
{code:java}
long cp = in.getCurrentPos();
in.seekSet(anchorTableOffset);
...
xd = readPosDeviceTable(cp, xdo);
yd = readPosDeviceTable(cp, ydo);
{code}
Per the OpenType spec, an Anchor Table format 3's {{xDeviceOffset}} /
{{yDeviceOffset}} are measured "from beginning of Anchor table" - so the base
must be {{anchorTableOffset}}, not {{cp}}. As written, the reader seeks into an
unrelated part of the font and parses whatever bytes it finds as a device table.
This has gone unnoticed because anchor device tables are rare in non-variable
fonts. In a variable font they are VariationIndex tables and are everywhere, so
the bad path is taken constantly.
h3. 2. A delta count of zero slips past the guard
{{readPosDeviceTable}} rejects a negative delta count:
{code:java}
int n = (es - ss) + 1;
if (n < 0) {
log.debug("invalid device table delta count: " + n + ", ignoring device
table");
return null;
}
{code}
but {{n == 0}} (ie {{es == ss - 1}}) passes, and then trips
{{GlyphPositioningTable.DeviceTable}}'s {{assert startSize <= endSize}}. So with
assertions enabled the misparse in (1) turns from a silently-ignored bad read
into an {{AssertionError}} that aborts font loading.
h3. Reproducing
Load {{NotoSans\[wght\].ttf}} - Fedora's {{google-noto-sans-vf-fonts}} package,
also available from Google Fonts - through
{{org.apache.fop.fonts.autodetect.FontInfoFinder}}, with
{{org.apache.fop.complexscripts}} at DEBUG. FOP 2.11 logs 22,700 device table
reads for that one font, thousands of them rejected as garbage:
{noformat}
DEBUG OTFAdvancedTypographicTableReader - invalid device table delta count:
-101, ignoring device table
DEBUG OTFAdvancedTypographicTableReader - invalid device table delta count:
-107, ignoring device table
DEBUG OTFAdvancedTypographicTableReader - invalid device table delta count:
-113, ignoring device table
{noformat}
Those counts are nonsense because the offsets are wrong. With the base
corrected,
the same tables are recognised as VariationIndex (delta format 0x8000) and are
cleanly ignored by the existing "unsupported device table delta format" branch.
To be precise about what I did and did not reproduce: on these particular fonts,
stock FOP 2.11 has every bad read caught by the {{n < 0}} guard, so no
{{AssertionError}} is thrown - the garbage reads are the observable symptom. The
identical code in a downstream fork (docx4j, which repackages this class) does
hit {{n == 0}} on the same font file and throws. Originally reported there as
https://github.com/plutext/docx4j/issues/686, by a user on Fedora with
{{-ea}} enabled:
{noformat}
java.lang.AssertionError
at
...GlyphPositioningTable$DeviceTable.<init>(GlyphPositioningTable.java:1778)
at ...OTFAdvancedTypographicTableReader.readPosDeviceTable(...:1732)
at ...OTFAdvancedTypographicTableReader.readPosAnchor(...:2063)
at
...OTFAdvancedTypographicTableReader.readMarkToLigaturePosTableFormat1(...:2350)
at ...OTFAdvancedTypographicTableReader.readGPOSSubtable(...:3127)
at ...OTFAdvancedTypographicTableReader.readAll(...:89)
at ...OpenFont.handleCharacterSpacing(OpenFont.java:902)
at ...OFFontLoader.read(OFFontLoader.java:123)
at ...FontLoader.getFont(FontLoader.java:130)
at ...FontInfoFinder.find(FontInfoFinder.java:268)
{noformat}
(package names differ in the fork; the code at those lines is yours, unmodified)
Whether a given font trips the assertion or merely reads garbage depends on what
happens to be at the wrong offset, so I would treat (1) as the defect to fix and
(2) as hardening.
h3. Effect
* With {{-ea}}: {{AssertionError}} during font loading. In the fork's case this
aborted discovery of every remaining font, not just the offending one.
* Without {{-ea}}: any bogus device table that survives the guard feeds wrong
adjustments into glyph positioning.
h3. Patch
Attached: {{FOP-anchor-device-table.patch}}. It applies cleanly to trunk and to
the {{fop-2_11}} tag - the file is byte-identical between them.
I verified in the downstream fork that the fix does not disturb ordinary fonts:
font discovery over a machine with 1242 system fonts produces byte-identical
results before and after, and the five Noto variable fonts that previously
failed
now load. Longest added line is 96 characters, so checkstyle should be happy.
Happy to raise this as a GitHub PR instead, or as well, if that is easier for
you.
Contribution is offered under the Apache License 2.0.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
