Refactor isProxyOrigin and getSourceOrigin to url.js

src/service/cid-impl.js

@@ -25,7 +25,7 @@
 import {assert} from '../asserts';
 import {getCookie, setCookie} from '../cookies';
 import {getService} from '../service';
-import {parseUrl} from '../url';
+import {getSourceOrigin, isProxyOrigin, parseUrl} from '../url';
 import {timer} from '../timer';
 import {viewerFor} from '../viewer';
 import {sha384Base64} from
@@ -132,7 +132,7 @@ function getExternalCid(cid, getCidStruct, persistenceConsent) {
   return getBaseCid(cid, persistenceConsent).then(baseCid => {
     return cid.sha384Base64_(
         baseCid +
-        getSourceOrigin(url) +
+        getProxySourceOrigin(url) +
         getCidStruct.scope);
   });
 }
@@ -170,47 +170,17 @@ function getOrCreateCookie(cid, getCidStruct, persistenceConsent) {
   return Promise.resolve(newCookie);
 }
 
-/**
- * Returns whether the URL has the origin of a proxy.
- * @param {!Url} url URL of an AMP document.
- * @return {boolean}
- * @visibleForTesting BUT if this is needed elsewhere it could be
- *     factored into its own package.
- */
-export function isProxyOrigin(url) {
-  const path = url.pathname.split('/');
-  const prefix = path[1];
-  // List of well known proxy hosts. New proxies must be added here
-  // to generate correct tokens.
-  return (url.origin == 'https://cdn.ampproject.org' ||
-      (url.origin.indexOf('http://localhost:') == 0 &&
-       (prefix == 'c' || prefix == 'v')));
-}
-
 /**
  * Returns the source origin of an AMP document for documents served
  * on a proxy origin. Throws an error if the doc is not on a proxy origin.
- * @param {!Url} url URL of an AMP document.
+ * @param {!Location} url URL of an AMP document.
  * @return {string} The source origin of the URL.
  * @visibleForTesting BUT if this is needed elsewhere it could be
  *     factored into its own package.
  */
-export function getSourceOrigin(url) {
+export function getProxySourceOrigin(url) {
   assert(isProxyOrigin(url), 'Expected proxy origin %s', url.origin);
-  // Example path that is being matched here.
-  // https://cdn.ampproject.org/c/s/www.origin.com/foo/
-  // The /s/ is optional and signals a secure origin.
-  const path = url.pathname.split('/');
-  const prefix = path[1];
-  assert(prefix == 'c' || prefix == 'v',
-      'Unknown path prefix in url %s', url.href);
-  const domainOrHttpsSignal = path[2];
-  const origin = domainOrHttpsSignal == 's'
-      ? 'https://' + path[3]
-      : 'http://' + domainOrHttpsSignal;
-  // Sanity test that what we found looks like a domain.
-  assert(origin.indexOf('.') > 0, 'Expected a . in origin %s', origin);
-  return origin;
+  return getSourceOrigin(url);
 }
 
 /**

src/url.js

@@ -160,3 +160,54 @@ export function removeFragment(url) {
   }
   return url.substring(0, index);
 }
+
+
+/**
+ * Returns whether the URL has the origin of a proxy.
+ * @param {string|!Location} url URL of an AMP document.
+ * @return {boolean}
+ */
+export function isProxyOrigin(url) {
+  if (typeof url == 'string') {
+    url = parseUrl(url);
+  }
+  const path = url.pathname.split('/');
+  const prefix = path[1];
+  // List of well known proxy hosts. New proxies must be added here.
+  return (url.origin == 'https://cdn.ampproject.org' ||
+      (url.origin.indexOf('http://localhost:') == 0 &&
+       (prefix == 'c' || prefix == 'v')));
+}
+
+/**
+ * Returns the source origin of an AMP document for documents served
+ * on a proxy origin or directly.
+ * @param {string|!Location} url URL of an AMP document.
+ * @return {string} The source origin of the URL.
+ */
+export function getSourceOrigin(url) {
+  if (typeof url == 'string') {
+    url = parseUrl(url);
+  }
+
+  // Not a proxy URL - return the URL's origin.
+  if (!isProxyOrigin(url)) {
+    return getOrigin(url);
+  }
+
+  // A proxy URL.
+  // Example path that is being matched here.
+  // https://cdn.ampproject.org/c/s/www.origin.com/foo/
+  // The /s/ is optional and signals a secure origin.
+  const path = url.pathname.split('/');
+  const prefix = path[1];
+  assert(prefix == 'c' || prefix == 'v',
+      'Unknown path prefix in url %s', url.href);
+  const domainOrHttpsSignal = path[2];
+  const origin = domainOrHttpsSignal == 's'
+      ? 'https://' + path[3]
+      : 'http://' + domainOrHttpsSignal;
+  // Sanity test that what we found looks like a domain.
+  assert(origin.indexOf('.') > 0, 'Expected a . in origin %s', origin);
+  return origin;
+}

test/functional/test-cid.js

@@ -15,7 +15,7 @@
  */
 
 import {cidFor} from '../../src/cid';
-import {installCidService, getSourceOrigin, isProxyOrigin} from
+import {installCidService, getProxySourceOrigin} from
     '../../src/service/cid-impl';
 import {parseUrl} from '../../src/url';
 import {timer} from '../../src/timer';
@@ -364,64 +364,10 @@ describe('cid', () => {
   }
 });
 
-describe('getSourceOrigin', () => {
-
-  function testOrigin(href, origin) {
-    it('should return the origin from ' + href, () => {
-      expect(getSourceOrigin(parseUrl(href))).to.equal(origin);
-    });
-  }
-
-  testOrigin(
-      'https://cdn.ampproject.org/v/www.origin.com/foo/?f=0',
-      'http://www.origin.com');
-  testOrigin(
-      'https://cdn.ampproject.org/v/s/www.origin.com/foo/?f=0',
-      'https://www.origin.com');
-  testOrigin(
-      'https://cdn.ampproject.org/c/www.origin.com/foo/?f=0',
-      'http://www.origin.com');
-  testOrigin(
-      'https://cdn.ampproject.org/c/s/www.origin.com/foo/?f=0',
-      'https://www.origin.com');
-  testOrigin(
-      'https://cdn.ampproject.org/c/s/origin.com/foo/?f=0',
-      'https://origin.com');
-
-  it('should fail on invalid source origin', () => {
-    expect(() => {
-      getSourceOrigin(parseUrl('https://cdn.ampproject.org/v/yyy/'));
-    }).to.throw(/Expected a \. in origin http:\/\/yyy/);
-  });
-
+describe('getProxySourceOrigin', () => {
   it('should fail on non-proxy origin', () => {
     expect(() => {
-      getSourceOrigin(parseUrl('https://abc.org/v/foo.com/'));
+      getProxySourceOrigin(parseUrl('https://abc.org/v/foo.com/'));
     }).to.throw(/Expected proxy origin/);
   });
 });
-
-describe('isProxyOrigin', () => {
-
-  function testProxyOrigin(href, bool) {
-    it('should return whether it is a proxy origin origin for ' + href, () => {
-      expect(isProxyOrigin(parseUrl(href))).to.equal(bool);
-    });
-  }
-
-  testProxyOrigin(
-      'https://cdn.ampproject.org/v/www.origin.com/foo/?f=0', true);
-  testProxyOrigin(
-      'http://localhost:123', false);
-  testProxyOrigin(
-      'http://localhost:123/c', true);
-  testProxyOrigin(
-      'http://localhost:123/v', true);
-  testProxyOrigin(
-      'https://cdn.ampproject.net/v/www.origin.com/foo/?f=0', false);
-  testProxyOrigin(
-      'https://medium.com/swlh/nobody-wants-your-app-6af1f7f69cb7', false);
-  testProxyOrigin(
-      'http://www.spiegel.de/politik/deutschland/angela-merkel-a-1062761.html',
-      false);
-});

test/functional/test-url.js

@@ -19,6 +19,8 @@ import {
   addParamsToUrl,
   assertHttpsUrl,
   getOrigin,
+  getSourceOrigin,
+  isProxyOrigin,
   parseQueryString,
   parseUrl,
   removeFragment
@@ -274,3 +276,59 @@ describe('addParamsToUrl', () => {
     expect(url).to.equal('https://www.ampproject.org/get/here?hello=world&foo=bar#hash-value');
   });
 });
+
+describe('isProxyOrigin', () => {
+
+  function testProxyOrigin(href, bool) {
+    it('should return whether it is a proxy origin origin for ' + href, () => {
+      expect(isProxyOrigin(parseUrl(href))).to.equal(bool);
+    });
+  }
+
+  testProxyOrigin(
+      'https://cdn.ampproject.org/v/www.origin.com/foo/?f=0', true);
+  testProxyOrigin(
+      'http://localhost:123', false);
+  testProxyOrigin(
+      'http://localhost:123/c', true);
+  testProxyOrigin(
+      'http://localhost:123/v', true);
+  testProxyOrigin(
+      'https://cdn.ampproject.net/v/www.origin.com/foo/?f=0', false);
+  testProxyOrigin(
+      'https://medium.com/swlh/nobody-wants-your-app-6af1f7f69cb7', false);
+  testProxyOrigin(
+      'http://www.spiegel.de/politik/deutschland/angela-merkel-a-1062761.html',
+      false);
+});
+
+describe('getSourceOrigin', () => {
+
+  function testOrigin(href, origin) {
+    it('should return the origin from ' + href, () => {
+      expect(getSourceOrigin(parseUrl(href))).to.equal(origin);
+    });
+  }
+
+  testOrigin(
+      'https://cdn.ampproject.org/v/www.origin.com/foo/?f=0',
+      'http://www.origin.com');
+  testOrigin(
+      'https://cdn.ampproject.org/v/s/www.origin.com/foo/?f=0',
+      'https://www.origin.com');
+  testOrigin(
+      'https://cdn.ampproject.org/c/www.origin.com/foo/?f=0',
+      'http://www.origin.com');
+  testOrigin(
+      'https://cdn.ampproject.org/c/s/www.origin.com/foo/?f=0',
+      'https://www.origin.com');
+  testOrigin(
+      'https://cdn.ampproject.org/c/s/origin.com/foo/?f=0',
+      'https://origin.com');
+
+  it('should fail on invalid source origin', () => {
+    expect(() => {
+      getSourceOrigin(parseUrl('https://cdn.ampproject.org/v/yyy/'));
+    }).to.throw(/Expected a \. in origin http:\/\/yyy/);
+  });
+});