fix: prevent infinite loop when bypassing sendBeacon() requests (#2353)

Author: kettanaito

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']])
+})

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
 }

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', () => {

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<RequestHandler> = []
@@ -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<RequestHandler> = [

src/core/utils/handleRequest.ts

@@ -46,8 +46,8 @@ export async function handleRequest(
 ): Promise<Response | undefined> {
   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

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 })
   }

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) {

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()

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)
+})

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')
 })

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',
+  )
+})