Merge pull request #9838 from Snuffleupagus/invalid-path-OPS

Error, rather than warn, once a number of invalid path operators are encountered in `EvaluatorPreprocessor.read` (bug 1443140)

src/core/evaluator.js

@@ -2932,6 +2932,8 @@ var EvaluatorPreprocessor = (function EvaluatorPreprocessorClosure() {
     t['null'] = null;
   });
 
+  const MAX_INVALID_PATH_OPS = 20;
+
   function EvaluatorPreprocessor(stream, xref, stateManager) {
     this.opMap = getOPMap();
     // TODO(mduan): pass array of knownCommands rather than this.opMap
@@ -2939,6 +2941,7 @@ var EvaluatorPreprocessor = (function EvaluatorPreprocessorClosure() {
     this.parser = new Parser(new Lexer(stream, this.opMap), false, xref);
     this.stateManager = stateManager;
     this.nonProcessedArgs = [];
+    this._numInvalidPathOPS = 0;
   }
 
   EvaluatorPreprocessor.prototype = {
@@ -2976,7 +2979,7 @@ var EvaluatorPreprocessor = (function EvaluatorPreprocessorClosure() {
           // Check that the command is valid
           var opSpec = this.opMap[cmd];
           if (!opSpec) {
-            warn('Unknown command "' + cmd + '"');
+            warn(`Unknown command "${cmd}".`);
             continue;
           }
 
@@ -3002,18 +3005,28 @@ var EvaluatorPreprocessor = (function EvaluatorPreprocessorClosure() {
             }
 
             if (argsLength < numArgs) {
+              const partialMsg = `command ${cmd}: expected ${numArgs} args, ` +
+                                 `but received ${argsLength} args.`;
+
+              // Incomplete path operators, in particular, can result in fairly
+              // chaotic rendering artifacts. Hence the following heuristics is
+              // used to error, rather than just warn, once a number of invalid
+              // path operators have been encountered (fixes bug1443140.pdf).
+              if ((fn >= OPS.moveTo && fn <= OPS.endPath) && // Path operator
+                  ++this._numInvalidPathOPS > MAX_INVALID_PATH_OPS) {
+                throw new FormatError(`Invalid ${partialMsg}`);
+              }
               // If we receive too few arguments, it's not possible to execute
               // the command, hence we skip the command.
-              warn('Skipping command ' + fn + ': expected ' + numArgs +
+              warn(`Skipping ${partialMsg}`);
-                   ' args, but received ' + argsLength + ' args.');
               if (args !== null) {
                 args.length = 0;
               }
               continue;
             }
           } else if (argsLength > numArgs) {
-            info('Command ' + fn + ': expected [0,' + numArgs +
+            info(`Command ${cmd}: expected [0, ${numArgs}] args, ` +
-                 '] args, but received ' + argsLength + ' args.');
+                 `but received ${argsLength} args.`);
           }
 
           // TODO figure out how to type-check vararg functions

test/pdfs/bug1443140.pdf.link

@@ -0,0 +1 @@
+https://web.archive.org/web/20180324105403/https://engineering.purdue.edu/~chengkok/papers/2005/p507-li.pdf

test/test_manifest.json

@@ -126,8 +126,7 @@
       "file": "pdfs/issue2391-1.pdf",
       "md5": "25ae9cb959612e7b343b55da63af2716",
       "rounds": 1,
-      "lastPage": 1,
+      "type": "eq"
-      "type": "load"
     },
     {  "id": "issue2391-2",
       "file": "pdfs/issue2391-2.pdf",
@@ -135,6 +134,15 @@
       "rounds": 1,
       "type": "eq"
     },
+    {  "id": "bug1443140",
+       "file": "pdfs/bug1443140.pdf",
+       "md5": "8f9347b0d5620537850b24b8385b0982",
+       "rounds": 1,
+       "link": true,
+       "firstPage": 4,
+       "lastPage": 4,
+       "type": "eq"
+    },
     {  "id": "issue2531",
       "file": "pdfs/issue2531.pdf",
       "md5": "c58e6642d8a6e2ddd5e07a543ef8f30d",

test/unit/evaluator_spec.js

@@ -216,6 +216,34 @@ describe('evaluator', function() {
         done();
       });
     });
+
+    it('should error if (many) path operators have too few arguments ' +
+       '(bug 1443140)', function(done) {
+      const NUM_INVALID_OPS = 25;
+      const tempArr = new Array(NUM_INVALID_OPS + 1);
+
+      // Non-path operators, should be ignored.
+      const invalidMoveText = tempArr.join('10 Td\n');
+      const moveTextStream = new StringStream(invalidMoveText);
+      runOperatorListCheck(partialEvaluator, moveTextStream,
+                           new ResourcesMock(), function(result) {
+        expect(result.argsArray).toEqual([]);
+        expect(result.fnArray).toEqual([]);
+        done();
+      });
+
+      // Path operators, should throw error.
+      const invalidLineTo = tempArr.join('20 l\n');
+      const lineToStream = new StringStream(invalidLineTo);
+      runOperatorListCheck(partialEvaluator, lineToStream, new ResourcesMock(),
+          function(error) {
+        expect(error instanceof FormatError).toEqual(true);
+        expect(error.message).toEqual(
+          'Invalid command l: expected 2 args, but received 1 args.');
+        done();
+      });
+    });
+
     it('should close opened saves', function(done) {
       var stream = new StringStream('qq');
       runOperatorListCheck(partialEvaluator, stream, new ResourcesMock(),