Skip to content

Commit

Permalink
Merge pull request #1419 from dvoytenko/access18b
Browse files Browse the repository at this point in the history
Refactor isProxyOrigin and getSourceOrigin to url.js
  • Loading branch information
dvoytenko committed Jan 14, 2016
2 parents f13a1de + 7750ce7 commit dcb7c6d
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 92 deletions.
40 changes: 5 additions & 35 deletions src/service/cid-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
});
}
Expand Down Expand Up @@ -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);
}

/**
Expand Down
51 changes: 51 additions & 0 deletions src/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
60 changes: 3 additions & 57 deletions test/functional/test-cid.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
});
58 changes: 58 additions & 0 deletions test/functional/test-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
addParamsToUrl,
assertHttpsUrl,
getOrigin,
getSourceOrigin,
isProxyOrigin,
parseQueryString,
parseUrl,
removeFragment
Expand Down Expand Up @@ -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/);
});
});

0 comments on commit dcb7c6d

Please sign in to comment.