Skip to content

Commit

Permalink
fix(graphql): clone request before parsing
Browse files Browse the repository at this point in the history
  • Loading branch information
kettanaito committed Aug 25, 2023
1 parent bb02e57 commit 1275e0c
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 4 deletions.
13 changes: 13 additions & 0 deletions src/core/utils/internal/parseGraphQLRequest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,16 @@ test('returns false given a GraphQL-incompatible request', async () => {
})
expect(await parseGraphQLRequest(postRequest)).toBeUndefined()
})

test('does not read the original request body', async () => {
const request = new Request(new URL('http://localhost/api'), {
method: 'POST',
body: JSON.stringify({ payload: 'value' }),
})

await parseGraphQLRequest(request)

// Must not read the original request body because GraphQL parsing
// is an internal operation that must not lock the body stream.
expect(request.bodyUsed).toBe(false)
})
11 changes: 7 additions & 4 deletions src/core/utils/internal/parseGraphQLRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,9 @@ function extractMultipartVariables<VariablesType extends GraphQLVariables>(
}

async function getGraphQLInput(request: Request): Promise<GraphQLInput | null> {
const url = new URL(request.url)

switch (request.method) {
case 'GET': {
const url = new URL(request.url)
const query = url.searchParams.get('query')
const variables = url.searchParams.get('variables') || ''

Expand All @@ -104,12 +103,16 @@ async function getGraphQLInput(request: Request): Promise<GraphQLInput | null> {
}

case 'POST': {
// Clone the request so we could read its body without locking
// the body stream to the downward consumers.
const requestClone = request.clone()

// Handle multipart body GraphQL operations.
if (
request.headers.get('content-type')?.includes('multipart/form-data')
) {
const responseJson = parseMultipartData<GraphQLMultipartRequestBody>(
await request.text(),
await requestClone.text(),
request.headers,
)

Expand Down Expand Up @@ -147,7 +150,7 @@ async function getGraphQLInput(request: Request): Promise<GraphQLInput | null> {
query: string
variables?: GraphQLVariables
operations?: any /** @todo Annotate this */
} = await request.json().catch(() => null)
} = await requestClone.json().catch(() => null)

if (requestJson?.query) {
const { query, variables } = requestJson
Expand Down
66 changes: 66 additions & 0 deletions test/node/rest-api/request/body/body-used.node.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/**
* @jest-environment node
*/
import { HttpResponse, http, graphql } from 'msw'
import { setupServer } from 'msw/node'
import * as express from 'express'
import { HttpServer } from '@open-draft/test-server/http'

const httpServer = new HttpServer((app) => {
app.post('/resource', express.json(), (req, res) => {
res.json({ response: `received: ${req.body.message}` })
})
})

const server = setupServer()

beforeAll(async () => {
server.listen()
await httpServer.listen()
})

afterEach(() => {
server.resetHandlers()
jest.restoreAllMocks()
})

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

it('does not read the body while parsing an unhandled request', async () => {
// Expecting an unhandled request warning in this test.
jest.spyOn(console, 'warn').mockImplementation(() => {})

const requestUrl = httpServer.http.url('/resource')
const response = await fetch(requestUrl, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({
message: 'Hello server',
}),
})
expect(await response.json()).toEqual({ response: `received: Hello server` })
})

it('does not read the body while parsing an unhandled request', async () => {
const requestUrl = httpServer.http.url('/resource')
server.use(
http.post(requestUrl, () => {
return HttpResponse.json({ mocked: true })
}),
)
const response = await fetch(requestUrl, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({
message: 'Hello server',
}),
})
expect(await response.json()).toEqual({ mocked: true })
})

0 comments on commit 1275e0c

Please sign in to comment.