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.

Sign up for GitHub

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

Jump to bottom

chore: use HeadersArray instead of Headers object on the server side #3512

Merged
merged 1 commit into from
Aug 18, 2020
Merged

chore: use HeadersArray instead of Headers object on the server side #3512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions src/browserContext.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export interface BrowserContext extends EventEmitter {
grantPermissions(permissions: string[], options?: { origin?: string }): Promise<void>;
clearPermissions(): Promise<void>;
setGeolocation(geolocation?: types.Geolocation): Promise<void>;
setExtraHTTPHeaders(headers: types.Headers): Promise<void>;
setExtraHTTPHeaders(headers: types.HeadersArray): Promise<void>;
setOffline(offline: boolean): Promise<void>;
setHTTPCredentials(httpCredentials?: types.Credentials): Promise<void>;
addInitScript(script: Function | string | { path?: string, content?: string }, arg?: any): Promise<void>;
Expand Down Expand Up @@ -103,7 +103,7 @@ export abstract class BrowserContextBase extends EventEmitter implements Browser
abstract _doClearPermissions(): Promise<void>;
abstract setGeolocation(geolocation?: types.Geolocation): Promise<void>;
abstract _doSetHTTPCredentials(httpCredentials?: types.Credentials): Promise<void>;
abstract setExtraHTTPHeaders(headers: types.Headers): Promise<void>;
abstract setExtraHTTPHeaders(headers: types.HeadersArray): Promise<void>;
abstract setOffline(offline: boolean): Promise<void>;
abstract _doAddInitScript(expression: string): Promise<void>;
abstract _doExposeBinding(binding: PageBinding): Promise<void>;
Expand Down Expand Up @@ -189,9 +189,11 @@ export abstract class BrowserContextBase extends EventEmitter implements Browser
const { username, password } = proxy;
if (username) {
this._options.httpCredentials = { username, password: password! };
this._options.extraHTTPHeaders = this._options.extraHTTPHeaders || {};
const token = Buffer.from(`${username}:${password}`).toString('base64');
this._options.extraHTTPHeaders['Proxy-Authorization'] = `Basic ${token}`;
this._options.extraHTTPHeaders = network.mergeHeaders([
this._options.extraHTTPHeaders,
network.singleHeader('Proxy-Authorization', `Basic ${token}`),
]);
}
}

Expand Down Expand Up @@ -236,8 +238,6 @@ export function validateBrowserContextOptions(options: types.BrowserContextOptio
if (!options.viewport && !options.noDefaultViewport)
options.viewport = { width: 1280, height: 720 };
verifyGeolocation(options.geolocation);
if (options.extraHTTPHeaders)
options.extraHTTPHeaders = network.verifyHeaders(options.extraHTTPHeaders);
}

export function verifyGeolocation(geolocation?: types.Geolocation) {
Expand Down
4 changes: 2 additions & 2 deletions src/chromium/crBrowser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,8 @@ export class CRBrowserContext extends BrowserContextBase {
await (page._delegate as CRPage).updateGeolocation();
}

async setExtraHTTPHeaders(headers: types.Headers): Promise<void> {
this._options.extraHTTPHeaders = network.verifyHeaders(headers);
async setExtraHTTPHeaders(headers: types.HeadersArray): Promise<void> {
this._options.extraHTTPHeaders = headers;
for (const page of this.pages())
await (page._delegate as CRPage).updateExtraHTTPHeaders();
}
Expand Down
13 changes: 3 additions & 10 deletions src/chromium/crNetworkManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import * as network from '../network';
import * as frames from '../frames';
import * as types from '../types';
import { CRPage } from './crPage';
import { headersObjectToArray } from '../converters';

export class CRNetworkManager {
private _client: CRSession;
Expand Down Expand Up @@ -239,7 +240,7 @@ export class CRNetworkManager {
const response = await this._client.send('Network.getResponseBody', { requestId: request._requestId });
return Buffer.from(response.body, response.base64Encoded ? 'base64' : 'utf8');
};
return new network.Response(request.request, responsePayload.status, responsePayload.statusText, headersObject(responsePayload.headers), getResponseBody);
return new network.Response(request.request, responsePayload.status, responsePayload.statusText, headersObjectToArray(responsePayload.headers), getResponseBody);
}

_handleRequestRedirect(request: InterceptableRequest, responsePayload: Protocol.Network.Response) {
Expand Down Expand Up @@ -350,7 +351,7 @@ class InterceptableRequest implements network.RouteDelegate {
if (postDataEntries && postDataEntries.length && postDataEntries[0].bytes)
postDataBuffer = Buffer.from(postDataEntries[0].bytes, 'base64');

this.request = new network.Request(allowInterception ? this : null, frame, redirectedFrom, documentId, url, type, method, postDataBuffer, headersObject(headers));
this.request = new network.Request(allowInterception ? this : null, frame, redirectedFrom, documentId, url, type, method, postDataBuffer, headersObjectToArray(headers));
}

async continue(overrides: types.NormalizedContinueOverrides) {
Expand Down Expand Up @@ -406,11 +407,3 @@ const errorReasons: { [reason: string]: Protocol.Network.ErrorReason } = {
'timedout': 'TimedOut',
'failed': 'Failed',
};

function headersObject(headers: Protocol.Network.Headers): types.Headers {
const result: types.Headers = {};
for (const key of Object.keys(headers))
result[key.toLowerCase()] = headers[key];
return result;
}

3 changes: 2 additions & 1 deletion src/chromium/crPage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import * as types from '../types';
import { ConsoleMessage } from '../console';
import * as sourceMap from '../utils/sourceMap';
import { rewriteErrorMessage } from '../utils/stackTrace';
import { headersArrayToObject } from '../converters';


const UTILITY_WORLD_NAME = '__playwright_utility_world__';
Expand Down Expand Up @@ -729,7 +730,7 @@ class FrameSession {
this._crPage._browserContext._options.extraHTTPHeaders,
this._page._state.extraHTTPHeaders
]);
await this._client.send('Network.setExtraHTTPHeaders', { headers });
await this._client.send('Network.setExtraHTTPHeaders', { headers: headersArrayToObject(headers, false /* lowerCase */) });
}

async _updateGeolocation(): Promise<void> {
Expand Down
60 changes: 6 additions & 54 deletions src/converters.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import * as mime from 'mime';
import * as path from 'path';
import * as util from 'util';
import * as types from './types';
import { helper, assert } from './helper';

export async function normalizeFilePayloads(files: string | types.FilePayload | string[] | types.FilePayload[]): Promise<types.FilePayload[]> {
let ff: string[] | types.FilePayload[];
Expand All @@ -43,65 +42,18 @@ export async function normalizeFilePayloads(files: string | types.FilePayload |
return filePayloads;
}

export async function normalizeFulfillParameters(params: types.FulfillResponse & { path?: string }): Promise<types.NormalizedFulfillResponse> {
let body = '';
let isBase64 = false;
let length = 0;
if (params.path) {
const buffer = await util.promisify(fs.readFile)(params.path);
body = buffer.toString('base64');
isBase64 = true;
length = buffer.length;
} else if (helper.isString(params.body)) {
body = params.body;
isBase64 = false;
length = Buffer.byteLength(body);
} else if (params.body) {
body = params.body.toString('base64');
isBase64 = true;
length = params.body.length;
}
const headers: types.Headers = {};
for (const header of Object.keys(params.headers || {}))
headers[header.toLowerCase()] = String(params.headers![header]);
if (params.contentType)
headers['content-type'] = String(params.contentType);
else if (params.path)
headers['content-type'] = mime.getType(params.path) || 'application/octet-stream';
if (length && !('content-length' in headers))
headers['content-length'] = String(length);

return {
status: params.status || 200,
headers: headersObjectToArray(headers),
body,
isBase64
};
}

export function normalizeContinueOverrides(overrides: types.ContinueOverrides): types.NormalizedContinueOverrides {
return {
method: overrides.method,
headers: overrides.headers ? headersObjectToArray(overrides.headers) : undefined,
postData: helper.isString(overrides.postData) ? Buffer.from(overrides.postData, 'utf8') : overrides.postData,
};
}

export function headersObjectToArray(headers: types.Headers): types.HeadersArray {
export function headersObjectToArray(headers: { [key: string]: string }): types.HeadersArray {
const result: types.HeadersArray = [];
for (const name in headers) {
if (!Object.is(headers[name], undefined)) {
const value = headers[name];
assert(helper.isString(value), `Expected value of header "${name}" to be String, but "${typeof value}" is found.`);
result.push({ name, value });
}
if (!Object.is(headers[name], undefined))
result.push({ name, value: headers[name] });
dgozman marked this conversation as resolved.
Show resolved Hide resolved
}
return result;
}

export function headersArrayToObject(headers: types.HeadersArray): types.Headers {
const result: types.Headers = {};
export function headersArrayToObject(headers: types.HeadersArray, lowerCase: boolean): { [key: string]: string } {
const result: { [key: string]: string } = {};
for (const { name, value } of headers)
result[name] = value;
result[lowerCase ? name.toLowerCase() : name] = value;
dgozman marked this conversation as resolved.
Show resolved Hide resolved
return result;
}
13 changes: 6 additions & 7 deletions src/firefox/ffBrowser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import { Page, PageBinding } from '../page';
import { ConnectionTransport, SlowMoTransport } from '../transport';
import * as types from '../types';
import { ConnectionEvents, FFConnection } from './ffConnection';
import { headersArray } from './ffNetworkManager';
import { FFPage } from './ffPage';
import { Protocol } from './protocol';

Expand Down Expand Up @@ -211,7 +210,7 @@ export class FFBrowserContext extends BrowserContextBase {
if (this._options.permissions)
promises.push(this.grantPermissions(this._options.permissions));
if (this._options.extraHTTPHeaders || this._options.locale)
promises.push(this.setExtraHTTPHeaders(this._options.extraHTTPHeaders || {}));
promises.push(this.setExtraHTTPHeaders(this._options.extraHTTPHeaders || []));
if (this._options.httpCredentials)
promises.push(this.setHTTPCredentials(this._options.httpCredentials));
if (this._options.geolocation)
Expand Down Expand Up @@ -294,12 +293,12 @@ export class FFBrowserContext extends BrowserContextBase {
await this._browser._connection.send('Browser.setGeolocationOverride', { browserContextId: this._browserContextId || undefined, geolocation: geolocation || null });
}

async setExtraHTTPHeaders(headers: types.Headers): Promise<void> {
this._options.extraHTTPHeaders = network.verifyHeaders(headers);
const allHeaders = { ...this._options.extraHTTPHeaders };
async setExtraHTTPHeaders(headers: types.HeadersArray): Promise<void> {
this._options.extraHTTPHeaders = headers;
let allHeaders = this._options.extraHTTPHeaders;
if (this._options.locale)
allHeaders['Accept-Language'] = this._options.locale;
await this._browser._connection.send('Browser.setExtraHTTPHeaders', { browserContextId: this._browserContextId || undefined, headers: headersArray(allHeaders) });
allHeaders = network.mergeHeaders([allHeaders, network.singleHeader('Accept-Language', this._options.locale)]);
await this._browser._connection.send('Browser.setExtraHTTPHeaders', { browserContextId: this._browserContextId || undefined, headers: allHeaders });
}

async setOffline(offline: boolean): Promise<void> {
Expand Down
19 changes: 2 additions & 17 deletions src/firefox/ffNetworkManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,7 @@ export class FFNetworkManager {
throw new Error(`Response body for ${request.request.method()} ${request.request.url()} was evicted!`);
return Buffer.from(response.base64body, 'base64');
};
const headers: types.Headers = {};
for (const {name, value} of event.headers)
headers[name.toLowerCase()] = value;
const response = new network.Response(request.request, event.status, event.statusText, headers, getResponseBody);
const response = new network.Response(request.request, event.status, event.statusText, event.headers, getResponseBody);
this._page._frameManager.requestReceivedResponse(response);
}

Expand Down Expand Up @@ -150,14 +147,11 @@ class InterceptableRequest implements network.RouteDelegate {
this._id = payload.requestId;
this._session = session;

const headers: types.Headers = {};
for (const {name, value} of payload.headers)
headers[name.toLowerCase()] = value;
let postDataBuffer = null;
if (payload.postData)
postDataBuffer = Buffer.from(payload.postData, 'base64');
this.request = new network.Request(payload.isIntercepted ? this : null, frame, redirectedFrom ? redirectedFrom.request : null, payload.navigationId,
payload.url, internalCauseToResourceType[payload.internalCause] || causeToResourceType[payload.cause] || 'other', payload.method, postDataBuffer, headers);
payload.url, internalCauseToResourceType[payload.internalCause] || causeToResourceType[payload.cause] || 'other', payload.method, postDataBuffer, payload.headers);
}

async continue(overrides: types.NormalizedContinueOverrides) {
Expand Down Expand Up @@ -188,12 +182,3 @@ class InterceptableRequest implements network.RouteDelegate {
});
}
}

export function headersArray(headers: types.Headers): Protocol.Network.HTTPHeader[] {
const result: Protocol.Network.HTTPHeader[] = [];
for (const name in headers) {
if (!Object.is(headers[name], undefined))
result.push({name, value: headers[name] + ''});
}
return result;
}
4 changes: 2 additions & 2 deletions src/firefox/ffPage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { FFBrowserContext } from './ffBrowser';
import { FFSession, FFSessionEvents } from './ffConnection';
import { FFExecutionContext } from './ffExecutionContext';
import { RawKeyboardImpl, RawMouseImpl } from './ffInput';
import { FFNetworkManager, headersArray } from './ffNetworkManager';
import { FFNetworkManager } from './ffNetworkManager';
import { Protocol } from './protocol';
import { selectors } from '../selectors';
import { rewriteErrorMessage } from '../utils/stackTrace';
Expand Down Expand Up @@ -272,7 +272,7 @@ export class FFPage implements PageDelegate {
}

async updateExtraHTTPHeaders(): Promise<void> {
await this._session.send('Network.setExtraHTTPHeaders', { headers: headersArray(this._page._state.extraHTTPHeaders || {}) });
await this._session.send('Network.setExtraHTTPHeaders', { headers: this._page._state.extraHTTPHeaders || [] });
}

async setViewportSize(viewportSize: types.Size): Promise<void> {
Expand Down
5 changes: 3 additions & 2 deletions src/frames.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,8 +428,9 @@ export class Frame {
return runNavigationTask(this, options, async progress => {
const waitUntil = verifyLifecycle('waitUntil', options.waitUntil === undefined ? 'load' : options.waitUntil);
progress.log(`navigating to "${url}", waiting until "${waitUntil}"`);
const headers = (this._page._state.extraHTTPHeaders || {});
let referer = headers['referer'] || headers['Referer'];
const headers = this._page._state.extraHTTPHeaders || [];
const refererHeader = headers.find(h => h.name === 'referer' || h.name === 'Referer');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it used to be like this before, but why not

h => h.name.toLowerCase() === 'referer'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea. I'll follow up 😄

let referer = refererHeader ? refererHeader.value : undefined;
if (options.referer !== undefined) {
if (referer !== undefined && referer !== options.referer)
throw new Error('"referer" is already specified as extra HTTP header');
Expand Down
Loading