Sakurai/pdf.js
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Avoid some redundant type checks in XRef.fetchUncompressed

Browse Source 
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.
This commit is contained in:
Jonas Jenwald 2017-08-31 16:11:00 +02:00
parent 47789b51c3
commit 772a5412a4
1 changed files with 10 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/core/obj.js
View File

@ -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;
}
}