Parse Type1 font files to determine the various `Length{n}` properties, instead of trusting the PDF file (issue 5686, issue 3928) #7066

Snuffleupagus · 2016-03-05T21:51:56Z

Make Type1Font more class-like, by adding closure

Note: Ignoring whitespace should simplify reviewing a great deal.
Snuffleupagus@a7978cc?w=1
Parse Type1 font files to determine the various Length{n} properties, instead of trusting the PDF file (issue 5686, issue 3928)

Please note: In order to save time, I've not yet included any reduced test-cases in this patch. I'm happy to include tests, but I'd like the patch to be reviewed first, so that I don't waste time creating reduced test-cases for a patch before knowing if it's acceptable.

Fixes Squares instead of spaces #5686.
Fixes Stops displaying the document, loops with 100% cpu power #3928.

Snuffleupagus · 2016-03-12T12:42:23Z

@brendandahl Can you please provide feedback on the general direction of this patch (so that I know if it's worthwhile to continue working on it)?

/botio-windows preview

brendandahl · 2016-03-14T22:24:14Z

@brendandahl Can you please provide feedback on the general direction of this patch (so that I know if it's worthwhile to continue working on it)?

General direction looks good. It may be worth looking at freetype within pdfium to see how they handle this case as well.

Snuffleupagus · 2016-03-15T22:12:40Z

It may be worth looking at freetype within pdfium to see how they handle this case as well.

Disclamer: My knowledge of C isn't what it should be, so I might be misunderstanding the code, but from a cursory look through the t1parse.c source code it appears that FreeType:

Parses the font file to locate the eexec string, to determine the length of the header block.
Parses the rest of the font file, i.e. the eexec block, not caring about a potential fixed-content block.

Again, I might not understand the FreeType code correctly! But if the above is approximately correct, it looks to me that this PR is doing something that is at least similar.

timvandermeij · 2016-03-16T20:10:31Z

/botio-linux preview

pdfjsbot · 2016-03-16T20:10:32Z

From: Bot.io (Linux)

Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://107.21.233.14:8877/697ac99d901da82/output.txt

pdfjsbot · 2016-03-16T20:11:29Z

From: Bot.io (Linux)

Success

Full output at http://107.21.233.14:8877/697ac99d901da82/output.txt

Total script time: 0.95 mins

Published

timvandermeij · 2016-03-16T20:11:40Z

/botio test

pdfjsbot · 2016-03-16T20:11:40Z

From: Bot.io (Windows)

Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://107.22.172.223:8877/b0ecdc4a095d159/output.txt

pdfjsbot · 2016-03-16T20:11:40Z

From: Bot.io (Linux)

Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://107.21.233.14:8877/58e31eff3d81724/output.txt

pdfjsbot · 2016-03-16T20:31:33Z

From: Bot.io (Windows)

Success

Full output at http://107.22.172.223:8877/b0ecdc4a095d159/output.txt

Total script time: 19.88 mins

Font tests: Passed
Unit tests: Passed
Regression tests: Passed

pdfjsbot · 2016-03-16T20:35:26Z

From: Bot.io (Linux)

Failed

Full output at http://107.21.233.14:8877/58e31eff3d81724/output.txt

Total script time: 23.76 mins

Font tests: Passed
Unit tests: Passed
Regression tests: FAILED

Image differences available at: http://107.21.233.14:8877/58e31eff3d81724/reftest-analyzer.html#web=eq.log

timvandermeij · 2016-03-16T22:24:18Z

/botio-linux test

pdfjsbot · 2016-03-16T22:24:19Z

From: Bot.io (Linux)

Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://107.21.233.14:8877/d12f941a2151932/output.txt

pdfjsbot · 2016-03-16T22:46:13Z

From: Bot.io (Linux)

Success

Full output at http://107.21.233.14:8877/d12f941a2151932/output.txt

Total script time: 21.89 mins

Font tests: Passed
Unit tests: Passed
Regression tests: Passed

brendandahl · 2016-03-30T18:52:15Z

src/core/fonts.js

+
+    var actualLength = 0;
+    while (true) {
+      var scanBytes = stream.peekBytes(headerBytesLength);


headerBytesLength is potentially undefined here, is that okay?

Yes, it's fine, since undefined just means that we'll get the entire stream. See the comment at https://github.com/mozilla/pdf.js/pull/7066/files#diff-d9f16d4d656cd2e962e0ad032dfba903R3681.

Actually, it seems kind of dumb to get the entire stream at once like that, since the header usually isn't that long compared to the total length of a Type1 font file.
So I've pushed a new version of the patch, that searches the stream in smaller chunks instead.

*Note:* Ignoring whitespace should simplify reviewing a great deal.

…s, instead of trusting the PDF file (issue 5686, issue 3928) Fixes 5686. Fixes 3928.

Snuffleupagus · 2016-03-31T10:54:45Z

/botio test

pdfjsbot · 2016-03-31T10:54:46Z

From: Bot.io (Linux)

Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.21.233.14:8877/ca23ca71aa97fc8/output.txt

pdfjsbot · 2016-03-31T10:54:46Z

From: Bot.io (Windows)

Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.22.172.223:8877/167e8522b655cb7/output.txt

pdfjsbot · 2016-03-31T11:14:40Z

From: Bot.io (Windows)

Success

Full output at http://107.22.172.223:8877/167e8522b655cb7/output.txt

Total script time: 19.91 mins

Font tests: Passed
Unit tests: Passed
Regression tests: Passed

pdfjsbot · 2016-03-31T11:16:38Z

From: Bot.io (Linux)

Success

Full output at http://107.21.233.14:8877/ca23ca71aa97fc8/output.txt

Total script time: 21.87 mins

Font tests: Passed
Unit tests: Passed
Regression tests: Passed

Snuffleupagus · 2016-04-01T08:42:02Z

/botio makeref

pdfjsbot · 2016-04-01T08:42:03Z

From: Bot.io (Linux)

Received

Command cmd_makeref from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.21.233.14:8877/5ca7ff768090e79/output.txt

pdfjsbot · 2016-04-01T08:42:03Z

From: Bot.io (Windows)

Received

Command cmd_makeref from @Snuffleupagus received. Current queue size: 0

Live output at: http://107.22.172.223:8877/2979a5d01841be9/output.txt

pdfjsbot · 2016-04-01T09:02:12Z

From: Bot.io (Windows)

Success

Full output at http://107.22.172.223:8877/2979a5d01841be9/output.txt

Total script time: 20.13 mins

Lint: Passed
Make references: Passed
Check references: Passed

pdfjsbot · 2016-04-01T09:03:36Z

From: Bot.io (Linux)

Success

Full output at http://107.21.233.14:8877/5ca7ff768090e79/output.txt

Total script time: 21.55 mins

Lint: Passed
Make references: Passed
Check references: Passed

Snuffleupagus added font-conversion font-type1 labels Mar 5, 2016

timvandermeij assigned brendandahl Mar 13, 2016

brendandahl reviewed Mar 30, 2016
View reviewed changes

Snuffleupagus added 2 commits March 31, 2016 11:00

Make Type1Font more class-like, by adding closure

c40df8a

*Note:* Ignoring whitespace should simplify reviewing a great deal.

Parse Type1 font files to determine the various Length{n} propertie…

05cf709

…s, instead of trusting the PDF file (issue 5686, issue 3928) Fixes 5686. Fixes 3928.

brendandahl merged commit 8910cea into mozilla:master Mar 31, 2016

Snuffleupagus deleted the Type1-headerBlockLength branch March 31, 2016 18:27

Snuffleupagus restored the Type1-headerBlockLength branch April 1, 2016 08:40

Snuffleupagus deleted the Type1-headerBlockLength branch April 1, 2016 10:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parse Type1 font files to determine the various `Length{n}` properties, instead of trusting the PDF file (issue 5686, issue 3928) #7066

Parse Type1 font files to determine the various `Length{n}` properties, instead of trusting the PDF file (issue 5686, issue 3928) #7066

Snuffleupagus commented Mar 5, 2016

Snuffleupagus commented Mar 12, 2016

brendandahl commented Mar 14, 2016

Snuffleupagus commented Mar 15, 2016

timvandermeij commented Mar 16, 2016

pdfjsbot commented Mar 16, 2016

pdfjsbot commented Mar 16, 2016

timvandermeij commented Mar 16, 2016

pdfjsbot commented Mar 16, 2016

pdfjsbot commented Mar 16, 2016

pdfjsbot commented Mar 16, 2016

pdfjsbot commented Mar 16, 2016

timvandermeij commented Mar 16, 2016

pdfjsbot commented Mar 16, 2016

pdfjsbot commented Mar 16, 2016

brendandahl Mar 30, 2016

Snuffleupagus Mar 30, 2016

Snuffleupagus Mar 31, 2016

Snuffleupagus commented Mar 31, 2016

pdfjsbot commented Mar 31, 2016

pdfjsbot commented Mar 31, 2016

pdfjsbot commented Mar 31, 2016

pdfjsbot commented Mar 31, 2016

Snuffleupagus commented Apr 1, 2016

pdfjsbot commented Apr 1, 2016

pdfjsbot commented Apr 1, 2016

pdfjsbot commented Apr 1, 2016

pdfjsbot commented Apr 1, 2016

Parse Type1 font files to determine the various Length{n} properties, instead of trusting the PDF file (issue 5686, issue 3928) #7066

Parse Type1 font files to determine the various Length{n} properties, instead of trusting the PDF file (issue 5686, issue 3928) #7066

Conversation

Snuffleupagus commented Mar 5, 2016

Snuffleupagus commented Mar 12, 2016

brendandahl commented Mar 14, 2016

Snuffleupagus commented Mar 15, 2016

timvandermeij commented Mar 16, 2016

pdfjsbot commented Mar 16, 2016

From: Bot.io (Linux)

Received

pdfjsbot commented Mar 16, 2016

From: Bot.io (Linux)

Success

Published

timvandermeij commented Mar 16, 2016

pdfjsbot commented Mar 16, 2016

From: Bot.io (Windows)

Received

pdfjsbot commented Mar 16, 2016

From: Bot.io (Linux)

Received

pdfjsbot commented Mar 16, 2016

From: Bot.io (Windows)

Success

pdfjsbot commented Mar 16, 2016

From: Bot.io (Linux)

Failed

timvandermeij commented Mar 16, 2016

pdfjsbot commented Mar 16, 2016

From: Bot.io (Linux)

Received

pdfjsbot commented Mar 16, 2016

From: Bot.io (Linux)

Success

brendandahl Mar 30, 2016

Choose a reason for hiding this comment

Snuffleupagus Mar 30, 2016

Choose a reason for hiding this comment

Snuffleupagus Mar 31, 2016

Choose a reason for hiding this comment

Snuffleupagus commented Mar 31, 2016

pdfjsbot commented Mar 31, 2016

From: Bot.io (Linux)

Received

pdfjsbot commented Mar 31, 2016

From: Bot.io (Windows)

Received

pdfjsbot commented Mar 31, 2016

From: Bot.io (Windows)

Success

pdfjsbot commented Mar 31, 2016

From: Bot.io (Linux)

Success

Snuffleupagus commented Apr 1, 2016

pdfjsbot commented Apr 1, 2016

From: Bot.io (Linux)

Received

pdfjsbot commented Apr 1, 2016

From: Bot.io (Windows)

Received

pdfjsbot commented Apr 1, 2016

From: Bot.io (Windows)

Success

pdfjsbot commented Apr 1, 2016

From: Bot.io (Linux)

Success

Parse Type1 font files to determine the various `Length{n}` properties, instead of trusting the PDF file (issue 5686, issue 3928) #7066

Parse Type1 font files to determine the various `Length{n}` properties, instead of trusting the PDF file (issue 5686, issue 3928) #7066