[
https://issues.apache.org/jira/browse/FOP-3328?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jason Harrop updated FOP-3328:
------------------------------
Attachment: FOP-3328-variable-font.fo
noto-variable.xconf
logging.properties
> [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
> Priority: Minor
> Attachments: FOP-3328-variable-font.fo,
> FOP-anchor-device-table.patch, logging.properties, noto-variable.xconf
>
> Original Estimate: 20m
> Remaining Estimate: 20m
>
> 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)
