Merge pull request #716 from hypothesis/resolve-relative-pdf-urls

Resolve relative URLs when getting PDF URL from PDF.js

src/annotator/guest.coffee

-baseURI = require('document-base-uri')
 extend = require('extend')
 raf = require('raf')
 scrollIntoView = require('scroll-into-view')
@@ -12,6 +11,7 @@ highlighter = require('./highlighter')
 rangeUtil = require('./range-util')
 selections = require('./selections')
 xpathRange = require('./anchoring/range')
+{ normalizeURI } = require('./util/url')
 
 animationPromise = (fn) ->
   return new Promise (resolve, reject) ->
@@ -21,17 +21,6 @@ animationPromise = (fn) ->
       catch error
         reject(error)
 
-# Normalize the URI for an annotation. This makes it absolute and strips
-# the fragment identifier.
-normalizeURI = (uri, baseURI) ->
-  # Convert to absolute URL
-  url = new URL(uri, baseURI)
-  # Remove the fragment identifier.
-  # This is done on the serialized URL rather than modifying `url.hash` due
-  # to a bug in Safari.
-  # See https://github.com/hypothesis/h/issues/3471#issuecomment-226713750
-  return url.toString().replace(/#.*/, '');
-
 module.exports = class Guest extends Delegator
   SHOW_HIGHLIGHTS_CLASS = 'annotator-highlights-always-on'
 
@@ -140,7 +129,7 @@ module.exports = class Guest extends Delegator
 
     return Promise.all([metadataPromise, uriPromise]).then ([metadata, href]) =>
       return {
-        uri: normalizeURI(href, baseURI),
+        uri: normalizeURI(href),
         metadata,
         frameIdentifier: this.frameIdentifier
       }

src/annotator/plugin/document.coffee

 $ = require('jquery')
-Plugin = require('../plugin')
-
 baseURI = require('document-base-uri')
 
+Plugin = require('../plugin')
+{ normalizeURI } = require('../util/url')
+
 ###
 ** Adapted from:
 ** https://github.com/openannotation/annotator/blob/v1.2.x/src/plugin/document.coffee
@@ -185,9 +186,7 @@ module.exports = class Document extends Plugin
 
   # Hack to get a absolute url from a possibly relative one
   _absoluteUrl: (url) ->
-    d = @document.createElement('a')
-    d.href = url
-    d.href
+    normalizeURI(url, @baseURI)
 
   # Get the true URI record when it's masked via a different protocol.
   # This happens when an href is set with a uri using the 'blob:' protocol

src/annotator/plugin/pdf-metadata.js

 'use strict';
 
+const { normalizeURI } = require('../util/url');
+
 /**
  * This PDFMetadata service extracts metadata about a loading/loaded PDF
  * document from a PDF.js PDFViewerApplication object.
@@ -74,11 +76,13 @@ function fingerprintToURN(fingerprint) {
 }
 
 function getPDFURL(app) {
+  const url = normalizeURI(app.url);
+
   // Local file:// URLs should not be saved in document metadata.
   // Entries in document.link should be URIs. In the case of
   // local files, omit the URL.
-  if (app.url.indexOf('file://') !== 0) {
-    return app.url;
+  if (url.indexOf('file://') !== 0) {
+    return url;
   }
 
   return null;

src/annotator/plugin/test/pdf-metadata-test.js

@@ -14,7 +14,7 @@ describe('pdf-metadata', function () {
     window.dispatchEvent(event);
 
     return pdfMetadata.getUri().then(function (uri) {
-      assert.equal(uri, 'http://fake.com');
+      assert.equal(uri, 'http://fake.com/');
     });
   });
 
@@ -25,7 +25,7 @@ describe('pdf-metadata', function () {
     };
     var pdfMetadata = new PDFMetadata(fakePDFViewerApplication);
     return pdfMetadata.getUri().then(function (uri) {
-      assert.equal(uri, 'http://fake.com');
+      assert.equal(uri, 'http://fake.com/');
     });
   });
 
@@ -41,7 +41,7 @@ describe('pdf-metadata', function () {
           'dc:title': 'fakeTitle',
         },
       },
-      url: 'http://fake.com',
+      url: 'http://fake.com/',
     };
 
     beforeEach(function () {
@@ -51,7 +51,7 @@ describe('pdf-metadata', function () {
     describe('#getUri', function () {
       it('returns the non-file URI', function() {
         return pdfMetadata.getUri().then(function (uri) {
-          assert.equal(uri, 'http://fake.com');
+          assert.equal(uri, 'http://fake.com/');
         });
       });
 
@@ -66,6 +66,20 @@ describe('pdf-metadata', function () {
           assert.equal(uri, 'urn:x-pdf:fakeFingerprint');
         });
       });
+
+      it('resolves relative URLs', () => {
+        var fakePDFViewerApplication = {
+          url: 'index.php?action=download&file_id=wibble',
+          documentFingerprint: 'fakeFingerprint',
+        };
+        var pdfMetadata = new PDFMetadata(fakePDFViewerApplication);
+
+        return pdfMetadata.getUri().then(uri => {
+          var expected = new URL(fakePDFViewerApplication.url,
+                                 document.location.href).toString();
+          assert.equal(uri, expected);
+        });
+      });
     });
 
     describe('#getMetadata', function () {
@@ -81,7 +95,7 @@ describe('pdf-metadata', function () {
         fakePDFViewerApplication.metadata.get = sinon.stub().returns('dcTitle');
 
         return pdfMetadata.getMetadata().then(function (actualMetadata) {
-          assert.deepEqual(expectedMetadata, actualMetadata);
+          assert.deepEqual(actualMetadata, expectedMetadata);
         });
       });
 

src/annotator/util/test/url-test.js

+'use strict';
+
+const { normalizeURI } = require('../url');
+
+describe('annotator.util.url', () => {
+  describe('normalizeURI', () => {
+    it('resolves relative URLs against the provided base URI', () => {
+      const base = 'http://example.com';
+      assert.equal(normalizeURI('index.html', base), `${base}/index.html`);
+    });
+
+    it('resolves relative URLs against the document URI, if no base URI is provided', () => {
+      // Strip filename from base URI.
+      const base = document.baseURI.replace(/\/[^/]*$/, '');
+      assert.equal(normalizeURI('foo.html'), `${base}/foo.html`);
+    });
+
+    it('does not modify absolute URIs', () => {
+      const url = 'http://example.com/wibble';
+      assert.equal(normalizeURI(url), url);
+    });
+
+    it('removes the fragment identifier', () => {
+      const url = 'http://example.com/wibble#fragment';
+      assert.equal(normalizeURI(url), 'http://example.com/wibble');
+    });
+
+    [
+      'file:///Users/jane/article.pdf',
+      'doi:10.1234/4567',
+    ].forEach(url => {
+      it('does not modify absolute non-HTTP/HTTPS URLs', () => {
+        assert.equal(normalizeURI(url), url);
+      });
+    });
+  });
+});

src/annotator/util/url.js

+'use strict';
+
+const baseURI = require('document-base-uri');
+
+/**
+ * Return a normalized version of a URI.
+ *
+ * This makes it absolute and strips the fragment identifier.
+ *
+ * @param {string} uri - Relative or absolute URL
+ * @param {string} [base] - Base URL to resolve relative to. Defaults to
+ *   the document's base URL.
+ */
+function normalizeURI(uri, base = baseURI) {
+  const absUrl = new URL(uri, base).href;
+
+  // Remove the fragment identifier.
+  // This is done on the serialized URL rather than modifying `url.hash` due to
+  // a bug in Safari.
+  // See https://github.com/hypothesis/h/issues/3471#issuecomment-226713750
+  return absUrl.toString().replace(/#.*/, '');
+}
+
+module.exports = {
+  normalizeURI,
+};