Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not allow to override cookie header #35168

Merged
merged 2 commits into from
Mar 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/src/api/class-route.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ The [`option: headers`] option applies to both the routed request and any redire

[`method: Route.continue`] will immediately send the request to the network, other matching handlers won't be invoked. Use [`method: Route.fallback`] If you want next matching handler in the chain to be invoked.

:::warning
The `Cookie` header cannot be overridden using this method. If a value is provided, it will be ignored, and the cookie will be loaded from the browser's cookie store. To set custom cookies, use [`method: BrowserContext.addCookies`].
:::

### option: Route.continue.url
* since: v1.8
- `url` <[string]>
Expand Down
5 changes: 5 additions & 0 deletions packages/playwright-client/types/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20786,6 +20786,11 @@ export interface Route {
* request to the network, other matching handlers won't be invoked. Use
* [route.fallback([options])](https://playwright.dev/docs/api/class-route#route-fallback) If you want next matching
* handler in the chain to be invoked.
*
* **NOTE** The `Cookie` header cannot be overridden using this method. If a value is provided, it will be ignored,
* and the cookie will be loaded from the browser's cookie store. To set custom cookies, use
* [browserContext.addCookies(cookies)](https://playwright.dev/docs/api/class-browsercontext#browser-context-add-cookies).
*
* @param options
*/
continue(options?: {
Expand Down
2 changes: 2 additions & 0 deletions packages/playwright-core/src/server/network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,8 @@ export class Route extends SdkObject {
if (oldUrl.protocol !== newUrl.protocol)
throw new Error('New URL must have same protocol as overridden URL');
}
if (overrides.headers)
overrides.headers = overrides.headers?.filter(header => header.name.toLowerCase() !== 'cookie');
this._request._setOverrides(overrides);
if (!overrides.isFallback)
this._request._context.emit(BrowserContext.Events.RequestContinued, this._request);
Expand Down
5 changes: 5 additions & 0 deletions packages/playwright-core/types/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20786,6 +20786,11 @@ export interface Route {
* request to the network, other matching handlers won't be invoked. Use
* [route.fallback([options])](https://playwright.dev/docs/api/class-route#route-fallback) If you want next matching
* handler in the chain to be invoked.
*
* **NOTE** The `Cookie` header cannot be overridden using this method. If a value is provided, it will be ignored,
* and the cookie will be loaded from the browser's cookie store. To set custom cookies, use
* [browserContext.addCookies(cookies)](https://playwright.dev/docs/api/class-browsercontext#browser-context-add-cookies).
*
* @param options
*/
continue(options?: {
Expand Down
4 changes: 2 additions & 2 deletions tests/library/har.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ it('should record request overrides', async ({ contextFactory, server }, testInf
expect(request.url).toBe(server.EMPTY_PAGE);
expect(request.method).toBe('POST');
expect(request.headers).toContainEqual({ name: 'custom', value: 'value' });
expect(request.cookies).toContainEqual({ name: 'foo', value: 'bar' });
expect(request.cookies).toEqual([]);
expect(request.postData).toEqual({ 'mimeType': 'text/plain', 'params': [], 'text': 'Hi!' });
});

Expand Down Expand Up @@ -508,7 +508,7 @@ it('should record failed request overrides', async ({ contextFactory, server },
expect(request.url).toBe(server.EMPTY_PAGE);
expect(request.method).toBe('POST');
expect(request.headers).toContainEqual({ name: 'custom', value: 'value' });
expect(request.cookies).toContainEqual({ name: 'foo', value: 'bar' });
expect(request.cookies).toEqual([]);
expect(request.postData).toEqual({ 'mimeType': 'text/plain', 'params': [], 'text': 'Hi!' });
});

Expand Down
105 changes: 102 additions & 3 deletions tests/page/page-request-continue.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,96 @@ it('should respect set-cookie in redirect response', {
expect.soft(await page.evaluate(() => document.cookie)).toBe('foo=bar');
});

it('continue should not propagate cookie override to redirects', {
annotation: [
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/35168' },
]
}, async ({ page, server, browserName }) => {
it.fixme(browserName === 'firefox', 'We currently clear all headers during interception in firefox');
server.setRoute('/set-cookie', (request, response) => {
response.writeHead(200, { 'Set-Cookie': 'foo=bar;' });
response.end();
});
await page.goto(server.PREFIX + '/set-cookie');
expect(await page.evaluate(() => document.cookie)).toBe('foo=bar');
server.setRedirect('/redirect', server.PREFIX + '/empty.html');
await page.route('**/redirect', route => {
void route.continue({
headers: {
...route.request().headers(),
cookie: 'override'
}
});
});
const [serverRequest] = await Promise.all([
server.waitForRequest('/empty.html'),
page.goto(server.PREFIX + '/redirect')
]);
expect(serverRequest.headers['cookie']).toBe('foo=bar');
});

it('continue should not override cookie', {
annotation: [
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/35168' },
]
}, async ({ page, server, browserName }) => {
it.fixme(browserName === 'firefox', 'We currently clear all headers during interception in firefox');
server.setRoute('/set-cookie', (request, response) => {
response.writeHead(200, { 'Set-Cookie': 'foo=bar;' });
response.end();
});
await page.goto(server.PREFIX + '/set-cookie');
expect(await page.evaluate(() => document.cookie)).toBe('foo=bar');
await page.route('**', route => {
void route.continue({
headers: {
...route.request().headers(),
cookie: 'override',
custom: 'value'
}
});
});
const [serverRequest] = await Promise.all([
server.waitForRequest('/empty.html'),
page.goto(server.EMPTY_PAGE)
]);
// Original cookie from the browser's cookie jar should be sent.
expect(serverRequest.headers['cookie']).toBe('foo=bar');
expect(serverRequest.headers['custom']).toBe('value');
});

it('redirect after continue should be able to delete cookie', {
annotation: [
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/35168' },
]
}, async ({ page, server }) => {
server.setRoute('/set-cookie', (request, response) => {
response.writeHead(200, { 'Set-Cookie': 'foo=bar;' });
response.end();
});
await page.goto(server.PREFIX + '/set-cookie');
expect(await page.evaluate(() => document.cookie)).toBe('foo=bar');

server.setRoute('/delete-cookie', (request, response) => {
response.writeHead(200, { 'Set-Cookie': 'foo=bar; expires=Thu, 01 Jan 1970 00:00:00 GMT' });
response.end();
});
server.setRedirect('/redirect', '/delete-cookie');
await page.route('**/redirect', route => {
void route.continue({
headers: {
...route.request().headers(),
}
});
});
await page.goto(server.PREFIX + '/redirect');
const [serverRequest] = await Promise.all([
server.waitForRequest('/empty.html'),
page.goto(server.EMPTY_PAGE)
]);
expect(serverRequest.headers['cookie']).toBeFalsy();
});

it('continue should propagate headers to redirects', {
annotation: [
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/28758' },
Expand Down Expand Up @@ -536,6 +626,7 @@ it('propagate headers same origin redirect', {
annotation: [
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/13106' },
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/32045' },
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/35154' },
]
}, async ({ page, server }) => {
await page.goto(server.PREFIX + '/empty.html');
Expand All @@ -547,7 +638,7 @@ it('propagate headers same origin redirect', {
'Access-Control-Allow-Origin': server.PREFIX,
'Access-Control-Allow-Credentials': 'true',
'Access-Control-Allow-Methods': 'POST, GET, OPTIONS, DELETE',
'Access-Control-Allow-Headers': 'authorization,custom',
'Access-Control-Allow-Headers': 'authorization,cookie,custom',
});
response.end();
return;
Expand All @@ -557,6 +648,7 @@ it('propagate headers same origin redirect', {
response.end('done');
});
await server.setRedirect('/redirect', '/something');
await page.evaluate(() => document.cookie = 'a=b');
const text = await page.evaluate(async url => {
const data = await fetch(url, {
headers: {
Expand All @@ -570,6 +662,7 @@ it('propagate headers same origin redirect', {
expect(text).toBe('done');
const serverRequest = await serverRequestPromise;
expect.soft(serverRequest.headers['authorization']).toBe('credentials');
expect.soft(serverRequest.headers['cookie']).toBe('a=b');
expect.soft(serverRequest.headers['custom']).toBe('foo');
});

Expand Down Expand Up @@ -620,6 +713,7 @@ it('propagate headers cross origin redirect', {
annotation: [
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/13106' },
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/32045' },
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/35154' },
]
}, async ({ page, server, isAndroid }) => {
it.fixme(isAndroid, 'receives authorization:credentials header');
Expand All @@ -633,7 +727,7 @@ it('propagate headers cross origin redirect', {
'Access-Control-Allow-Origin': server.PREFIX,
'Access-Control-Allow-Credentials': 'true',
'Access-Control-Allow-Methods': 'POST, GET, OPTIONS, DELETE',
'Access-Control-Allow-Headers': 'authorization,custom',
'Access-Control-Allow-Headers': 'authorization,cookie,custom',
});
response.end();
return;
Expand All @@ -649,6 +743,7 @@ it('propagate headers cross origin redirect', {
response.writeHead(301, { location: `${server.CROSS_PROCESS_PREFIX}/something` });
response.end();
});
await page.evaluate(() => document.cookie = 'a=b');
const text = await page.evaluate(async url => {
const data = await fetch(url, {
headers: {
Expand All @@ -663,13 +758,15 @@ it('propagate headers cross origin redirect', {
const serverRequest = await serverRequestPromise;
// Authorization header not propagated to cross-origin redirect.
expect.soft(serverRequest.headers['authorization']).toBeFalsy();
expect.soft(serverRequest.headers['cookie']).toBeFalsy();
expect.soft(serverRequest.headers['custom']).toBe('foo');
});

it('propagate headers cross origin redirect after interception', {
annotation: [
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/13106' },
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/32045' },
{ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/35154' },
]
}, async ({ page, server, browserName }) => {
await page.goto(server.PREFIX + '/empty.html');
Expand All @@ -681,7 +778,7 @@ it('propagate headers cross origin redirect after interception', {
'Access-Control-Allow-Origin': server.PREFIX,
'Access-Control-Allow-Credentials': 'true',
'Access-Control-Allow-Methods': 'POST, GET, OPTIONS, DELETE',
'Access-Control-Allow-Headers': 'authorization,custom',
'Access-Control-Allow-Headers': 'authorization,cookie,custom',
});
response.end();
return;
Expand All @@ -697,6 +794,7 @@ it('propagate headers cross origin redirect after interception', {
response.writeHead(301, { location: `${server.CROSS_PROCESS_PREFIX}/something` });
response.end();
});
await page.evaluate(() => document.cookie = 'a=b');
await page.route('**/redirect', async route => {
await route.continue({
headers: {
Expand All @@ -721,6 +819,7 @@ it('propagate headers cross origin redirect after interception', {
expect.soft(serverRequest.headers['authorization']).toBeFalsy();
else
expect.soft(serverRequest.headers['authorization']).toBe('credentials');
expect.soft(serverRequest.headers['cookie']).toBeFalsy();
expect.soft(serverRequest.headers['custom']).toBe('foo');
});

Expand Down
9 changes: 5 additions & 4 deletions tests/page/page-route.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,9 @@ it('should properly return navigation response when URL has cookies', async ({ p
expect(response.status()).toBe(200);
});

it('should override cookie header', async ({ page, server, browserName }) => {
it('should not override cookie header', async ({ page, server, browserName }) => {
it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/16773' });
it.fail(browserName !== 'firefox' && !browserName.includes('bidi'));
it.fixme(browserName === 'firefox', 'We currently clear all headers during interception in firefox');

await page.goto(server.EMPTY_PAGE);
await page.evaluate(() => document.cookie = 'original=value');
Expand All @@ -184,8 +184,9 @@ it('should override cookie header', async ({ page, server, browserName }) => {
page.goto(server.EMPTY_PAGE),
]);

expect(cookieValueInRoute).toBe('original=value');
expect(serverReq.headers['cookie']).toBe('overridden=value');
if (browserName !== 'webkit')
expect.soft(cookieValueInRoute).toBe('original=value');
expect.soft(serverReq.headers['cookie']).toBe('original=value');
});

it('should show custom HTTP headers', async ({ page, server }) => {
Expand Down
Loading