From 7d7fe3c618464fd6d9d01afd4e679ba336ddd1ac Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Thu, 31 Mar 2022 19:21:21 -0700 Subject: [PATCH] fix(route): remove cors option, compare origin (#13231) --- docs/src/api/class-route.md | 9 --- .../playwright-core/src/client/network.ts | 3 +- .../playwright-core/src/protocol/channels.ts | 2 - .../playwright-core/src/protocol/protocol.yml | 5 -- .../playwright-core/src/protocol/validator.ts | 1 - .../playwright-core/src/server/network.ts | 31 ++++++---- packages/playwright-core/types/types.d.ts | 9 --- tests/page/page-route.spec.ts | 57 +++++++++---------- 8 files changed, 48 insertions(+), 69 deletions(-) diff --git a/docs/src/api/class-route.md b/docs/src/api/class-route.md index afae387bf2..61dd9e3ad4 100644 --- a/docs/src/api/class-route.md +++ b/docs/src/api/class-route.md @@ -226,15 +226,6 @@ is resolved relative to the current working directory. [APIResponse] to fulfill route's request with. Individual fields of the response (such as headers) can be overridden using fulfill options. -### option: Route.fulfill.cors -- `cors` <[CorsMode]<"allow"|"none">> - -Wheb set to "allow" or omitted, the fulfilled response will have -["Access-Control-Allow-Origin"](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin) -header set to request's origin. If the option is set to "none" then -[CORS](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS) headers won't be added to the response. -Note that all CORS headers configured via `headers` option will take precedence. - ## method: Route.request - returns: <[Request]> diff --git a/packages/playwright-core/src/client/network.ts b/packages/playwright-core/src/client/network.ts index e069954748..372eb3b6e4 100644 --- a/packages/playwright-core/src/client/network.ts +++ b/packages/playwright-core/src/client/network.ts @@ -239,7 +239,7 @@ export class Route extends ChannelOwner implements api.Ro await this._raceWithPageClose(this._channel.abort({ errorCode })); } - async fulfill(options: { response?: api.APIResponse, status?: number, headers?: Headers, contentType?: string, cors?: 'allow' | 'none', body?: string | Buffer, path?: string } = {}) { + async fulfill(options: { response?: api.APIResponse, status?: number, headers?: Headers, contentType?: string, body?: string | Buffer, path?: string } = {}) { let fetchResponseUid; let { status: statusOption, headers: headersOption, body } = options; if (options.response) { @@ -282,7 +282,6 @@ export class Route extends ChannelOwner implements api.Ro await this._raceWithPageClose(this._channel.fulfill({ status: statusOption || 200, headers: headersObjectToArray(headers), - cors: options.cors, body, isBase64, fetchResponseUid diff --git a/packages/playwright-core/src/protocol/channels.ts b/packages/playwright-core/src/protocol/channels.ts index 9cf89a9a56..b2d2b657f0 100644 --- a/packages/playwright-core/src/protocol/channels.ts +++ b/packages/playwright-core/src/protocol/channels.ts @@ -3139,7 +3139,6 @@ export type RouteContinueResult = void; export type RouteFulfillParams = { status?: number, headers?: NameValue[], - cors?: 'allow' | 'none', body?: string, isBase64?: boolean, fetchResponseUid?: string, @@ -3147,7 +3146,6 @@ export type RouteFulfillParams = { export type RouteFulfillOptions = { status?: number, headers?: NameValue[], - cors?: 'allow' | 'none', body?: string, isBase64?: boolean, fetchResponseUid?: string, diff --git a/packages/playwright-core/src/protocol/protocol.yml b/packages/playwright-core/src/protocol/protocol.yml index 494a248e4b..62fb42b633 100644 --- a/packages/playwright-core/src/protocol/protocol.yml +++ b/packages/playwright-core/src/protocol/protocol.yml @@ -2454,11 +2454,6 @@ Route: headers: type: array? items: NameValue - cors: - type: enum? - literals: - - allow - - none body: string? isBase64: boolean? fetchResponseUid: string? diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index 902eb04d5e..ee6fcc8d2b 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -1171,7 +1171,6 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { scheme.RouteFulfillParams = tObject({ status: tOptional(tNumber), headers: tOptional(tArray(tType('NameValue'))), - cors: tOptional(tEnum(['allow', 'none'])), body: tOptional(tString), isBase64: tOptional(tBoolean), fetchResponseUid: tOptional(tString), diff --git a/packages/playwright-core/src/server/network.ts b/packages/playwright-core/src/server/network.ts index 57759f7c89..abce84c6a3 100644 --- a/packages/playwright-core/src/server/network.ts +++ b/packages/playwright-core/src/server/network.ts @@ -266,18 +266,7 @@ export class Route extends SdkObject { } } const headers = [...(overrides.headers || [])]; - if (overrides.cors !== 'none') { - const corsHeader = headers.find(({ name }) => name === 'access-control-allow-origin'); - // See https://github.com/microsoft/playwright/issues/12929 - if (!corsHeader) { - const origin = this._request.headerValue('origin'); - if (origin) { - headers.push({ name: 'access-control-allow-origin', value: origin }); - headers.push({ name: 'access-control-allow-credentials', value: 'true' }); - headers.push({ name: 'vary', value: 'Origin' }); - } - } - } + this._maybeAddCorsHeaders(headers); await this._delegate.fulfill({ status: overrides.status || 200, headers, @@ -286,6 +275,24 @@ export class Route extends SdkObject { }); } + // See https://github.com/microsoft/playwright/issues/12929 + private _maybeAddCorsHeaders(headers: NameValue[]) { + const origin = this._request.headerValue('origin'); + if (!origin) + return; + const requestUrl = new URL(this._request.url()); + if (!requestUrl.protocol.startsWith('http')) + return; + if (requestUrl.origin === origin.trim()) + return; + const corsHeader = headers.find(({ name }) => name === 'access-control-allow-origin'); + if (corsHeader) + return; + headers.push({ name: 'access-control-allow-origin', value: origin }); + headers.push({ name: 'access-control-allow-credentials', value: 'true' }); + headers.push({ name: 'vary', value: 'Origin' }); + } + async continue(overrides: types.NormalizedContinueOverrides = {}) { this._startHandling(); if (overrides.url) { diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts index cbcfc9c53d..2f0225d3b5 100644 --- a/packages/playwright-core/types/types.d.ts +++ b/packages/playwright-core/types/types.d.ts @@ -14728,15 +14728,6 @@ export interface Route { */ contentType?: string; - /** - * Wheb set to "allow" or omitted, the fulfilled response will have - * ["Access-Control-Allow-Origin"](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin) - * header set to request's origin. If the option is set to "none" then - * [CORS](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS) headers won't be added to the response. Note that all - * CORS headers configured via `headers` option will take precedence. - */ - cors?: "allow"|"none"; - /** * Response headers. Header values will be converted to a string. */ diff --git a/tests/page/page-route.spec.ts b/tests/page/page-route.spec.ts index 5037ae0631..457b8a27d8 100644 --- a/tests/page/page-route.spec.ts +++ b/tests/page/page-route.spec.ts @@ -516,10 +516,9 @@ it('should not fulfill with redirect status', async ({ page, server, browserName it('should support cors with GET', async ({ page, server, browserName }) => { await page.goto(server.EMPTY_PAGE); await page.route('**/cars*', async (route, request) => { - const headers = request.url().endsWith('allow') ? { 'access-control-allow-origin': '*' } : {}; + const headers = { 'access-control-allow-origin': request.url().endsWith('allow') ? '*' : 'none' }; await route.fulfill({ contentType: 'application/json', - cors: 'none', headers, status: 200, body: JSON.stringify(['electric', 'gas']), @@ -574,7 +573,31 @@ it('should add Access-Control-Allow-Origin by default when fulfill', async ({ pa expect(await response.headerValue('Access-Control-Allow-Origin')).toBe(server.PREFIX); }); -it('should respect cors false', async ({ page, server, browserName }) => { +it('should allow null origin for about:blank', async ({ page, server, browserName }) => { + await page.route('**/something', async route => { + await route.fulfill({ + contentType: 'text/plain', + status: 200, + body: 'done', + }); + }); + + const [response, text] = await Promise.all([ + page.waitForResponse(server.CROSS_PROCESS_PREFIX + '/something'), + page.evaluate(async url => { + const data = await fetch(url, { + method: 'GET', + headers: { 'X-PINGOTHER': 'pingpong' } + }); + return data.text(); + }, server.CROSS_PROCESS_PREFIX + '/something') + ]); + expect(text).toBe('done'); + expect(await response.headerValue('Access-Control-Allow-Origin')).toBe('null'); +}); + +it('should respect cors overrides', async ({ page, server, browserName }) => { + await page.goto(server.EMPTY_PAGE); server.setRoute('/something', (request, response) => { if (request.method === 'OPTIONS') { response.writeHead(204, { @@ -589,37 +612,13 @@ it('should respect cors false', async ({ page, server, browserName }) => { response.writeHead(404, { 'Access-Control-Allow-Origin': '*' }); response.end('NOT FOUND'); }); - // First check the browser will send preflight request when interception is OFF. + // Fetch request should fail when CORS header doesn't include the origin. { await page.route('**/something', async route => { await route.fulfill({ contentType: 'text/plain', status: 200, - body: 'done', - }); - }); - - const [response, text] = await Promise.all([ - page.waitForResponse(server.CROSS_PROCESS_PREFIX + '/something'), - page.evaluate(async url => { - const data = await fetch(url, { - method: 'GET', - headers: { 'X-PINGOTHER': 'pingpong' } - }); - return data.text(); - }, server.CROSS_PROCESS_PREFIX + '/something') - ]); - expect(text).toBe('done'); - expect(await response.headerValue('Access-Control-Allow-Origin')).toBe('null'); - } - - // Fetch request should when CORS headers are missing on the response. - { - await page.route('**/something', async route => { - await route.fulfill({ - contentType: 'text/plain', - status: 200, - cors: 'none', + headers: { 'Access-Control-Allow-Origin': 'http://non-existent' }, body: 'done', }); });