[api-minor] Let `Catalog_getPageIndex` check that the `Ref` actually points to a /Page dictionary

Currently the `getPageIndex` method will happily return `0`, even if the `Ref` parameter doesn't actually point to a proper /Page dictionary.
Having the API trust that the consumer is doing the right thing seems error-prone, hence this patch which adds a check for this case.

Given that the `Catalog_getPageIndex` method isn't used in any hot part of the codebase, this extra check shouldn't be a problem.
(Note: in the standard viewer, it is only ever used from `PDFLinkService_navigateTo` if a destination needs to be resolved during document loading, which isn't common enough to be an issue IMHO.)

src/core/obj.js

@@ -56,6 +56,7 @@ var isName = corePrimitives.isName;
 var isCmd = corePrimitives.isCmd;
 var isDict = corePrimitives.isDict;
 var isRef = corePrimitives.isRef;
+var isRefsEqual = corePrimitives.isRefsEqual;
 var isStream = corePrimitives.isStream;
 var CipherTransformFactory = coreCrypto.CipherTransformFactory;
 var Lexer = coreParser.Lexer;
@@ -522,7 +523,7 @@ var Catalog = (function CatalogClosure() {
       return capability.promise;
     },
 
-    getPageIndex: function Catalog_getPageIndex(ref) {
+    getPageIndex: function Catalog_getPageIndex(pageRef) {
       // The page tree nodes have the count of all the leaves below them. To get
       // how many pages are before we just have to walk up the tree and keep
       // adding the count of siblings to the left of the node.
@@ -531,15 +532,21 @@ var Catalog = (function CatalogClosure() {
         var total = 0;
         var parentRef;
         return xref.fetchAsync(kidRef).then(function (node) {
+          if (isRefsEqual(kidRef, pageRef) && !isDict(node, 'Page') &&
+              !(isDict(node) && !node.has('Type') && node.has('Contents'))) {
+            throw new Error('The reference does not point to a /Page Dict.');
+          }
           if (!node) {
             return null;
           }
+          assert(isDict(node), 'node must be a Dict.');
           parentRef = node.getRaw('Parent');
           return node.getAsync('Parent');
         }).then(function (parent) {
           if (!parent) {
             return null;
           }
+          assert(isDict(parent), 'parent must be a Dict.');
           return parent.getAsync('Kids');
         }).then(function (kids) {
           if (!kids) {
@@ -549,7 +556,7 @@ var Catalog = (function CatalogClosure() {
           var found = false;
           for (var i = 0; i < kids.length; i++) {
             var kid = kids[i];
-            assert(isRef(kid), 'kids must be a ref');
+            assert(isRef(kid), 'kid must be a Ref.');
             if (kid.num === kidRef.num) {
               found = true;
               break;
@@ -585,7 +592,7 @@ var Catalog = (function CatalogClosure() {
         });
       }
 
-      return next(ref);
+      return next(pageRef);
     }
   };
 

src/core/primitives.js

@@ -290,6 +290,10 @@ function isRef(v) {
   return v instanceof Ref;
 }
 
+function isRefsEqual(v1, v2) {
+  return v1.num === v2.num && v1.gen === v2.gen;
+}
+
 function isStream(v) {
   return typeof v === 'object' && v !== null && v.getBytes !== undefined;
 }
@@ -304,5 +308,6 @@ exports.isCmd = isCmd;
 exports.isDict = isDict;
 exports.isName = isName;
 exports.isRef = isRef;
+exports.isRefsEqual = isRefsEqual;
 exports.isStream = isStream;
 }));

src/display/api.js

@@ -1683,7 +1683,12 @@ var WorkerTransport = (function WorkerTransportClosure() {
     },
 
     getPageIndex: function WorkerTransport_getPageIndexByRef(ref) {
-      return this.messageHandler.sendWithPromise('GetPageIndex', { ref: ref });
+      return this.messageHandler.sendWithPromise('GetPageIndex', { ref: ref }).
+        then(function (pageIndex) {
+          return pageIndex;
+        }, function (reason) {
+          return Promise.reject(new Error(reason));
+        });
     },
 
     getAnnotations: function WorkerTransport_getAnnotations(pageIndex, intent) {

test/unit/api_spec.js

@@ -360,6 +360,16 @@ describe('api', function() {
         done.fail(reason);
       });
     });
+    it('gets invalid page index', function (done) {
+      var ref = { num: 3, gen: 0 }; // Reference to a font dictionary.
+      var promise = doc.getPageIndex(ref);
+      promise.then(function () {
+        done.fail('shall fail for invalid page reference.');
+      }, function (data) {
+        expect(data instanceof Error).toEqual(true);
+        done();
+      });
+    });
 
     it('gets destinations, from /Dests dictionary', function(done) {
       var promise = doc.getDestinations();

test/unit/primitives_spec.js

@@ -1,5 +1,5 @@
 /* globals expect, it, describe, beforeEach, Name, Dict, Ref, RefSet, Cmd,
-           jasmine, isDict */
+           jasmine, isDict, isRefsEqual */
 
 'use strict';
 
@@ -158,4 +158,18 @@ describe('primitives', function() {
       expect(isDict(dict, 'Page')).toEqual(true);
     });
   });
+
+  describe('isRefsEqual', function () {
+    it('should handle different Refs pointing to the same object', function () {
+      var ref1 = new Ref(1, 0);
+      var ref2 = new Ref(1, 0);
+      expect(isRefsEqual(ref1, ref2)).toEqual(true);
+    });
+
+    it('should handle Refs pointing to different objects', function () {
+      var ref1 = new Ref(1, 0);
+      var ref2 = new Ref(2, 0);
+      expect(isRefsEqual(ref1, ref2)).toEqual(false);
+    });
+  });
 });