fix: duplicate and expired cookies #25761
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,208 @@ | ||
| // FIXME: currently cookies aren't cleared properly in headless mode with webkit between tests, as the below tests (excluding cy.origin) pass headfully locally. | ||
| describe('misc cookie tests', { browser: '!webkit' }, () => { | ||
|
There was a problem hiding this comment. Any reason we can add these tests to the existing e2e/cookie_behavior spec? There was a problem hiding this comment. main reason is just how large the spec already is. I almost think we should rename it, just not sold on something yet There was a problem hiding this comment. I guess I'd rather have one place to look for tests related to a topic so it's easier to see if we have coverage rather than searching multiple files 🤷🏻♀️ There was a problem hiding this comment. I agree, but considering how flaky that spec currently is, I don't want real issues to get lost in the noise. Not a great reason, but I also think the two specs are accomplishing two different things. This one is more for expiry and duplication bug fixes, and the cookie behavior tests are mostly checking attachment for xhr/fetch requests. Would it be better to break the There was a problem hiding this comment. maybe group the various cookie tests under a cookie directory? There was a problem hiding this comment. I'm gonna have a follow up to this PR that is going to address types, as well as refactoring some of these tests and addressing some of the cookie behavior flake |
||
| // NOTE: For this test to work correctly, we need to have a FQDN, not localhost (www.foobar.com). | ||
| // FIXES: https://github.com/cypress-io/cypress/issues/25174 (cookies are duplicated with prepended dot (.)) | ||
| it('does not duplicate cookies with a prepended dot for cookies that are stored inside the server side cookie jar (host only)', () => { | ||
| cy.visit('https://www.foobar.com:3502/fixtures/trigger-cross-origin-redirect-to-self.html') | ||
|
|
||
| // does a 302 redirect back to www.foobar.com primary-origin page, but sets a sameSite=None cookie | ||
| cy.get('[data-cy="cookie-cross-origin-redirects-host-only"]').click() | ||
|
|
||
| cy.getCookies({ domain: 'www.foobar.com' }).then((cookies) => { | ||
| expect(cookies).to.have.length(1) | ||
|
|
||
| const singleCookie = cookies[0] | ||
|
|
||
| expect(singleCookie).to.have.property('name', 'foo') | ||
| expect(singleCookie).to.have.property('value', 'bar') | ||
| expect(singleCookie).to.have.property('domain', 'www.foobar.com') | ||
| }) | ||
| }) | ||
|
|
||
| it('does not duplicate cookies with a prepended dot for cookies that are stored inside the server side cookie jar (non-host only)', () => { | ||
| cy.visit('https://www.foobar.com:3502/fixtures/trigger-cross-origin-redirect-to-self.html') | ||
|
|
||
| // does a 302 redirect back to www.foobar.com primary-origin page, but sets a sameSite=None cookie | ||
| cy.get('[data-cy="cookie-cross-origin-redirects"]').click() | ||
|
|
||
| cy.getCookies({ domain: 'www.foobar.com' }).then((cookies) => { | ||
| expect(cookies).to.have.length(1) | ||
|
|
||
| const singleCookie = cookies[0] | ||
|
|
||
| expect(singleCookie).to.have.property('name', 'foo') | ||
| expect(singleCookie).to.have.property('value', 'bar') | ||
| expect(singleCookie).to.have.property('domain', '.www.foobar.com') | ||
| }) | ||
| }) | ||
|
|
||
| /** | ||
| * FIXES: | ||
| * https://github.com/cypress-io/cypress/issues/25205 (cookies set with expired time with value deleted show up as set with value deleted) | ||
| * https://github.com/cypress-io/cypress/issues/25495 (session cookies set with expired time with value deleted show up as set with value deleted) | ||
| * https://github.com/cypress-io/cypress/issues/25148 (cannot log into azure, shows cookies are disabled/blocked) | ||
| */ | ||
| describe('expiring cookies', { browser: '!webkit' }, () => { | ||
| before(() => { | ||
| cy.origin(`https://app.foobar.com:3503`, () => { | ||
| window.makeRequest = Cypress.require('../../../support/utils').makeRequestForCookieBehaviorTests | ||
| }) | ||
| }) | ||
|
|
||
| describe('removes cookies that are set with an expired expiry time from the server side cookie jar / browser via CDP', () => { | ||
| it('works with Max-Age=0', () => { | ||
| cy.visit(`https://www.foobar.com:3502/fixtures/primary-origin.html`) | ||
|
|
||
| cy.visit(`https://app.foobar.com:3503/fixtures/secondary-origin.html`) | ||
| cy.origin(`https://app.foobar.com:3503`, () => { | ||
| cy.window().then((win) => { | ||
| return cy.wrap(window.makeRequest(win, `/set-cookie?cookie=foo=bar; Domain=.foobar.com;`)) | ||
| }) | ||
|
|
||
| cy.getCookie('foo').its('value').should('eq', 'bar') | ||
|
|
||
| cy.window().then((win) => { | ||
| return cy.wrap(window.makeRequest(win, `/set-cookie?cookie=foo=deleted; Domain=.foobar.com; Max-Age=0;`)) | ||
| }) | ||
|
|
||
| cy.getCookie('foo').should('eq', null) | ||
| }) | ||
| }) | ||
|
|
||
| it('works with expires=Thu, 01-Jan-1970 00:00:01 GMT', () => { | ||
| cy.visit(`https://www.foobar.com:3502/fixtures/primary-origin.html`) | ||
|
|
||
| cy.visit(`https://app.foobar.com:3503/fixtures/secondary-origin.html`) | ||
| cy.origin(`https://app.foobar.com:3503`, () => { | ||
| cy.window().then((win) => { | ||
| return cy.wrap(window.makeRequest(win, `/set-cookie?cookie=foo=bar; Domain=.foobar.com;`)) | ||
| }) | ||
|
|
||
| cy.getCookie('foo').its('value').should('eq', 'bar') | ||
|
|
||
| cy.window().then((win) => { | ||
| return cy.wrap(window.makeRequest(win, `/set-cookie?cookie=foo=deleted; Domain=.foobar.com; expires=Thu, 01-Jan-1970 00:00:01 GMT;`)) | ||
| }) | ||
|
|
||
| cy.getCookie('foo').should('eq', null) | ||
| }) | ||
| }) | ||
|
|
||
| it('works with expires=Tues, 01-Jan-1980 00:00:01 GMT', () => { | ||
| cy.visit(`https://www.foobar.com:3502/fixtures/primary-origin.html`) | ||
|
|
||
| cy.visit(`https://app.foobar.com:3503/fixtures/secondary-origin.html`) | ||
| cy.origin(`https://app.foobar.com:3503`, () => { | ||
| cy.window().then((win) => { | ||
| return cy.wrap(window.makeRequest(win, `/set-cookie?cookie=foo=bar; Domain=.foobar.com;`)) | ||
| }) | ||
|
|
||
| cy.getCookie('foo').its('value').should('eq', 'bar') | ||
|
|
||
| cy.window().then((win) => { | ||
| return cy.wrap(window.makeRequest(win, `/set-cookie?cookie=foo=deleted; Domain=.foobar.com; expires=Tues, 01-Jan-1980 00:00:01 GMT; Max-Age=0;`)) | ||
| }) | ||
|
|
||
| cy.getCookie('foo').should('eq', null) | ||
| }) | ||
| }) | ||
|
|
||
| it('work with expires=Thu, 01-Jan-1970 00:00:01 GMT and Max-Age=0', () => { | ||
| cy.visit(`https://www.foobar.com:3502/fixtures/primary-origin.html`) | ||
|
|
||
| cy.visit(`https://app.foobar.com:3503/fixtures/secondary-origin.html`) | ||
| cy.origin(`https://app.foobar.com:3503`, () => { | ||
| cy.window().then((win) => { | ||
| return cy.wrap(window.makeRequest(win, `/set-cookie?cookie=foo=bar; Domain=.foobar.com;`)) | ||
| }) | ||
|
|
||
| cy.getCookie('foo').its('value').should('eq', 'bar') | ||
|
|
||
| cy.window().then((win) => { | ||
| return cy.wrap(window.makeRequest(win, `/set-cookie?cookie=foo=deleted; Domain=.foobar.com; expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0;`)) | ||
| }) | ||
|
|
||
| cy.getCookie('foo').should('eq', null) | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| describe('removes cookies that are set with an expired expiry time from the document.cookie patch / browser via CDP', () => { | ||
| it('works with Max-Age=0', () => { | ||
| cy.visit(`https://www.foobar.com:3502/fixtures/primary-origin.html`) | ||
|
|
||
| cy.visit(`https://app.foobar.com:3503/fixtures/secondary-origin.html`) | ||
| cy.origin(`https://app.foobar.com:3503`, () => { | ||
| cy.window().then((win) => { | ||
| win.document.cookie = 'foo=bar' | ||
| }) | ||
|
|
||
| cy.getCookie('foo').its('value').should('eq', 'bar') | ||
|
|
||
| cy.window().then((win) => { | ||
| win.document.cookie = 'foo=deleted; expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0;' | ||
| }) | ||
|
|
||
| cy.getCookie('foo').should('eq', null) | ||
| }) | ||
| }) | ||
|
|
||
| it('works with expires=Thu, 01-Jan-1970 00:00:01 GMT', () => { | ||
| cy.visit(`https://www.foobar.com:3502/fixtures/primary-origin.html`) | ||
|
|
||
| cy.visit(`https://app.foobar.com:3503/fixtures/secondary-origin.html`) | ||
| cy.origin(`https://app.foobar.com:3503`, () => { | ||
| cy.window().then((win) => { | ||
| win.document.cookie = 'foo=bar' | ||
| }) | ||
|
|
||
| cy.getCookie('foo').its('value').should('eq', 'bar') | ||
|
|
||
| cy.window().then((win) => { | ||
| win.document.cookie = 'foo=deleted; Max-Age=0' | ||
| }) | ||
|
|
||
| cy.getCookie('foo').should('eq', null) | ||
| }) | ||
| }) | ||
|
|
||
| it('works with expires=Tues, 01-Jan-1980 00:00:01 GMT', () => { | ||
| cy.visit(`https://www.foobar.com:3502/fixtures/primary-origin.html`) | ||
|
|
||
| cy.visit(`https://app.foobar.com:3503/fixtures/secondary-origin.html`) | ||
| cy.origin(`https://app.foobar.com:3503`, () => { | ||
| cy.window().then((win) => { | ||
| win.document.cookie = 'foo=bar' | ||
| }) | ||
|
|
||
| cy.getCookie('foo').its('value').should('eq', 'bar') | ||
|
|
||
| cy.window().then((win) => { | ||
| win.document.cookie = 'foo=deleted; expires=Tues, 01-Jan-1980 00:00:01 GMT' | ||
| }) | ||
|
|
||
| cy.getCookie('foo').should('eq', null) | ||
| }) | ||
| }) | ||
|
|
||
| it('expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0', () => { | ||
| cy.visit(`https://www.foobar.com:3502/fixtures/primary-origin.html`) | ||
|
|
||
| cy.visit(`https://app.foobar.com:3503/fixtures/secondary-origin.html`) | ||
| cy.origin(`https://app.foobar.com:3503`, () => { | ||
| cy.window().then((win) => { | ||
| win.document.cookie = 'foo=bar' | ||
| }) | ||
|
|
||
| cy.getCookie('foo').its('value').should('eq', 'bar') | ||
|
|
||
| cy.window().then((win) => { | ||
| win.document.cookie = 'foo=deleted; expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0' | ||
| }) | ||
|
|
||
| cy.getCookie('foo').should('eq', null) | ||
| }) | ||
| }) | ||
| }) | ||
| }) | ||
| }) | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| <!DOCTYPE html> | ||
| <html> | ||
| <head> | ||
| </head> | ||
| <body> | ||
| <a data-cy="cookie-cross-origin-redirects-host-only">Trigger Cross Origin Redirect Loop that sets host only cookie</a> | ||
| <a data-cy="cookie-cross-origin-redirects">Trigger Cross Origin Redirect Loop that sets non host only cookie</a> | ||
| <script> | ||
| function setHref (dataCy, redirect, cookie) { | ||
| const redirectEncoded = encodeURIComponent(redirect) | ||
| const cookieEncoded = encodeURIComponent(cookie) | ||
|
|
||
| document.querySelector(`[data-cy="${dataCy}"]`).href = ( | ||
| `/set-same-site-none-cookie-on-redirect?redirect=${redirectEncoded}&cookie=${cookieEncoded}` | ||
| ) | ||
| } | ||
| setHref('cookie-cross-origin-redirects-host-only', `${window.location.origin}/fixtures/primary-origin.html`, 'foo=bar') | ||
| setHref('cookie-cross-origin-redirects', `${window.location.origin}/fixtures/primary-origin.html`, `foo=bar; domain=.${window.location.hostname}`) | ||
| </script> | ||
| </body> | ||
| </html> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to change the logic here for the duplicate cookie work to apply the actual cookie domain or default
hostOnlydomain to the set cookie here. This is to make sure the cookie doesn't double up with a non host only equivalent in CDP. This should be fine for two reasons: