From 9bed87f5dcaa4a17ea68a512123ae63a96857efc Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Sat, 4 Mar 2017 12:34:29 +0100
Subject: [PATCH] Return `undefined` instead of `Dict.empty` from
 `Page.getInheritedPageProp` for non-existent properties to prevent possible
 future bugs

*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.
---
 src/core/document.js | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/core/document.js b/src/core/document.js
index 815687ff8..93b47543f 100644
--- a/src/core/document.js
+++ b/src/core/document.js
@@ -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() {