From 0d5b41ce7b345f1b8548f0f06f23fc26405dea2c Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Thu, 2 Sep 2021 20:48:23 -0700 Subject: [PATCH] feat(headers): add Headers.headers that would mimic the behavior of the deprecated getters (#8665) --- docs/src/api/class-headers.md | 5 +- src/client/network.ts | 14 +++-- src/client/types.ts | 5 +- src/common/types.ts | 1 + src/server/chromium/crNetworkManager.ts | 4 +- src/server/firefox/ffNetworkManager.ts | 14 ++++- src/server/network.ts | 14 ++++- src/server/supplements/har/harTracer.ts | 15 ++--- src/server/webkit/wkInterceptableRequest.ts | 3 +- src/utils/utils.ts | 14 ++++- tests/har.spec.ts | 12 ++-- tests/page/page-network-request.spec.ts | 61 +++++++++++++------ tests/page/page-network-response.spec.ts | 49 +++++++++++++++ tests/page/page-network-sizes.spec.ts | 4 +- tests/page/page-route.spec.ts | 7 +-- .../page/page-set-extra-http-headers.spec.ts | 1 - types/types.d.ts | 5 +- 17 files changed, 166 insertions(+), 62 deletions(-) diff --git a/docs/src/api/class-headers.md b/docs/src/api/class-headers.md index df0adca001..0263f52d7d 100644 --- a/docs/src/api/class-headers.md +++ b/docs/src/api/class-headers.md @@ -25,6 +25,7 @@ Header name, case-insensitive. Returns all header names in this headers collection. ## method: Headers.headers -- returns: <[Array]<{ name: string, value: string }>> +- returns: <[Object]<[string], [string]>> -Returns all raw headers. +Returns all headers as a dictionary. Header names are normalized to lower case, multi-value headers are concatenated +using comma. diff --git a/src/client/network.ts b/src/client/network.ts index 11a7c8ba01..eb08964954 100644 --- a/src/client/network.ts +++ b/src/client/network.ts @@ -652,7 +652,10 @@ export class RawHeaders implements api.Headers { } get(name: string): string | null { - return this.getAll(name)[0] || null; + const values = this.getAll(name); + if (!values) + return null; + return values.join(', '); } getAll(name: string): string[] { @@ -660,10 +663,13 @@ export class RawHeaders implements api.Headers { } headerNames(): string[] { - return [...new Set(this._headersArray.map(h => h.name))]; + return [...this._headersMap.keys()]; } - headers(): HeadersArray { - return this._headersArray; + headers(): Headers { + const result: Headers = {}; + for (const name of this._headersMap.keys()) + result[name] = this.get(name)!; + return result; } } diff --git a/src/client/types.ts b/src/client/types.ts index cd720adc83..0bf730888f 100644 --- a/src/client/types.ts +++ b/src/client/types.ts @@ -16,9 +16,9 @@ */ import * as channels from '../protocol/channels'; -import type { NameValue, Size } from '../common/types'; +import type { Size } from '../common/types'; import type { ParsedStackTrace } from '../utils/stackTrace'; -export { Size, Point, Rect, Quad, URLMatch, TimeoutOptions } from '../common/types'; +export { Size, Point, Rect, Quad, URLMatch, TimeoutOptions, HeadersArray } from '../common/types'; type LoggerSeverity = 'verbose' | 'info' | 'warning' | 'error'; export interface Logger { @@ -32,7 +32,6 @@ export interface ClientSideInstrumentation { export type StrictOptions = { strict?: boolean }; export type Headers = { [key: string]: string }; -export type HeadersArray = NameValue[]; export type Env = { [key: string]: string | number | boolean | undefined }; export type WaitForEventOptions = Function | { predicate?: Function, timeout?: number }; diff --git a/src/common/types.ts b/src/common/types.ts index 9d361770e6..1e9fc72322 100644 --- a/src/common/types.ts +++ b/src/common/types.ts @@ -21,3 +21,4 @@ export type Quad = [ Point, Point, Point, Point ]; export type URLMatch = string | RegExp | ((url: URL) => boolean); export type TimeoutOptions = { timeout?: number }; export type NameValue = { name: string, value: string }; +export type HeadersArray = NameValue[]; diff --git a/src/server/chromium/crNetworkManager.ts b/src/server/chromium/crNetworkManager.ts index 200b2b2b31..a9a0bbd546 100644 --- a/src/server/chromium/crNetworkManager.ts +++ b/src/server/chromium/crNetworkManager.ts @@ -675,10 +675,10 @@ class ResponseExtraInfoTracker { const response = info.responses[index]; const requestExtraInfo = info.requestWillBeSentExtraInfo[index]; if (response && requestExtraInfo) - response.setRawRequestHeaders(headersObjectToArray(requestExtraInfo.headers)); + response.setRawRequestHeaders(headersObjectToArray(requestExtraInfo.headers, '\n')); const responseExtraInfo = info.responseReceivedExtraInfo[index]; if (response && responseExtraInfo) - response.setRawResponseHeaders(headersObjectToArray(responseExtraInfo.headers)); + response.setRawResponseHeaders(headersObjectToArray(responseExtraInfo.headers, '\n')); } private _checkFinished(info: RequestInfo) { diff --git a/src/server/firefox/ffNetworkManager.ts b/src/server/firefox/ffNetworkManager.ts index d57909bec6..e0b0cdaf60 100644 --- a/src/server/firefox/ffNetworkManager.ts +++ b/src/server/firefox/ffNetworkManager.ts @@ -23,6 +23,7 @@ import * as frames from '../frames'; import * as types from '../types'; import { Protocol } from './protocol'; import { InterceptedResponse } from '../network'; +import { HeadersArray } from '../../server/types'; export class FFNetworkManager { private _session: FFSession; @@ -96,7 +97,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, event.headers, timing, getResponseBody); + const response = new network.Response(request.request, event.status, event.statusText, parseMultivalueHeaders(event.headers), timing, getResponseBody); if (event?.remoteIPAddress && typeof event?.remotePort === 'number') { response._serverAddrFinished({ ipAddress: event.remoteIPAddress, @@ -252,3 +253,14 @@ 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/src/server/network.ts b/src/server/network.ts index e2b0731dc4..6cc47594eb 100644 --- a/src/server/network.ts +++ b/src/server/network.ts @@ -446,21 +446,31 @@ export class Response extends SdkObject { headersSize += 8; // httpVersion; headersSize += 3; // statusCode; headersSize += this.statusText().length; - const headers = this._rawResponseHeadersPromise ? await this._rawResponseHeadersPromise : this._headers; + const headers = await this._bestEffortResponseHeaders(); for (const header of headers) headersSize += header.name.length + header.value.length + 4; // 4 = ': ' + '\r\n' headersSize += 2; // '\r\n' return headersSize; } + private async _bestEffortResponseHeaders(): Promise { + return this._rawResponseHeadersPromise ? await this._rawResponseHeadersPromise : this._headers; + } + async sizes(): Promise { await this._finishedPromise; const requestHeadersSize = await this._requestHeadersSize(); const responseHeadersSize = await this._responseHeadersSize(); let { bodySize, encodedBodySize, transferSize } = this._request.responseSize; + if (!bodySize) { + const headers = await this._bestEffortResponseHeaders(); + const contentLength = headers.find(h => h.name.toLowerCase() === 'content-length')?.value; + bodySize = contentLength ? +contentLength : 0; + } if (!encodedBodySize && transferSize) { // Chromium only populates transferSize - encodedBodySize = transferSize - responseHeadersSize; + // Firefox can return 0 transferSize + encodedBodySize = Math.max(0, transferSize - responseHeadersSize); // Firefox only populate transferSize. if (!bodySize) bodySize = encodedBodySize; diff --git a/src/server/supplements/har/harTracer.ts b/src/server/supplements/har/harTracer.ts index 0760c4a352..a1f93a8693 100644 --- a/src/server/supplements/har/harTracer.ts +++ b/src/server/supplements/har/harTracer.ts @@ -252,8 +252,9 @@ export class HarTracer { status: response.status(), statusText: response.statusText(), httpVersion: response.httpVersion(), - cookies: cookiesForHar(response.headerValue('set-cookie'), '\n'), - headers: response.headers().map(header => ({ name: header.name, value: header.value })), + // These are bad values that will be overwritten bellow. + cookies: [], + headers: [], content: { size: -1, mimeType: 'x-unknown', @@ -292,12 +293,12 @@ export class HarTracer { })); this._addBarrier(page, response.rawRequestHeaders().then(headers => { for (const header of headers.filter(header => header.name.toLowerCase() === 'cookie')) - harEntry.request.cookies.push(...cookiesForHar(header.value, ';')); + harEntry.request.cookies.push(...header.value.split(';').map(parseCookie)); harEntry.request.headers = headers; })); this._addBarrier(page, response.rawResponseHeaders().then(headers => { for (const header of headers.filter(header => header.name.toLowerCase() === 'set-cookie')) - harEntry.response.cookies.push(...cookiesForHar(header.value, '\n')); + harEntry.response.cookies.push(parseCookie(header.value)); harEntry.response.headers = headers; const contentType = headers.find(header => header.name.toLowerCase() === 'content-type'); if (contentType) @@ -365,12 +366,6 @@ function postDataForHar(request: network.Request, content: 'omit' | 'sha1' | 'em return result; } -function cookiesForHar(header: string | undefined, separator: string): har.Cookie[] { - if (!header) - return []; - return header.split(separator).map(c => parseCookie(c)); -} - function parseCookie(c: string): har.Cookie { const cookie: har.Cookie = { name: '', diff --git a/src/server/webkit/wkInterceptableRequest.ts b/src/server/webkit/wkInterceptableRequest.ts index 729670be82..7283714be7 100644 --- a/src/server/webkit/wkInterceptableRequest.ts +++ b/src/server/webkit/wkInterceptableRequest.ts @@ -89,7 +89,8 @@ export class WKInterceptableRequest { requestStart: timingPayload ? wkMillisToRoundishMillis(timingPayload.requestStart) : -1, responseStart: timingPayload ? wkMillisToRoundishMillis(timingPayload.responseStart) : -1, }; - return new network.Response(this.request, responsePayload.status, responsePayload.statusText, headersObjectToArray(responsePayload.headers), timing, getResponseBody); + const setCookieSeparator = process.platform === 'linux' ? '\n' : ','; + return new network.Response(this.request, responsePayload.status, responsePayload.statusText, headersObjectToArray(responsePayload.headers, ',', setCookieSeparator), timing, getResponseBody); } } diff --git a/src/utils/utils.ts b/src/utils/utils.ts index 302ca4f454..51495e47c0 100644 --- a/src/utils/utils.ts +++ b/src/utils/utils.ts @@ -246,11 +246,19 @@ export async function mkdirIfNeeded(filePath: string) { type HeadersArray = { name: string, value: string }[]; type HeadersObject = { [key: string]: string }; -export function headersObjectToArray(headers: HeadersObject): HeadersArray { +export function headersObjectToArray(headers: HeadersObject, separator?: string, setCookieSeparator?: string): HeadersArray { + if (!setCookieSeparator) + setCookieSeparator = separator; const result: HeadersArray = []; for (const name in headers) { - if (!Object.is(headers[name], undefined)) - result.push({ name, value: headers[name] }); + const values = headers[name]; + if (separator) { + const sep = name.toLowerCase() === 'set-cookie' ? setCookieSeparator : separator; + for (const value of values.split(sep!)) + result.push({ name, value: value.trim() }); + } else { + result.push({ name, value: values }); + } } return result; } diff --git a/tests/har.spec.ts b/tests/har.spec.ts index 981f89381f..d915b9ef41 100644 --- a/tests/har.spec.ts +++ b/tests/har.spec.ts @@ -194,9 +194,7 @@ it('should include cookies', async ({ contextFactory, server }, testInfo) => { ]); }); -it('should include set-cookies', async ({ contextFactory, server, browserName, platform }, testInfo) => { - it.fail(browserName === 'webkit' && platform === 'darwin', 'Does not work yet'); - +it('should include set-cookies', async ({ contextFactory, server }, testInfo) => { const { page, getLog } = await pageWithHar(contextFactory, testInfo); server.setRoute('/empty.html', (req, res) => { res.setHeader('Set-Cookie', [ @@ -214,18 +212,20 @@ it('should include set-cookies', async ({ contextFactory, server, browserName, p expect(new Date(cookies[2].expires).valueOf()).toBeGreaterThan(Date.now()); }); -it('should include set-cookies with comma', async ({ contextFactory, server }, testInfo) => { +it('should include set-cookies with comma', async ({ contextFactory, server, browserName }, testInfo) => { + it.fixme(browserName === 'webkit', 'We get "name1=val, ue1, name2=val, ue2" as a header value'); const { page, getLog } = await pageWithHar(contextFactory, testInfo); server.setRoute('/empty.html', (req, res) => { res.setHeader('Set-Cookie', [ - 'name1=val,ue1', + 'name1=val, ue1', 'name2=val, ue2', ]); res.end(); }); await page.goto(server.EMPTY_PAGE); const log = await getLog(); const cookies = log.entries[0].response.cookies; - expect(cookies[0]).toEqual({ name: 'name1', value: 'val,ue1' }); + expect(cookies[0]).toEqual({ name: 'name1', value: 'val, ue1' }); + expect(cookies[1]).toEqual({ name: 'name2', value: 'val, ue2' }); }); it('should include secure set-cookies', async ({ contextFactory, httpsServer }, testInfo) => { diff --git a/tests/page/page-network-request.spec.ts b/tests/page/page-network-request.spec.ts index 1046650e75..4d32fd0f15 100644 --- a/tests/page/page-network-request.spec.ts +++ b/tests/page/page-network-request.spec.ts @@ -90,10 +90,7 @@ it('should get the same headers as the server', async ({ page, server, browserNa }); const response = await page.goto(server.PREFIX + '/empty.html'); const headers = await response.request().allHeaders(); - const result = {}; - for (const header of headers.headers()) - result[header.name.toLowerCase()] = header.value; - expect(result).toEqual(serverRequest.headers); + expect(headers.headers()).toEqual(serverRequest.headers); }); it('should get the same headers as the server CORS', async ({page, server, browserName, platform}) => { @@ -114,11 +111,7 @@ it('should get the same headers as the server CORS', async ({page, server, brows expect(text).toBe('done'); const response = await responsePromise; const headers = await response.request().allHeaders(); - const result = {}; - for (const header of headers.headers()) - result[header.name.toLowerCase()] = header.value; - - expect(result).toEqual(serverRequest.headers); + expect(headers.headers()).toEqual(serverRequest.headers); }); it('should return postData', async ({page, server, isAndroid}) => { @@ -273,17 +266,28 @@ it('should return navigation bit when navigating to image', async ({page, server expect(requests[0].isNavigationRequest()).toBe(true); }); -it('should report raw headers', async ({ page, server, browserName }) => { - const response = await page.goto(server.EMPTY_PAGE); - const requestHeaders = await response.request().allHeaders(); - expect(requestHeaders.headerNames().map(h => h.toLowerCase())).toContain('accept'); - expect(requestHeaders.getAll('host')).toHaveLength(1); - expect(requestHeaders.get('host')).toBe(`localhost:${server.PORT}`); +it('should report all headers', async ({ page, server, browserName, platform }) => { + const expectedHeaders = {}; + server.setRoute('/headers', (req, res) => { + for (let i = 0; i < req.rawHeaders.length; i += 2) + expectedHeaders[req.rawHeaders[i].toLowerCase()] = req.rawHeaders[i + 1]; + res.end(); + }); - const responseHeaders = await response.allHeaders(); - expect(responseHeaders.headerNames().map(h => h.toLowerCase())).toContain('content-type'); - expect(responseHeaders.getAll('content-type')).toHaveLength(1); - expect(responseHeaders.get('content-type')).toBe('text/html; charset=utf-8'); + await page.goto(server.EMPTY_PAGE); + const [request] = await Promise.all([ + page.waitForRequest('**/*'), + page.evaluate(() => fetch('/headers', { + headers: [ + ['header-a', 'value-a'], + ['header-b', 'value-b'], + ['header-a', 'value-a-1'], + ['header-a', 'value-a-2'], + ] + })) + ]); + const headers = await request.allHeaders(); + expect(headers.headers()).toEqual(expectedHeaders); }); it('should report raw response headers in redirects', async ({ page, server, browserName }) => { @@ -310,3 +314,22 @@ it('should report raw response headers in redirects', async ({ page, server, bro expect(redirectChain).toEqual(expectedUrls); expect(headersChain).toEqual(expectedHeaders); }); + +it('should report all cookies in one header', async ({ page, server }) => { + const expectedHeaders = {}; + server.setRoute('/headers', (req, res) => { + for (let i = 0; i < req.rawHeaders.length; i += 2) + expectedHeaders[req.rawHeaders[i]] = req.rawHeaders[i + 1]; + res.end(); + }); + + await page.goto(server.EMPTY_PAGE); + await page.evaluate(() => { + document.cookie = 'myCookie=myValue'; + document.cookie = 'myOtherCookie=myOtherValue'; + }); + const response = await page.goto(server.EMPTY_PAGE); + const headers = await response.request().allHeaders(); + const cookie = headers.get('cookie'); + expect(cookie).toBe('myCookie=myValue; myOtherCookie=myOtherValue'); +}); diff --git a/tests/page/page-network-response.spec.ts b/tests/page/page-network-response.spec.ts index 1c2090bf24..34fd0f96c4 100644 --- a/tests/page/page-network-response.spec.ts +++ b/tests/page/page-network-response.spec.ts @@ -117,3 +117,52 @@ it('should return status text', async ({page, server}) => { const response = await page.goto(server.PREFIX + '/cool'); expect(response.statusText()).toBe('cool!'); }); + +it('should report all headers', async ({ page, server }) => { + const expectedHeaders = { + 'header-a': ['value-a', 'value-a-1', 'value-a-2'], + 'header-b': ['value-b'], + }; + server.setRoute('/headers', (req, res) => { + res.writeHead(200, expectedHeaders); + res.end(); + }); + + await page.goto(server.EMPTY_PAGE); + const [response] = await Promise.all([ + page.waitForResponse('**/*'), + page.evaluate(() => fetch('/headers')) + ]); + const headers = await response.allHeaders(); + const actualHeaders = {}; + for (const name of headers.headerNames()) + actualHeaders[name] = headers.getAll(name); + delete actualHeaders['Keep-Alive']; + delete actualHeaders['keep-alive']; + delete actualHeaders['Connection']; + delete actualHeaders['connection']; + delete actualHeaders['Date']; + delete actualHeaders['date']; + delete actualHeaders['Transfer-Encoding']; + delete actualHeaders['transfer-encoding']; + expect(actualHeaders).toEqual(expectedHeaders); +}); + +it('should report multiple set-cookie headers', async ({ page, server }) => { + server.setRoute('/headers', (req, res) => { + res.writeHead(200, { + 'Set-Cookie': ['a=b', 'c=d'] + }); + res.write('\r\n'); + res.end(); + }); + + await page.goto(server.EMPTY_PAGE); + const [response] = await Promise.all([ + page.waitForResponse('**/*'), + page.evaluate(() => fetch('/headers')) + ]); + const headers = await response.allHeaders(); + const cookies = headers.getAll('set-cookie'); + expect(cookies).toEqual(['a=b', 'c=d']); +}); diff --git a/tests/page/page-network-sizes.spec.ts b/tests/page/page-network-sizes.spec.ts index 42db1290be..7168eb2f68 100644 --- a/tests/page/page-network-sizes.spec.ts +++ b/tests/page/page-network-sizes.spec.ts @@ -55,8 +55,8 @@ it('should set bodySize, headersSize, and transferSize', async ({page, server, b ]); const sizes = await response.request().sizes(); expect(sizes.responseBodySize).toBe(6); - expect(sizes.responseHeadersSize).toBeGreaterThanOrEqual(150); - expect(sizes.responseTransferSize).toBeGreaterThanOrEqual(160); + expect(sizes.responseHeadersSize).toBeGreaterThanOrEqual(100); + expect(sizes.responseTransferSize).toBeGreaterThanOrEqual(100); }); it('should set bodySize to 0 when there was no response body', async ({page, server, browserName, platform}) => { diff --git a/tests/page/page-route.spec.ts b/tests/page/page-route.spec.ts index fa6edba6bc..9aeaa0b716 100644 --- a/tests/page/page-route.spec.ts +++ b/tests/page/page-route.spec.ts @@ -98,10 +98,9 @@ it('should work when header manipulation headers with redirect', async ({page, s // @see https://github.com/GoogleChrome/puppeteer/issues/4743 it('should be able to remove headers', async ({page, server}) => { await page.goto(server.EMPTY_PAGE); - await page.route('**/*', route => { - const headers = Object.assign({}, route.request().headers(), { - foo: undefined, // remove "foo" header - }); + await page.route('**/*', async route => { + const headers = { ...route.request().headers() }; + delete headers['foo']; route.continue({ headers }); }); diff --git a/tests/page/page-set-extra-http-headers.spec.ts b/tests/page/page-set-extra-http-headers.spec.ts index 971f79caf8..d03d6f1672 100644 --- a/tests/page/page-set-extra-http-headers.spec.ts +++ b/tests/page/page-set-extra-http-headers.spec.ts @@ -20,7 +20,6 @@ import { test as it, expect } from './pageTest'; it('should work', async ({page, server}) => { await page.setExtraHTTPHeaders({ foo: 'bar', - baz: undefined, }); const [request] = await Promise.all([ server.waitForRequest('/empty.html'), diff --git a/types/types.d.ts b/types/types.d.ts index f9da8ce9f7..d3f239d7ef 100644 --- a/types/types.d.ts +++ b/types/types.d.ts @@ -12682,9 +12682,10 @@ export interface Headers { headerNames(): Array; /** - * Returns all raw headers. + * Returns all headers as a dictionary. Header names are normalized to lower case, multi-value headers are concatenated + * using comma. */ - headers(): Array<{ name: string, value: string }>; + headers(): { [key: string]: string; }; } /**