534352 - fonts with extra sentinel value in cmap table refuse to load

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Jonathan Kew [:jfkthame]]

In TrueType fonts, the list of character ranges in a format-4 'cmap' subtable is supposed to end with a "sentinel" entry for [0xFFFF,0xFFFF], presumably so that code can rely on this to terminate a search.

However, it appears that some font-creation tools may have appended an extra, redundant [0xFFFF,0xFFFF] entry to tables that already included it. An example is the font in https://bugzilla.mozilla.org/attachment.cgi?id=408927 (see bug 496573 comment #25). This causes our cmap validation at http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/src/gfxFontUtils.cpp#322 to reject the font, although this particular anomaly would be harmless.

If we relax our validation here, the example font will then load correctly.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: patch v1: relax validation to permit redundant sentinel value; includes reftest

With this patch, which allows a redundant cmap4 range starting at 0xFFFF, the Prototype.ttf font loads as expected. Although this is strictly speaking a font error, this particular anomaly seems safe enough that we can allow the font to be used.

Comment 2

[John Daggett (:jtd)]

Actually, I regret having added all the persnickety cmap ordering checks, they cause problems with a number of fonts that work just fine otherwise.  We definitely want to keep the out-of-Unicode range check, but maybe we should get rid of these ordering checks altogether?  Bug 526869 is another example of this type of problem.

Comment 3

[Jonathan Kew [:jfkthame]]

Pushed http://hg.mozilla.org/mozilla-central/rev/1f0e04dc2b21

Comment 4

[Jonathan Kew [:jfkthame]]

(In reply to comment #2)
> Actually, I regret having added all the persnickety cmap ordering checks, they
> cause problems with a number of fonts that work just fine otherwise.  We
> definitely want to keep the out-of-Unicode range check, but maybe we should get
> rid of these ordering checks altogether?  Bug 526869 is another example of this
> type of problem.

I'd be reluctant to drop them completely, unless we see that there are in fact lots of cases where they are causing problems.

As a general rule, I think we should write to the standards, and encourage others to conform to them. Then, we can consider individual cases where we may choose to relax our criteria, where our judgment is that there is a significant benefit and a sufficiently low risk (not just in our own code, but also in the system APIs we'll be passing out-of-spec data on to).

In this example, ISTM that a redundant (duplicated) terminator looks pretty safe, when thinking about how code is likely to use the cmap tables. Allowing overlapping ranges elsewhere is going a step further; and then what about general disordered entries? Might that lead to problems for cmap-interpreting code that assumes correct ordering? I'd want to think pretty carefully about the possible implications before abandoning the checks entirely.