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.

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

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

Merged
merged 4 commits into from
Nov 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 25 additions & 14 deletions src/core/bypass.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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 () => {
Expand All @@ -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']])
})
12 changes: 7 additions & 5 deletions src/core/bypass.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 1 addition & 3 deletions src/core/passthrough.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
/**
* @vitest-environment node
*/
// @vitest-environment node
import { passthrough } from './passthrough'

it('creates a 302 response with the intention header', () => {
Expand Down
8 changes: 4 additions & 4 deletions src/core/utils/handleRequest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> = []
Expand All @@ -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> = [
Expand Down
4 changes: 2 additions & 2 deletions src/core/utils/handleRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 8 additions & 6 deletions src/mockServiceWorker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
}
Expand Down
23 changes: 23 additions & 0 deletions src/node/SetupServerCommonApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
14 changes: 14 additions & 0 deletions test/browser/rest-api/send-beacon.mocks.ts
Original file line number Diff line number Diff line change
@@ -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()
56 changes: 56 additions & 0 deletions test/browser/rest-api/send-beacon.test.ts
Original file line number Diff line number Diff line change
@@ -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)
})
8 changes: 4 additions & 4 deletions test/node/graphql-api/response-patching.node.test.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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')
})
80 changes: 80 additions & 0 deletions test/node/rest-api/response-patching.node.test.ts
Original file line number Diff line number Diff line change
@@ -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',
)
})
Loading