Merge pull request #7116 from Snuffleupagus/refactor-LinkAnnotation-tests

Refactor `LinkAnnotation` slightly, improve handling of the `GoToR` action, and add unit-tests

src/core/annotation.js

@@ -37,6 +37,8 @@ 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;
@@ -705,11 +707,12 @@ var LinkAnnotation = (function LinkAnnotationClosure() {
     var data = this.data;
     data.annotationType = AnnotationType.LINK;
 
-    var action = dict.get('A');
+    var action = dict.get('A'), url, dest;
     if (action && isDict(action)) {
       var linkType = action.get('S').name;
-      if (linkType === 'URI') {
+      switch (linkType) {
-        var url = action.get('URI');
+        case 'URI':
+          url = action.get('URI');
           if (isName(url)) {
             // Some bad PDFs do not put parentheses around relative URLs.
             url = '/' + url.name;
@@ -718,55 +721,79 @@ var LinkAnnotation = (function LinkAnnotationClosure() {
           }
           // TODO: pdf spec mentions urls can be relative to a Base
           // entry in the dictionary.
-        if (!isValidUrl(url, false)) {
+          break;
-          url = '';
+
-        }
+        case 'GoTo':
-        // According to ISO 32000-1:2008, section 12.6.4.7,
+          dest = action.get('D');
-        // URI should to be encoded in 7-bit ASCII.
+          break;
-        // Some bad PDFs may have URIs in UTF-8 encoding, see Bugzilla 1122280.
+
-        try {
+        case 'GoToR':
-          data.url = stringToUTF8String(url);
-        } catch (e) {
-          // Fall back to a simple copy.
-          data.url = url;
-        }
-      } else if (linkType === 'GoTo') {
-        data.dest = action.get('D');
-      } else if (linkType === 'GoToR') {
           var urlDict = action.get('F');
           if (isDict(urlDict)) {
-          // We assume that the 'url' is a Filspec dictionary
+            // We assume that we found a FileSpec dictionary
-          // and fetch the url without checking any further
+            // and fetch the URL without checking any further.
-          url = urlDict.get('F') || '';
+            url = urlDict.get('F') || null;
+          } else if (isString(urlDict)) {
+            url = urlDict;
           }
 
-        // TODO: pdf reference says that GoToR
+          // NOTE: the destination is relative to the *remote* document.
-        // can also have 'NewWindow' attribute
+          var remoteDest = action.get('D');
-        if (!isValidUrl(url, false)) {
+          if (remoteDest) {
-          url = '';
+            if (isName(remoteDest)) {
+              remoteDest = remoteDest.name;
+            }
+            if (isString(remoteDest) && isString(url)) {
+              var baseUrl = url.split('#')[0];
+              url = baseUrl + '#' + remoteDest;
+            }
+          }
+          // The 'NewWindow' property, equal to `LinkTarget.BLANK`.
+          var newWindow = action.get('NewWindow');
+          if (isBool(newWindow)) {
+            data.newWindow = newWindow;
           }
-        data.url = url;
+          break;
-        data.dest = action.get('D');
+
-      } else if (linkType === 'Named') {
+        case 'Named':
           data.action = action.get('N').name;
-      } else {
+          break;
+
+        default:
           warn('unrecognized link type: ' + linkType);
       }
-    } else if (dict.has('Dest')) {
+    } else if (dict.has('Dest')) { // Simple destination link.
-      // simple destination link
+      dest = dict.get('Dest');
-      var 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 (url && url.indexOf('www.') === 0) {
+    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;
+    }
+  }
+
   Util.inherit(LinkAnnotation, Annotation, {});
 
   return LinkAnnotation;

src/display/annotation_layer.js

@@ -31,6 +31,7 @@ var AnnotationBorderStyleType = sharedUtil.AnnotationBorderStyleType;
 var AnnotationType = sharedUtil.AnnotationType;
 var Util = sharedUtil.Util;
 var addLinkAttributes = displayDOMUtils.addLinkAttributes;
+var LinkTarget = displayDOMUtils.LinkTarget;
 var getFilenameFromUrl = displayDOMUtils.getFilenameFromUrl;
 var warn = sharedUtil.warn;
 var CustomStyle = displayDOMUtils.CustomStyle;
@@ -278,13 +279,16 @@ var LinkAnnotationElement = (function LinkAnnotationElementClosure() {
       this.container.className = 'linkAnnotation';
 
       var link = document.createElement('a');
-      addLinkAttributes(link, { url: this.data.url });
+      addLinkAttributes(link, {
+        url: this.data.url,
+        target: (this.data.newWindow ? LinkTarget.BLANK : undefined),
+      });
 
       if (!this.data.url) {
         if (this.data.action) {
           this._bindNamedAction(link, this.data.action);
         } else {
-          this._bindLink(link, ('dest' in this.data) ? this.data.dest : null);
+          this._bindLink(link, (this.data.dest || null));
         }
       }
 

src/display/dom_utils.js

@@ -114,15 +114,15 @@ var LinkTargetStringMap = [
 /**
  * @typedef ExternalLinkParameters
  * @typedef {Object} ExternalLinkParameters
- * @property {string} url
+ * @property {string} url - An absolute URL.
- * @property {LinkTarget} target
+ * @property {LinkTarget} target - The link target.
- * @property {string} rel
+ * @property {string} rel - The link relationship.
  */
 
 /**
  * Adds various attributes (href, title, target, rel) to hyperlinks.
  * @param {HTMLLinkElement} link - The link element.
- * @param {ExternalLinkParameters} params - An object with the properties.
+ * @param {ExternalLinkParameters} params
  */
 function addLinkAttributes(link, params) {
   var url = params && params.url;
@@ -134,7 +134,7 @@ function addLinkAttributes(link, params) {
       target = getDefaultSetting('externalLinkTarget');
     }
     link.target = LinkTargetStringMap[target];
-    // Strip referrer from the URL.
+
     var rel = params.rel;
     if (typeof rel === 'undefined') {
       rel = getDefaultSetting('externalLinkRel');

src/shared/util.js

@@ -312,7 +312,7 @@ function isSameOrigin(baseUrl, otherUrl) {
 
 // Validates if URL is safe and allowed, e.g. to avoid XSS.
 function isValidUrl(url, allowRelative) {
-  if (!url) {
+  if (!url || typeof url !== 'string') {
     return false;
   }
   // RFC 3986 (http://tools.ietf.org/html/rfc3986#section-3.1)

test/unit/annotation_layer_spec.js

@@ -1,10 +1,20 @@
 /* globals expect, it, describe, Dict, Name, Annotation, AnnotationBorderStyle,
-           AnnotationBorderStyleType, AnnotationFlag, PDFJS, 
+           AnnotationBorderStyleType, AnnotationType, AnnotationFlag, PDFJS,
-           beforeEach, afterEach, stringToBytes */
+           beforeEach, afterEach, stringToBytes, AnnotationFactory, Ref,
+           beforeAll, afterAll */
 
 'use strict';
 
 describe('Annotation layer', function() {
+  function XrefMock(queue) {
+    this.queue = queue || [];
+  }
+  XrefMock.prototype = {
+    fetchIfRef: function() {
+      return this.queue.shift();
+    }
+  };
+
   describe('Annotation', function() {
     it('should set and get flags', function() {
       var dict = new Dict();
@@ -174,6 +184,150 @@ describe('Annotation layer', function() {
     });
   });
 
+  describe('LinkAnnotation', function() {
+    var annotationFactory;
+
+    beforeAll(function (done) {
+      annotationFactory = new AnnotationFactory();
+      done();
+    });
+
+    afterAll(function () {
+      annotationFactory = null;
+    });
+
+    it('should correctly parse a URI action', function() {
+      var actionDict = new Dict();
+      actionDict.set('Type', Name.get('Action'));
+      actionDict.set('S', Name.get('URI'));
+      actionDict.set('URI', 'http://www.ctan.org/tex-archive/info/lshort');
+
+      var annotationDict = new Dict();
+      annotationDict.set('Type', Name.get('Annot'));
+      annotationDict.set('Subtype', Name.get('Link'));
+      annotationDict.set('A', actionDict);
+
+      var xrefMock = new XrefMock([annotationDict]);
+      var annotationRef = new Ref(820, 0);
+
+      var annotation = annotationFactory.create(xrefMock, annotationRef);
+      var data = annotation.data;
+      expect(data.annotationType).toEqual(AnnotationType.LINK);
+
+      expect(data.url).toEqual('http://www.ctan.org/tex-archive/info/lshort');
+      expect(data.dest).toBeUndefined();
+    });
+
+    it('should correctly parse a URI action, where the URI entry ' +
+       'is missing a protocol', function() {
+      var actionDict = new Dict();
+      actionDict.set('Type', Name.get('Action'));
+      actionDict.set('S', Name.get('URI'));
+      actionDict.set('URI', 'www.hmrc.gov.uk');
+
+      var annotationDict = new Dict();
+      annotationDict.set('Type', Name.get('Annot'));
+      annotationDict.set('Subtype', Name.get('Link'));
+      annotationDict.set('A', actionDict);
+
+      var xrefMock = new XrefMock([annotationDict]);
+      var annotationRef = new Ref(353, 0);
+
+      var annotation = annotationFactory.create(xrefMock, annotationRef);
+      var data = annotation.data;
+      expect(data.annotationType).toEqual(AnnotationType.LINK);
+
+      expect(data.url).toEqual('http://www.hmrc.gov.uk');
+      expect(data.dest).toBeUndefined();
+    });
+
+    it('should correctly parse a GoTo action', function() {
+      var actionDict = new Dict();
+      actionDict.set('Type', Name.get('Action'));
+      actionDict.set('S', Name.get('GoTo'));
+      actionDict.set('D', 'page.157');
+
+      var annotationDict = new Dict();
+      annotationDict.set('Type', Name.get('Annot'));
+      annotationDict.set('Subtype', Name.get('Link'));
+      annotationDict.set('A', actionDict);
+
+      var xrefMock = new XrefMock([annotationDict]);
+      var annotationRef = new Ref(798, 0);
+
+      var annotation = annotationFactory.create(xrefMock, annotationRef);
+      var data = annotation.data;
+      expect(data.annotationType).toEqual(AnnotationType.LINK);
+
+      expect(data.url).toBeUndefined();
+      expect(data.dest).toEqual('page.157');
+    });
+
+    it('should correctly parse a GoToR action, where the FileSpec entry ' +
+       'is a string containing a relative URL', function() {
+      var actionDict = new Dict();
+      actionDict.set('Type', Name.get('Action'));
+      actionDict.set('S', Name.get('GoToR'));
+      actionDict.set('F', '../../0021/002156/215675E.pdf');
+      actionDict.set('D', '15');
+
+      var annotationDict = new Dict();
+      annotationDict.set('Type', Name.get('Annot'));
+      annotationDict.set('Subtype', Name.get('Link'));
+      annotationDict.set('A', actionDict);
+
+      var xrefMock = new XrefMock([annotationDict]);
+      var annotationRef = new Ref(489, 0);
+
+      var annotation = annotationFactory.create(xrefMock, annotationRef);
+      var data = annotation.data;
+      expect(data.annotationType).toEqual(AnnotationType.LINK);
+
+      expect(data.url).toBeUndefined();
+      expect(data.dest).toBeUndefined();
+      expect(data.newWindow).toBeFalsy();
+    });
+
+    it('should correctly parse a Named action', function() {
+      var actionDict = new Dict();
+      actionDict.set('Type', Name.get('Action'));
+      actionDict.set('S', Name.get('Named'));
+      actionDict.set('N', Name.get('GoToPage'));
+
+      var annotationDict = new Dict();
+      annotationDict.set('Type', Name.get('Annot'));
+      annotationDict.set('Subtype', Name.get('Link'));
+      annotationDict.set('A', actionDict);
+
+      var xrefMock = new XrefMock([annotationDict]);
+      var annotationRef = new Ref(12, 0);
+
+      var annotation = annotationFactory.create(xrefMock, annotationRef);
+      var data = annotation.data;
+      expect(data.annotationType).toEqual(AnnotationType.LINK);
+
+      expect(data.url).toBeUndefined();
+      expect(data.action).toEqual('GoToPage');
+    });
+
+    it('should correctly parse a simple Dest', function() {
+      var annotationDict = new Dict();
+      annotationDict.set('Type', Name.get('Annot'));
+      annotationDict.set('Subtype', Name.get('Link'));
+      annotationDict.set('Dest', Name.get('LI0'));
+
+      var xrefMock = new XrefMock([annotationDict]);
+      var annotationRef = new Ref(583, 0);
+
+      var annotation = annotationFactory.create(xrefMock, annotationRef);
+      var data = annotation.data;
+      expect(data.annotationType).toEqual(AnnotationType.LINK);
+
+      expect(data.url).toBeUndefined();
+      expect(data.dest).toEqual('LI0');
+    });
+  });
+
   describe('FileAttachmentAnnotation', function() {
     var loadingTask;
     var annotations;