From a8561a06883583560e5ad4d57f718bdc4752f0c1 Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Thu, 9 Aug 2018 16:47:10 -0700 Subject: [PATCH 1/5] Simply removed the dead `getViewerUrl` function --- src/service/viewer-impl.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/service/viewer-impl.js b/src/service/viewer-impl.js index 30ced57a4241..ae882133736c 100644 --- a/src/service/viewer-impl.js +++ b/src/service/viewer-impl.js @@ -801,16 +801,6 @@ export class Viewer { return this.resolvedViewerUrl_; } - /** - * Returns the promise that will yield the viewer URL value. It's by default - * the current page's URL. The trusted viewers are allowed to override this - * value. - * @return {!Promise} - */ - getViewerUrl() { - return this.viewerUrl_; - } - /** * Possibly return the messaging origin if set. This would be the origin * of the parent viewer. From c160dabdb0e1ddfb1eb410ddfdcdd15da07e31b2 Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Thu, 9 Aug 2018 17:26:32 -0700 Subject: [PATCH 2/5] Removed tests with calls to getViewerUrl --- test/functional/test-viewer.js | 129 --------------------------------- 1 file changed, 129 deletions(-) diff --git a/test/functional/test-viewer.js b/test/functional/test-viewer.js index 481b3022ea25..b3b437957a5e 100644 --- a/test/functional/test-viewer.js +++ b/test/functional/test-viewer.js @@ -1499,135 +1499,6 @@ describe('Viewer', () => { const viewer = new Viewer(ampdoc); expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); }); - - it('should always return current location for top-level window', () => { - windowApi.parent = windowApi; - windowApi.location.href = 'https://acme.org/doc1#hash'; - const viewer = new Viewer(ampdoc); - expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); - return viewer.getViewerUrl().then(viewerUrl => { - expect(viewerUrl).to.equal('https://acme.org/doc1'); - expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); - expect(errorStub).to.have.not.been.called; - }); - }); - - it('should NOT allow override if not iframed', () => { - windowApi.parent = windowApi; - windowApi.location.href = 'https://acme.org/doc1'; - windowApi.location.hash = '#origin=g.com&viewerUrl=' + - encodeURIComponent('https://acme.org/viewer'); - const viewer = new Viewer(ampdoc); - expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); - return viewer.getViewerUrl().then(viewerUrl => { - expect(viewerUrl).to.equal('https://acme.org/doc1'); - expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); - expect(errorStub).to.have.not.been.called; - }); - }); - - it('should NOT allow override if not trusted', () => { - windowApi.parent = {}; - windowApi.location.href = 'https://acme.org/doc1'; - windowApi.location.hash = '#origin=g.com&viewerUrl=' + - encodeURIComponent('https://acme.org/viewer'); - windowApi.location.ancestorOrigins = ['https://untrusted.com']; - const viewer = new Viewer(ampdoc); - expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); - return viewer.getViewerUrl().then(viewerUrl => { - expect(viewerUrl).to.equal('https://acme.org/doc1'); - expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); - expect(errorStub).to.be.calledOnce; - expect(errorStub.calledWith('Viewer', - sinon.match(arg => { - return !!arg.match(/Untrusted viewer url override/); - }))).to.be.true; - }); - }); - - it('should NOT allow override if ancestor is empty', () => { - windowApi.parent = {}; - windowApi.location.href = 'https://acme.org/doc1'; - windowApi.location.hash = '#origin=g.com&viewerUrl=' + - encodeURIComponent('https://acme.org/viewer'); - windowApi.location.ancestorOrigins = []; - const viewer = new Viewer(ampdoc); - expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); - return viewer.getViewerUrl().then(viewerUrl => { - expect(viewerUrl).to.equal('https://acme.org/doc1'); - expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); - expect(errorStub).to.be.calledOnce; - expect(errorStub.calledWith('Viewer', - sinon.match(arg => { - return !!arg.match(/Untrusted viewer url override/); - }))).to.be.true; - }); - }); - - it('should allow partial override if async not trusted', () => { - windowApi.parent = {}; - windowApi.location.href = 'https://acme.org/doc1'; - windowApi.location.hash = '#origin=g.com&viewerUrl=' + - encodeURIComponent('https://acme.org/viewer'); - const viewer = new Viewer(ampdoc); - expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); - viewer.setMessageDeliverer(() => {}, 'https://untrusted.com'); - return viewer.getViewerUrl().then(viewerUrl => { - expect(viewerUrl).to.equal('https://acme.org/doc1'); - expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); - expect(errorStub).to.be.calledOnce; - expect(errorStub.calledWith('Viewer', - sinon.match(arg => { - return !!arg.match(/Untrusted viewer url override/); - }))).to.be.true; - }); - }); - - it('should allow full override if async trusted', () => { - windowApi.parent = {}; - windowApi.location.href = 'https://acme.org/doc1'; - windowApi.location.hash = '#origin=g.com&viewerUrl=' + - encodeURIComponent('https://acme.org/viewer'); - const viewer = new Viewer(ampdoc); - expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); - viewer.setMessageDeliverer(() => {}, 'https://google.com'); - return viewer.getViewerUrl().then(viewerUrl => { - expect(viewerUrl).to.equal('https://acme.org/viewer'); - expect(viewer.getResolvedViewerUrl()) - .to.equal('https://acme.org/viewer'); - expect(errorStub).to.have.not.been.called; - }); - }); - - it('should allow override if iframed and trusted', () => { - windowApi.parent = {}; - windowApi.location.href = 'https://acme.org/doc1'; - windowApi.location.hash = '#origin=g.com&viewerUrl=' + - encodeURIComponent('https://acme.org/viewer'); - windowApi.location.ancestorOrigins = ['https://google.com']; - const viewer = new Viewer(ampdoc); - expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); - return viewer.getViewerUrl().then(viewerUrl => { - expect(viewerUrl).to.equal('https://acme.org/viewer'); - expect(viewer.getResolvedViewerUrl()) - .to.equal('https://acme.org/viewer'); - expect(errorStub).to.have.not.been.called; - }); - }); - - it('should ignore override to empty if iframed and trusted', () => { - windowApi.parent = {}; - windowApi.location.href = 'https://acme.org/doc1'; - windowApi.location.hash = '#origin=g.com&viewerUrl='; - windowApi.location.ancestorOrigins = ['https://google.com']; - const viewer = new Viewer(ampdoc); - expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); - return viewer.getViewerUrl().then(viewerUrl => { - expect(viewerUrl).to.equal('https://acme.org/doc1'); - expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); - expect(errorStub).to.have.not.been.called; - }); - }); }); describe('viewerOrigin', () => { From 3109e1857be06bce1998b5612dd3879ea56526ee Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Fri, 17 Aug 2018 14:25:14 -0700 Subject: [PATCH 3/5] Marked the getViewerUrl at visible for testing --- src/service/viewer-impl.js | 18 ++++- test/functional/test-viewer.js | 132 ++++++++++++++++++++++++++++++++- 2 files changed, 148 insertions(+), 2 deletions(-) diff --git a/src/service/viewer-impl.js b/src/service/viewer-impl.js index ae882133736c..7f08a20fe314 100644 --- a/src/service/viewer-impl.js +++ b/src/service/viewer-impl.js @@ -801,6 +801,17 @@ export class Viewer { return this.resolvedViewerUrl_; } + /** + * Returns the promise that will yield the viewer URL value. It's by default + * the current page's URL. The trusted viewers are allowed to override this + * value. + * @return {!Promise} + * @visibleForTesting + */ + getViewerUrl() { + return this.viewerUrl_; + } + /** * Possibly return the messaging origin if set. This would be the origin * of the parent viewer. @@ -880,7 +891,12 @@ export class Viewer { isTrustedViewerOrigin_(urlString) { /** @const {!Location} */ const url = parseUrlDeprecated(urlString); - if (url.protocol != 'https:') { + const {protocol} = url; + // Mobile WebView x-thread is allowed. + if (protocol == 'x-thread:') { + return true; + } + if (protocol != 'https:') { // Non-https origins are never trusted. return false; } diff --git a/test/functional/test-viewer.js b/test/functional/test-viewer.js index b3b437957a5e..7aad9eb6dcef 100644 --- a/test/functional/test-viewer.js +++ b/test/functional/test-viewer.js @@ -1291,7 +1291,7 @@ describe('Viewer', () => { }); } - describe('should trust domain variations', () => { + describe('should trust trusted viewer origins', () => { test('https://google.com', true); test('https://www.google.com', true); test('https://news.google.com', true); @@ -1308,6 +1308,7 @@ describe('Viewer', () => { test('https://abc.www.google.com', true); test('https://google.cat', true); test('https://www.google.cat', true); + test('x-thread://', true); }); describe('should not trust host as referrer with http', () => { @@ -1499,6 +1500,135 @@ describe('Viewer', () => { const viewer = new Viewer(ampdoc); expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); }); + + it('should always return current location for top-level window', () => { + windowApi.parent = windowApi; + windowApi.location.href = 'https://acme.org/doc1#hash'; + const viewer = new Viewer(ampdoc); + expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); + return viewer.getViewerUrl().then(viewerUrl => { + expect(viewerUrl).to.equal('https://acme.org/doc1'); + expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); + expect(errorStub).to.have.not.been.called; + }); + }); + + it('should NOT allow override if not iframed', () => { + windowApi.parent = windowApi; + windowApi.location.href = 'https://acme.org/doc1'; + windowApi.location.hash = '#origin=g.com&viewerUrl=' + + encodeURIComponent('https://acme.org/viewer'); + const viewer = new Viewer(ampdoc); + expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); + return viewer.getViewerUrl().then(viewerUrl => { + expect(viewerUrl).to.equal('https://acme.org/doc1'); + expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); + expect(errorStub).to.have.not.been.called; + }); + }); + + it('should NOT allow override if not trusted', () => { + windowApi.parent = {}; + windowApi.location.href = 'https://acme.org/doc1'; + windowApi.location.hash = '#origin=g.com&viewerUrl=' + + encodeURIComponent('https://acme.org/viewer'); + windowApi.location.ancestorOrigins = ['https://untrusted.com']; + const viewer = new Viewer(ampdoc); + expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); + return viewer.getViewerUrl().then(viewerUrl => { + expect(viewerUrl).to.equal('https://acme.org/doc1'); + expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); + expect(errorStub).to.be.calledOnce; + expect(errorStub.calledWith('Viewer', + sinon.match(arg => { + return !!arg.match(/Untrusted viewer url override/); + }))).to.be.true; + }); + }); + + it('should NOT allow override if ancestor is empty', () => { + windowApi.parent = {}; + windowApi.location.href = 'https://acme.org/doc1'; + windowApi.location.hash = '#origin=g.com&viewerUrl=' + + encodeURIComponent('https://acme.org/viewer'); + windowApi.location.ancestorOrigins = []; + const viewer = new Viewer(ampdoc); + expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); + return viewer.getViewerUrl().then(viewerUrl => { + expect(viewerUrl).to.equal('https://acme.org/doc1'); + expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); + expect(errorStub).to.be.calledOnce; + expect(errorStub.calledWith('Viewer', + sinon.match(arg => { + return !!arg.match(/Untrusted viewer url override/); + }))).to.be.true; + }); + }); + + it('should allow partial override if async not trusted', () => { + windowApi.parent = {}; + windowApi.location.href = 'https://acme.org/doc1'; + windowApi.location.hash = '#origin=g.com&viewerUrl=' + + encodeURIComponent('https://acme.org/viewer'); + const viewer = new Viewer(ampdoc); + expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); + viewer.setMessageDeliverer(() => {}, 'https://untrusted.com'); + return viewer.getViewerUrl().then(viewerUrl => { + expect(viewerUrl).to.equal('https://acme.org/doc1'); + expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); + expect(errorStub).to.be.calledOnce; + expect(errorStub.calledWith('Viewer', + sinon.match(arg => { + return !!arg.match(/Untrusted viewer url override/); + }))).to.be.true; + }); + }); + + it('should allow full override if async trusted', () => { + windowApi.parent = {}; + windowApi.location.href = 'https://acme.org/doc1'; + windowApi.location.hash = '#origin=g.com&viewerUrl=' + + encodeURIComponent('https://acme.org/viewer'); + const viewer = new Viewer(ampdoc); + expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); + viewer.setMessageDeliverer(() => {}, 'https://google.com'); + return viewer.getViewerUrl().then(viewerUrl => { + expect(viewerUrl).to.equal('https://acme.org/viewer'); + expect(viewer.getResolvedViewerUrl()) + .to.equal('https://acme.org/viewer'); + expect(errorStub).to.have.not.been.called; + }); + }); + + it('should allow override if iframed and trusted', () => { + windowApi.parent = {}; + windowApi.location.href = 'https://acme.org/doc1'; + windowApi.location.hash = '#origin=g.com&viewerUrl=' + + encodeURIComponent('https://acme.org/viewer'); + windowApi.location.ancestorOrigins = ['https://google.com']; + const viewer = new Viewer(ampdoc); + expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); + return viewer.getViewerUrl().then(viewerUrl => { + expect(viewerUrl).to.equal('https://acme.org/viewer'); + expect(viewer.getResolvedViewerUrl()) + .to.equal('https://acme.org/viewer'); + expect(errorStub).to.have.not.been.called; + }); + }); + + it('should ignore override to empty if iframed and trusted', () => { + windowApi.parent = {}; + windowApi.location.href = 'https://acme.org/doc1'; + windowApi.location.hash = '#origin=g.com&viewerUrl='; + windowApi.location.ancestorOrigins = ['https://google.com']; + const viewer = new Viewer(ampdoc); + expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); + return viewer.getViewerUrl().then(viewerUrl => { + expect(viewerUrl).to.equal('https://acme.org/doc1'); + expect(viewer.getResolvedViewerUrl()).to.equal('https://acme.org/doc1'); + expect(errorStub).to.have.not.been.called; + }); + }); }); describe('viewerOrigin', () => { From 9830eb6f0e92febc9fff938949602109dcab0342 Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Fri, 17 Aug 2018 14:56:57 -0700 Subject: [PATCH 4/5] Removed som random code that got added by syncing with master --- src/service/viewer-impl.js | 5 ----- test/functional/test-viewer.js | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/service/viewer-impl.js b/src/service/viewer-impl.js index 7f08a20fe314..bc7ccc41c20b 100644 --- a/src/service/viewer-impl.js +++ b/src/service/viewer-impl.js @@ -891,11 +891,6 @@ export class Viewer { isTrustedViewerOrigin_(urlString) { /** @const {!Location} */ const url = parseUrlDeprecated(urlString); - const {protocol} = url; - // Mobile WebView x-thread is allowed. - if (protocol == 'x-thread:') { - return true; - } if (protocol != 'https:') { // Non-https origins are never trusted. return false; diff --git a/test/functional/test-viewer.js b/test/functional/test-viewer.js index 7aad9eb6dcef..286b90b20c52 100644 --- a/test/functional/test-viewer.js +++ b/test/functional/test-viewer.js @@ -1291,7 +1291,7 @@ describe('Viewer', () => { }); } - describe('should trust trusted viewer origins', () => { + describe('should trust domain variations', () => { test('https://google.com', true); test('https://www.google.com', true); test('https://news.google.com', true); From e1a784784c32b1b8f11a0bd07442668e7bf6ca13 Mon Sep 17 00:00:00 2001 From: Aaron Turner Date: Fri, 17 Aug 2018 14:59:02 -0700 Subject: [PATCH 5/5] Removed more random code that got added by the sync --- src/service/viewer-impl.js | 2 +- test/functional/test-viewer.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/service/viewer-impl.js b/src/service/viewer-impl.js index bc7ccc41c20b..3a4b9428f2ef 100644 --- a/src/service/viewer-impl.js +++ b/src/service/viewer-impl.js @@ -891,7 +891,7 @@ export class Viewer { isTrustedViewerOrigin_(urlString) { /** @const {!Location} */ const url = parseUrlDeprecated(urlString); - if (protocol != 'https:') { + if (url.protocol != 'https:') { // Non-https origins are never trusted. return false; } diff --git a/test/functional/test-viewer.js b/test/functional/test-viewer.js index 286b90b20c52..481b3022ea25 100644 --- a/test/functional/test-viewer.js +++ b/test/functional/test-viewer.js @@ -1308,7 +1308,6 @@ describe('Viewer', () => { test('https://abc.www.google.com', true); test('https://google.cat', true); test('https://www.google.cat', true); - test('x-thread://', true); }); describe('should not trust host as referrer with http', () => {