Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to recover valid format 3 FDSelect data from broken CFF fonts (bug 1146106) #7361

Merged
merged 1 commit into from
Jun 7, 2016
Merged

Attempt to recover valid format 3 FDSelect data from broken CFF fonts (bug 1146106) #7361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions src/core/cff_parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -856,20 +856,27 @@ var CFFParser = (function CFFParserClosure() {
var start = pos;
var bytes = this.bytes;
var format = bytes[pos++];
var fdSelect = [];
var i;
var fdSelect = [], rawBytes;
var i, invalidFirstGID = false;

switch (format) {
case 0:
for (i = 0; i < length; ++i) {
var id = bytes[pos++];
fdSelect.push(id);
}
rawBytes = bytes.subarray(start, pos);
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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do the simpler approach for now so it's easier to follow. Hopefully we won't have to touch this code again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me; I've pushed an updated patch.

warn('parseFDSelect: The first range must have a first GID of 0' +
' -- trying to recover.');
invalidFirstGID = true;
first = 0;
}
var fdIndex = bytes[pos++];
var next = (bytes[pos] << 8) | bytes[pos + 1];
for (var j = first; j < next; ++j) {
Expand All @@ -878,13 +885,19 @@ var CFFParser = (function CFFParserClosure() {
}
// Advance past the sentinel(next).
pos += 2;
rawBytes = bytes.subarray(start, pos);

if (invalidFirstGID) {
rawBytes[3] = rawBytes[4] = 0; // Adjust the first range, first GID.
}
break;
default:
error('Unknown fdselect format ' + format);
error('parseFDSelect: Unknown format "' + format + '".');
break;
}
var end = pos;
return new CFFFDSelect(fdSelect, bytes.subarray(start, end));
assert(fdSelect.length === length, 'parseFDSelect: Invalid font data.');

return new CFFFDSelect(fdSelect, rawBytes);
}
};
return CFFParser;
Expand Down
1 change: 1 addition & 0 deletions test/pdfs/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
!bug1050040.pdf
!bug1200096.pdf
!bug1068432.pdf
!bug1146106.pdf
!issue5564_reduced.pdf
!canvas.pdf
!bug1132849.pdf
Expand Down
Binary file added test/pdfs/bug1146106.pdf
View file
Open in desktop
Binary file not shown.
7 changes: 7 additions & 0 deletions test/test_manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,13 @@
"link": false,
"type": "eq"
},
{ "id": "bug1146106",
"file": "pdfs/bug1146106.pdf",
"md5": "a323d3766da49ee40f7d5dff0aeb0cc1",
"rounds": 1,
"link": false,
"type": "eq"
},
{ "id": "issue1512",
"file": "pdfs/issue1512r.pdf",
"md5": "af48ede2658d99cca423147085c6609b",
Expand Down
35 changes: 28 additions & 7 deletions test/unit/cff_parser_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,24 +262,45 @@ describe('CFFParser', function() {
var bytes = new Uint8Array([0x00, // format
0x00, // gid: 0 fd: 0
0x01 // gid: 1 fd: 1
]);
parser.bytes = bytes;
]);
parser.bytes = bytes.slice();
var fdSelect = parser.parseFDSelect(0, 2);

expect(fdSelect.fdSelect).toEqual([0, 1]);
expect(fdSelect.raw).toEqual(bytes);
});

it('parses fdselect format 3', function() {
var bytes = new Uint8Array([0x03, // format
0x00, 0x02, // range count
0x00, 0x00, // first gid
0x09, // font dict 1 id
0x00, 0x02, // nex gid
0x0a, // font dict 2 gid
0x00, 0x02, // next gid
0x0a, // font dict 2 id
0x00, 0x04 // sentinel (last gid)
]);
parser.bytes = bytes;
var fdSelect = parser.parseFDSelect(0, 2);
]);
parser.bytes = bytes.slice();
var fdSelect = parser.parseFDSelect(0, 4);

expect(fdSelect.fdSelect).toEqual([9, 9, 0xa, 0xa]);
expect(fdSelect.raw).toEqual(bytes);
});

it('parses invalid fdselect format 3 (bug 1146106)', function() {
var bytes = new Uint8Array([0x03, // format
0x00, 0x02, // range count
0x00, 0x01, // first gid (invalid)
0x09, // font dict 1 id
0x00, 0x02, // next gid
0x0a, // font dict 2 id
0x00, 0x04 // sentinel (last gid)
]);
parser.bytes = bytes.slice();
var fdSelect = parser.parseFDSelect(0, 4);

expect(fdSelect.fdSelect).toEqual([9, 9, 0xa, 0xa]);
bytes[3] = bytes[4] = 0x00; // The adjusted first range, first gid.
expect(fdSelect.raw).toEqual(bytes);
});

// TODO fdArray
Expand Down