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: parse multipart/form-data requests correctly #39

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 3 additions & 8 deletions src/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { Emitter } from 'strict-event-emitter'
import type { RequestHandler as ExpressMiddleware } from 'express'
import type { LifeCycleEventsMap, RequestHandler } from 'msw'

const encoder = new TextEncoder()
const emitter = new Emitter<LifeCycleEventsMap>()

export function createMiddleware(
Expand All @@ -18,10 +17,8 @@ export function createMiddleware(
const method = req.method || 'GET'

// Ensure the request body input passed to the MockedRequest
// is always a string. Custom middleware like "express.json()"
// may coerce "req.body" to be an Object.
const requestBody =
typeof req.body === 'string' ? req.body : JSON.stringify(req.body)
// is always the raw body from express req.
// "express.raw({ type: '*/*' })" must be used to get "req.body".

const mockedRequest = new Request(
// Treat all relative URLs as the ones coming from the server.
Expand All @@ -31,9 +28,7 @@ export function createMiddleware(
headers: new Headers(req.headers as HeadersInit),
credentials: 'omit',
// Request with GET/HEAD method cannot have body.
body: ['GET', 'HEAD'].includes(method)
? undefined
: encoder.encode(requestBody),
body: ['GET', 'HEAD'].includes(method) ? undefined : req.body,
},
)

Expand Down
9 changes: 7 additions & 2 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@ import express from 'express'
import { HttpHandler } from 'msw'
import { createMiddleware } from './middleware'

export function createServer(...handlers: Array<HttpHandler>): express.Express {
type ParserOptions = Parameters<typeof express.raw>[0]

export function createServer(
parserOptions?: ParserOptions,
...handlers: Array<HttpHandler>
): express.Express {
const app = express()

app.use(express.json())
app.use(express.raw({ type: '*/*', limit: '20mb', ...parserOptions }))
app.use(createMiddleware(...handlers))
app.use((_req, res) => {
res.status(404).json({
Expand Down
66 changes: 34 additions & 32 deletions test/with-express-json.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,50 +3,52 @@
*/
import fetch from 'node-fetch'
import express from 'express'
import { HttpServer } from '@open-draft/test-server/http'
import { HttpResponse, http } from 'msw'
import { createMiddleware } from '../src'

const httpServer = new HttpServer((app) => {
// Apply a request body JSON middleware.
app.use(express.json())

app.use(
createMiddleware(
http.post<never, { firstName: string }>('/user', async ({ request }) => {
const { firstName } = await request.json()

return HttpResponse.json(
{ firstName },
{
headers: {
'x-my-header': 'value',
},
import httpModule from 'http'
import { AddressInfo } from 'net'

const app = express()

app.use(express.raw({ type: '*/*' }))
Copy link
Member

Choose a reason for hiding this comment

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

What do you think if we bake in express.raw() into createMiddleware() itself?

Applying it to the entire server is too much anyway since people can attach handlers-based routes to an existing Express server.

In other words, having the raw body parser is a prerequisite of the middleware. So it must be defined there, next to the middleware.


app.use(
createMiddleware(
http.post<never, { firstName: string }>('/user', async ({ request }) => {
const { firstName } = await request.json()

return HttpResponse.json(
{ firstName },
{
headers: {
'x-my-header': 'value',
},
)
}),
),
)

app.use((_req, res) => {
res.status(404).json({
error: 'Mock not found',
})
},
)
}),
),
)

app.use((_req, res) => {
res.status(404).json({
error: 'Mock not found',
})

return app
})

beforeAll(async () => {
await httpServer.listen()
const httpServer = httpModule.createServer(app)

beforeAll(() => {
httpServer.listen({ port: 0 })
})

afterAll(async () => {
await httpServer.close()
httpServer.close()
})

it('supports usage alongside with the "express.json()" middleware', async () => {
const res = await fetch(httpServer.http.url('/user'), {
it('supports usage with json payload', async () => {
const { port } = httpServer.address() as AddressInfo
const res = await fetch(`http://localhost:${port}/user`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
Expand Down
69 changes: 69 additions & 0 deletions test/with-express-multipart-form.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/**
* @jest-environment node
*/
import express from 'express'
import { HttpResponse, http } from 'msw'
import { createMiddleware } from '../src'

import httpModule from 'http'
import { AddressInfo } from 'net'

const app = express()

app.use(express.raw({ type: '*/*' }))

app.use(
createMiddleware(
http.post<never, { firstName: string }>('/user', async ({ request }) => {
const form = await request.formData()

const field1 = form.get('field1')
const field2 = form.get('field2')

if (!field1 || typeof field1 !== 'string') return HttpResponse.error()
if (!field2 || typeof field2 !== 'string') return HttpResponse.error()

return HttpResponse.json(
{ field1, field2 },
{
headers: {
'x-my-header': 'value',
},
},
)
}),
),
)

app.use((_req, res) => {
res.status(404).json({
error: 'Mock not found',
})
})

const httpServer = httpModule.createServer(app)

beforeAll(() => {
httpServer.listen({ port: 0 })
})

afterAll(async () => {
httpServer.close()
})

it('supports usage with multipart/form-data payload', async () => {
const form = new FormData()

form.append('field1', 'value1')
form.append('field2', 'value2')

const { port } = httpServer.address() as AddressInfo
const res = await fetch(`http://localhost:${port}/user`, {
method: 'POST',
body: form,
})

expect(res.status).toBe(200)
expect(res.headers.get('x-my-header')).toBe('value')
expect(await res.json()).toEqual({ field1: 'value1', field2: 'value2' })
})