From 96c551cecf32e01ffbc3734a1dfeabc7bc67ea3c Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Tue, 19 May 2020 12:26:40 -0400 Subject: [PATCH] fix(rewriter): rewrite accesses to document.location (#7418) --- packages/driver/src/cypress/resolvers.ts | 11 +++++- .../integration/cypress/resolvers_spec.ts | 35 +++++++++--------- packages/driver/ts/internal-types.d.ts | 6 ++-- packages/rewriter/test/unit/js-spec.ts | 8 +++++ .../__snapshots__/6_visit_spec.coffee.js | 16 ++++++--- .../integration/source_rewriting_spec.js | 36 +++++++++++++++++++ .../settimeout_redirect_document_href.html | 13 +++++++ .../static/settimeout_redirect_href_hash.html | 13 +++++++ ...imeout_redirect_set_document_location.html | 13 +++++++ ...t_redirect_set_document_location_hash.html | 13 +++++++ ...redirect_set_window_document_location.html | 13 +++++++ ...timeout_redirect_window_document_href.html | 13 +++++++ 12 files changed, 165 insertions(+), 25 deletions(-) create mode 100644 packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_document_href.html create mode 100644 packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_href_hash.html create mode 100644 packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_set_document_location.html create mode 100644 packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_set_document_location_hash.html create mode 100644 packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_set_window_document_location.html create mode 100644 packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_window_document_href.html diff --git a/packages/driver/src/cypress/resolvers.ts b/packages/driver/src/cypress/resolvers.ts index ba50858c9d..9c49f032d8 100644 --- a/packages/driver/src/cypress/resolvers.ts +++ b/packages/driver/src/cypress/resolvers.ts @@ -14,8 +14,17 @@ export function resolveWindowReference (this: typeof $Cypress, currentWindow: Wi const getTargetValue = () => { const targetValue = accessedObject[accessedProp] + const accessingDocument = dom.isDocument(accessedObject) + const hasLocation = dom.isWindow(accessedObject) || accessingDocument + + if (hasLocation && accessedProp === 'location') { + if (accessingDocument) { + // `document.location` is the same reference as `window.location`. + // use `window.location` so that the location Proxy can be cached in the same + // place on `window` regardless of if it is accessed via `document` or `window` + accessedObject = currentWindow + } - if (dom.isWindow(accessedObject) && accessedProp === 'location') { const targetLocation = resolveLocationReference(accessedObject) if (isValPassed) { diff --git a/packages/driver/test/cypress/integration/cypress/resolvers_spec.ts b/packages/driver/test/cypress/integration/cypress/resolvers_spec.ts index 39755d0418..e161e5b00a 100644 --- a/packages/driver/test/cypress/integration/cypress/resolvers_spec.ts +++ b/packages/driver/test/cypress/integration/cypress/resolvers_spec.ts @@ -24,7 +24,6 @@ describe('src/cypress/resolvers', function () { // @ts-ignore cy.spy(unboundFn, 'bind') - // @ts-ignore const actual = Cypress.resolveWindowReference({}, unboundFnWindow, 'parent') expect(actual).to.be.instanceOf(Function) @@ -32,16 +31,26 @@ describe('src/cypress/resolvers', function () { expect(unboundFn.bind).to.be.calledWith(unboundFnWindow) }) - it('returns proxied location object if prop is location', function () { - const contentWindow = Cypress.state('$autIframe')!.prop('contentWindow') - // @ts-ignore - const actual = Cypress.resolveWindowReference(contentWindow, contentWindow, 'location') + context('returns proxied location object', function () { + it('if prop is location and obj is a Window', function () { + const contentWindow = Cypress.state('$autIframe')!.prop('contentWindow') + const actual = Cypress.resolveWindowReference(contentWindow, contentWindow, 'location') - cy.stub(Cypress.dom, 'isWindow').withArgs(contentWindow).returns(true) - cy.stub(Cypress.dom, 'isJquery').withArgs(contentWindow).returns(false) + cy.stub(Cypress.dom, 'isWindow').withArgs(contentWindow).returns(true) + cy.stub(Cypress.dom, 'isJquery').withArgs(contentWindow).returns(false) - // @ts-ignore - expect(actual).to.eq(Cypress.resolveLocationReference(contentWindow)) + expect(actual).to.eq(Cypress.resolveLocationReference(contentWindow)) + }) + + it('if prop is location and obj is a Document', function () { + const contentWindow = Cypress.state('$autIframe')!.prop('contentWindow') + const { document } = contentWindow + const actual = Cypress.resolveWindowReference(contentWindow, document, 'location') + + cy.stub(Cypress.dom, 'isDocument').withArgs(document).returns(true) + + expect(actual).to.eq(Cypress.resolveLocationReference(contentWindow)) + }) }) context('window reference selection', function () { @@ -131,7 +140,6 @@ describe('src/cypress/resolvers', function () { isJquery.withArgs(frame).returns(false) }) - // @ts-ignore const actual = Cypress.resolveWindowReference(currentWindow, accessedObject, accessedProp) expect(actual).to.eq(expected) @@ -151,7 +159,6 @@ describe('src/cypress/resolvers', function () { }) it('.href setter sets location.href with resolved URL', () => { - // @ts-ignore const loc = Cypress.resolveLocationReference(fakeWindow) loc.href = 'foo' @@ -160,7 +167,6 @@ describe('src/cypress/resolvers', function () { }) it('.assign() calls location.assign with resolved URL', () => { - // @ts-ignore const loc = Cypress.resolveLocationReference(fakeWindow) loc.assign('foo') @@ -169,7 +175,6 @@ describe('src/cypress/resolvers', function () { }) it('.replace() calls location.replace with resolved URL', () => { - // @ts-ignore const loc = Cypress.resolveLocationReference(fakeWindow) loc.replace('foo') @@ -178,7 +183,6 @@ describe('src/cypress/resolvers', function () { }) it('calls through to unintercepted functions', () => { - // @ts-ignore const loc = Cypress.resolveLocationReference(fakeWindow) loc.someFn('foo') @@ -187,7 +191,6 @@ describe('src/cypress/resolvers', function () { }) it('calls through to unintercepted setters + getters', () => { - // @ts-ignore const loc = Cypress.resolveLocationReference(fakeWindow) expect(loc.someProp).to.eq('foo') @@ -199,9 +202,7 @@ describe('src/cypress/resolvers', function () { }) it('returns the same object between calls', () => { - // @ts-ignore const loc1 = Cypress.resolveLocationReference(fakeWindow) - // @ts-ignore const loc2 = Cypress.resolveLocationReference(fakeWindow) expect(loc1).to.eq(loc2) diff --git a/packages/driver/ts/internal-types.d.ts b/packages/driver/ts/internal-types.d.ts index fdd04977f2..009384161c 100644 --- a/packages/driver/ts/internal-types.d.ts +++ b/packages/driver/ts/internal-types.d.ts @@ -2,13 +2,15 @@ // TODO: find a better place for this declare namespace Cypress { - interface Cypress { + // TODO: how to pull these from resolvers.ts? can't import in a d.ts file... + resolveWindowReference: any + resolveLocationReference: any + /** * Access and set Cypress's internal state. */ state: State - } interface State { diff --git a/packages/rewriter/test/unit/js-spec.ts b/packages/rewriter/test/unit/js-spec.ts index ee0c474f44..4c46abfeba 100644 --- a/packages/rewriter/test/unit/js-spec.ts +++ b/packages/rewriter/test/unit/js-spec.ts @@ -138,6 +138,14 @@ describe('js rewriter', function () { 'window.location = "bar"', `globalThis.top.Cypress.resolveWindowReference(globalThis, window, 'location', "bar")`, ], + [ + 'document.location.href = "bar"', + `${match('document', 'location')}.href = "bar"`, + ], + [ + 'document.location = "bar"', + `globalThis.top.Cypress.resolveWindowReference(globalThis, document, 'location', "bar")`, + ], ] .forEach(([string, expected]) => { if (!expected) { diff --git a/packages/server/__snapshots__/6_visit_spec.coffee.js b/packages/server/__snapshots__/6_visit_spec.coffee.js index 49e83784ac..f498b38934 100644 --- a/packages/server/__snapshots__/6_visit_spec.coffee.js +++ b/packages/server/__snapshots__/6_visit_spec.coffee.js @@ -779,10 +779,16 @@ exports['e2e visit / low response timeout / passes with experimentalSourceRewrit it can relative redirect in a settimeout ✓ with location.href ✓ with window.location.href + ✓ with document.location.href + ✓ with window.document.location.href + ✓ with location.href = #hash ✓ with location.replace() ✓ with location.assign() ✓ with location = ... ✓ with window.location = ... + ✓ with document.location = ... + ✓ with window.document.location = ... + ✓ with document.location = #hash ✓ with location.search ✓ with location.pathname can load some well-known sites in a timely manner @@ -793,15 +799,15 @@ exports['e2e visit / low response timeout / passes with experimentalSourceRewrit - http://github.com - 12 passing + 18 passing 6 pending (Results) ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ - │ Tests: 18 │ - │ Passing: 12 │ + │ Tests: 24 │ + │ Passing: 18 │ │ Failing: 0 │ │ Pending: 6 │ │ Skipped: 0 │ @@ -825,9 +831,9 @@ exports['e2e visit / low response timeout / passes with experimentalSourceRewrit Spec Tests Passing Failing Pending Skipped ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ - │ ✔ source_rewriting_spec.js XX:XX 18 12 - 6 - │ + │ ✔ source_rewriting_spec.js XX:XX 24 18 - 6 - │ └────────────────────────────────────────────────────────────────────────────────────────────────┘ - ✔ All specs passed! XX:XX 18 12 - 6 - + ✔ All specs passed! XX:XX 24 18 - 6 - ` diff --git a/packages/server/test/support/fixtures/projects/e2e/cypress/integration/source_rewriting_spec.js b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/source_rewriting_spec.js index 06dd41f008..28b1e98b09 100644 --- a/packages/server/test/support/fixtures/projects/e2e/cypress/integration/source_rewriting_spec.js +++ b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/source_rewriting_spec.js @@ -27,6 +27,24 @@ describe('source rewriting spec', function () { cy.location('pathname').should('eq', '/static/index.html') }) + it('with document.location.href', function () { + cy.visit('/static/settimeout_redirect_document_href.html') + cy.location('pathname').should('eq', '/static/index.html') + }) + + it('with window.document.location.href', function () { + cy.visit('/static/settimeout_redirect_window_document_href.html') + cy.location('pathname').should('eq', '/static/index.html') + }) + + it('with location.href = #hash', function () { + cy.visit('/static/settimeout_redirect_href_hash.html') + cy.location().should('include', { + pathname: '/static/settimeout_redirect_href_hash.html', + hash: '#foo', + }) + }) + it('with location.replace()', function () { cy.visit('/static/settimeout_redirect_replace.html') cy.location('pathname').should('eq', '/static/index.html') @@ -47,6 +65,24 @@ describe('source rewriting spec', function () { cy.location('pathname').should('eq', '/static/index.html') }) + it('with document.location = ...', function () { + cy.visit('/static/settimeout_redirect_set_document_location.html') + cy.location('pathname').should('eq', '/static/index.html') + }) + + it('with window.document.location = ...', function () { + cy.visit('/static/settimeout_redirect_set_window_document_location.html') + cy.location('pathname').should('eq', '/static/index.html') + }) + + it('with document.location = #hash', function () { + cy.visit('/static/settimeout_redirect_set_document_location_hash.html') + cy.location().should('include', { + pathname: '/static/settimeout_redirect_set_document_location_hash.html', + hash: '#foo', + }) + }) + it('with location.search', function () { cy.visit('/static/settimeout_redirect_search.html') cy.location().should('include', { diff --git a/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_document_href.html b/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_document_href.html new file mode 100644 index 0000000000..cbf072151a --- /dev/null +++ b/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_document_href.html @@ -0,0 +1,13 @@ + + + + Document + + + + + diff --git a/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_href_hash.html b/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_href_hash.html new file mode 100644 index 0000000000..869ad2acba --- /dev/null +++ b/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_href_hash.html @@ -0,0 +1,13 @@ + + + + Document + + + + + diff --git a/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_set_document_location.html b/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_set_document_location.html new file mode 100644 index 0000000000..d6b5f8ca2b --- /dev/null +++ b/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_set_document_location.html @@ -0,0 +1,13 @@ + + + + Document + + + + + diff --git a/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_set_document_location_hash.html b/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_set_document_location_hash.html new file mode 100644 index 0000000000..f9751fbc69 --- /dev/null +++ b/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_set_document_location_hash.html @@ -0,0 +1,13 @@ + + + + Document + + + + + diff --git a/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_set_window_document_location.html b/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_set_window_document_location.html new file mode 100644 index 0000000000..48a2702e0b --- /dev/null +++ b/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_set_window_document_location.html @@ -0,0 +1,13 @@ + + + + Document + + + + + diff --git a/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_window_document_href.html b/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_window_document_href.html new file mode 100644 index 0000000000..af2457bcd3 --- /dev/null +++ b/packages/server/test/support/fixtures/projects/e2e/static/settimeout_redirect_window_document_href.html @@ -0,0 +1,13 @@ + + + + Document + + + + +