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

Return undefined instead of Dict.empty from Page.getInheritedPageProp for non-existent properties to prevent possible future bugs

Browse Source 
*This is something that I noticed while working on PR 8126, which is (more) fallout from PR 6065.*

In general, it's actually *not* correct to return `Dict.empty` as the default value for non-existent properties. Please note that a prior PR, see https://github.com/mozilla/pdf.js/pull/5957#issuecomment-103112698, asked for that behaviour but I don't think that's right.

Obviously for properties that are (or should) be `Dict`s it makes sense, however certain properties can be e.g. Strings or Arrays instead. In the latter case, returning `Dict.empty` is just plain wrong, and it's quite fascinating that this hasn't caused any errors in practice. (The existing validation in the various getters has actually saved us here.)

Also, when looking at this code again, it seemed unnecessary to duplicate the `MAX_LOOP_COUNT` check since we could just return immediately instead.
This commit is contained in:
Jonas Jenwald 2017-03-04 12:34:29 +01:00
parent 1eb96d7ca9
commit 9bed87f5dc
1 changed files with 6 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/core/document.js
View File

@ -119,16 +119,15 @@ var Page = (function PageClosure() {
valueArray.push(value);
}
if (++loopCount > MAX_LOOP_COUNT) {
warn('Page_getInheritedPageProp: maximum loop count exceeded.');
break;
warn('getInheritedPageProp: maximum loop count exceeded for ' + key);
return valueArray ? valueArray[0] : undefined;
}
dict = dict.get('Parent');
}
if (!valueArray) {
return Dict.empty;
return undefined;
}
if (valueArray.length === 1 || !isDict(valueArray[0]) ||
loopCount > MAX_LOOP_COUNT) {
if (valueArray.length === 1 || !isDict(valueArray[0])) {
return valueArray[0];
}
return Dict.merge(this.xref, valueArray);
@ -142,7 +141,8 @@ var Page = (function PageClosure() {
// For robustness: The spec states that a \Resources entry has to be
// present, but can be empty. Some document omit it still, in this case
// we return an empty dictionary.
return shadow(this, 'resources', this.getInheritedPageProp('Resources'));
return shadow(this, 'resources',
this.getInheritedPageProp('Resources') || Dict.empty);
},
get mediaBox() {