From 9cc86bd6cbc505fa70534075cfe76ce5d4b754d4 Mon Sep 17 00:00:00 2001 From: Devin Rousso Date: Fri, 18 Sep 2026 11:27:46 -0600 Subject: [PATCH] fix(firefox): preserve response header field boundaries splitting response headers on commas corrupts values and invents field boundaries use the original fields reported by Juggler without parsing them again reuse Chromium's `Set-Cookie` splitter for fulfillment so multiple cookies remain separate fields --- docs/src/api/class-response.md | 3 +- packages/isomorphic/headers.ts | 14 ++ packages/playwright-client/types/types.d.ts | 5 +- .../src/server/chromium/crNetworkManager.ts | 16 +- .../src/server/firefox/ffNetworkManager.ts | 17 +- packages/playwright-core/types/types.d.ts | 5 +- tests/page/firefox-network-response.spec.ts | 172 ++++++++++++++++++ 7 files changed, 198 insertions(+), 34 deletions(-) create mode 100644 tests/page/firefox-network-response.spec.ts diff --git a/docs/src/api/class-response.md b/docs/src/api/class-response.md index 555d2210c096d..03fa2f5e47e3f 100644 --- a/docs/src/api/class-response.md +++ b/docs/src/api/class-response.md @@ -54,8 +54,9 @@ You can use [`method: Response.allHeaders`] for complete list of headers that in - `name` <[string]> Name of the header. - `value` <[string]> Value of the header. -An array with all the request HTTP headers associated with this response. Unlike [`method: Response.allHeaders`], header names are NOT lower-cased. +An array with all the response HTTP headers associated with this response. Unlike [`method: Response.allHeaders`], header names are NOT lower-cased. Headers with multiple entries, such as `Set-Cookie`, appear in the array multiple times. +Some browser network stacks combine multiple field values before reporting them, so separate entries are not always available. ## async method: Response.headerValue * since: v1.15 diff --git a/packages/isomorphic/headers.ts b/packages/isomorphic/headers.ts index e228e5312f230..fa9303b5e25ed 100644 --- a/packages/isomorphic/headers.ts +++ b/packages/isomorphic/headers.ts @@ -42,3 +42,17 @@ export function headersArrayToObject(headers: HeadersArray, lowerCase: boolean): result[lowerCase ? name.toLowerCase() : name] = value; return result; } + +export function splitSetCookieHeader(headers: HeadersArray): HeadersArray { + const index = headers.findIndex(({ name }) => name.toLowerCase() === 'set-cookie'); + if (index === -1) + return headers; + + const header = headers[index]; + const values = header.value.split('\n'); + if (values.length === 1) + return headers; + const result = headers.slice(); + result.splice(index, 1, ...values.map(value => ({ name: header.name, value }))); + return result; +} diff --git a/packages/playwright-client/types/types.d.ts b/packages/playwright-client/types/types.d.ts index cd297c4c18d49..ff0aae20a5d97 100644 --- a/packages/playwright-client/types/types.d.ts +++ b/packages/playwright-client/types/types.d.ts @@ -22715,9 +22715,10 @@ export interface Response { headers(): { [key: string]: string; }; /** - * An array with all the request HTTP headers associated with this response. Unlike + * An array with all the response HTTP headers associated with this response. Unlike * [response.allHeaders()](https://playwright.dev/docs/api/class-response#response-all-headers), header names are NOT - * lower-cased. Headers with multiple entries, such as `Set-Cookie`, appear in the array multiple times. + * lower-cased. Headers with multiple entries, such as `Set-Cookie`, appear in the array multiple times. Some browser + * network stacks combine multiple field values before reporting them, so separate entries are not always available. */ headersArray(): Promise header.name.toLowerCase() !== 'cookie'); } -function splitSetCookieHeader(headers: types.HeadersArray): types.HeadersArray { - const index = headers.findIndex(({ name }) => name.toLowerCase() === 'set-cookie'); - if (index === -1) - return headers; - - const header = headers[index]; - const values = header.value.split('\n'); - if (values.length === 1) - return headers; - const result = headers.slice(); - result.splice(index, 1, ...values.map(value => ({ name: header.name, value }))); - return result; -} - const errorReasons: { [reason: string]: Protocol.Network.ErrorReason } = { 'aborted': 'Aborted', 'accessdenied': 'AccessDenied', diff --git a/packages/playwright-core/src/server/firefox/ffNetworkManager.ts b/packages/playwright-core/src/server/firefox/ffNetworkManager.ts index b6b52620fd1b1..9f08caea6ada1 100644 --- a/packages/playwright-core/src/server/firefox/ffNetworkManager.ts +++ b/packages/playwright-core/src/server/firefox/ffNetworkManager.ts @@ -16,11 +16,11 @@ */ import { eventsHelper } from '@utils/eventsHelper'; +import { splitSetCookieHeader } from '@isomorphic/headers'; import * as network from '../network'; import type { FFSession } from './ffConnection'; import type { FFPage } from './ffPage'; -import type { HeadersArray } from '../../server/types'; import type { RegisteredListener } from '@utils/eventsHelper'; import type * as frames from '../frames'; import type * as types from '../types'; @@ -115,7 +115,7 @@ export class FFNetworkManager { requestStart: relativeToStart(event.timing.requestStart), responseStart: relativeToStart(event.timing.responseStart), }; - const response = new network.Response(request.request, event.status, event.statusText, parseMultivalueHeaders(event.headers), timing, getResponseBody, event.fromServiceWorker); + const response = new network.Response(request.request, event.status, event.statusText, event.headers, timing, getResponseBody, event.fromServiceWorker); if (event?.remoteIPAddress && typeof event?.remotePort === 'number') { response._serverAddrFinished({ ipAddress: event.remoteIPAddress, @@ -264,7 +264,7 @@ class FFRouteImpl implements network.RouteDelegate { requestId: this._request._id, status: response.status, statusText: network.statusText(response.status), - headers: response.headers, + headers: splitSetCookieHeader(response.headers), base64body, }); } @@ -276,14 +276,3 @@ class FFRouteImpl implements network.RouteDelegate { }); } } - -function parseMultivalueHeaders(headers: HeadersArray) { - const result: HeadersArray = []; - for (const header of headers) { - const separator = header.name.toLowerCase() === 'set-cookie' ? '\n' : ','; - const tokens = header.value.split(separator).map(s => s.trim()); - for (const token of tokens) - result.push({ name: header.name, value: token }); - } - return result; -} diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts index cd297c4c18d49..ff0aae20a5d97 100644 --- a/packages/playwright-core/types/types.d.ts +++ b/packages/playwright-core/types/types.d.ts @@ -22715,9 +22715,10 @@ export interface Response { headers(): { [key: string]: string; }; /** - * An array with all the request HTTP headers associated with this response. Unlike + * An array with all the response HTTP headers associated with this response. Unlike * [response.allHeaders()](https://playwright.dev/docs/api/class-response#response-all-headers), header names are NOT - * lower-cased. Headers with multiple entries, such as `Set-Cookie`, appear in the array multiple times. + * lower-cased. Headers with multiple entries, such as `Set-Cookie`, appear in the array multiple times. Some browser + * network stacks combine multiple field values before reporting them, so separate entries are not always available. */ headersArray(): Promise browserName !== 'firefox' || isBidi, 'Tests the Firefox network observer.'); + +async function expectHeaderFields(response: Response, expected: HeadersArray) { + const headers = await response.headersArray(); + const allHeaders = await response.allHeaders(); + for (const name of new Set(expected.map(header => header.name.toLowerCase()))) { + const fields = expected.filter(header => header.name.toLowerCase() === name); + const values = fields.map(header => header.value); + expect(headers.filter(header => header.name.toLowerCase() === name)).toEqual(fields); + expect(await response.headerValues(name)).toEqual(values); + const combined = values.join(name === 'set-cookie' ? '\n' : ', '); + expect(await response.headerValue(name)).toBe(combined); + expect(allHeaders[name]).toBe(combined); + } +} + +async function fetchResponse(page: Page, url: string) { + const [response, fetched] = await Promise.all([ + page.waitForResponse(url), + page.evaluate(async url => { + const response = await fetch(url); + return { status: response.status, body: await response.text() }; + }, url), + ]); + return { response, fetched }; +} + +for (const status of [200, 302]) { + it(`should preserve original response header fields for ${status} responses`, async ({ page, server }) => { + const headers = [ + { name: 'Date', value: 'Wed, 21 Oct 2037 07:28:00 GMT' }, + { name: 'X-Literal', value: 'first, "second, third"' }, + { name: 'X-Identical', value: 'same' }, + { name: 'X-Identical', value: 'same' }, + { name: 'X-Repeat', value: 'first, literal' }, + { name: 'x-repeat', value: 'second' }, + { name: 'Set-Cookie', value: 'a=b; Expires=Wed, 21 Oct 2037 07:28:00 GMT' }, + { name: 'Set-Cookie', value: 'c=d; Expires=Wed, 21 Oct 2037 07:28:00 GMT; Path=/' }, + { name: 'WWW-Authenticate', value: 'Digest realm="one, two", qop="auth, auth-int"' }, + { name: 'WWW-Authenticate', value: 'Basic realm="three, four"' }, + { name: 'Proxy-Authenticate', value: 'Basic realm="five, six"' }, + { name: 'Proxy-Authenticate', value: 'Basic realm="five, six"' }, + ]; + server.setRoute('/headers', (request, response) => { + response.writeHead(status, [ + 'Content-Type', 'text/html', + ...status === 302 ? ['Location', '/empty.html'] : [], + ...headers.flatMap(({ name, value }) => [name, value]), + ]); + response.end('body'); + }); + const finalResponse = await page.goto(server.PREFIX + '/headers'); + const response = status === 302 ? await finalResponse.request().redirectedFrom().response() : finalResponse; + expect(response.status()).toBe(status); + await expectHeaderFields(response, headers); + }); +} + +it('should preserve synthesized response header fields', async ({ page, server }) => { + const cookies = [ + 'a=b; Expires=Wed, 21 Oct 2037 07:28:00 GMT', + 'c=d; Expires=Wed, 21 Oct 2037 07:28:00 GMT; Path=/', + ]; + await page.route('**/headers', route => route.fulfill({ + contentType: 'text/html', + headers: { + 'X-Literal': 'first, "second, third"', + 'Date': 'Wed, 21 Oct 2037 07:28:00 GMT', + 'Set-Cookie': cookies.join('\n'), + 'WWW-Authenticate': 'Digest realm="one, two", qop="auth, auth-int"', + }, + body: 'fulfilled', + })); + const response = await page.goto(server.PREFIX + '/headers'); + expect(await response.text()).toBe('fulfilled'); + await expectHeaderFields(response, [ + { name: 'x-literal', value: 'first, "second, third"' }, + { name: 'date', value: 'Wed, 21 Oct 2037 07:28:00 GMT' }, + ...cookies.map(value => ({ name: 'set-cookie', value })), + { name: 'www-authenticate', value: 'Digest realm="one, two", qop="auth, auth-int"' }, + ]); +}); + +it('should report current effective fields after cache revalidation', async ({ page, server }) => { + const validators: (string | undefined)[] = []; + server.setRoute('/cached-headers', (request, response) => { + validators.push(request.headers['if-none-match']); + if (validators.length === 1) { + response.writeHead(200, { + 'Content-Type': 'text/plain', + 'ETag': '"v1"', + 'Cache-Control': 'max-age=0, must-revalidate', + 'X-Keep': ['first, literal', 'second'], + 'X-Change': ['old, literal', 'old-second'], + 'Set-Cookie': ['old=value; Expires=Wed, 21 Oct 2037 07:28:00 GMT', 'old2=value; Path=/'], + 'WWW-Authenticate': ['Basic realm="old, realm"', 'Basic realm="old-second"'], + 'Proxy-Authenticate': ['Basic realm="old, proxy"', 'Basic realm="old-proxy-second"'], + }); + response.end('cached-body'); + return; + } + response.writeHead(304, { + 'Cache-Control': 'max-age=3600', + 'X-Change': ['new, literal', 'new-second'], + 'Set-Cookie': ['new=value; Expires=Wed, 21 Oct 2037 07:28:00 GMT', 'new2=value; Path=/'], + 'WWW-Authenticate': ['Basic realm="new, realm"', 'Basic realm="new-second"'], + 'Proxy-Authenticate': ['Basic realm="new, proxy"', 'Basic realm="new-proxy-second"'], + }); + response.end(); + }); + + await page.goto(server.EMPTY_PAGE); + const url = server.PREFIX + '/cached-headers'; + const initial = await fetchResponse(page, url); + const revalidated = await fetchResponse(page, url); + const cached = await fetchResponse(page, url); + + expect(validators).toEqual([undefined, '"v1"']); + for (const { fetched } of [initial, revalidated, cached]) + expect(fetched).toEqual({ status: 200, body: 'cached-body' }); + expect([initial.response.status(), revalidated.response.status(), cached.response.status()]).toEqual([200, 304, 200]); + expect(await initial.response.headerValue('x-change')).toBe('old, literal, old-second'); + expect(await revalidated.response.headerValues('x-keep')).toEqual([]); + expect(await revalidated.response.headerValues('x-change')).toEqual(['new, literal', 'new-second']); + expect(await revalidated.response.headerValues('set-cookie')).toEqual([ + 'new=value; Expires=Wed, 21 Oct 2037 07:28:00 GMT', + 'new2=value; Path=/', + ]); + expect(await revalidated.response.headerValues('www-authenticate')).toEqual(['Basic realm="new, realm"', 'Basic realm="new-second"']); + expect(await revalidated.response.headerValues('proxy-authenticate')).toEqual(['Basic realm="new, proxy"', 'Basic realm="new-proxy-second"']); + expect(await cached.response.headerValues('x-keep')).toEqual(['first, literal, second']); + expect(await cached.response.headerValues('x-change')).toEqual(['new, literal, new-second']); + for (const name of ['set-cookie', 'www-authenticate', 'proxy-authenticate']) + expect(await cached.response.headerValues(name)).toEqual([]); +}); + +it('should report a standalone 304 response without waiting for a cache merge', async ({ page, server }) => { + server.setRoute('/standalone-304', (request, response) => { + response.writeHead(304, { + 'X-Literal': 'standalone, value', + 'Cache-Control': 'no-store', + }); + response.end(); + }); + await page.goto(server.EMPTY_PAGE); + const { response, fetched } = await fetchResponse(page, server.PREFIX + '/standalone-304'); + expect(response.status()).toBe(304); + expect(fetched).toEqual({ status: 304, body: '' }); + expect(await response.headerValues('x-literal')).toEqual(['standalone, value']); + expect(await response.finished()).toBeNull(); +});