From 84476da26e8ae232429061ab1c0106a770cc59cb Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 14 Jul 2020 13:00:35 +0200 Subject: [PATCH 1/4] Handle lookup errors "silently" in `PartialEvaluator.hasBlendModes` (PR 11680 follow-up) Given that this method is used during what's essentially a *pre*-parsing stage, before the actual OperatorList parsing occurs, on second thought it doesn't seem at all necessary to warn and trigger fallback in cases where there's lookup errors. *Please note:* Any any errors will still be either suppressed or thrown, according to the `ignoreErrors` option, during the *actual* OperatorList parsing. --- src/core/evaluator.js | 38 ++++++++++---------------------------- 1 file changed, 10 insertions(+), 28 deletions(-) diff --git a/src/core/evaluator.js b/src/core/evaluator.js index 3a1296f1d..6951e954e 100644 --- a/src/core/evaluator.js +++ b/src/core/evaluator.js @@ -264,20 +264,11 @@ class PartialEvaluator { if (ex instanceof MissingDataException) { throw ex; } - if (this.options.ignoreErrors) { - if (graphicState instanceof Ref) { - // Avoid parsing a corrupt ExtGState more than once. - processed[graphicState.toString()] = true; - } - // Error(s) in the ExtGState -- sending unsupported feature - // notification and allow parsing/rendering to continue. - this.handler.send("UnsupportedFeature", { - featureId: UNSUPPORTED_FEATURES.errorExtGState, - }); - warn(`hasBlendModes - ignoring ExtGState: "${ex}".`); - continue; - } - throw ex; + // Avoid parsing a corrupt ExtGState more than once. + processed[graphicState.toString()] = true; + + info(`hasBlendModes - ignoring ExtGState: "${ex}".`); + continue; } } if (!(graphicState instanceof Dict)) { @@ -326,20 +317,11 @@ class PartialEvaluator { if (ex instanceof MissingDataException) { throw ex; } - if (this.options.ignoreErrors) { - if (xObject instanceof Ref) { - // Avoid parsing a corrupt XObject more than once. - processed[xObject.toString()] = true; - } - // Error(s) in the XObject -- sending unsupported feature - // notification and allow parsing/rendering to continue. - this.handler.send("UnsupportedFeature", { - featureId: UNSUPPORTED_FEATURES.errorXObject, - }); - warn(`hasBlendModes - ignoring XObject: "${ex}".`); - continue; - } - throw ex; + // Avoid parsing a corrupt XObject more than once. + processed[xObject.toString()] = true; + + info(`hasBlendModes - ignoring XObject: "${ex}".`); + continue; } } if (!isStream(xObject)) { From 15fa3f8518a1e400370d04604a5bc08198284293 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 15 Jul 2020 12:30:30 +0200 Subject: [PATCH 2/4] Remove a redundant `/XObject` stream dictionary `objId` check in `PartialEvaluator.hasBlendModes` (PR 6971 follow-up) This case should no longer happen, given the `instanceof Ref` branch just above (added in PR 6971). Also, I've run the entire test-suite locally with `continue` replaced by `throw new Error(...)` and didn't find any problems. --- src/core/evaluator.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/core/evaluator.js b/src/core/evaluator.js index 6951e954e..aacfeed57 100644 --- a/src/core/evaluator.js +++ b/src/core/evaluator.js @@ -328,9 +328,6 @@ class PartialEvaluator { continue; } if (xObject.dict.objId) { - if (processed[xObject.dict.objId]) { - continue; // Stream has objId and was processed already. - } processed[xObject.dict.objId] = true; } var xResources = xObject.dict.get("Resources"); From f20aeb93433cb2e34b5507f7d3565c6d5512c2ce Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 15 Jul 2020 11:51:45 +0200 Subject: [PATCH 3/4] Slightly simplify the code in `PartialEvaluator.hasBlendModes`, e.g. by using `for...of` loops - Replace the existing loops with `for...of` variants instead. - Make use of `continue`, to reduce indentation and to make the code (slightly) easier to follow, when checking `/Resources` entries. --- src/core/evaluator.js | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/core/evaluator.js b/src/core/evaluator.js index aacfeed57..6edeebe73 100644 --- a/src/core/evaluator.js +++ b/src/core/evaluator.js @@ -249,10 +249,7 @@ class PartialEvaluator { // First check the current resources for blend modes. var graphicStates = node.get("ExtGState"); if (graphicStates instanceof Dict) { - var graphicStatesKeys = graphicStates.getKeys(); - for (let i = 0, ii = graphicStatesKeys.length; i < ii; i++) { - const key = graphicStatesKeys[i]; - + for (const key of graphicStates.getKeys()) { let graphicState = graphicStates.getRaw(key); if (graphicState instanceof Ref) { if (processed[graphicState.toString()]) { @@ -286,8 +283,8 @@ class PartialEvaluator { continue; } if (bm !== undefined && Array.isArray(bm)) { - for (let j = 0, jj = bm.length; j < jj; j++) { - if (bm[j] instanceof Name && bm[j].name !== "Normal") { + for (const element of bm) { + if (element instanceof Name && element.name !== "Normal") { return true; } } @@ -299,10 +296,7 @@ class PartialEvaluator { if (!(xObjects instanceof Dict)) { continue; } - var xObjectsKeys = xObjects.getKeys(); - for (let i = 0, ii = xObjectsKeys.length; i < ii; i++) { - const key = xObjectsKeys[i]; - + for (const key of xObjects.getKeys()) { var xObject = xObjects.getRaw(key); if (xObject instanceof Ref) { if (processed[xObject.toString()]) { @@ -331,15 +325,17 @@ class PartialEvaluator { processed[xObject.dict.objId] = true; } var xResources = xObject.dict.get("Resources"); + if (!(xResources instanceof Dict)) { + continue; + } // Checking objId to detect an infinite loop. - if ( - xResources instanceof Dict && - (!xResources.objId || !processed[xResources.objId]) - ) { - nodes.push(xResources); - if (xResources.objId) { - processed[xResources.objId] = true; - } + if (xResources.objId && processed[xResources.objId]) { + continue; + } + + nodes.push(xResources); + if (xResources.objId) { + processed[xResources.objId] = true; } } } From b3480842b3380ca3827512a11a5afc01a3f2cb84 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 15 Jul 2020 12:05:05 +0200 Subject: [PATCH 4/4] Use a `RefSet`, rather than a plain Object, for tracking already processed nodes in `PartialEvaluator.hasBlendModes` --- src/core/evaluator.js | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/core/evaluator.js b/src/core/evaluator.js index 6edeebe73..bed5ab850 100644 --- a/src/core/evaluator.js +++ b/src/core/evaluator.js @@ -44,6 +44,7 @@ import { isStream, Name, Ref, + RefSet, } from "./primitives.js"; import { ErrorFont, @@ -237,9 +238,9 @@ class PartialEvaluator { return false; } - var processed = Object.create(null); + const processed = new RefSet(); if (resources.objId) { - processed[resources.objId] = true; + processed.put(resources.objId); } var nodes = [resources], @@ -252,7 +253,7 @@ class PartialEvaluator { for (const key of graphicStates.getKeys()) { let graphicState = graphicStates.getRaw(key); if (graphicState instanceof Ref) { - if (processed[graphicState.toString()]) { + if (processed.has(graphicState)) { continue; // The ExtGState has already been processed. } try { @@ -262,7 +263,7 @@ class PartialEvaluator { throw ex; } // Avoid parsing a corrupt ExtGState more than once. - processed[graphicState.toString()] = true; + processed.put(graphicState); info(`hasBlendModes - ignoring ExtGState: "${ex}".`); continue; @@ -272,7 +273,7 @@ class PartialEvaluator { continue; } if (graphicState.objId) { - processed[graphicState.objId] = true; + processed.put(graphicState.objId); } const bm = graphicState.get("BM"); @@ -299,7 +300,7 @@ class PartialEvaluator { for (const key of xObjects.getKeys()) { var xObject = xObjects.getRaw(key); if (xObject instanceof Ref) { - if (processed[xObject.toString()]) { + if (processed.has(xObject)) { // The XObject has already been processed, and by avoiding a // redundant `xref.fetch` we can *significantly* reduce the load // time for badly generated PDF files (fixes issue6961.pdf). @@ -312,7 +313,7 @@ class PartialEvaluator { throw ex; } // Avoid parsing a corrupt XObject more than once. - processed[xObject.toString()] = true; + processed.put(xObject); info(`hasBlendModes - ignoring XObject: "${ex}".`); continue; @@ -322,20 +323,20 @@ class PartialEvaluator { continue; } if (xObject.dict.objId) { - processed[xObject.dict.objId] = true; + processed.put(xObject.dict.objId); } var xResources = xObject.dict.get("Resources"); if (!(xResources instanceof Dict)) { continue; } // Checking objId to detect an infinite loop. - if (xResources.objId && processed[xResources.objId]) { + if (xResources.objId && processed.has(xResources.objId)) { continue; } nodes.push(xResources); if (xResources.objId) { - processed[xResources.objId] = true; + processed.put(xResources.objId); } } }