From b74c813353f53cc43aca6d1ee039f157c5a1d523 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 11 Aug 2018 15:40:08 +0200 Subject: [PATCH 1/2] Re-factor `destinations`/`getDestination`, in the `Catalog`, to reduce unnecessary duplication Currently, these two methods contain the same boilerplate code for getting the /Dests data. --- src/core/obj.js | 75 +++++++++++++++++-------------------------------- 1 file changed, 26 insertions(+), 49 deletions(-) diff --git a/src/core/obj.js b/src/core/obj.js index 02af40abf6621..e345d23f65603 100644 --- a/src/core/obj.js +++ b/src/core/obj.js @@ -28,6 +28,10 @@ import { ChunkedStream } from './chunked_stream'; import { CipherTransformFactory } from './crypto'; import { ColorSpace } from './colorspace'; +function fetchDestination(dest) { + return isDict(dest) ? dest.get('D') : dest; +} + var Catalog = (function CatalogClosure() { function Catalog(pdfManager, xref) { this.pdfManager = pdfManager; @@ -173,64 +177,37 @@ var Catalog = (function CatalogClosure() { // shadow the prototype getter return shadow(this, 'numPages', obj); }, - get destinations() { - function fetchDestination(dest) { - return isDict(dest) ? dest.get('D') : dest; - } - - var xref = this.xref; - var dests = {}, nameTreeRef, nameDictionaryRef; - var obj = this.catDict.get('Names'); - if (obj && obj.has('Dests')) { - nameTreeRef = obj.getRaw('Dests'); - } else if (this.catDict.has('Dests')) { - nameDictionaryRef = this.catDict.get('Dests'); - } - if (nameDictionaryRef) { - // reading simple destination dictionary - obj = nameDictionaryRef; - obj.forEach(function catalogForEach(key, value) { - if (!value) { - return; - } - dests[key] = fetchDestination(value); - }); - } - if (nameTreeRef) { - var nameTree = new NameTree(nameTreeRef, xref); - var names = nameTree.getAll(); - for (var name in names) { + get destinations() { + const obj = this._readDests(), dests = Object.create(null); + if (obj instanceof NameTree) { + const names = obj.getAll(); + for (let name in names) { dests[name] = fetchDestination(names[name]); } + } else if (obj instanceof Dict) { + obj.forEach(function(key, value) { + if (value) { + dests[key] = fetchDestination(value); + } + }); } return shadow(this, 'destinations', dests); }, - getDestination: function Catalog_getDestination(destinationId) { - function fetchDestination(dest) { - return isDict(dest) ? dest.get('D') : dest; + getDestination(destinationId) { + const obj = this._readDests(); + if (obj instanceof NameTree || obj instanceof Dict) { + return fetchDestination(obj.get(destinationId) || null); } - - var xref = this.xref; - var dest = null, nameTreeRef, nameDictionaryRef; - var obj = this.catDict.get('Names'); + return null; + }, + _readDests() { + const obj = this.catDict.get('Names'); if (obj && obj.has('Dests')) { - nameTreeRef = obj.getRaw('Dests'); - } else if (this.catDict.has('Dests')) { - nameDictionaryRef = this.catDict.get('Dests'); - } - - if (nameDictionaryRef) { // Simple destination dictionary. - var value = nameDictionaryRef.get(destinationId); - if (value) { - dest = fetchDestination(value); - } - } - if (nameTreeRef) { - var nameTree = new NameTree(nameTreeRef, xref); - dest = fetchDestination(nameTree.get(destinationId)); + return new NameTree(obj.getRaw('Dests'), this.xref); + } else if (this.catDict.has('Dests')) { // Simple destination dictionary. + return this.catDict.get('Dests'); } - return dest; }, get pageLabels() { From 1179584fd63a41747269bf32e93ae418f632bf22 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 11 Aug 2018 16:00:48 +0200 Subject: [PATCH 2/2] Reject `getDestination`, in the API, for non-string inputs Note how e.g. the `getPage` method does basic validation of the input. --- src/display/api.js | 5 ++++- test/unit/api_spec.js | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/display/api.js b/src/display/api.js index f462d5bc762b7..9afadc3ce90c8 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -627,7 +627,7 @@ var PDFDocumentProxy = (function PDFDocumentProxyClosure() { return this.transport.getDestinations(); }, /** - * @param {string} id The named destination to get. + * @param {string} id - The named destination to get. * @return {Promise} A promise that is resolved with all information * of the given named destination. */ @@ -2119,6 +2119,9 @@ var WorkerTransport = (function WorkerTransportClosure() { }, getDestination: function WorkerTransport_getDestination(id) { + if (typeof id !== 'string') { + return Promise.reject(new Error('Invalid destination request.')); + } return this.messageHandler.sendWithPromise('GetDestination', { id, }); diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index d6dcd08f22896..35f415f2a7773 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -583,6 +583,32 @@ describe('api', function() { }); }); + it('gets non-string destination', function(done) { + let numberPromise = doc.getDestination(4.3); + let booleanPromise = doc.getDestination(true); + let arrayPromise = doc.getDestination([ + { num: 17, gen: 0, }, { name: 'XYZ', }, 0, 841.89, null]); + + numberPromise = numberPromise.then(function() { + throw new Error('shall fail for non-string destination.'); + }, function(reason) { + expect(reason instanceof Error).toEqual(true); + }); + booleanPromise = booleanPromise.then(function() { + throw new Error('shall fail for non-string destination.'); + }, function(reason) { + expect(reason instanceof Error).toEqual(true); + }); + arrayPromise = arrayPromise.then(function() { + throw new Error('shall fail for non-string destination.'); + }, function(reason) { + expect(reason instanceof Error).toEqual(true); + }); + + Promise.all([numberPromise, booleanPromise, arrayPromise]).then( + done, done.fail); + }); + it('gets non-existent page labels', function (done) { var promise = doc.getPageLabels(); promise.then(function (data) {