Attempt to recover valid format 3 FDSelect data from broken CFF fonts (bug 1146106)
#7361
Conversation
Snuffleupagus
added
font-conversion
4-work-in-progress
font-type1
font-type0
and removed
font-type1
labels
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/23fcd2249cbb6ae/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/1d6f8a46b60ce37/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/23fcd2249cbb6ae/output.txt Total script time: 20.91 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/1d6f8a46b60ce37/output.txt Total script time: 27.66 mins
|
| break; | ||
| case 3: | ||
| var rangesCount = (bytes[pos++] << 8) | bytes[pos++]; | ||
| for (i = 0; i < rangesCount; ++i) { | ||
| var first = (bytes[pos++] << 8) | bytes[pos++]; | ||
| if (i === 0 && first !== 0) { |
There was a problem hiding this comment.
Would it be possible to just set the invalid flag and first = 0 here, then just fix up the rawbytes below instead of re-calling parseFDSelect?
There was a problem hiding this comment.
Sure, we can do that. It's actually what I did in the initial patch, but then I got a bit paranoid that if we encounter sufficiently weird FDSelect data, that it could unnecessarily breaking things :-)
E.g. what happens if the first range is wrong, but the rest of the FDSelect data is weird enough that the total length becomes correct anyway?
However, given that I've not seen that happen, it probably just a theoretical concern. So, if you want me to, I'll remove the recoveryMode and just do the adjustment directly!
There was a problem hiding this comment.
Let's do the simpler approach for now so it's easier to follow. Hopefully we won't have to touch this code again.
There was a problem hiding this comment.
Works for me; I've pushed an updated patch.
…ts (bug 1146106) According to the CFF specification, see http://partners.adobe.com/public/developer/en/font/5176.CFF.pdf#G3.46884, for `format 3` FDSelect data: "The first range must have a ‘first’ GID of 0". Since the PDF file (attached in the bug) violates that part of the specification, this patch tries to recover valid FDSelect data to prevent OTS from rejecting the font. Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1146106.
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @brendandahl received. Current queue size: 0 Live output at: http://107.22.172.223:8877/e8a6fc2eec6d392/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_test from @brendandahl received. Current queue size: 0 Live output at: http://107.21.233.14:8877/6942ead328e25b8/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/e8a6fc2eec6d392/output.txt Total script time: 21.28 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/6942ead328e25b8/output.txt Total script time: 27.45 mins
|
|
/botio makeref |
From: Bot.io (Windows)ReceivedCommand cmd_makeref from @brendandahl received. Current queue size: 0 Live output at: http://107.22.172.223:8877/3cf83f276c26687/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_makeref from @brendandahl received. Current queue size: 0 Live output at: http://107.21.233.14:8877/38fad8f1b757dc2/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/3cf83f276c26687/output.txt Total script time: 21.24 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/38fad8f1b757dc2/output.txt Total script time: 26.06 mins
|
|
Thanks! |
According to the CFF specification, see http://partners.adobe.com/public/developer/en/font/5176.CFF.pdf#G3.46884, for
format 3FDSelect data: "The first range must have a ‘first’ GID of 0".Since the PDF file (attached in the bug) violates that part of the specification, this patch tries to recover valid FDSelect data to prevent OTS from rejecting the font.
Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1146106.
r? @brendandahl
Edit: Compared to the first version of this patch, the latest version contains a number of improvements, hence I'm updating it from
f?tor?.Please also note that this patch, as far as I know, fixes the only currently remaining font regression.