From 772a5412a4c659078b984bcf58cbbe2c135373db Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 31 Aug 2017 16:11:00 +0200 Subject: [PATCH] Avoid some redundant type checks in `XRef.fetchUncompressed` When looking briefly at using `Number.isInteger`/`Number.isNan` rather than `isInt`/`isNaN`, I noticed that there's a couple of not entirely straightforward cases to consider. At first I really couldn't understand why `parseInt` is being used like it is in `XRef.fetchUncompressed`, since the `num` and `gen` properties of an object reference should *always* be integers. However, doing a bit of code archaeology pointed to PR 4348, and it thus seem that this was a very deliberate change. Since I didn't want to inadvertently introduce any regressions, I've kept the `parseInt` calls intact but moved them to occur *only* when actually necessary.[1] Secondly, I noticed that there's a redundant `isCmd` check for an edge-case of broken operators. Since we're throwing a `FormatError` if `obj3` isn't a command, we don't need to repeat that check. In practice, this patch could perhaps be considered as a micro-optimization, but considering that `XRef.fetchUncompressed` can be called *many* thousand times when loading larger PDF documents these changes at least cannot hurt. --- [1] I even ran all tests locally, with an added `assert(Number.isInteger(obj1) && Number.isInteger(obj2));` check, and everything passed with flying colours. However, since it appears that this was in fact necessary at one point, one possible explanation is that the failing test-case(s) have now been replaced by reduced ones. --- src/core/obj.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/core/obj.js b/src/core/obj.js index 95924a6fe..841da2cc3 100644 --- a/src/core/obj.js +++ b/src/core/obj.js @@ -1346,16 +1346,21 @@ var XRef = (function XRefClosure() { var obj1 = parser.getObj(); var obj2 = parser.getObj(); var obj3 = parser.getObj(); - if (!isInt(obj1) || parseInt(obj1, 10) !== num || - !isInt(obj2) || parseInt(obj2, 10) !== gen || - !isCmd(obj3)) { + + if (!Number.isInteger(obj1)) { + obj1 = parseInt(obj1, 10); + } + if (!Number.isInteger(obj2)) { + obj2 = parseInt(obj2, 10); + } + if (obj1 !== num || obj2 !== gen || !isCmd(obj3)) { throw new FormatError('bad XRef entry'); } - if (!isCmd(obj3, 'obj')) { + if (obj3.cmd !== 'obj') { // some bad PDFs use "obj1234" and really mean 1234 if (obj3.cmd.indexOf('obj') === 0) { num = parseInt(obj3.cmd.substring(3), 10); - if (!isNaN(num)) { + if (!Number.isNaN(num)) { return num; } }