Skip to content

Commit 61f5185

Browse files
authored
fix: destroy IncomingMessage and invalid assertion (#834)
Change so that we destroy IncomingMessage without closing the socket. Also fixes an invalid assertion that was triggered by the test.
1 parent dce5e4d commit 61f5185

File tree

3 files changed

+61
-4
lines changed

3 files changed

+61
-4
lines changed

lib/client.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,8 +1520,7 @@ function write (client, request) {
15201520
socket.write('\r\n0\r\n\r\n', 'ascii')
15211521
}
15221522

1523-
assert.strictEqual(socket[kParser].timeoutType, TIMEOUT_HEADERS)
1524-
if (socket[kParser].timeout) {
1523+
if (socket[kParser].timeout && socket[kParser].timeoutType === TIMEOUT_HEADERS) {
15251524
// istanbul ignore else: only for jest
15261525
if (socket[kParser].timeout.refresh) {
15271526
socket[kParser].timeout.refresh()

lib/core/util.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,11 @@ function destroy (stream, err) {
130130
}
131131

132132
if (typeof stream.destroy === 'function') {
133-
if (err || Object.getPrototypeOf(stream).constructor !== IncomingMessage) {
134-
stream.destroy(err)
133+
if (Object.getPrototypeOf(stream).constructor === IncomingMessage) {
134+
// See: https://github.com/nodejs/node/pull/38505/files
135+
stream.socket = null
135136
}
137+
stream.destroy(err)
136138
} else if (err) {
137139
process.nextTick((stream, err) => {
138140
stream.emit('error', err)

test/http-req-destroy.js

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
'use strict'
2+
3+
const { test } = require('tap')
4+
const undici = require('..')
5+
const { createServer } = require('http')
6+
const { Readable } = require('stream')
7+
8+
test('do not kill req socket', (t) => {
9+
t.plan(3)
10+
11+
const server1 = createServer((req, res) => {
12+
undici.request(`http://localhost:${server2.address().port}`, {
13+
method: 'POST',
14+
body: req
15+
}, (err, response) => {
16+
t.error(err)
17+
setTimeout(() => {
18+
response.body.on('data', buf => {
19+
res.write(buf)
20+
setTimeout(() => {
21+
res.end()
22+
}, 100)
23+
})
24+
}, 100)
25+
})
26+
})
27+
t.teardown(server1.close.bind(server1))
28+
29+
const server2 = createServer((req, res) => {
30+
setTimeout(() => {
31+
req.pipe(res)
32+
}, 100)
33+
})
34+
t.teardown(server2.close.bind(server2))
35+
36+
server1.listen(0, () => {
37+
const r = new Readable({ read () {} })
38+
r.push('hello')
39+
undici.request(`http://localhost:${server1.address().port}`, {
40+
method: 'POST',
41+
body: r
42+
}, (err, response) => {
43+
t.error(err)
44+
const bufs = []
45+
response.body.on('data', (buf) => {
46+
bufs.push(buf)
47+
r.push(null)
48+
})
49+
response.body.on('end', () => {
50+
t.equal('hello', Buffer.concat(bufs).toString('utf8'))
51+
})
52+
})
53+
})
54+
55+
server2.listen(0)
56+
})

0 commit comments

Comments
 (0)