diff --git a/docs/api/puppeteer.deletecookiesrequest.md b/docs/api/puppeteer.deletecookiesrequest.md index ae206306afc..e74f197d436 100644 --- a/docs/api/puppeteer.deletecookiesrequest.md +++ b/docs/api/puppeteer.deletecookiesrequest.md @@ -12,9 +12,9 @@ export interface DeleteCookiesRequest ## Properties -| Property | Modifiers | Type | Description | Default | -| -------- | --------------------- | ------ | --------------------------------------------------------------------------------------------------- | ------- | -| domain | optional | string | If specified, deletes only cookies with the exact domain. | | -| name | | string | Name of the cookies to remove. | | -| path | optional | string | If specified, deletes only cookies with the exact path. | | -| url | optional | string | If specified, deletes all the cookies with the given name where domain and path match provided URL. | | +| Property | Modifiers | Type | Description | Default | +| -------- | --------------------- | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | +| domain | optional | string | If specified, deletes only cookies with the exact domain. | | +| name | | string | Name of the cookies to remove. | | +| path | optional | string | If specified, deletes only cookies with the exact path. | | +| url | optional | string | If specified, deletes all the cookies with the given name where domain and path match provided URL. Otherwise, deletes only cookies related to the current page's domain. | | diff --git a/packages/puppeteer-core/src/bidi/Page.ts b/packages/puppeteer-core/src/bidi/Page.ts index 4e4279ecb61..c662496a187 100644 --- a/packages/puppeteer-core/src/bidi/Page.ts +++ b/packages/puppeteer-core/src/bidi/Page.ts @@ -28,6 +28,7 @@ import {Coverage} from '../cdp/Coverage.js'; import {EmulationManager} from '../cdp/EmulationManager.js'; import {Tracing} from '../cdp/Tracing.js'; import type {Cookie, CookieParam, CookieSameSite} from '../common/Cookie.js'; +import type {DeleteCookiesRequest} from '../common/Cookie.js'; import {UnsupportedOperation} from '../common/Errors.js'; import {EventEmitter} from '../common/EventEmitter.js'; import type {PDFOptions} from '../common/PDFOptions.js'; @@ -559,9 +560,6 @@ export class BidiPage extends Page { ), }; - // TODO: delete cookie before setting them. - // await this.deleteCookie(bidiCookie); - if (cookie.partitionKey !== undefined) { await this.browserContext().userContext.setCookie( bidiCookie, @@ -573,8 +571,32 @@ export class BidiPage extends Page { } } - override deleteCookie(): never { - throw new UnsupportedOperation(); + override async deleteCookie( + ...cookies: DeleteCookiesRequest[] + ): Promise { + await Promise.all( + cookies.map(async deleteCookieRequest => { + const cookieUrl = deleteCookieRequest.url ?? this.url(); + const normalizedUrl = URL.canParse(cookieUrl) + ? new URL(cookieUrl) + : undefined; + + const domain = deleteCookieRequest.domain ?? normalizedUrl?.hostname; + assert( + domain !== undefined, + `At least one of the url and domain needs to be specified` + ); + + const filter = { + domain: domain, + name: deleteCookieRequest.name, + ...(deleteCookieRequest.path !== undefined + ? {path: deleteCookieRequest.path} + : {}), + }; + await this.#frame.browsingContext.deleteCookie(filter); + }) + ); } override async removeExposedFunction(name: string): Promise { diff --git a/packages/puppeteer-core/src/bidi/core/BrowsingContext.ts b/packages/puppeteer-core/src/bidi/core/BrowsingContext.ts index 6ba600c0e56..07309576a35 100644 --- a/packages/puppeteer-core/src/bidi/core/BrowsingContext.ts +++ b/packages/puppeteer-core/src/bidi/core/BrowsingContext.ts @@ -547,4 +547,24 @@ export class BrowsingContext extends EventEmitter<{ this.#disposables.dispose(); super[disposeSymbol](); } + + @throwIfDisposed(context => { + // SAFETY: Disposal implies this exists. + return context.#reason!; + }) + async deleteCookie( + ...cookieFilters: Bidi.Storage.CookieFilter[] + ): Promise { + await Promise.all( + cookieFilters.map(async filter => { + await this.#session.send('storage.deleteCookies', { + filter: filter, + partition: { + type: 'context', + context: this.id, + }, + }); + }) + ); + } } diff --git a/packages/puppeteer-core/src/bidi/core/Connection.ts b/packages/puppeteer-core/src/bidi/core/Connection.ts index 90aa16144db..9c26a035038 100644 --- a/packages/puppeteer-core/src/bidi/core/Connection.ts +++ b/packages/puppeteer-core/src/bidi/core/Connection.ts @@ -137,6 +137,10 @@ export interface Commands { returnType: Bidi.EmptyResult; }; + 'storage.deleteCookies': { + params: Bidi.Storage.DeleteCookiesParameters; + returnType: Bidi.Storage.DeleteCookiesResult; + }; 'storage.getCookies': { params: Bidi.Storage.GetCookiesParameters; returnType: Bidi.Storage.GetCookiesResult; diff --git a/packages/puppeteer-core/src/common/Cookie.ts b/packages/puppeteer-core/src/common/Cookie.ts index f9bafa20dba..c9f7283075b 100644 --- a/packages/puppeteer-core/src/common/Cookie.ts +++ b/packages/puppeteer-core/src/common/Cookie.ts @@ -172,7 +172,7 @@ export interface DeleteCookiesRequest { name: string; /** * If specified, deletes all the cookies with the given name where domain and path match - * provided URL. + * provided URL. Otherwise, deletes only cookies related to the current page's domain. */ url?: string; /** diff --git a/test/TestExpectations.json b/test/TestExpectations.json index d77f1a49a3f..cf4bc0d9def 100644 --- a/test/TestExpectations.json +++ b/test/TestExpectations.json @@ -276,6 +276,13 @@ "parameters": ["firefox", "webDriverBiDi"], "expectations": ["SKIP"] }, + { + "testIdPattern": "[cookies.spec] Cookie specs Page.deleteCookie should delete cookie for specified URL regardless of the current page", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["firefox"], + "expectations": ["SKIP"], + "comment": "The test relies on the default page partition key do not contain the source origin. This is not the case for Firefox." + }, { "testIdPattern": "[coverage.spec] *", "platforms": ["darwin", "linux", "win32"], @@ -1081,22 +1088,46 @@ "expectations": ["FAIL"] }, { - "testIdPattern": "[cookies.spec] Cookie specs Page.deleteCookie should work", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["chrome", "webDriverBiDi"], - "expectations": ["FAIL"] - }, - { - "testIdPattern": "[cookies.spec] Cookie specs Page.deleteCookie should work", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["cdp", "firefox"], - "expectations": ["FAIL"] - }, - { - "testIdPattern": "[cookies.spec] Cookie specs Page.deleteCookie should work", + "testIdPattern": "[cookies.spec] Cookie specs Page.deleteCookie should delete cookie", "platforms": ["darwin", "linux", "win32"], "parameters": ["firefox", "webDriverBiDi"], - "expectations": ["FAIL"] + "expectations": ["FAIL"], + "comment": "Firefox default partition key is inconsistent: #12004" + }, + { + "testIdPattern": "[cookies.spec] Cookie specs Page.deleteCookie should delete cookie", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["cdp", "firefox"], + "expectations": ["FAIL"], + "comment": "Not supported with cdp" + }, + { + "testIdPattern": "[cookies.spec] Cookie specs Page.deleteCookie should delete cookie for specified URL", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["firefox", "webDriverBiDi"], + "expectations": ["FAIL"], + "comment": "Firefox default partition key is inconsistent: #12004" + }, + { + "testIdPattern": "[cookies.spec] Cookie specs Page.deleteCookie should delete cookie for specified URL", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["cdp", "firefox"], + "expectations": ["FAIL"], + "comment": "Not supported with cdp" + }, + { + "testIdPattern": "[cookies.spec] Cookie specs Page.deleteCookie should not delete cookie for different domain", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["firefox", "webDriverBiDi"], + "expectations": ["FAIL"], + "comment": "Firefox default partition key is inconsistent: #12004" + }, + { + "testIdPattern": "[cookies.spec] Cookie specs Page.deleteCookie should not delete cookie for different domain", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["cdp", "firefox"], + "expectations": ["FAIL"], + "comment": "Not supported with cdp" }, { "testIdPattern": "[cookies.spec] Cookie specs Page.setCookie should be able to set insecure cookie for HTTP website", @@ -1194,12 +1225,6 @@ "parameters": ["cdp", "firefox"], "expectations": ["FAIL", "PASS"] }, - { - "testIdPattern": "[defaultbrowsercontext.spec] DefaultBrowserContext page.deleteCookie() should work", - "platforms": ["darwin", "linux", "win32"], - "parameters": ["chrome", "webDriverBiDi"], - "expectations": ["FAIL"] - }, { "testIdPattern": "[defaultbrowsercontext.spec] DefaultBrowserContext page.deleteCookie() should work", "platforms": ["darwin", "linux", "win32"], diff --git a/test/src/cookies.spec.ts b/test/src/cookies.spec.ts index dbe265059af..5a6d2475956 100644 --- a/test/src/cookies.spec.ts +++ b/test/src/cookies.spec.ts @@ -558,7 +558,7 @@ describe('Cookie specs', () => { }); describe('Page.deleteCookie', function () { - it('should work', async () => { + it('should delete cookie', async () => { const {page, server} = await getTestState(); await page.goto(server.EMPTY_PAGE); @@ -584,5 +584,139 @@ describe('Cookie specs', () => { 'cookie1=1; cookie3=3' ); }); + it('should not delete cookie for different domain', async () => { + const {page, server} = await getTestState(); + const COOKIE_DESTINATION_URL = 'https://example.com'; + const COOKIE_NAME = 'some_cookie_name'; + + await page.goto(server.EMPTY_PAGE); + // Set a cookie for the current page. + await page.setCookie({ + name: COOKIE_NAME, + value: 'local page cookie value', + }); + expect(await page.cookies()).toHaveLength(1); + + // Set a cookie for different domain. + await page.setCookie({ + url: COOKIE_DESTINATION_URL, + name: COOKIE_NAME, + value: 'COOKIE_DESTINATION_URL cookie value', + }); + expect(await page.cookies(COOKIE_DESTINATION_URL)).toHaveLength(1); + + await page.deleteCookie({name: COOKIE_NAME}); + + // Verify the cookie is deleted for the current page. + expect(await page.cookies()).toHaveLength(0); + + // Verify the cookie is not deleted for different domain. + await expectCookieEquals(await page.cookies(COOKIE_DESTINATION_URL), [ + { + name: COOKIE_NAME, + value: 'COOKIE_DESTINATION_URL cookie value', + domain: 'example.com', + path: '/', + sameParty: false, + expires: -1, + size: 51, + httpOnly: false, + secure: true, + session: true, + sourceScheme: 'Secure', + }, + ]); + }); + it('should delete cookie for specified URL', async () => { + const {page, server} = await getTestState(); + const COOKIE_DESTINATION_URL = 'https://example.com'; + const COOKIE_NAME = 'some_cookie_name'; + + await page.goto(server.EMPTY_PAGE); + // Set a cookie for the current page. + await page.setCookie({ + name: COOKIE_NAME, + value: 'some_cookie_value', + }); + expect(await page.cookies()).toHaveLength(1); + + // Set a cookie for specified URL. + await page.setCookie({ + url: COOKIE_DESTINATION_URL, + name: COOKIE_NAME, + value: 'another_cookie_value', + }); + expect(await page.cookies(COOKIE_DESTINATION_URL)).toHaveLength(1); + + // Delete the cookie for specified URL. + await page.deleteCookie({ + url: COOKIE_DESTINATION_URL, + name: COOKIE_NAME, + }); + + // Verify the cookie is deleted for specified URL. + expect(await page.cookies(COOKIE_DESTINATION_URL)).toHaveLength(0); + + // Verify the cookie is not deleted for the current page. + await expectCookieEquals(await page.cookies(), [ + { + name: COOKIE_NAME, + value: 'some_cookie_value', + domain: 'localhost', + path: '/', + sameParty: false, + expires: -1, + size: 33, + httpOnly: false, + secure: false, + session: true, + sourceScheme: 'NonSecure', + }, + ]); + }); + it('should delete cookie for specified URL regardless of the current page', async () => { + // This test verifies the page.deleteCookie method deletes cookies for the custom + // destination URL, even if it was set from another page. Depending on the cookie + // partitioning implementation, this test case does not pass, if source origin is in + // the default cookie partition. + + const {page, server} = await getTestState(); + const COOKIE_DESTINATION_URL = 'https://example.com'; + const COOKIE_NAME = 'some_cookie_name'; + const URL_1 = server.EMPTY_PAGE; + const URL_2 = server.CROSS_PROCESS_PREFIX + '/empty.html'; + + await page.goto(URL_1); + // Set a cookie for the COOKIE_DESTINATION from URL_1. + await page.setCookie({ + url: COOKIE_DESTINATION_URL, + name: COOKIE_NAME, + value: 'Cookie from URL_1', + }); + expect(await page.cookies(COOKIE_DESTINATION_URL)).toHaveLength(1); + + await page.goto(URL_2); + // Set a cookie for the COOKIE_DESTINATION from URL_2. + await page.setCookie({ + url: COOKIE_DESTINATION_URL, + name: COOKIE_NAME, + value: 'Cookie from URL_2', + }); + expect(await page.cookies(COOKIE_DESTINATION_URL)).toHaveLength(1); + + // Delete the cookie for the COOKIE_DESTINATION from URL_2. + await page.deleteCookie({ + name: COOKIE_NAME, + url: COOKIE_DESTINATION_URL, + }); + + // Expect the cookie for the COOKIE_DESTINATION from URL_2 is deleted. + expect(await page.cookies(COOKIE_DESTINATION_URL)).toHaveLength(0); + + // Navigate back to the URL_1. + await page.goto(server.EMPTY_PAGE); + // Expect the cookie for the COOKIE_DESTINATION from URL_1 is deleted. + expect(await page.cookies(COOKIE_DESTINATION_URL)).toHaveLength(0); + }); }); });