From 62b17ad36e305659b7658e92f37fa32b7a6d8ef0 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 12 Feb 2016 18:05:19 +0100 Subject: [PATCH 1/3] Add a linked `load` test for issue 6549 --- test/pdfs/issue6549.pdf.link | 1 + test/test_manifest.json | 9 +++++++++ 2 files changed, 10 insertions(+) create mode 100644 test/pdfs/issue6549.pdf.link diff --git a/test/pdfs/issue6549.pdf.link b/test/pdfs/issue6549.pdf.link new file mode 100644 index 000000000..9da866501 --- /dev/null +++ b/test/pdfs/issue6549.pdf.link @@ -0,0 +1 @@ +http://web.archive.org/web/20150306062422/http://www.kokuyo-st.co.jp/stationery/camiapp/CamiApp_Sample.pdf diff --git a/test/test_manifest.json b/test/test_manifest.json index e796620dc..589f96ba0 100644 --- a/test/test_manifest.json +++ b/test/test_manifest.json @@ -781,6 +781,15 @@ "rounds": 1, "type": "load" }, + { "id": "issue6549", + "file": "pdfs/issue6549.pdf", + "md5": "699aeea73a6f45375022ffc6cc80f12a", + "link": true, + "rounds": 1, + "firstPage": 2, + "lastPage": 3, + "type": "load" + }, { "id": "ibwa-bad", "file": "pdfs/ibwa-bad.pdf", "md5": "6ca059d32b74ac2688ae06f727fee755", From 93ea866f0141dbad92edc7be0555b1510582ee2b Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 12 Feb 2016 18:15:49 +0100 Subject: [PATCH 2/3] Remove `getAll` from `EvaluatorPreprocessor_read` For the operators that we currently support, the arguments are not `Dict`s, which means that it's not really necessary to use `Dict_getAll` in `EvaluatorPreprocessor_read`. Also, I do think that if/when we support operators that use `Dict`s as arguments, that should be dealt with in the corresponding `case` in `PartialEvaluator_getOperatorList` which handles the operator. The only reason that I can find for using `Dict_getAll` like that, is that prior to PR 6550 we would just append certain (currently unsupported) operators without doing any further processing/checking. But as issue 6549 showed, that can lead to issues in practice, which is why it was changed. In an effort to prevent possible issue with unsupported operators, this patch simply ignores operators with `Dict` arguments in `PartialEvaluator_getOperatorList`. --- src/core/evaluator.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/core/evaluator.js b/src/core/evaluator.js index 2fa42d7b6..df4fa0953 100644 --- a/src/core/evaluator.js +++ b/src/core/evaluator.js @@ -1005,9 +1005,20 @@ var PartialEvaluator = (function PartialEvaluatorClosure() { // but doing so is meaningless without knowing the semantics. continue; default: - // Note: Let's hope that the ignored operator does not have any - // non-serializable arguments, otherwise postMessage will throw + // Note: Ignore the operator if it has `Dict` arguments, since + // those are non-serializable, otherwise postMessage will throw // "An object could not be cloned.". + if (args !== null) { + for (i = 0, ii = args.length; i < ii; i++) { + if (args[i] instanceof Dict) { + break; + } + } + if (i < ii) { + warn('getOperatorList - ignoring operator: ' + fn); + continue; + } + } } operatorList.addOp(fn, args); } @@ -2555,7 +2566,7 @@ var EvaluatorPreprocessor = (function EvaluatorPreprocessorClosure() { if (!args) { args = []; } - args.push((obj instanceof Dict ? obj.getAll() : obj)); + args.push(obj); assert(args.length <= 33, 'Too many arguments'); } } From 1ee016b005ff3265b82f784bbde253bd99657d94 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 12 Feb 2016 18:16:45 +0100 Subject: [PATCH 3/3] Remove `Dict_getAll` since it is now unused `Dict_getAll` is problematic for a number of reasons. First of all, as issue 6961 shows, it can be really bad for performance, since it dereferences all indirect objects. Second of all, all the derefencing can lead to data being unncessarily requested when ranged/chunked loading is used, thus unnecessarily delaying rendering. Note: For cases where `Dict_getAll` was previously used, `Dict_getKeys` in combination with `Dict_get` can be used instead. This has the advantage that data isn't requested until it's actually needed. --- src/core/primitives.js | 70 ------------------------------------------ 1 file changed, 70 deletions(-) diff --git a/src/core/primitives.js b/src/core/primitives.js index 410a38470..730bacb1f 100644 --- a/src/core/primitives.js +++ b/src/core/primitives.js @@ -67,24 +67,6 @@ var Dict = (function DictClosure() { return nonSerializable; // creating closure on some variable }; - var GETALL_DICTIONARY_TYPES_WHITELIST = { - 'Background': true, - 'ExtGState': true, - 'Halftone': true, - 'Layout': true, - 'Mask': true, - 'Pagination': true, - 'Printing': true - }; - - function isRecursionAllowedFor(dict) { - if (!isName(dict.Type)) { - return true; - } - var dictType = dict.Type.name; - return GETALL_DICTIONARY_TYPES_WHITELIST[dictType] === true; - } - // xref is optional function Dict(xref) { // Map should only be used internally, use functions below to access. @@ -162,58 +144,6 @@ var Dict = (function DictClosure() { return this.map[key]; }, - // creates new map and dereferences all Refs - getAll: function Dict_getAll() { - var all = Object.create(null); - var queue = null; - var key, obj; - for (key in this.map) { - obj = this.get(key); - if (obj instanceof Dict) { - if (isRecursionAllowedFor(obj)) { - (queue || (queue = [])).push({target: all, key: key, obj: obj}); - } else { - all[key] = this.getRaw(key); - } - } else { - all[key] = obj; - } - } - if (!queue) { - return all; - } - - // trying to take cyclic references into the account - var processed = Object.create(null); - while (queue.length > 0) { - var item = queue.shift(); - var itemObj = item.obj; - var objId = itemObj.objId; - if (objId && objId in processed) { - item.target[item.key] = processed[objId]; - continue; - } - var dereferenced = Object.create(null); - for (key in itemObj.map) { - obj = itemObj.get(key); - if (obj instanceof Dict) { - if (isRecursionAllowedFor(obj)) { - queue.push({target: dereferenced, key: key, obj: obj}); - } else { - dereferenced[key] = itemObj.getRaw(key); - } - } else { - dereferenced[key] = obj; - } - } - if (objId) { - processed[objId] = dereferenced; - } - item.target[item.key] = dereferenced; - } - return all; - }, - getKeys: function Dict_getKeys() { return Object.keys(this.map); },