From e64bc1fd1387d43c3447c7018eaa16f1794f0d2a Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 30 Sep 2016 16:08:03 +0200 Subject: [PATCH 1/4] Move parsing of destination dictionaries to a helper function This not only reduces code duplication, but it also allow us to easily support the same kind of URLs we currently do for Link annotations in the Outline as well. --- src/core/annotation.js | 102 ++--------------------------- src/core/obj.js | 133 ++++++++++++++++++++++++++++++++------ test/unit/api_spec.js | 1 + web/pdf_outline_viewer.js | 7 +- 4 files changed, 126 insertions(+), 117 deletions(-) diff --git a/src/core/annotation.js b/src/core/annotation.js index 4539db43d4249..37c3315966070 100644 --- a/src/core/annotation.js +++ b/src/core/annotation.js @@ -38,14 +38,11 @@ var AnnotationFlag = sharedUtil.AnnotationFlag; var AnnotationType = sharedUtil.AnnotationType; var OPS = sharedUtil.OPS; var Util = sharedUtil.Util; -var isBool = sharedUtil.isBool; var isString = sharedUtil.isString; var isArray = sharedUtil.isArray; var isInt = sharedUtil.isInt; -var isValidUrl = sharedUtil.isValidUrl; var stringToBytes = sharedUtil.stringToBytes; var stringToPDFString = sharedUtil.stringToPDFString; -var stringToUTF8String = sharedUtil.stringToUTF8String; var warn = sharedUtil.warn; var Dict = corePrimitives.Dict; var isDict = corePrimitives.isDict; @@ -53,6 +50,7 @@ var isName = corePrimitives.isName; var isRef = corePrimitives.isRef; var Stream = coreStream.Stream; var ColorSpace = coreColorSpace.ColorSpace; +var Catalog = coreObj.Catalog; var ObjectLoader = coreObj.ObjectLoader; var FileSpec = coreObj.FileSpec; var OperatorList = coreEvaluator.OperatorList; @@ -842,103 +840,13 @@ var LinkAnnotation = (function LinkAnnotationClosure() { function LinkAnnotation(params) { Annotation.call(this, params); - var dict = params.dict; var data = this.data; data.annotationType = AnnotationType.LINK; - var action = dict.get('A'), url, dest; - if (action && isDict(action)) { - var linkType = action.get('S').name; - switch (linkType) { - case 'URI': - url = action.get('URI'); - if (isName(url)) { - // Some bad PDFs do not put parentheses around relative URLs. - url = '/' + url.name; - } else if (url) { - url = addDefaultProtocolToUrl(url); - } - // TODO: pdf spec mentions urls can be relative to a Base - // entry in the dictionary. - break; - - case 'GoTo': - dest = action.get('D'); - break; - - case 'GoToR': - var urlDict = action.get('F'); - if (isDict(urlDict)) { - // We assume that we found a FileSpec dictionary - // and fetch the URL without checking any further. - url = urlDict.get('F') || null; - } else if (isString(urlDict)) { - url = urlDict; - } - - // NOTE: the destination is relative to the *remote* document. - var remoteDest = action.get('D'); - if (remoteDest) { - if (isName(remoteDest)) { - remoteDest = remoteDest.name; - } - if (isString(url)) { - var baseUrl = url.split('#')[0]; - if (isString(remoteDest)) { - // In practice, a named destination may contain only a number. - // If that happens, use the '#nameddest=' form to avoid the link - // redirecting to a page, instead of the correct destination. - url = baseUrl + '#' + - (/^\d+$/.test(remoteDest) ? 'nameddest=' : '') + remoteDest; - } else if (isArray(remoteDest)) { - url = baseUrl + '#' + JSON.stringify(remoteDest); - } - } - } - // The 'NewWindow' property, equal to `LinkTarget.BLANK`. - var newWindow = action.get('NewWindow'); - if (isBool(newWindow)) { - data.newWindow = newWindow; - } - break; - - case 'Named': - data.action = action.get('N').name; - break; - - default: - warn('unrecognized link type: ' + linkType); - } - } else if (dict.has('Dest')) { // Simple destination link. - dest = dict.get('Dest'); - } - - if (url) { - if (isValidUrl(url, /* allowRelative = */ false)) { - data.url = tryConvertUrlEncoding(url); - } - } - if (dest) { - data.dest = isName(dest) ? dest.name : dest; - } - } - - // Lets URLs beginning with 'www.' default to using the 'http://' protocol. - function addDefaultProtocolToUrl(url) { - if (isString(url) && url.indexOf('www.') === 0) { - return ('http://' + url); - } - return url; - } - - function tryConvertUrlEncoding(url) { - // According to ISO 32000-1:2008, section 12.6.4.7, URIs should be encoded - // in 7-bit ASCII. Some bad PDFs use UTF-8 encoding, see Bugzilla 1122280. - try { - return stringToUTF8String(url); - } catch (e) { - return url; - } + Catalog.parseDestDictionary({ + destDict: params.dict, + resultObj: data, + }); } Util.inherit(LinkAnnotation, Annotation, {}); diff --git a/src/core/obj.js b/src/core/obj.js index a839d95e38019..254f1d444b2fd 100644 --- a/src/core/obj.js +++ b/src/core/obj.js @@ -41,6 +41,7 @@ var createPromiseCapability = sharedUtil.createPromiseCapability; var error = sharedUtil.error; var info = sharedUtil.info; var isArray = sharedUtil.isArray; +var isBool = sharedUtil.isBool; var isInt = sharedUtil.isInt; var isString = sharedUtil.isString; var shadow = sharedUtil.shadow; @@ -152,23 +153,11 @@ var Catalog = (function CatalogClosure() { } assert(outlineDict.has('Title'), 'Invalid outline item'); - var actionDict = outlineDict.get('A'), dest = null, url = null; - if (actionDict) { - var destEntry = actionDict.get('D'); - if (destEntry) { - dest = destEntry; - } else { - var uriEntry = actionDict.get('URI'); - if (isString(uriEntry) && isValidUrl(uriEntry, false)) { - url = uriEntry; - } - } - } else if (outlineDict.has('Dest')) { - dest = outlineDict.getRaw('Dest'); - if (isName(dest)) { - dest = dest.name; - } - } + var data = { url: null, dest: null, }; + Catalog.parseDestDictionary({ + destDict: outlineDict, + resultObj: data, + }); var title = outlineDict.get('Title'); var flags = outlineDict.get('F') || 0; @@ -179,8 +168,9 @@ var Catalog = (function CatalogClosure() { rgbColor = ColorSpace.singletons.rgb.getRgb(color, 0); } var outlineItem = { - dest: dest, - url: url, + dest: data.dest, + url: data.url, + newWindow: data.newWindow, title: stringToPDFString(title), color: rgbColor, count: outlineDict.get('Count'), @@ -595,6 +585,111 @@ var Catalog = (function CatalogClosure() { } }; + /** + * Helper function used to parse the contents of destination dictionaries. + * @param {Dict} destDict - The dictionary containing the destination. + * @param {Object} resultObj - The object where the parsed destination + * properties will be placed. + */ + Catalog.parseDestDictionary = function Catalog_parseDestDictionary(params) { + // Lets URLs beginning with 'www.' default to using the 'http://' protocol. + function addDefaultProtocolToUrl(url) { + if (isString(url) && url.indexOf('www.') === 0) { + return ('http://' + url); + } + return url; + } + // According to ISO 32000-1:2008, section 12.6.4.7, URIs should be encoded + // in 7-bit ASCII. Some bad PDFs use UTF-8 encoding, see Bugzilla 1122280. + function tryConvertUrlEncoding(url) { + try { + return stringToUTF8String(url); + } catch (e) { + return url; + } + } + + var destDict = params.destDict; + var resultObj = params.resultObj; + + var action = destDict.get('A'), url, dest; + if (action && isDict(action)) { + var linkType = action.get('S').name; + switch (linkType) { + case 'URI': + url = action.get('URI'); + if (isName(url)) { + // Some bad PDFs do not put parentheses around relative URLs. + url = '/' + url.name; + } else if (url) { + url = addDefaultProtocolToUrl(url); + } + // TODO: pdf spec mentions urls can be relative to a Base + // entry in the dictionary. + break; + + case 'GoTo': + dest = action.get('D'); + break; + + case 'GoToR': + var urlDict = action.get('F'); + if (isDict(urlDict)) { + // We assume that we found a FileSpec dictionary + // and fetch the URL without checking any further. + url = urlDict.get('F') || null; + } else if (isString(urlDict)) { + url = urlDict; + } + + // NOTE: the destination is relative to the *remote* document. + var remoteDest = action.get('D'); + if (remoteDest) { + if (isName(remoteDest)) { + remoteDest = remoteDest.name; + } + if (isString(url)) { + var baseUrl = url.split('#')[0]; + if (isString(remoteDest)) { + // In practice, a named destination may contain only a number. + // If that happens, use the '#nameddest=' form to avoid the link + // redirecting to a page, instead of the correct destination. + url = baseUrl + '#' + + (/^\d+$/.test(remoteDest) ? 'nameddest=' : '') + remoteDest; + } else if (isArray(remoteDest)) { + url = baseUrl + '#' + JSON.stringify(remoteDest); + } + } + } + // The 'NewWindow' property, equal to `LinkTarget.BLANK`. + var newWindow = action.get('NewWindow'); + if (isBool(newWindow)) { + resultObj.newWindow = newWindow; + } + break; + + case 'Named': + resultObj.action = action.get('N').name; + break; + + default: + warn('Catalog_parseDestDictionary: Unrecognized link type "' + + linkType + '".'); + } + } else if (destDict.has('Dest')) { // Simple destination link. + dest = destDict.get('Dest'); + } + + if (url) { + if (isValidUrl(url, /* allowRelative = */ false)) { + resultObj.url = tryConvertUrlEncoding(url); + } + } + if (dest) { + resultObj.dest = isName(dest) ? dest.name : dest; + } + }; + return Catalog; })(); diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 1e0e03a6d5deb..f599f5ab28cb4 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -644,6 +644,7 @@ describe('api', function() { expect(typeof outlineItemTwo.title).toEqual('string'); expect(outlineItemTwo.dest).toEqual(null); expect(outlineItemTwo.url).toEqual('http://google.com'); + expect(outlineItemTwo.newWindow).toBeUndefined(); var outlineItemOne = outline[1]; expect(outlineItemOne.bold).toEqual(false); diff --git a/web/pdf_outline_viewer.js b/web/pdf_outline_viewer.js index 8d4eedf0cb1be..028e7a5eae30d 100644 --- a/web/pdf_outline_viewer.js +++ b/web/pdf_outline_viewer.js @@ -26,6 +26,8 @@ } }(this, function (exports, pdfjsLib) { +var PDFJS = pdfjsLib.PDFJS; + var DEFAULT_TITLE = '\u2013'; /** @@ -82,7 +84,10 @@ var PDFOutlineViewer = (function PDFOutlineViewerClosure() { */ _bindLink: function PDFOutlineViewer_bindLink(element, item) { if (item.url) { - pdfjsLib.addLinkAttributes(element, { url: item.url }); + pdfjsLib.addLinkAttributes(element, { + url: item.url, + target: (item.newWindow ? PDFJS.LinkTarget.BLANK : undefined), + }); return; } var linkService = this.linkService; From 42f07c62624361204fd1892e2a635c08b5d9c9f5 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 30 Sep 2016 16:32:22 +0200 Subject: [PATCH 2/4] [api-minor] Use the `new URL` constructor when validating URLs in annotations and the outline, as a complement to only checking the protocol, and add a bit more validation to `Catalog_parseDestDictionary` Note that this will automatically reject any relative URL. To make the API more useful to consumers, URLs that are rejected will be available via the `unsafeUrl` property in the data object returned by `PDFPageProxy_getAnnotations`. The patch also adds a bit more validation of the data for `Named` actions. --- src/core/obj.js | 40 +++++++++++++++++++++------ test/unit/annotation_layer_spec.js | 44 +++++++++++++++++++++++++++--- test/unit/api_spec.js | 2 +- 3 files changed, 73 insertions(+), 13 deletions(-) diff --git a/src/core/obj.js b/src/core/obj.js index 254f1d444b2fd..bbbfe232c56ed 100644 --- a/src/core/obj.js +++ b/src/core/obj.js @@ -594,7 +594,7 @@ var Catalog = (function CatalogClosure() { Catalog.parseDestDictionary = function Catalog_parseDestDictionary(params) { // Lets URLs beginning with 'www.' default to using the 'http://' protocol. function addDefaultProtocolToUrl(url) { - if (isString(url) && url.indexOf('www.') === 0) { + if (url.indexOf('www.') === 0) { return ('http://' + url); } return url; @@ -610,10 +610,18 @@ var Catalog = (function CatalogClosure() { } var destDict = params.destDict; + if (!isDict(destDict)) { + warn('Catalog_parseDestDictionary: "destDict" must be a dictionary.'); + return; + } var resultObj = params.resultObj; + if (typeof resultObj !== 'object') { + warn('Catalog_parseDestDictionary: "resultObj" must be an object.'); + return; + } var action = destDict.get('A'), url, dest; - if (action && isDict(action)) { + if (isDict(action)) { var linkType = action.get('S').name; switch (linkType) { case 'URI': @@ -621,7 +629,7 @@ var Catalog = (function CatalogClosure() { if (isName(url)) { // Some bad PDFs do not put parentheses around relative URLs. url = '/' + url.name; - } else if (url) { + } else if (isString(url)) { url = addDefaultProtocolToUrl(url); } // TODO: pdf spec mentions urls can be relative to a Base @@ -669,24 +677,40 @@ var Catalog = (function CatalogClosure() { break; case 'Named': - resultObj.action = action.get('N').name; + var namedAction = action.get('N'); + if (isName(namedAction)) { + resultObj.action = namedAction.name; + } break; default: warn('Catalog_parseDestDictionary: Unrecognized link type "' + linkType + '".'); + break; } } else if (destDict.has('Dest')) { // Simple destination link. dest = destDict.get('Dest'); } - if (url) { - if (isValidUrl(url, /* allowRelative = */ false)) { - resultObj.url = tryConvertUrlEncoding(url); + if (isString(url)) { + url = tryConvertUrlEncoding(url); + var absoluteUrl; + try { + absoluteUrl = new URL(url).href; + } catch (ex) { /* `new URL()` will throw on incorrect data. */ } + + if (isValidUrl(absoluteUrl, /* allowRelative = */ false)) { + resultObj.url = absoluteUrl; } + resultObj.unsafeUrl = url; } if (dest) { - resultObj.dest = isName(dest) ? dest.name : dest; + if (isName(dest)) { + dest = dest.name; + } + if (isString(dest) || isArray(dest)) { + resultObj.dest = dest; + } } }; diff --git a/test/unit/annotation_layer_spec.js b/test/unit/annotation_layer_spec.js index 89b02fe7312f6..276f5bf698d43 100644 --- a/test/unit/annotation_layer_spec.js +++ b/test/unit/annotation_layer_spec.js @@ -275,6 +275,8 @@ describe('Annotation layer', function() { expect(data.annotationType).toEqual(AnnotationType.LINK); expect(data.url).toEqual('http://www.ctan.org/tex-archive/info/lshort'); + expect(data.unsafeUrl).toEqual( + 'http://www.ctan.org/tex-archive/info/lshort'); expect(data.dest).toBeUndefined(); }); @@ -299,7 +301,8 @@ describe('Annotation layer', function() { var data = annotation.data; expect(data.annotationType).toEqual(AnnotationType.LINK); - expect(data.url).toEqual('http://www.hmrc.gov.uk'); + expect(data.url).toEqual('http://www.hmrc.gov.uk/'); + expect(data.unsafeUrl).toEqual('http://www.hmrc.gov.uk'); expect(data.dest).toBeUndefined(); }); @@ -331,6 +334,8 @@ describe('Annotation layer', function() { expect(data.annotationType).toEqual(AnnotationType.LINK); expect(data.url).toEqual( + new URL(stringToUTF8String('http://www.example.com/üöä')).href); + expect(data.unsafeUrl).toEqual( stringToUTF8String('http://www.example.com/üöä')); expect(data.dest).toBeUndefined(); }); @@ -356,6 +361,7 @@ describe('Annotation layer', function() { expect(data.annotationType).toEqual(AnnotationType.LINK); expect(data.url).toBeUndefined(); + expect(data.unsafeUrl).toBeUndefined(); expect(data.dest).toEqual('page.157'); }); @@ -382,7 +388,8 @@ describe('Annotation layer', function() { var data = annotation.data; expect(data.annotationType).toEqual(AnnotationType.LINK); - expect(data.url).toBeUndefined(); // ../../0013/001346/134685E.pdf#4.3 + expect(data.url).toBeUndefined(); + expect(data.unsafeUrl).toEqual('../../0013/001346/134685E.pdf#4.3'); expect(data.dest).toBeUndefined(); expect(data.newWindow).toEqual(true); }); @@ -410,6 +417,8 @@ describe('Annotation layer', function() { expect(data.annotationType).toEqual(AnnotationType.LINK); expect(data.url).toEqual('http://www.example.com/test.pdf#nameddest=15'); + expect(data.unsafeUrl).toEqual( + 'http://www.example.com/test.pdf#nameddest=15'); expect(data.dest).toBeUndefined(); expect(data.newWindow).toBeFalsy(); }); @@ -436,8 +445,10 @@ describe('Annotation layer', function() { var data = annotation.data; expect(data.annotationType).toEqual(AnnotationType.LINK); - expect(data.url).toEqual('http://www.example.com/test.pdf#' + - '[14,{"name":"XYZ"},null,298.043,null]'); + expect(data.url).toEqual(new URL('http://www.example.com/test.pdf#' + + '[14,{"name":"XYZ"},null,298.043,null]').href); + expect(data.unsafeUrl).toEqual('http://www.example.com/test.pdf#' + + '[14,{"name":"XYZ"},null,298.043,null]'); expect(data.dest).toBeUndefined(); expect(data.newWindow).toBeFalsy(); }); @@ -463,6 +474,7 @@ describe('Annotation layer', function() { expect(data.annotationType).toEqual(AnnotationType.LINK); expect(data.url).toBeUndefined(); + expect(data.unsafeUrl).toBeUndefined(); expect(data.action).toEqual('GoToPage'); }); @@ -482,8 +494,32 @@ describe('Annotation layer', function() { expect(data.annotationType).toEqual(AnnotationType.LINK); expect(data.url).toBeUndefined(); + expect(data.unsafeUrl).toBeUndefined(); expect(data.dest).toEqual('LI0'); }); + + it('should correctly parse a simple Dest, with explicit destination array', + function() { + var annotationDict = new Dict(); + annotationDict.set('Type', Name.get('Annot')); + annotationDict.set('Subtype', Name.get('Link')); + annotationDict.set('Dest', [new Ref(17, 0), Name.get('XYZ'), + 0, 841.89, null]); + + var annotationRef = new Ref(10, 0); + var xref = new XRefMock([ + { ref: annotationRef, data: annotationDict, } + ]); + + var annotation = annotationFactory.create(xref, annotationRef); + var data = annotation.data; + expect(data.annotationType).toEqual(AnnotationType.LINK); + + expect(data.url).toBeUndefined(); + expect(data.unsafeUrl).toBeUndefined(); + expect(data.dest).toEqual([{ num: 17, gen: 0, }, { name: 'XYZ' }, + 0, 841.89, null]); + }); }); describe('TextWidgetAnnotation', function() { diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index f599f5ab28cb4..e593a37250e7f 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -643,7 +643,7 @@ describe('api', function() { var outlineItemTwo = outline[2]; expect(typeof outlineItemTwo.title).toEqual('string'); expect(outlineItemTwo.dest).toEqual(null); - expect(outlineItemTwo.url).toEqual('http://google.com'); + expect(outlineItemTwo.url).toEqual('http://google.com/'); expect(outlineItemTwo.newWindow).toBeUndefined(); var outlineItemOne = outline[1]; From 71a781ee5c9e548c16d8e612f41fbc45f1082fc5 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 3 Oct 2016 14:35:29 +0200 Subject: [PATCH 3/4] Deprecate the `isValidUrl` utility function and replace it with `createValidAbsoluteUrl`/`isValidProtocal` functions instead, since the main URL validation is now done using the `new URL` constructor --- src/core/obj.js | 12 ++++------- src/display/dom_utils.js | 9 ++++++++ src/display/global.js | 2 +- src/main_loader.js | 4 ++-- src/pdf.js | 4 +++- src/shared/util.js | 46 +++++++++++++++++++++++++--------------- web/download_manager.js | 3 +-- 7 files changed, 49 insertions(+), 31 deletions(-) diff --git a/src/core/obj.js b/src/core/obj.js index bbbfe232c56ed..c8a564c7e9080 100644 --- a/src/core/obj.js +++ b/src/core/obj.js @@ -48,7 +48,7 @@ var shadow = sharedUtil.shadow; var stringToPDFString = sharedUtil.stringToPDFString; var stringToUTF8String = sharedUtil.stringToUTF8String; var warn = sharedUtil.warn; -var isValidUrl = sharedUtil.isValidUrl; +var createValidAbsoluteUrl = sharedUtil.createValidAbsoluteUrl; var Util = sharedUtil.Util; var Ref = corePrimitives.Ref; var RefSet = corePrimitives.RefSet; @@ -694,13 +694,9 @@ var Catalog = (function CatalogClosure() { if (isString(url)) { url = tryConvertUrlEncoding(url); - var absoluteUrl; - try { - absoluteUrl = new URL(url).href; - } catch (ex) { /* `new URL()` will throw on incorrect data. */ } - - if (isValidUrl(absoluteUrl, /* allowRelative = */ false)) { - resultObj.url = absoluteUrl; + var absoluteUrl = createValidAbsoluteUrl(url); + if (absoluteUrl) { + resultObj.url = absoluteUrl.href; } resultObj.unsafeUrl = url; } diff --git a/src/display/dom_utils.js b/src/display/dom_utils.js index a41eda2f8dadb..5894bf4d13816 100644 --- a/src/display/dom_utils.js +++ b/src/display/dom_utils.js @@ -28,6 +28,8 @@ var removeNullCharacters = sharedUtil.removeNullCharacters; var warn = sharedUtil.warn; +var deprecated = sharedUtil.deprecated; +var createValidAbsoluteUrl = sharedUtil.createValidAbsoluteUrl; /** * Optimised CSS custom property getter/setter. @@ -229,9 +231,16 @@ function isExternalLinkTargetSet() { } } +function isValidUrl(url, allowRelative) { + deprecated('isValidUrl(), please use createValidAbsoluteUrl() instead.'); + var baseUrl = allowRelative ? 'http://example.com' : null; + return createValidAbsoluteUrl(url, baseUrl) !== null; +} + exports.CustomStyle = CustomStyle; exports.addLinkAttributes = addLinkAttributes; exports.isExternalLinkTargetSet = isExternalLinkTargetSet; +exports.isValidUrl = isValidUrl; exports.getFilenameFromUrl = getFilenameFromUrl; exports.LinkTarget = LinkTarget; exports.hasCanvasTypedArrays = hasCanvasTypedArrays; diff --git a/src/display/global.js b/src/display/global.js index 04c451ab468c2..c5f9f23eb82ec 100644 --- a/src/display/global.js +++ b/src/display/global.js @@ -76,7 +76,7 @@ PDFJS.VERBOSITY_LEVELS = sharedUtil.VERBOSITY_LEVELS; PDFJS.OPS = sharedUtil.OPS; PDFJS.UNSUPPORTED_FEATURES = sharedUtil.UNSUPPORTED_FEATURES; - PDFJS.isValidUrl = sharedUtil.isValidUrl; + PDFJS.isValidUrl = displayDOMUtils.isValidUrl; PDFJS.shadow = sharedUtil.shadow; PDFJS.createBlob = sharedUtil.createBlob; PDFJS.createObjectURL = function PDFJS_createObjectURL(data, contentType) { diff --git a/src/main_loader.js b/src/main_loader.js index e6dc4920e2040..a1660908d9e25 100644 --- a/src/main_loader.js +++ b/src/main_loader.js @@ -55,12 +55,12 @@ exports.UnexpectedResponseException = sharedUtil.UnexpectedResponseException; exports.OPS = sharedUtil.OPS; exports.UNSUPPORTED_FEATURES = sharedUtil.UNSUPPORTED_FEATURES; - exports.isValidUrl = sharedUtil.isValidUrl; + exports.isValidUrl = displayDOMUtils.isValidUrl; + exports.createValidAbsoluteUrl = sharedUtil.createValidAbsoluteUrl; exports.createObjectURL = sharedUtil.createObjectURL; exports.removeNullCharacters = sharedUtil.removeNullCharacters; exports.shadow = sharedUtil.shadow; exports.createBlob = sharedUtil.createBlob; exports.getFilenameFromUrl = displayDOMUtils.getFilenameFromUrl; exports.addLinkAttributes = displayDOMUtils.addLinkAttributes; - })); diff --git a/src/pdf.js b/src/pdf.js index 941920320d4ae..0322f8b2acad0 100644 --- a/src/pdf.js +++ b/src/pdf.js @@ -64,7 +64,9 @@ exports.OPS = pdfjsLibs.pdfjsSharedUtil.OPS; exports.UNSUPPORTED_FEATURES = pdfjsLibs.pdfjsSharedUtil.UNSUPPORTED_FEATURES; - exports.isValidUrl = pdfjsLibs.pdfjsSharedUtil.isValidUrl; + exports.isValidUrl = pdfjsLibs.pdfjsDisplayDOMUtils.isValidUrl; + exports.createValidAbsoluteUrl = + pdfjsLibs.pdfjsSharedUtil.createValidAbsoluteUrl; exports.createObjectURL = pdfjsLibs.pdfjsSharedUtil.createObjectURL; exports.removeNullCharacters = pdfjsLibs.pdfjsSharedUtil.removeNullCharacters; diff --git a/src/shared/util.js b/src/shared/util.js index a9554fca769df..de33f2be943f8 100644 --- a/src/shared/util.js +++ b/src/shared/util.js @@ -332,30 +332,42 @@ function isSameOrigin(baseUrl, otherUrl) { return base.origin === other.origin; } -// Validates if URL is safe and allowed, e.g. to avoid XSS. -function isValidUrl(url, allowRelative) { - if (!url || typeof url !== 'string') { +// Checks if URLs use one of the whitelisted protocols, e.g. to avoid XSS. +function isValidProtocol(url) { + if (!url) { return false; } - // RFC 3986 (http://tools.ietf.org/html/rfc3986#section-3.1) - // scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." ) - var protocol = /^[a-z][a-z0-9+\-.]*(?=:)/i.exec(url); - if (!protocol) { - return allowRelative; - } - protocol = protocol[0].toLowerCase(); - switch (protocol) { - case 'http': - case 'https': - case 'ftp': - case 'mailto': - case 'tel': + switch (url.protocol) { + case 'http:': + case 'https:': + case 'ftp:': + case 'mailto:': + case 'tel:': return true; default: return false; } } +/** + * Attempts to create a valid absolute URL (utilizing `isValidProtocol`). + * @param {URL|string} url - An absolute, or relative, URL. + * @param {URL|string} baseUrl - An absolute URL. + * @returns Either a valid {URL}, or `null` otherwise. + */ +function createValidAbsoluteUrl(url, baseUrl) { + if (!url) { + return null; + } + try { + var absoluteUrl = baseUrl ? new URL(url, baseUrl) : new URL(url); + if (isValidProtocol(absoluteUrl)) { + return absoluteUrl; + } + } catch (ex) { /* `new URL()` will throw on incorrect data. */ } + return null; +} + function shadow(obj, prop, value) { Object.defineProperty(obj, prop, { value: value, enumerable: true, @@ -2431,7 +2443,7 @@ exports.isNum = isNum; exports.isString = isString; exports.isSpace = isSpace; exports.isSameOrigin = isSameOrigin; -exports.isValidUrl = isValidUrl; +exports.createValidAbsoluteUrl = createValidAbsoluteUrl; exports.isLittleEndian = isLittleEndian; exports.isEvalSupported = isEvalSupported; exports.loadJpegStream = loadJpegStream; diff --git a/web/download_manager.js b/web/download_manager.js index e58f046c1687b..75c1510388b74 100644 --- a/web/download_manager.js +++ b/web/download_manager.js @@ -67,10 +67,9 @@ if (typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC || CHROME')) { DownloadManager.prototype = { downloadUrl: function DownloadManager_downloadUrl(url, filename) { - if (!pdfjsLib.isValidUrl(url, true)) { + if (!pdfjsLib.createValidAbsoluteUrl(url, 'http://example.com')) { return; // restricted/invalid URL } - download(url + '#pdfjs.action=download', filename); }, From d284cfd5ebe7b6b5d3961f8c5d3a477456ebb516 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 1 Oct 2016 12:05:07 +0200 Subject: [PATCH 4/4] [api-minor] Add support for relative URLs, in both annotations and the outline, by adding a `docBaseUrl` parameter to `PDFJS.getDocument` (bug 766086) Note that in `FIREFOX/MOZCENTRAL/CHROME` builds of the standard viewer the `docBaseUrl` parameter will be set by default, since in that case it makes sense to use the current URL as a base. For the `GENERIC` viewer, or the API itself, it doesn't make sense to try and set the `docBaseUrl` by default. However, custom deployments/implementations may still find the parameter useful. --- src/core/annotation.js | 5 +- src/core/document.js | 1 + src/core/obj.js | 6 +- src/core/pdf_manager.js | 24 +++++- src/core/worker.js | 7 +- src/display/api.js | 4 + test/pdfs/.gitignore | 1 + test/pdfs/bug766086.pdf | Bin 0 -> 860 bytes test/unit/annotation_layer_spec.js | 128 ++++++++++++++++++++++------- test/unit/api_spec.js | 62 ++++++++++++++ web/app.js | 28 ++++--- 11 files changed, 219 insertions(+), 47 deletions(-) create mode 100644 test/pdfs/bug766086.pdf diff --git a/src/core/annotation.js b/src/core/annotation.js index 37c3315966070..c0338cd749b54 100644 --- a/src/core/annotation.js +++ b/src/core/annotation.js @@ -64,11 +64,12 @@ AnnotationFactory.prototype = /** @lends AnnotationFactory.prototype */ { /** * @param {XRef} xref * @param {Object} ref + * @param {PDFManager} pdfManager * @param {string} uniquePrefix * @param {Object} idCounters * @returns {Annotation} */ - create: function AnnotationFactory_create(xref, ref, + create: function AnnotationFactory_create(xref, ref, pdfManager, uniquePrefix, idCounters) { var dict = xref.fetchIfRef(ref); if (!isDict(dict)) { @@ -88,6 +89,7 @@ AnnotationFactory.prototype = /** @lends AnnotationFactory.prototype */ { ref: isRef(ref) ? ref : null, subtype: subtype, id: id, + pdfManager: pdfManager, }; switch (subtype) { @@ -846,6 +848,7 @@ var LinkAnnotation = (function LinkAnnotationClosure() { Catalog.parseDestDictionary({ destDict: params.dict, resultObj: data, + docBaseUrl: params.pdfManager.docBaseUrl, }); } diff --git a/src/core/document.js b/src/core/document.js index 710597a4dfc42..a4aad52898f3f 100644 --- a/src/core/document.js +++ b/src/core/document.js @@ -329,6 +329,7 @@ var Page = (function PageClosure() { for (var i = 0, n = annotationRefs.length; i < n; ++i) { var annotationRef = annotationRefs[i]; var annotation = annotationFactory.create(this.xref, annotationRef, + this.pdfManager, this.uniquePrefix, this.idCounters); if (annotation) { diff --git a/src/core/obj.js b/src/core/obj.js index c8a564c7e9080..7c47b6ea88b12 100644 --- a/src/core/obj.js +++ b/src/core/obj.js @@ -157,6 +157,7 @@ var Catalog = (function CatalogClosure() { Catalog.parseDestDictionary({ destDict: outlineDict, resultObj: data, + docBaseUrl: this.pdfManager.docBaseUrl, }); var title = outlineDict.get('Title'); var flags = outlineDict.get('F') || 0; @@ -590,6 +591,8 @@ var Catalog = (function CatalogClosure() { * @param {Dict} destDict - The dictionary containing the destination. * @param {Object} resultObj - The object where the parsed destination * properties will be placed. + * @param {string} docBaseUrl - (optional) The document base URL that is used + * when attempting to recover valid absolute URLs from relative ones. */ Catalog.parseDestDictionary = function Catalog_parseDestDictionary(params) { // Lets URLs beginning with 'www.' default to using the 'http://' protocol. @@ -619,6 +622,7 @@ var Catalog = (function CatalogClosure() { warn('Catalog_parseDestDictionary: "resultObj" must be an object.'); return; } + var docBaseUrl = params.docBaseUrl || null; var action = destDict.get('A'), url, dest; if (isDict(action)) { @@ -694,7 +698,7 @@ var Catalog = (function CatalogClosure() { if (isString(url)) { url = tryConvertUrlEncoding(url); - var absoluteUrl = createValidAbsoluteUrl(url); + var absoluteUrl = createValidAbsoluteUrl(url, docBaseUrl); if (absoluteUrl) { resultObj.url = absoluteUrl.href; } diff --git a/src/core/pdf_manager.js b/src/core/pdf_manager.js index bd5f7933d52fc..15656da0c35d8 100644 --- a/src/core/pdf_manager.js +++ b/src/core/pdf_manager.js @@ -31,6 +31,9 @@ }(this, function (exports, sharedUtil, coreStream, coreChunkedStream, coreDocument) { +var warn = sharedUtil.warn; +var createValidAbsoluteUrl = sharedUtil.createValidAbsoluteUrl; +var shadow = sharedUtil.shadow; var NotImplementedException = sharedUtil.NotImplementedException; var MissingDataException = sharedUtil.MissingDataException; var createPromiseCapability = sharedUtil.createPromiseCapability; @@ -49,6 +52,19 @@ var BasePdfManager = (function BasePdfManagerClosure() { return this._docId; }, + get docBaseUrl() { + var docBaseUrl = null; + if (this._docBaseUrl) { + var absoluteUrl = createValidAbsoluteUrl(this._docBaseUrl); + if (absoluteUrl) { + docBaseUrl = absoluteUrl.href; + } else { + warn('Invalid absolute docBaseUrl: "' + this._docBaseUrl + '".'); + } + } + return shadow(this, 'docBaseUrl', docBaseUrl); + }, + onLoadedStream: function BasePdfManager_onLoadedStream() { throw new NotImplementedException(); }, @@ -110,8 +126,10 @@ var BasePdfManager = (function BasePdfManagerClosure() { })(); var LocalPdfManager = (function LocalPdfManagerClosure() { - function LocalPdfManager(docId, data, password, evaluatorOptions) { + function LocalPdfManager(docId, data, password, evaluatorOptions, + docBaseUrl) { this._docId = docId; + this._docBaseUrl = docBaseUrl; this.evaluatorOptions = evaluatorOptions; var stream = new Stream(data); this.pdfDocument = new PDFDocument(this, stream, password); @@ -158,8 +176,10 @@ var LocalPdfManager = (function LocalPdfManagerClosure() { })(); var NetworkPdfManager = (function NetworkPdfManagerClosure() { - function NetworkPdfManager(docId, pdfNetworkStream, args, evaluatorOptions) { + function NetworkPdfManager(docId, pdfNetworkStream, args, evaluatorOptions, + docBaseUrl) { this._docId = docId; + this._docBaseUrl = docBaseUrl; this.msgHandler = args.msgHandler; this.evaluatorOptions = evaluatorOptions; diff --git a/src/core/worker.js b/src/core/worker.js index 423bb08627ca3..96746197d9f27 100644 --- a/src/core/worker.js +++ b/src/core/worker.js @@ -480,6 +480,7 @@ var WorkerMessageHandler = { var WorkerTasks = []; var docId = docParams.docId; + var docBaseUrl = docParams.docBaseUrl; var workerHandlerName = docParams.docId + '_worker'; var handler = new MessageHandler(workerHandlerName, docId, port); @@ -544,7 +545,7 @@ var WorkerMessageHandler = { if (source.data) { try { pdfManager = new LocalPdfManager(docId, source.data, source.password, - evaluatorOptions); + evaluatorOptions, docBaseUrl); pdfManagerCapability.resolve(pdfManager); } catch (ex) { pdfManagerCapability.reject(ex); @@ -593,7 +594,7 @@ var WorkerMessageHandler = { length: fullRequest.contentLength, disableAutoFetch: disableAutoFetch, rangeChunkSize: source.rangeChunkSize - }, evaluatorOptions); + }, evaluatorOptions, docBaseUrl); pdfManagerCapability.resolve(pdfManager); cancelXHRs = null; }).catch(function (reason) { @@ -610,7 +611,7 @@ var WorkerMessageHandler = { // the data is array, instantiating directly from it try { pdfManager = new LocalPdfManager(docId, pdfFile, source.password, - evaluatorOptions); + evaluatorOptions, docBaseUrl); pdfManagerCapability.resolve(pdfManager); } catch (ex) { pdfManagerCapability.reject(ex); diff --git a/src/display/api.js b/src/display/api.js index d695f1c2cb190..425dfbc900441 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -129,6 +129,9 @@ if (typeof PDFJSDev !== 'undefined' && * 2^16 = 65536. * @property {PDFWorker} worker - The worker that will be used for the loading * and parsing of the PDF data. + * @property {string} docBaseUrl - (optional) The base URL of the document, + * used when attempting to recover valid absolute URLs for annotations, and + * outline items, that (incorrectly) only specify relative URLs. */ /** @@ -301,6 +304,7 @@ function _fetchDocument(worker, source, pdfDataRangeTransport, docId) { disableCreateObjectURL: getDefaultSetting('disableCreateObjectURL'), postMessageTransfers: getDefaultSetting('postMessageTransfers') && !isPostMessageTransfersDisabled, + docBaseUrl: source.docBaseUrl, }).then(function (workerId) { if (worker.destroyed) { throw new Error('Worker was destroyed'); diff --git a/test/pdfs/.gitignore b/test/pdfs/.gitignore index 5559a84b4c632..f07e5921fe392 100644 --- a/test/pdfs/.gitignore +++ b/test/pdfs/.gitignore @@ -46,6 +46,7 @@ !arial_unicode_ab_cidfont.pdf !arial_unicode_en_cidfont.pdf !asciihexdecode.pdf +!bug766086.pdf !bug793632.pdf !bug1020858.pdf !bug1050040.pdf diff --git a/test/pdfs/bug766086.pdf b/test/pdfs/bug766086.pdf new file mode 100644 index 0000000000000000000000000000000000000000..42900a21bde8e9d27891691139b7aeea6fdf9aa1 GIT binary patch literal 860 zcmZWoL2lbH5WMdz_F}*}*d!H^N`WAN9os!LMGZ$l4@M7+MXOA0GLYyZ?I(N45897( zNn47M5QHQwcV}jYtHE-%7>RKKgTMd&ynsNC{^1ef`Wo0$*QUjcLMve9uP1}-RtLTF zHQd~QaaGEU=Y7d1TeY~pj3wWI-TH$ID7xZ=cU~Iv`qHIt3}v6_CeHalQ27b!v#G3} z`e$5oie#Ll1dM_7YM#Zm7=G1qM~y_k*yar^-X1( zBcUL!`@=&>Cfr%~gAj2Y-HIwECr%uW$BZ`TnMe<*SSF+@8Z7+b$a400bk8Vyj z!For?$UgegC$Zg8>_7Im3AmOhIEoJ(b7DKQK6dyjSGOkh&q}*D?P%rqI%0RKBPCtM z+--biU5)IkbraXxF3^dJ80RF>u%6`3xH^1Cc@;C-&=|dkX$d4R<5*vAAl9v8aT zm?E8TVj_E=mlbqnZ}Rf|eV)wE@Is^Q&NPrFYvn!7G?_v_#l8J)NNe&!HzA2vQo&#_ H|FVGp-ILv4 literal 0 HcmV?d00001 diff --git a/test/unit/annotation_layer_spec.js b/test/unit/annotation_layer_spec.js index 276f5bf698d43..16d461716dc18 100644 --- a/test/unit/annotation_layer_spec.js +++ b/test/unit/annotation_layer_spec.js @@ -27,15 +27,24 @@ describe('Annotation layer', function() { }, }; - var annotationFactory; + function PDFManagerMock(params) { + this.docBaseUrl = params.docBaseUrl || null; + } + PDFManagerMock.prototype = {}; + + var annotationFactory, pdfManagerMock; beforeAll(function (done) { annotationFactory = new AnnotationFactory(); + pdfManagerMock = new PDFManagerMock({ + docBaseUrl: null, + }); done(); }); afterAll(function () { annotationFactory = null; + pdfManagerMock = null; }); describe('AnnotationFactory', function () { @@ -49,7 +58,8 @@ describe('Annotation layer', function() { { ref: annotationRef, data: annotationDict, } ]); - var annotation = annotationFactory.create(xref, annotationRef); + var annotation = annotationFactory.create(xref, annotationRef, + pdfManagerMock); var data = annotation.data; expect(data.annotationType).toEqual(AnnotationType.LINK); @@ -66,8 +76,10 @@ describe('Annotation layer', function() { var uniquePrefix = 'p0_', idCounters = { obj: 0, }; var annotation1 = annotationFactory.create(xref, annotationDict, + pdfManagerMock, uniquePrefix, idCounters); var annotation2 = annotationFactory.create(xref, annotationDict, + pdfManagerMock, uniquePrefix, idCounters); var data1 = annotation1.data, data2 = annotation2.data; expect(data1.annotationType).toEqual(AnnotationType.LINK); @@ -86,7 +98,8 @@ describe('Annotation layer', function() { { ref: annotationRef, data: annotationDict, } ]); - var annotation = annotationFactory.create(xref, annotationRef); + var annotation = annotationFactory.create(xref, annotationRef, + pdfManagerMock); var data = annotation.data; expect(data.annotationType).toBeUndefined(); }); @@ -270,7 +283,8 @@ describe('Annotation layer', function() { { ref: annotationRef, data: annotationDict, } ]); - var annotation = annotationFactory.create(xref, annotationRef); + var annotation = annotationFactory.create(xref, annotationRef, + pdfManagerMock); var data = annotation.data; expect(data.annotationType).toEqual(AnnotationType.LINK); @@ -297,7 +311,8 @@ describe('Annotation layer', function() { { ref: annotationRef, data: annotationDict, } ]); - var annotation = annotationFactory.create(xref, annotationRef); + var annotation = annotationFactory.create(xref, annotationRef, + pdfManagerMock); var data = annotation.data; expect(data.annotationType).toEqual(AnnotationType.LINK); @@ -329,7 +344,8 @@ describe('Annotation layer', function() { { ref: annotationRef, data: annotationDict, } ]); - var annotation = annotationFactory.create(xref, annotationRef); + var annotation = annotationFactory.create(xref, annotationRef, + pdfManagerMock); var data = annotation.data; expect(data.annotationType).toEqual(AnnotationType.LINK); @@ -356,7 +372,8 @@ describe('Annotation layer', function() { { ref: annotationRef, data: annotationDict, } ]); - var annotation = annotationFactory.create(xref, annotationRef); + var annotation = annotationFactory.create(xref, annotationRef, + pdfManagerMock); var data = annotation.data; expect(data.annotationType).toEqual(AnnotationType.LINK); @@ -384,7 +401,8 @@ describe('Annotation layer', function() { { ref: annotationRef, data: annotationDict, } ]); - var annotation = annotationFactory.create(xref, annotationRef); + var annotation = annotationFactory.create(xref, annotationRef, + pdfManagerMock); var data = annotation.data; expect(data.annotationType).toEqual(AnnotationType.LINK); @@ -394,6 +412,38 @@ describe('Annotation layer', function() { expect(data.newWindow).toEqual(true); }); + it('should correctly parse a GoToR action, containing a relative URL, ' + + 'with the "docBaseUrl" parameter specified', function() { + var actionDict = new Dict(); + actionDict.set('Type', Name.get('Action')); + actionDict.set('S', Name.get('GoToR')); + actionDict.set('F', '../../0013/001346/134685E.pdf'); + actionDict.set('D', '4.3'); + + var annotationDict = new Dict(); + annotationDict.set('Type', Name.get('Annot')); + annotationDict.set('Subtype', Name.get('Link')); + annotationDict.set('A', actionDict); + + var annotationRef = new Ref(489, 0); + var xref = new XRefMock([ + { ref: annotationRef, data: annotationDict, } + ]); + var pdfManager = new PDFManagerMock({ + docBaseUrl: 'http://www.example.com/test/pdfs/qwerty.pdf', + }); + + var annotation = annotationFactory.create(xref, annotationRef, + pdfManager); + var data = annotation.data; + expect(data.annotationType).toEqual(AnnotationType.LINK); + + expect(data.url).toEqual( + 'http://www.example.com/0013/001346/134685E.pdf#4.3'); + expect(data.unsafeUrl).toEqual('../../0013/001346/134685E.pdf#4.3'); + expect(data.dest).toBeUndefined(); + }); + it('should correctly parse a GoToR action, with named destination', function() { var actionDict = new Dict(); @@ -412,7 +462,8 @@ describe('Annotation layer', function() { { ref: annotationRef, data: annotationDict, } ]); - var annotation = annotationFactory.create(xref, annotationRef); + var annotation = annotationFactory.create(xref, annotationRef, + pdfManagerMock); var data = annotation.data; expect(data.annotationType).toEqual(AnnotationType.LINK); @@ -441,7 +492,8 @@ describe('Annotation layer', function() { { ref: annotationRef, data: annotationDict, } ]); - var annotation = annotationFactory.create(xref, annotationRef); + var annotation = annotationFactory.create(xref, annotationRef, + pdfManagerMock); var data = annotation.data; expect(data.annotationType).toEqual(AnnotationType.LINK); @@ -469,7 +521,8 @@ describe('Annotation layer', function() { { ref: annotationRef, data: annotationDict, } ]); - var annotation = annotationFactory.create(xref, annotationRef); + var annotation = annotationFactory.create(xref, annotationRef, + pdfManagerMock); var data = annotation.data; expect(data.annotationType).toEqual(AnnotationType.LINK); @@ -489,7 +542,8 @@ describe('Annotation layer', function() { { ref: annotationRef, data: annotationDict, } ]); - var annotation = annotationFactory.create(xref, annotationRef); + var annotation = annotationFactory.create(xref, annotationRef, + pdfManagerMock); var data = annotation.data; expect(data.annotationType).toEqual(AnnotationType.LINK); @@ -511,7 +565,8 @@ describe('Annotation layer', function() { { ref: annotationRef, data: annotationDict, } ]); - var annotation = annotationFactory.create(xref, annotationRef); + var annotation = annotationFactory.create(xref, annotationRef, + pdfManagerMock); var data = annotation.data; expect(data.annotationType).toEqual(AnnotationType.LINK); @@ -545,7 +600,8 @@ describe('Annotation layer', function() { { ref: textWidgetRef, data: textWidgetDict, } ]); - var textWidgetAnnotation = annotationFactory.create(xref, textWidgetRef); + var textWidgetAnnotation = annotationFactory.create(xref, textWidgetRef, + pdfManagerMock); expect(textWidgetAnnotation.data.textAlignment).toEqual(null); expect(textWidgetAnnotation.data.maxLen).toEqual(null); expect(textWidgetAnnotation.data.readOnly).toEqual(false); @@ -564,7 +620,8 @@ describe('Annotation layer', function() { { ref: textWidgetRef, data: textWidgetDict, } ]); - var textWidgetAnnotation = annotationFactory.create(xref, textWidgetRef); + var textWidgetAnnotation = annotationFactory.create(xref, textWidgetRef, + pdfManagerMock); expect(textWidgetAnnotation.data.textAlignment).toEqual(null); expect(textWidgetAnnotation.data.maxLen).toEqual(null); expect(textWidgetAnnotation.data.readOnly).toEqual(false); @@ -584,7 +641,8 @@ describe('Annotation layer', function() { { ref: textWidgetRef, data: textWidgetDict, } ]); - var textWidgetAnnotation = annotationFactory.create(xref, textWidgetRef); + var textWidgetAnnotation = annotationFactory.create(xref, textWidgetRef, + pdfManagerMock); expect(textWidgetAnnotation.data.textAlignment).toEqual(1); expect(textWidgetAnnotation.data.maxLen).toEqual(20); expect(textWidgetAnnotation.data.readOnly).toEqual(true); @@ -599,7 +657,8 @@ describe('Annotation layer', function() { { ref: textWidgetRef, data: textWidgetDict, } ]); - var textWidgetAnnotation = annotationFactory.create(xref, textWidgetRef); + var textWidgetAnnotation = annotationFactory.create(xref, textWidgetRef, + pdfManagerMock); expect(textWidgetAnnotation.data.comb).toEqual(false); }); @@ -612,7 +671,8 @@ describe('Annotation layer', function() { { ref: textWidgetRef, data: textWidgetDict, } ]); - var textWidgetAnnotation = annotationFactory.create(xref, textWidgetRef); + var textWidgetAnnotation = annotationFactory.create(xref, textWidgetRef, + pdfManagerMock); expect(textWidgetAnnotation.data.comb).toEqual(true); }); @@ -636,9 +696,8 @@ describe('Annotation layer', function() { { ref: textWidgetRef, data: textWidgetDict, } ]); - var textWidgetAnnotation = annotationFactory.create(xref, - textWidgetRef); - + var textWidgetAnnotation = annotationFactory.create(xref, textWidgetRef, + pdfManagerMock); var valid = (invalidFieldFlags.length === 0); expect(textWidgetAnnotation.data.comb).toEqual(valid); @@ -673,7 +732,8 @@ describe('Annotation layer', function() { ]); var choiceWidgetAnnotation = annotationFactory.create(xref, - choiceWidgetRef); + choiceWidgetRef, + pdfManagerMock); var data = choiceWidgetAnnotation.data; expect(data.annotationType).toEqual(AnnotationType.WIDGET); expect(data.options).toEqual([]); @@ -700,7 +760,8 @@ describe('Annotation layer', function() { ]); var choiceWidgetAnnotation = annotationFactory.create(xref, - choiceWidgetRef); + choiceWidgetRef, + pdfManagerMock); var data = choiceWidgetAnnotation.data; expect(data.annotationType).toEqual(AnnotationType.WIDGET); expect(data.options).toEqual(expected); @@ -727,7 +788,8 @@ describe('Annotation layer', function() { ]); var choiceWidgetAnnotation = annotationFactory.create(xref, - choiceWidgetRef); + choiceWidgetRef, + pdfManagerMock); var data = choiceWidgetAnnotation.data; expect(data.annotationType).toEqual(AnnotationType.WIDGET); expect(data.options).toEqual(expected); @@ -744,7 +806,8 @@ describe('Annotation layer', function() { ]); var choiceWidgetAnnotation = annotationFactory.create(xref, - choiceWidgetRef); + choiceWidgetRef, + pdfManagerMock); var data = choiceWidgetAnnotation.data; expect(data.annotationType).toEqual(AnnotationType.WIDGET); expect(data.fieldValue).toEqual(fieldValue); @@ -761,7 +824,8 @@ describe('Annotation layer', function() { ]); var choiceWidgetAnnotation = annotationFactory.create(xref, - choiceWidgetRef); + choiceWidgetRef, + pdfManagerMock); var data = choiceWidgetAnnotation.data; expect(data.annotationType).toEqual(AnnotationType.WIDGET); expect(data.fieldValue).toEqual([fieldValue]); @@ -774,7 +838,8 @@ describe('Annotation layer', function() { ]); var choiceWidgetAnnotation = annotationFactory.create(xref, - choiceWidgetRef); + choiceWidgetRef, + pdfManagerMock); var data = choiceWidgetAnnotation.data; expect(data.annotationType).toEqual(AnnotationType.WIDGET); expect(data.readOnly).toEqual(false); @@ -791,7 +856,8 @@ describe('Annotation layer', function() { ]); var choiceWidgetAnnotation = annotationFactory.create(xref, - choiceWidgetRef); + choiceWidgetRef, + pdfManagerMock); var data = choiceWidgetAnnotation.data; expect(data.annotationType).toEqual(AnnotationType.WIDGET); expect(data.readOnly).toEqual(false); @@ -810,7 +876,8 @@ describe('Annotation layer', function() { ]); var choiceWidgetAnnotation = annotationFactory.create(xref, - choiceWidgetRef); + choiceWidgetRef, + pdfManagerMock); var data = choiceWidgetAnnotation.data; expect(data.annotationType).toEqual(AnnotationType.WIDGET); expect(data.readOnly).toEqual(true); @@ -869,7 +936,8 @@ describe('Annotation layer', function() { { ref: popupRef, data: popupDict, } ]); - var popupAnnotation = annotationFactory.create(xref, popupRef); + var popupAnnotation = annotationFactory.create(xref, popupRef, + pdfManagerMock); var data = popupAnnotation.data; expect(data.annotationType).toEqual(AnnotationType.POPUP); diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index e593a37250e7f..45d4843327b7c 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -789,6 +789,68 @@ describe('api', function() { done.fail(reason); }); }); + + it('gets annotations containing relative URLs (bug 766086)', + function (done) { + var url = new URL('../pdfs/bug766086.pdf', window.location).href; + + var defaultLoadingTask = PDFJS.getDocument(url); + var defaultPromise = defaultLoadingTask.promise.then(function (pdfDoc) { + return pdfDoc.getPage(1).then(function (pdfPage) { + return pdfPage.getAnnotations(); + }); + }); + + var docBaseUrlLoadingTask = PDFJS.getDocument({ + url: url, + docBaseUrl: 'http://www.example.com/test/pdfs/qwerty.pdf', + }); + var docBaseUrlPromise = docBaseUrlLoadingTask.promise.then( + function (pdfDoc) { + return pdfDoc.getPage(1).then(function (pdfPage) { + return pdfPage.getAnnotations(); + }); + }); + + var invalidDocBaseUrlLoadingTask = PDFJS.getDocument({ + url: url, + docBaseUrl: 'qwerty.pdf', + }); + var invalidDocBaseUrlPromise = invalidDocBaseUrlLoadingTask.promise.then( + function (pdfDoc) { + return pdfDoc.getPage(1).then(function (pdfPage) { + return pdfPage.getAnnotations(); + }); + }); + + Promise.all([defaultPromise, docBaseUrlPromise, + invalidDocBaseUrlPromise]).then(function (data) { + var defaultAnnotations = data[0]; + var docBaseUrlAnnotations = data[1]; + var invalidDocBaseUrlAnnotations = data[2]; + + expect(defaultAnnotations[0].url).toBeUndefined(); + expect(defaultAnnotations[0].unsafeUrl).toEqual( + '../../0021/002156/215675E.pdf#nameddest=15'); + + expect(docBaseUrlAnnotations[0].url).toEqual( + 'http://www.example.com/0021/002156/215675E.pdf#nameddest=15'); + expect(docBaseUrlAnnotations[0].unsafeUrl).toEqual( + '../../0021/002156/215675E.pdf#nameddest=15'); + + expect(invalidDocBaseUrlAnnotations[0].url).toBeUndefined(); + expect(invalidDocBaseUrlAnnotations[0].unsafeUrl).toEqual( + '../../0021/002156/215675E.pdf#nameddest=15'); + + defaultLoadingTask.destroy(); + docBaseUrlLoadingTask.destroy(); + invalidDocBaseUrlLoadingTask.destroy(); + done(); + }).catch(function (reason) { + done.fail(reason); + }); + }); + it('gets text content', function (done) { var defaultPromise = page.getTextContent(); var parametersPromise = page.getTextContent({ diff --git a/web/app.js b/web/app.js index eb3cbe202d1ad..73cc29911f8b5 100644 --- a/web/app.js +++ b/web/app.js @@ -180,6 +180,7 @@ var PDFViewerApplication = { preferenceDefaultZoomValue: '', isViewerEmbedded: (window.parent !== window), url: '', + baseUrl: '', externalServices: DefaultExernalServices, // called once when the document is loaded @@ -522,6 +523,7 @@ var PDFViewerApplication = { setTitleUsingUrl: function pdfViewSetTitleUsingUrl(url) { this.url = url; + this.baseUrl = url.split('#')[0]; try { this.setTitle(decodeURIComponent( pdfjsLib.getFilenameFromUrl(url)) || url); @@ -614,6 +616,11 @@ var PDFViewerApplication = { this.setTitleUsingUrl(file.originalUrl); parameters.url = file.url; } + if (typeof PDFJSDev !== 'undefined' && + PDFJSDev.test('FIREFOX || MOZCENTRAL || CHROME')) { + parameters.docBaseUrl = this.baseUrl; + } + if (args) { for (var prop in args) { parameters[prop] = args[prop]; @@ -682,7 +689,7 @@ var PDFViewerApplication = { downloadManager.downloadUrl(url, filename); } - var url = this.url.split('#')[0]; + var url = this.baseUrl; var filename = getPDFFileNameFromURL(url); var downloadManager = this.downloadManager; downloadManager.onerror = function (err) { @@ -719,14 +726,15 @@ var PDFViewerApplication = { return; } this.fellback = true; - var url = this.url.split('#')[0]; - this.externalServices.fallback({ featureId: featureId, url: url }, - function response(download) { - if (!download) { - return; - } - PDFViewerApplication.download(); - }); + this.externalServices.fallback({ + featureId: featureId, + url: this.baseUrl, + }, function response(download) { + if (!download) { + return; + } + PDFViewerApplication.download(); + }); } }, @@ -856,7 +864,7 @@ var PDFViewerApplication = { if (typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) { baseDocumentUrl = null; } else if (PDFJSDev.test('FIREFOX || MOZCENTRAL')) { - baseDocumentUrl = this.url.split('#')[0]; + baseDocumentUrl = this.baseUrl; } else if (PDFJSDev.test('CHROME')) { baseDocumentUrl = location.href.split('#')[0]; }