Fix searching for end of inline (EI) images with ASCII85Decode filters (bug 1077808) #5383

Snuffleupagus · 2014-10-04T22:25:03Z

Based on http://www.adobe.com/content/dam/Adobe/en/devnet/acrobat/pdfs/PDF32000_2008.pdf#page=33, this is a tentative patch to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1077808.

Snuffleupagus · 2014-10-04T22:27:18Z

/botio-linux preview

pdfjsbot · 2014-10-04T22:27:18Z

From: Bot.io (Linux)

Received

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

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

pdfjsbot · 2014-10-04T22:28:07Z

From: Bot.io (Linux)

Success

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

Total script time: 0.80 mins

Published

Viewer: http://107.21.233.14:8877/a716f99d5968d46/web/viewer.html
B2G Viewer: http://107.21.233.14:8877/a716f99d5968d46/extensions/b2g/content/web/viewer.html
Extension: http://107.21.233.14:8877/a716f99d5968d46/extensions/firefox/pdf.js.xpi
Extension (AMO): http://107.21.233.14:8877/a716f99d5968d46/extensions/firefox/pdf.js.amo.xpi

Snuffleupagus · 2014-10-04T22:28:58Z

/botio test

pdfjsbot · 2014-10-04T22:28:59Z

From: Bot.io (Windows)

Received

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

Live output at: http://107.22.172.223:8877/2dd802736ea69ac/output.txt

pdfjsbot · 2014-10-04T22:28:59Z

From: Bot.io (Linux)

Received

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

Live output at: http://107.21.233.14:8877/4bc0931aadc8d7e/output.txt

pdfjsbot · 2014-10-04T22:31:53Z

From: Bot.io (Windows)

Failed

Full output at http://107.22.172.223:8877/2dd802736ea69ac/output.txt

Total script time: 2.90 mins

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

Image differences available at: http://107.22.172.223:8877/2dd802736ea69ac/reftest-analyzer.html#web=eq.log

pdfjsbot · 2014-10-04T22:51:55Z

From: Bot.io (Linux)

Success

Full output at http://107.21.233.14:8877/4bc0931aadc8d7e/output.txt

Total script time: 22.94 mins

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

CodingFabian · 2014-10-22T19:04:31Z

src/core/parser.js

+        if (isName(filter)) {
+          filterNames = [filter.name];
+        } else if (isArray(filter)) {
+          for (i = 0, ii = filter.length; i < ii; ++i) {


not sure how long that list typically is. but i think that you could move this code to a separate function which directly checks every element and does a quick return if one is found. This should be shorter in code and execution time.

something like this:
https://gist.github.com/CodingFabian/0731224202f8447cb805

yurydelendik · 2014-11-11T14:40:02Z

Hmm, we had that at one point Snuffleupagus@c68dbd7#diff-6947033678b93d106e25614dd972e66fL123 . We need to verify why we need to revert that decision.

yurydelendik · 2014-11-11T14:43:20Z

#1246

yurydelendik · 2014-11-11T14:47:05Z

/botio-windows preview

pdfjsbot · 2014-11-11T14:47:06Z

From: Bot.io (Windows)

Received

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

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

pdfjsbot · 2014-11-11T14:48:05Z

From: Bot.io (Windows)

Success

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

Total script time: 0.98 mins

Published

Viewer: http://107.22.172.223:8877/e8072358c6db18e/web/viewer.html
B2G Viewer: http://107.22.172.223:8877/e8072358c6db18e/extensions/b2g/content/web/viewer.html
Extension: http://107.22.172.223:8877/e8072358c6db18e/extensions/firefox/pdf.js.xpi
Extension (AMO): http://107.22.172.223:8877/e8072358c6db18e/extensions/firefox/pdf.js.amo.xpi

yurydelendik · 2014-11-11T14:51:43Z

I verified. It regresses the ADP stuff. Can you add at the last "if" check condition to do that only for ASCII85.

Snuffleupagus · 2014-11-12T15:53:05Z

After thinking some more about this, I believe that it may not be such a great idea to rely only on the existence of white-space before the EI to decide that we've found the end of an ASCII85Decode stream. Hence I've basically reverted the patch to its first version, which checks that the EOD (end-of-data) marker has been found when we think that we have encountered the EI.

yurydelendik · 2014-11-12T16:22:10Z

That's fine as well. Can you extract/refactor searching into two separate functions: for non-ASCII85 (which will just scan for '~') and regular search? In the future we can improve searching so other filter types, e.g. for HEX we can jump by 2*length first or walk-by-tags for JPX/JBIG/JPEG.

P.S. no need creating getFirstFilterName closure-function.

yurydelendik · 2014-11-12T16:25:14Z

I wonder if we can add e.g. skipToEnd() for some streams (if that exists, rely on it)

Snuffleupagus · 2014-11-12T22:34:15Z

@yurydelendik I've attempted to refactor this along the lines you suggested above. Please let me know if this is going in the right direction!

yurydelendik · 2014-11-12T23:39:38Z

Yes, that looks good. One thing: I looks like after default search your stream stop after 'EI', but for AHx and A85 before. Will that be a problem?

Snuffleupagus · 2014-11-13T12:25:48Z

I looks like after default search your stream stop after 'EI', but for AHx and A85 before. Will that be a problem?

I thought about that, but since the tests passed it seemed fine. It's really good that you asked though, because I found that it does break things. One indication that something went wrong was that text selection stopped working.
I've added a helper function to advance the stream to after the EI, and I verified that the final stream position is now the same as before.

yurydelendik · 2014-11-13T13:59:41Z

/botio test

pdfjsbot · 2014-11-13T13:59:41Z

From: Bot.io (Windows)

Received

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

Live output at: http://107.22.172.223:8877/12f3a8ad7e6ea18/output.txt

pdfjsbot · 2014-11-13T13:59:41Z

From: Bot.io (Linux)

Received

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

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

pdfjsbot · 2014-11-13T14:02:43Z

From: Bot.io (Windows)

Failed

Full output at http://107.22.172.223:8877/12f3a8ad7e6ea18/output.txt

Total script time: 3.02 mins

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

Image differences available at: http://107.22.172.223:8877/12f3a8ad7e6ea18/reftest-analyzer.html#web=eq.log

yurydelendik · 2014-11-13T14:12:21Z

/botio-windows test

pdfjsbot · 2014-11-13T14:12:22Z

From: Bot.io (Windows)

Received

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

Live output at: http://107.22.172.223:8877/7ae97467b69cff2/output.txt

pdfjsbot · 2014-11-13T14:23:48Z

From: Bot.io (Linux)

Success

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

Total script time: 24.11 mins

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

pdfjsbot · 2014-11-13T14:33:03Z

From: Bot.io (Windows)

Success

Full output at http://107.22.172.223:8877/7ae97467b69cff2/output.txt

Total script time: 20.67 mins

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

yurydelendik · 2014-11-13T14:37:42Z

@Snuffleupagus, looks good, can you squash commits?

Snuffleupagus · 2014-11-13T15:00:47Z

can you squash commits?

Done.

Snuffleupagus · 2014-12-09T12:09:36Z

/botio test

pdfjsbot · 2014-12-09T12:09:36Z

From: Bot.io (Windows)

Received

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

Live output at: http://107.22.172.223:8877/07879f16db8599f/output.txt

pdfjsbot · 2014-12-09T12:09:36Z

From: Bot.io (Linux)

Received

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

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

pdfjsbot · 2014-12-09T12:26:26Z

From: Bot.io (Windows)

Success

Full output at http://107.22.172.223:8877/07879f16db8599f/output.txt

Total script time: 16.83 mins

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

pdfjsbot · 2014-12-09T12:32:20Z

From: Bot.io (Linux)

Success

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

Total script time: 22.73 mins

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

yurydelendik · 2014-12-15T15:58:46Z

src/core/parser.js

+      var FF = 0xFF, D9 = 0xD9;
+      var startPos = stream.pos, ch, length;
+      while ((ch = stream.getByte()) !== -1) {
+        if (ch === FF && stream.peekByte() === D9) {


Thinking out loud: I don't remember if you need to ignore (0xFF 0xFF)*2 0xF9 sequences

I've removed this part of the patch, since I'd rather get the main part landed now in order to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1077808 (given that the PR has been open for over two months).
Once this PR is merged, I can submit a followup which changes the inline JPEG image case.

…s (bug 1077808) This patch changes searching for the end of inline image streams to rely on the EOD marker for the filters: ASCII85Decode and ASCIIHexDecode.

Snuffleupagus · 2014-12-15T17:51:10Z

/botio test

pdfjsbot · 2014-12-15T17:51:11Z

From: Bot.io (Windows)

Received

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

Live output at: http://107.22.172.223:8877/23f11e1868ea98f/output.txt

pdfjsbot · 2014-12-15T17:51:11Z

From: Bot.io (Linux)

Received

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

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

pdfjsbot · 2014-12-15T18:08:24Z

From: Bot.io (Windows)

Success

Full output at http://107.22.172.223:8877/23f11e1868ea98f/output.txt

Total script time: 17.20 mins

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

pdfjsbot · 2014-12-15T18:13:40Z

From: Bot.io (Linux)

Success

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

Total script time: 22.48 mins

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

yurydelendik · 2014-12-18T16:03:10Z

/botio makeref

pdfjsbot · 2014-12-18T16:03:11Z

From: Bot.io (Windows)

Received

Command cmd_makeref from @yurydelendik received. Current queue size: 1

Live output at: http://107.22.172.223:8877/0bf6c17cb2af150/output.txt

pdfjsbot · 2014-12-18T16:03:11Z

From: Bot.io (Linux)

Received

Command cmd_makeref from @yurydelendik received. Current queue size: 1

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

yurydelendik · 2014-12-18T16:03:22Z

Thank you for the patch

Fix searching for end of inline (EI) images with ASCII85Decode filters (bug 1077808)

pdfjsbot · 2014-12-18T16:32:47Z

From: Bot.io (Windows)

Success

Full output at http://107.22.172.223:8877/0bf6c17cb2af150/output.txt

Total script time: 17.22 mins

Lint: Passed
Make references: Passed
Check references: Passed

pdfjsbot · 2014-12-18T16:43:51Z

From: Bot.io (Linux)

Success

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

Total script time: 22.70 mins

Lint: Passed
Make references: Passed
Check references: Passed

Fix searching for end of inline (EI) images with ASCII85Decode filters (bug 1077808)

Snuffleupagus added the core label Oct 4, 2014

Snuffleupagus changed the title ~~Fix searching for end of inline (EI) images with ASCII85Decode filters (bug 1070788)~~ Fix searching for end of inline (EI) images with ASCII85Decode filters (bug 1077808) Oct 4, 2014

CodingFabian reviewed Oct 22, 2014
View reviewed changes

Snuffleupagus mentioned this pull request Oct 23, 2014

Change bug878194 to an |eq| test #5437

Merged

yurydelendik self-assigned this Oct 23, 2014

yurydelendik reviewed Dec 15, 2014
View reviewed changes

Fix searching for end of inline (EI) images with ASCII85Decode filter…

184880a

…s (bug 1077808) This patch changes searching for the end of inline image streams to rely on the EOD marker for the filters: ASCII85Decode and ASCIIHexDecode.

yurydelendik added a commit that referenced this pull request Dec 18, 2014

Merge pull request #5383 from Snuffleupagus/bug-1077808

f4fa7aa

Fix searching for end of inline (EI) images with ASCII85Decode filters (bug 1077808)

yurydelendik merged commit f4fa7aa into mozilla:master Dec 18, 2014

Snuffleupagus deleted the bug-1077808 branch December 18, 2014 16:44

Snuffleupagus mentioned this pull request Jan 9, 2015

Refactor searching for end of inline (EI) JPEG image streams #5631

Merged

speedplane pushed a commit to speedplane/pdf.js that referenced this pull request Feb 24, 2015

Merge pull request mozilla#5383 from Snuffleupagus/bug-1077808

f279c65

Fix searching for end of inline (EI) images with ASCII85Decode filters (bug 1077808)

Snuffleupagus mentioned this pull request Jul 5, 2017

Fix a stupid spelling error in the ASCII85Decode name in Parser.makeInlineImage (issue 8613) #8615

Merged

Fix searching for end of inline (EI) images with ASCII85Decode filters (bug 1077808) #5383

Fix searching for end of inline (EI) images with ASCII85Decode filters (bug 1077808) #5383

Conversation

Snuffleupagus commented Oct 4, 2014

Snuffleupagus commented Oct 4, 2014

pdfjsbot commented Oct 4, 2014

From: Bot.io (Linux)

Received

pdfjsbot commented Oct 4, 2014

From: Bot.io (Linux)

Success

Published

Snuffleupagus commented Oct 4, 2014

pdfjsbot commented Oct 4, 2014

From: Bot.io (Windows)

Received

pdfjsbot commented Oct 4, 2014

From: Bot.io (Linux)

Received

pdfjsbot commented Oct 4, 2014

From: Bot.io (Windows)

Failed

pdfjsbot commented Oct 4, 2014

From: Bot.io (Linux)

Success

CodingFabian Oct 22, 2014

Choose a reason for hiding this comment

yurydelendik commented Nov 11, 2014

yurydelendik commented Nov 11, 2014

yurydelendik commented Nov 11, 2014

pdfjsbot commented Nov 11, 2014

From: Bot.io (Windows)

Received

pdfjsbot commented Nov 11, 2014

From: Bot.io (Windows)

Success

Published

yurydelendik commented Nov 11, 2014

Snuffleupagus commented Nov 12, 2014

yurydelendik commented Nov 12, 2014

yurydelendik commented Nov 12, 2014

Snuffleupagus commented Nov 12, 2014

yurydelendik commented Nov 12, 2014

Snuffleupagus commented Nov 13, 2014

yurydelendik commented Nov 13, 2014

pdfjsbot commented Nov 13, 2014

From: Bot.io (Windows)

Received

pdfjsbot commented Nov 13, 2014

From: Bot.io (Linux)

Received

pdfjsbot commented Nov 13, 2014

From: Bot.io (Windows)

Failed

yurydelendik commented Nov 13, 2014

pdfjsbot commented Nov 13, 2014

From: Bot.io (Windows)

Received

pdfjsbot commented Nov 13, 2014

From: Bot.io (Linux)

Success

pdfjsbot commented Nov 13, 2014

From: Bot.io (Windows)

Success

yurydelendik commented Nov 13, 2014

Snuffleupagus commented Nov 13, 2014

Snuffleupagus commented Dec 9, 2014

pdfjsbot commented Dec 9, 2014

From: Bot.io (Windows)

Received

pdfjsbot commented Dec 9, 2014

From: Bot.io (Linux)

Received

pdfjsbot commented Dec 9, 2014

From: Bot.io (Windows)

Success

pdfjsbot commented Dec 9, 2014

From: Bot.io (Linux)

Success

yurydelendik Dec 15, 2014