From 2fa98c327acc51189f87789d9155c4ec57be2299 Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Sun, 10 Nov 2024 22:07:07 +0100 Subject: [PATCH] fix: prevent infinite loop when bypassing `sendBeacon()` requests (#2353) --- src/core/bypass.test.ts | 39 +++++---- src/core/bypass.ts | 12 +-- src/core/passthrough.test.ts | 4 +- src/core/utils/handleRequest.test.ts | 8 +- src/core/utils/handleRequest.ts | 4 +- src/mockServiceWorker.js | 14 ++-- src/node/SetupServerCommonApi.ts | 23 ++++++ test/browser/rest-api/send-beacon.mocks.ts | 14 ++++ test/browser/rest-api/send-beacon.test.ts | 56 +++++++++++++ .../response-patching.node.test.ts | 8 +- .../rest-api/response-patching.node.test.ts | 80 +++++++++++++++++++ 11 files changed, 224 insertions(+), 38 deletions(-) create mode 100644 test/browser/rest-api/send-beacon.mocks.ts create mode 100644 test/browser/rest-api/send-beacon.test.ts create mode 100644 test/node/rest-api/response-patching.node.test.ts diff --git a/src/core/bypass.test.ts b/src/core/bypass.test.ts index bef858bcb..b75901c91 100644 --- a/src/core/bypass.test.ts +++ b/src/core/bypass.test.ts @@ -9,24 +9,21 @@ it('returns bypassed request given a request url string', async () => { // Relative URLs are rebased against the current location. expect(request.method).toBe('GET') expect(request.url).toBe('https://api.example.com/resource') - expect(Object.fromEntries(request.headers.entries())).toEqual({ - 'x-msw-intention': 'bypass', - }) + expect(Array.from(request.headers)).toEqual([['accept', 'msw/passthrough']]) }) it('returns bypassed request given a request url', async () => { const request = bypass(new URL('/resource', 'https://api.example.com')) expect(request.url).toBe('https://api.example.com/resource') - expect(Object.fromEntries(request.headers)).toEqual({ - 'x-msw-intention': 'bypass', - }) + expect(Array.from(request.headers)).toEqual([['accept', 'msw/passthrough']]) }) it('returns bypassed request given request instance', async () => { const original = new Request('http://localhost/resource', { method: 'POST', headers: { + accept: '*/*', 'X-My-Header': 'value', }, body: 'hello world', @@ -40,10 +37,11 @@ it('returns bypassed request given request instance', async () => { expect(original.bodyUsed).toBe(false) expect(bypassedRequestBody).toEqual(await original.text()) - expect(Object.fromEntries(request.headers.entries())).toEqual({ - ...Object.fromEntries(original.headers.entries()), - 'x-msw-intention': 'bypass', - }) + expect(Array.from(request.headers)).toEqual([ + ['accept', '*/*, msw/passthrough'], + ['content-type', 'text/plain;charset=UTF-8'], + ['x-my-header', 'value'], + ]) }) it('allows modifying the bypassed request instance', async () => { @@ -57,13 +55,26 @@ it('allows modifying the bypassed request instance', async () => { }) expect(request.method).toBe('PUT') - expect(Object.fromEntries(request.headers.entries())).toEqual({ - 'x-msw-intention': 'bypass', - 'x-modified-header': 'yes', - }) + expect(Array.from(request.headers)).toEqual([ + ['accept', 'msw/passthrough'], + ['x-modified-header', 'yes'], + ]) expect(original.bodyUsed).toBe(false) expect(request.bodyUsed).toBe(false) expect(await request.text()).toBe('hello world') expect(original.bodyUsed).toBe(false) }) + +it('supports bypassing "keepalive: true" requests', async () => { + const original = new Request('http://localhost/resource', { + method: 'POST', + keepalive: true, + }) + const request = bypass(original) + + expect(request.method).toBe('POST') + expect(request.url).toBe('http://localhost/resource') + expect(request.body).toBeNull() + expect(Array.from(request.headers)).toEqual([['accept', 'msw/passthrough']]) +}) diff --git a/src/core/bypass.ts b/src/core/bypass.ts index 26d3afb2f..91cbd15e4 100644 --- a/src/core/bypass.ts +++ b/src/core/bypass.ts @@ -34,11 +34,13 @@ export function bypass(input: BypassRequestInput, init?: RequestInit): Request { const requestClone = request.clone() - // Set the internal header that would instruct MSW - // to bypass this request from any further request matching. - // Unlike "passthrough()", bypass is meant for performing - // additional requests within pending request resolution. - requestClone.headers.set('x-msw-intention', 'bypass') + /** + * Send the internal request header that would instruct MSW + * to perform this request as-is, ignoring any matching handlers. + * @note Use the `accept` header to support scenarios when the + * request cannot have headers (e.g. `sendBeacon` requests). + */ + requestClone.headers.append('accept', 'msw/passthrough') return requestClone } diff --git a/src/core/passthrough.test.ts b/src/core/passthrough.test.ts index 06a727bd3..5a7a5ee21 100644 --- a/src/core/passthrough.test.ts +++ b/src/core/passthrough.test.ts @@ -1,6 +1,4 @@ -/** - * @vitest-environment node - */ +// @vitest-environment node import { passthrough } from './passthrough' it('creates a 302 response with the intention header', () => { diff --git a/src/core/utils/handleRequest.test.ts b/src/core/utils/handleRequest.test.ts index 5e9e896c5..c8d87d617 100644 --- a/src/core/utils/handleRequest.test.ts +++ b/src/core/utils/handleRequest.test.ts @@ -46,13 +46,13 @@ afterEach(() => { vi.resetAllMocks() }) -test('returns undefined for a request with the "x-msw-intention" header equal to "bypass"', async () => { +test('returns undefined for a request with the "accept: msw/passthrough" header equal to "bypass"', async () => { const { emitter, events } = setup() const requestId = createRequestId() const request = new Request(new URL('http://localhost/user'), { headers: new Headers({ - 'x-msw-intention': 'bypass', + accept: 'msw/passthrough', }), }) const handlers: Array = [] @@ -79,12 +79,12 @@ test('returns undefined for a request with the "x-msw-intention" header equal to expect(handleRequestOptions.onMockedResponse).not.toHaveBeenCalled() }) -test('does not bypass a request with "x-msw-intention" header set to arbitrary value', async () => { +test('does not bypass a request with "accept: msw/*" header set to arbitrary value', async () => { const { emitter } = setup() const request = new Request(new URL('http://localhost/user'), { headers: new Headers({ - 'x-msw-intention': 'invalid', + acceot: 'msw/invalid', }), }) const handlers: Array = [ diff --git a/src/core/utils/handleRequest.ts b/src/core/utils/handleRequest.ts index 1f5780bbc..b7cd2599e 100644 --- a/src/core/utils/handleRequest.ts +++ b/src/core/utils/handleRequest.ts @@ -46,8 +46,8 @@ export async function handleRequest( ): Promise { emitter.emit('request:start', { request, requestId }) - // Perform bypassed requests (i.e. wrapped in "bypass()") as-is. - if (request.headers.get('x-msw-intention') === 'bypass') { + // Perform requests wrapped in "bypass()" as-is. + if (request.headers.get('accept')?.includes('msw/passthrough')) { emitter.emit('request:end', { request, requestId }) handleRequestOptions?.onPassthroughResponse?.(request) return diff --git a/src/mockServiceWorker.js b/src/mockServiceWorker.js index 18592f3d5..364d44023 100644 --- a/src/mockServiceWorker.js +++ b/src/mockServiceWorker.js @@ -192,12 +192,14 @@ async function getResponse(event, client, requestId) { const requestClone = request.clone() function passthrough() { - const headers = Object.fromEntries(requestClone.headers.entries()) - - // Remove internal MSW request header so the passthrough request - // complies with any potential CORS preflight checks on the server. - // Some servers forbid unknown request headers. - delete headers['x-msw-intention'] + // Cast the request headers to a new Headers instance + // so the headers can be manipulated with. + const headers = new Headers(requestClone.headers) + + // Remove the "accept" header value that marked this request as passthrough. + // This prevents request alteration and also keeps it compliant with the + // user-defined CORS policies. + headers.delete('accept', 'msw/passthrough') return fetch(requestClone, { headers }) } diff --git a/src/node/SetupServerCommonApi.ts b/src/node/SetupServerCommonApi.ts index 0d2104119..0f054749a 100644 --- a/src/node/SetupServerCommonApi.ts +++ b/src/node/SetupServerCommonApi.ts @@ -67,6 +67,29 @@ export class SetupServerCommonApi .filter(isHandlerKind('RequestHandler')), this.resolvedOptions, this.emitter, + { + onPassthroughResponse(request) { + const acceptHeader = request.headers.get('accept') + + /** + * @note Remove the internal bypass request header. + * In the browser, this is done by the worker script. + * In Node.js, it has to be done here. + */ + if (acceptHeader) { + const nextAcceptHeader = acceptHeader.replace( + /(,\s+)?msw\/passthrough/, + '', + ) + + if (nextAcceptHeader) { + request.headers.set('accept', nextAcceptHeader) + } else { + request.headers.delete('accept') + } + } + }, + }, ) if (response) { diff --git a/test/browser/rest-api/send-beacon.mocks.ts b/test/browser/rest-api/send-beacon.mocks.ts new file mode 100644 index 000000000..6eb46ceca --- /dev/null +++ b/test/browser/rest-api/send-beacon.mocks.ts @@ -0,0 +1,14 @@ +import { http, bypass } from 'msw' +import { setupWorker } from 'msw/browser' + +const worker = setupWorker( + http.post('/analytics', ({ request }) => { + return new Response(request.body) + }), + http.post('*/analytics-bypass', ({ request }) => { + const nextRequest = bypass(request) + return fetch(nextRequest) + }), +) + +worker.start() diff --git a/test/browser/rest-api/send-beacon.test.ts b/test/browser/rest-api/send-beacon.test.ts new file mode 100644 index 000000000..f2d530ba0 --- /dev/null +++ b/test/browser/rest-api/send-beacon.test.ts @@ -0,0 +1,56 @@ +import { test, expect } from '../playwright.extend' + +test('supports mocking a response to a "sendBeacon" request', async ({ + loadExample, + page, +}) => { + await loadExample(require.resolve('./send-beacon.mocks.ts')) + + const isQueuedPromise = page.evaluate(() => { + return navigator.sendBeacon( + '/analytics', + JSON.stringify({ event: 'pageview' }), + ) + }) + + const response = await page.waitForResponse((response) => { + return response.url().endsWith('/analytics') + }) + + expect(response.status()).toBe(200) + // Technically, "sendBeacon" responses don't send any body back. + // We use this body only to verify that the request body was accessible + // in the request handlers. + await expect(response.text()).resolves.toBe('{"event":"pageview"}') + + // Must return true, indicating that the server has queued the sent data. + await expect(isQueuedPromise).resolves.toBe(true) +}) + +test('supports bypassing "sendBeacon" requests', async ({ + loadExample, + page, +}) => { + const { compilation } = await loadExample( + require.resolve('./send-beacon.mocks.ts'), + { + beforeNavigation(compilation) { + compilation.use((router) => { + router.post('/analytics-bypass', (_req, res) => { + res.status(200).end() + }) + }) + }, + }, + ) + + const url = new URL('./analytics-bypass', compilation.previewUrl).href + const isQueuedPromise = page.evaluate((url) => { + return navigator.sendBeacon(url, JSON.stringify({ event: 'pageview' })) + }, url) + + const response = await page.waitForResponse(url) + expect(response.status()).toBe(200) + + await expect(isQueuedPromise).resolves.toBe(true) +}) diff --git a/test/node/graphql-api/response-patching.node.test.ts b/test/node/graphql-api/response-patching.node.test.ts index 6e05614ed..86fe2346a 100644 --- a/test/node/graphql-api/response-patching.node.test.ts +++ b/test/node/graphql-api/response-patching.node.test.ts @@ -1,6 +1,4 @@ -/** - * @vitest-environment node - */ +// @vitest-environment node import { bypass, graphql, HttpResponse } from 'msw' import { setupServer } from 'msw/node' import { graphql as executeGraphql, buildSchema } from 'graphql' @@ -29,6 +27,8 @@ const server = setupServer( const httpServer = new HttpServer((app) => { app.post('/graphql', async (req, res) => { + console.log('pass:', req.headers) + const result = await executeGraphql({ schema: buildSchema(gql` type User { @@ -112,7 +112,7 @@ test('patches a GraphQL response', async () => { firstName: 'Christian', lastName: 'Maverick', }) - expect(res.data?.requestHeaders).toHaveProperty('x-msw-intention', 'bypass') + expect(res.data?.requestHeaders).toHaveProperty('accept', '*/*') expect(res.data?.requestHeaders).not.toHaveProperty('_headers') expect(res.data?.requestHeaders).not.toHaveProperty('_names') }) diff --git a/test/node/rest-api/response-patching.node.test.ts b/test/node/rest-api/response-patching.node.test.ts new file mode 100644 index 000000000..f6a6205ef --- /dev/null +++ b/test/node/rest-api/response-patching.node.test.ts @@ -0,0 +1,80 @@ +// @vitest-environment node +import { http, bypass } from 'msw' +import { setupServer } from 'msw/node' +import express from 'express' +import { HttpServer } from '@open-draft/test-server/http' + +const httpServer = new HttpServer((app) => { + app.use('/resource', (_req, res, next) => { + res.setHeader('access-control-allow-headers', '*') + next() + }) + app.post('/resource', express.text(), (req, res) => { + res.json({ + text: req.body, + requestHeaders: req.headers, + }) + }) +}) + +const server = setupServer() + +beforeAll(async () => { + server.listen() + await httpServer.listen() +}) + +afterEach(() => { + server.resetHandlers() +}) + +afterAll(async () => { + server.close() + await httpServer.close() +}) + +it('supports patching an original HTTP response', async () => { + server.use( + http.post(httpServer.http.url('/resource'), async ({ request }) => { + const originalResponse = await fetch(bypass(request)) + const { text, requestHeaders } = await originalResponse.json() + return new Response(text.toUpperCase(), { headers: requestHeaders }) + }), + ) + + const response = await fetch(httpServer.http.url('/resource'), { + method: 'POST', + body: 'world', + }) + + await expect(response.text()).resolves.toBe('WORLD') + + // Must not contain the internal bypass request header. + expect(Object.fromEntries(response.headers)).toHaveProperty('accept', '*/*') +}) + +it('preserves request "accept" header when patching a response', async () => { + server.use( + http.post(httpServer.http.url('/resource'), async ({ request }) => { + const originalResponse = await fetch(bypass(request)) + const { text, requestHeaders } = await originalResponse.json() + return new Response(text.toUpperCase(), { headers: requestHeaders }) + }), + ) + + const response = await fetch(httpServer.http.url('/resource'), { + method: 'POST', + headers: { + accept: 'application/json', + }, + body: 'world', + }) + + await expect(response.text()).resolves.toBe('WORLD') + + // Must not contain the internal bypass request header. + expect(Object.fromEntries(response.headers)).toHaveProperty( + 'accept', + 'application/json', + ) +})