Skip to content

Commit aa726e8

Browse files
kettanaitomikicho
andauthored
fix(ClientRequest): allow GET requests with body (#715)
Co-authored-by: Michael Solomon <github@tltv.co.il>
1 parent 6bfa4e1 commit aa726e8

File tree

3 files changed

+91
-17
lines changed

3 files changed

+91
-17
lines changed

src/interceptors/ClientRequest/MockHttpSocket.ts

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -488,22 +488,20 @@ export class MockHttpSocket extends MockSocket {
488488
// If this Socket is reused for multiple requests,
489489
// this ensures that each request gets its own stream.
490490
// One Socket instance can only handle one request at a time.
491-
if (canHaveBody) {
492-
this.requestStream = new Readable({
493-
/**
494-
* @note Provide the `read()` method so a `Readable` could be
495-
* used as the actual request body (the stream calls "read()").
496-
* We control the queue in the onRequestBody/End functions.
497-
*/
498-
read: () => {
499-
// If the user attempts to read the request body,
500-
// flush the write buffer to trigger the callbacks.
501-
// This way, if the request stream ends in the write callback,
502-
// it will indeed end correctly.
503-
this.flushWriteBuffer()
504-
},
505-
})
506-
}
491+
this.requestStream = new Readable({
492+
/**
493+
* @note Provide the `read()` method so a `Readable` could be
494+
* used as the actual request body (the stream calls "read()").
495+
* We control the queue in the onRequestBody/End functions.
496+
*/
497+
read: () => {
498+
// If the user attempts to read the request body,
499+
// flush the write buffer to trigger the callbacks.
500+
// This way, if the request stream ends in the write callback,
501+
// it will indeed end correctly.
502+
this.flushWriteBuffer()
503+
},
504+
})
507505

508506
const requestId = createRequestId()
509507
this.request = new Request(url, {

test/modules/WebSocket/compliance/websocket.events.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,13 +236,13 @@ it('emits "error" event on passthrough client connection failure', async () => {
236236
expect(errorListener).toHaveBeenCalledTimes(1)
237237
})
238238

239+
expect(ws.readyState).toBe(ws.CLOSED)
239240
expect(openListener).not.toHaveBeenCalled()
240241
/**
241242
* @note The update in `ws` makes it dispatch the "close" event
242243
* if the handshake receives a network error (or non-101 response).
243244
*/
244245
expect(closeListener).toHaveBeenCalledOnce()
245-
expect(ws.readyState).toBe(ws.CLOSED)
246246
})
247247

248248
it('does not emit "error" event on mocked error code closures', async () => {
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/**
2+
* @see https://github.com/nock/nock/issues/2826
3+
*/
4+
import { it, expect, beforeAll, afterAll } from 'vitest'
5+
import http from 'node:http'
6+
import { DeferredPromise } from '@open-draft/deferred-promise'
7+
import { waitForClientRequest } from '../../../helpers'
8+
import { ClientRequestInterceptor } from '../../../../src/interceptors/ClientRequest'
9+
10+
const interceptor = new ClientRequestInterceptor()
11+
12+
const httpServer = new http.Server((req, res) => {
13+
if (req.url === '/resource') {
14+
req.pipe(res)
15+
return
16+
}
17+
18+
res.statusCode = 404
19+
res.end()
20+
})
21+
22+
beforeAll(async () => {
23+
interceptor.apply()
24+
const serverListenPromise = new DeferredPromise<void>()
25+
httpServer.listen(52203, '127.0.0.1', () => {
26+
serverListenPromise.resolve()
27+
})
28+
await serverListenPromise
29+
})
30+
31+
afterAll(() => {
32+
interceptor.removeAllListeners()
33+
})
34+
35+
afterAll(async () => {
36+
interceptor.dispose()
37+
const serverClosePromise = new DeferredPromise<void>()
38+
httpServer.close((error) => {
39+
if (error) {
40+
serverClosePromise.reject(error)
41+
}
42+
serverClosePromise.resolve()
43+
})
44+
await serverClosePromise
45+
})
46+
47+
it('allows an HTTP GET request with a body', async () => {
48+
const interceptedRequestPromise = new DeferredPromise<Request>()
49+
50+
interceptor.on('request', ({ request }) => {
51+
interceptedRequestPromise.resolve(request)
52+
})
53+
54+
const request = http.request('http://127.0.0.1:52203/resource', {
55+
method: 'GET',
56+
headers: {
57+
'content-type': 'text/plain',
58+
'content-length': '11',
59+
},
60+
})
61+
62+
/**
63+
* @note Although it is invalid to have GET requests with a body
64+
* per Fetch API specification, you should still be able to perform
65+
* such requests in Node.js without the library throwing an error.
66+
*/
67+
request.write('hello world')
68+
request.end()
69+
70+
const { text } = await waitForClientRequest(request)
71+
await expect(text()).resolves.toBe('hello world')
72+
73+
const interceptedRequest = await interceptedRequestPromise
74+
// The Fetch API representation of this request must NOT have any body.
75+
expect(interceptedRequest.body).toBeNull()
76+
})

0 commit comments

Comments
 (0)