diff --git a/.circleci/workflows.yml b/.circleci/workflows.yml
index 939160e8b4e..566e6cd6363 100644
--- a/.circleci/workflows.yml
+++ b/.circleci/workflows.yml
@@ -28,7 +28,7 @@ mainBuildFilters: &mainBuildFilters
only:
- develop
- /^release\/\d+\.\d+\.\d+$/
- - 'tgriesser/spike/spike'
+ - 'fix-duplicate-and-expired-cookies'
# usually we don't build Mac app - it takes a long time
# but sometimes we want to really confirm we are doing the right thing
@@ -37,7 +37,7 @@ macWorkflowFilters: &darwin-workflow-filters
when:
or:
- equal: [ develop, << pipeline.git.branch >> ]
- - equal: [ 'tgriesser/spike/spike', << pipeline.git.branch >> ]
+ - equal: [ 'fix-duplicate-and-expired-cookies', << pipeline.git.branch >> ]
- matches:
pattern: /^release\/\d+\.\d+\.\d+$/
value: << pipeline.git.branch >>
@@ -46,7 +46,7 @@ linuxArm64WorkflowFilters: &linux-arm64-workflow-filters
when:
or:
- equal: [ develop, << pipeline.git.branch >> ]
- - equal: [ 'tgriesser/spike/spike', << pipeline.git.branch >> ]
+ - equal: [ 'fix-duplicate-and-expired-cookies', << pipeline.git.branch >> ]
- matches:
pattern: /^release\/\d+\.\d+\.\d+$/
value: << pipeline.git.branch >>
@@ -65,7 +65,7 @@ windowsWorkflowFilters: &windows-workflow-filters
or:
- equal: [ develop, << pipeline.git.branch >> ]
# use the following branch as well to ensure that v8 snapshot cache updates are fully tested
- - equal: [ 'update-v8-snapshot-cache-on-develop', << pipeline.git.branch >> ]
+ - equal: [ 'update-v8-snapshot-cache-on-develop', 'fix-duplicate-and-expired-cookies', << pipeline.git.branch >> ]
- matches:
pattern: /^release\/\d+\.\d+\.\d+$/
value: << pipeline.git.branch >>
@@ -131,7 +131,7 @@ commands:
- run:
name: Check current branch to persist artifacts
command: |
- if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "tgriesser/spike/spike" ]]; then
+ if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "fix-duplicate-and-expired-cookies" ]]; then
echo "Not uploading artifacts or posting install comment for this branch."
circleci-agent step halt
fi
diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md
index 4fb680375b2..974f0ad1ab5 100644
--- a/cli/CHANGELOG.md
+++ b/cli/CHANGELOG.md
@@ -1,4 +1,12 @@
+## 12.6.1
+
+_Released 03/1/2023 (PENDING)_
+
+**Bugfixes:**
+
+- Fixed an issue where cookies were being duplicated with the same hostname, but a prepended dot. Fixed an issue where cookies may not be expiring correctly. Fixes [#25174](https://github.com/cypress-io/cypress/issues/25174), [#25205](https://github.com/cypress-io/cypress/issues/25205) and [#25495](https://github.com/cypress-io/cypress/issues/25495).
+
## 12.6.0
_Released 02/15/2023_
diff --git a/packages/driver/cypress/e2e/e2e/origin/cookie_login.cy.ts b/packages/driver/cypress/e2e/e2e/origin/cookie_login.cy.ts
index 679bb32419c..e54da595783 100644
--- a/packages/driver/cypress/e2e/e2e/origin/cookie_login.cy.ts
+++ b/packages/driver/cypress/e2e/e2e/origin/cookie_login.cy.ts
@@ -723,7 +723,7 @@ describe('cy.origin - cookie login', { browser: '!webkit' }, () => {
cy.getCookie('key').then((cookie) => {
expect(Cypress._.omit(cookie, 'expiry')).to.deep.equal({
- domain: '.www.foobar.com',
+ domain: 'www.foobar.com',
httpOnly: false,
name: 'key',
path: '/fixtures',
@@ -851,7 +851,7 @@ describe('cy.origin - cookie login', { browser: '!webkit' }, () => {
cy.get('[data-cy="doc-cookie"]').invoke('text').should('equal', 'name=value')
cy.getCookie('name').then((cookie) => {
expect(Cypress._.omit(cookie, 'expiry')).to.deep.equal({
- domain: '.www.foobar.com',
+ domain: 'www.foobar.com',
httpOnly: false,
name: 'name',
path: '/',
diff --git a/packages/driver/cypress/e2e/e2e/origin/cookie_misc.cy.ts b/packages/driver/cypress/e2e/e2e/origin/cookie_misc.cy.ts
new file mode 100644
index 00000000000..cf5aee56a85
--- /dev/null
+++ b/packages/driver/cypress/e2e/e2e/origin/cookie_misc.cy.ts
@@ -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' }, () => {
+ // 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)
+ })
+ })
+ })
+ })
+})
diff --git a/packages/driver/cypress/fixtures/trigger-cross-origin-redirect-to-self.html b/packages/driver/cypress/fixtures/trigger-cross-origin-redirect-to-self.html
new file mode 100644
index 00000000000..3dc647db692
--- /dev/null
+++ b/packages/driver/cypress/fixtures/trigger-cross-origin-redirect-to-self.html
@@ -0,0 +1,21 @@
+
+
+
+
+
+ Trigger Cross Origin Redirect Loop that sets host only cookie
+ Trigger Cross Origin Redirect Loop that sets non host only cookie
+
+
+
diff --git a/packages/driver/cypress/plugins/server.js b/packages/driver/cypress/plugins/server.js
index a27eba20791..66efc48a5d0 100644
--- a/packages/driver/cypress/plugins/server.js
+++ b/packages/driver/cypress/plugins/server.js
@@ -296,6 +296,17 @@ const createApp = (port) => {
.sendStatus(200)
})
+ app.get('/set-same-site-none-cookie-on-redirect', (req, res) => {
+ const { redirect, cookie } = req.query
+ const cookieDecoded = decodeURIComponent(cookie)
+
+ const cookieVal = `${cookieDecoded}; SameSite=None; Secure`
+
+ res
+ .header('Set-Cookie', cookieVal)
+ .redirect(302, redirect)
+ })
+
app.get('/test-request-credentials', (req, res) => {
const origin = cors.getOrigin(req['headers']['referer'])
diff --git a/packages/runner/injection/patches/cookies.ts b/packages/runner/injection/patches/cookies.ts
index 3193d062f72..586191311de 100644
--- a/packages/runner/injection/patches/cookies.ts
+++ b/packages/runner/injection/patches/cookies.ts
@@ -6,18 +6,25 @@ import {
import { Cookie as ToughCookie } from 'tough-cookie'
import type { AutomationCookie } from '@packages/server/lib/automation/cookies'
+function isHostOnlyCookie (domain) {
+ return domain[0] !== '.'
+}
+
const parseDocumentCookieString = (documentCookieString: string): AutomationCookie[] => {
if (!documentCookieString || !documentCookieString.trim().length) return []
return documentCookieString.split(';').map((cookieString) => {
const [name, value] = cookieString.split('=')
+ let cookieDomain = location.hostname
+
return {
name: name.trim(),
value: value.trim(),
- domain: location.hostname,
+ domain: cookieDomain,
expiry: null,
httpOnly: false,
+ hostOnly: isHostOnlyCookie(cookieDomain),
maxAge: null,
path: null,
sameSite: 'lax',
@@ -88,9 +95,33 @@ export const patchDocumentCookie = (requestCookies: AutomationCookie[]) => {
const stringValue = `${newValue}`
const parsedCookie = CookieJar.parse(stringValue)
+ if (parsedCookie?.hostOnly === null) {
+ // we want to make sure the hostOnly property is respected when syncing with CDP/extension to prevent duplicates.
+ // in the case it is not set, we need to calculate it
+ parsedCookie.hostOnly = isHostOnlyCookie(parsedCookie.domain || domain)
+ }
+
// if result is undefined, it was invalid and couldn't be parsed
if (!parsedCookie) return getDocumentCookieValue()
+ // if the cookie is expired, remove it in our cookie jar
+ // and via setting it inside our automation client with the correct expiry.
+ // This will have the effect of removing the cookie
+ if (parsedCookie.expiryTime() < Date.now()) {
+ cookieJar.removeCookie({
+ name: parsedCookie.key,
+ path: parsedCookie.path || '/',
+ domain: parsedCookie.domain as string,
+ })
+
+ // send the cookie to the server so it can be removed from the browser
+ // via aututomation. If the cookie expiry is set inside the server-side cookie jar,
+ // the cookie will be automatically removed.
+ sendCookieToServer(toughCookieToAutomationCookie(parsedCookie, domain))
+
+ return getDocumentCookieValue()
+ }
+
// we should be able to pass in parsedCookie here instead of the string
// value, but tough-cookie doesn't recognize it using an instanceof
// check and throws an error. because we can't, we have to massage
diff --git a/packages/server/lib/automation/cookies.ts b/packages/server/lib/automation/cookies.ts
index ee45dcc8970..a1211d89d6c 100644
--- a/packages/server/lib/automation/cookies.ts
+++ b/packages/server/lib/automation/cookies.ts
@@ -5,8 +5,9 @@ import { isHostOnlyCookie } from '../browsers/cdp_automation'
export interface AutomationCookie {
domain: string
- expiry: number | null
+ expiry: 'Infinity' | '-Infinity' | number | null
httpOnly: boolean
+ hostOnly: boolean
maxAge: 'Infinity' | '-Infinity' | number | null
name: string
path: string | null
@@ -31,9 +32,20 @@ const normalizeCookieProps = function (props) {
return props
}
+ // if the cookie is stored inside the server side cookie jar,
+ // we want to make the automation client aware so the domain property
+ // isn't mutated to prevent duplicate setting of cookies from different contexts.
+ // This should be handled by the hostOnly property
+
const cookie = _.pick(props, COOKIE_PROPERTIES)
- if (props.expiry != null) {
+ if (props.expiry === '-Infinity') {
+ cookie.expiry = -Infinity
+ // set the cookie to expired so when set, the cookie is removed
+ cookie.expirationDate = 0
+ } else if (props.expiry === 'Infinity') {
+ cookie.expiry = null
+ } else if (props.expiry != null) {
// when sending cookie props we need to convert
// expiry to expirationDate
delete cookie.expiry
diff --git a/packages/server/lib/util/cookies.ts b/packages/server/lib/util/cookies.ts
index 650c8c7b927..f0f46c17138 100644
--- a/packages/server/lib/util/cookies.ts
+++ b/packages/server/lib/util/cookies.ts
@@ -16,8 +16,11 @@ export const toughCookieToAutomationCookie = (toughCookie: Cookie, defaultDomain
return {
domain: toughCookie.domain || defaultDomain,
- expiry: isFinite(expiry) ? expiry / 1000 : null,
+ // if expiry is Infinity or -Infinity, this operation is a no-op
+ expiry: (expiry === Infinity || expiry === -Infinity) ? expiry.toString() as '-Infinity' | 'Infinity' : expiry / 1000,
httpOnly: toughCookie.httpOnly,
+ // we want to make sure the hostOnly property is respected when syncing with CDP/extension to prevent duplicates
+ hostOnly: toughCookie.hostOnly || false,
maxAge: toughCookie.maxAge,
name: toughCookie.key,
path: toughCookie.path,
@@ -28,10 +31,23 @@ export const toughCookieToAutomationCookie = (toughCookie: Cookie, defaultDomain
}
export const automationCookieToToughCookie = (automationCookie: AutomationCookie, defaultDomain: string): Cookie => {
+ let expiry: Date | undefined = undefined
+
+ if (automationCookie.expiry != null) {
+ if (isFinite(automationCookie.expiry as number)) {
+ expiry = new Date(automationCookie.expiry as number * 1000)
+ } else if (automationCookie.expiry === '-Infinity') {
+ // if negative Infinity, the cookie is Date(0), has expired and is slated to be removed
+ expiry = new Date(0)
+ }
+ }
+
return new Cookie({
domain: automationCookie.domain || defaultDomain,
- expires: automationCookie.expiry != null && isFinite(automationCookie.expiry) ? new Date(automationCookie.expiry * 1000) : undefined,
+ expires: expiry,
httpOnly: automationCookie.httpOnly,
+ // we want to make sure the hostOnly property is respected when syncing with CDP/extension to prevent duplicates
+ hostOnly: automationCookie.hostOnly || false,
maxAge: automationCookie.maxAge || 'Infinity',
key: automationCookie.name,
path: automationCookie.path || undefined,
diff --git a/packages/server/test/unit/browsers/cdp_automation_spec.ts b/packages/server/test/unit/browsers/cdp_automation_spec.ts
index 8a7a03704b3..b54e262b478 100644
--- a/packages/server/test/unit/browsers/cdp_automation_spec.ts
+++ b/packages/server/test/unit/browsers/cdp_automation_spec.ts
@@ -204,6 +204,23 @@ context('lib/browsers/cdp_automation', () => {
})
})
+ it('resolves with the cookie props (host only)', function () {
+ this.sendDebuggerCommand
+ .withArgs('Network.setCookie', { domain: 'google.com', name: 'session', value: 'key', path: '/' })
+ .resolves({ success: true })
+ .withArgs('Network.getAllCookies')
+ .resolves({
+ cookies: [
+ { name: 'session', value: 'key', path: '/', domain: 'google.com', secure: false, httpOnly: false },
+ ],
+ })
+
+ return this.onRequest('set:cookie', { domain: 'google.com', name: 'session', value: 'key', path: '/', hostOnly: true })
+ .then((resp) => {
+ expect(resp).to.deep.eq({ domain: 'google.com', expirationDate: undefined, hostOnly: true, httpOnly: false, name: 'session', value: 'key', path: '/', secure: false, sameSite: undefined })
+ })
+ })
+
it('rejects with error', function () {
return this.onRequest('set:cookie', { domain: 'foo', path: '/bar' })
.then(() => {