search

khaledhosny / ots

Sanitizer for OpenType
BSD 3-Clause "New" or "Revised" License
266 stars 64 forks source link

[`CFF2`] Lift `uint16 VariationStore.length` limitation #290

Open behdad opened 1 month ago

behdad commented 1 month ago

See this for details: https://github.com/harfbuzz/boring-expansion-spec/issues/159

I suggest we implement that already. That is, if the VariationStore struct's length field is 65,535, then allow the embedded ItemVariationStore to reach out to the rest of the font file.

@justvanrossum has been building a font that hit that limit.

We are going to also implement this change in FontTools. HarfBuzz already renders such fonts fine, and I will test FreeType and report.

khaledhosny commented 1 month ago

What does Microsoft and Apple rasterizer do with such fonts? We wouldn’t usually allow fonts they can’t handle, and specially if they don’t fail gracefully. cc @jfkthame

jfkthame commented 1 month ago

What does Microsoft and Apple rasterizer do with such fonts? We wouldn’t usually allow fonts they can’t handle, and specially if they don’t fail gracefully. cc @jfkthame

Yeah, that's the same question I was going to ask. We don't want OTS accepting fonts that will then cause issues in either DirectWrite or Core Text.

Are there some examples of such fonts that folks could test with the various rasterizers?

behdad commented 1 month ago

What does Microsoft and Apple rasterizer do with such fonts? We wouldn’t usually allow fonts they can’t handle, and specially if they don’t fail gracefully. cc @jfkthame

Yeah, that's the same question I was going to ask. We don't want OTS accepting fonts that will then cause issues in either DirectWrite or Core Text.

I can install the font in on the Mac just fine, and renders fine. I cannot test on Windows.

Are there some examples of such fonts that folks could test with the various rasterizers?

I have a test font from @justvanrossum that is proprietary. I can get it sorted to share with you or @khaledhosny if you can test on Windows.

jfkthame commented 1 month ago

Windows 10 refuses to install the font; it tells me it's "not a valid font file".

I was able to load it as a webfont resource in Firefox after hacking the OTS code to skip validation of the CFF2 table, and then it does seem to render the glyphs as expected (including weight variations), though I don't know how to test whether it would be able to use the whole of the VariationStore, or only whatever's in the first 64K.

(Just patching ParseVariationStore wasn't sufficient to get through OTS, as the CFF2 table was also rejected for invalid CharStrings; I had to make it skip CFF2 validation altogether.)

behdad commented 1 month ago

(Just patching ParseVariationStore wasn't sufficient to get through OTS, as the CFF2 table was also rejected for invalid CharStrings; I had to make it skip CFF2 validation altogether.)

Yes, there seems to be an stack overflow with the test font. I'm investigating that. If I succeed we can test again to see if it installs.

behdad commented 4 weeks ago

Windows 10 refuses to install the font; it tells me it's "not a valid font file".

I was able to load it as a webfont resource in Firefox after hacking the OTS code to skip validation of the CFF2 table, and then it does seem to render the glyphs as expected (including weight variations), though I don't know how to test whether it would be able to use the whole of the VariationStore, or only whatever's in the first 64K.

I sent you a version of the font that doesn't have the stack overflow. Can you test if that one installs?

So, based on your testing on Windows, do you think we can land this change?

jfkthame commented 2 weeks ago

Windows still refuses to install the font, insisting that it is "not a valid font file".

So while we'd be able to use such a file as a webfont (with the OTS change), the fact that it can't be installed in Windows still makes me hesitant to move forward. If OTS accepts a font without reporting any errors, users should be able to expect that font to work in all major current environments, and that surely includes Windows.

behdad commented 2 weeks ago

This change is on its way to ISO OFF and then OpenType. ISTM that we need different profiles in OTS soon.

jfkthame commented 2 weeks ago

Yes, I wondered if something like that might be the answer. So in browsers that can handle the font, we could configure it to allow this, but a standalone OTS validation tool could... reject it? Issue a warning? Provide an option so the user can choose the level of support?