Commit 66fe2d9
stream: avoid destroying http1 objects
http1 objects are coupled with their corresponding
res/req and cannot be treated independently as
normal streams. Add a special exception for this
in the pipeline cleanup.
Fixes: #32184
Backport-PR-URL: #32212
PR-URL: #32197
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>1 parent 77e5b50 commit 66fe2d9
2 files changed
Lines changed: 45 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -25,8 +25,20 @@ let EE; | |||
| 25 | 25 | let PassThrough; | |
| 26 | 26 | let createReadableStreamAsyncIterator; | |
| 27 | 27 | ||
| 28 | - function isRequest(stream) { | ||
| 29 | - return stream && stream.setHeader && typeof stream.abort === 'function'; | ||
| 28 | + function isIncoming(stream) { | ||
| 29 | + return ( | ||
| 30 | + stream.socket && | ||
| 31 | + typeof stream.complete === 'boolean' && | ||
| 32 | + ArrayIsArray(stream.rawTrailers) && | ||
| 33 | + ArrayIsArray(stream.rawHeaders) | ||
| 34 | + ); | ||
| 35 | + } | ||
| 36 | + | ||
| 37 | + function isOutgoing(stream) { | ||
| 38 | + return ( | ||
| 39 | + stream.socket && | ||
| 40 | + typeof stream.setHeader === 'function' | ||
| 41 | + ); | ||
| 30 | 42 | } | |
| 31 | 43 | ||
| 32 | 44 | function destroyer(stream, reading, writing, final, callback) { | |
@@ -37,10 +49,18 @@ function destroyer(stream, reading, writing, final, callback) { | |||
| 37 | 49 | eos(stream, { readable: reading, writable: writing }, (err) => { | |
| 38 | 50 | if (destroyed) return; | |
| 39 | 51 | destroyed = true; | |
| 40 | - const readable = stream.readable || isRequest(stream); | ||
| 41 | - if (err || !final || !readable) { | ||
| 52 | + | ||
| 53 | + if (!err && (isIncoming(stream) || isOutgoing(stream))) { | ||
| 54 | + // http/1 request objects have a coupling to their response and should | ||
| 55 | + // not be prematurely destroyed. Assume they will handle their own | ||
| 56 | + // lifecycle. | ||
| 57 | + return callback(); | ||
| 58 | + } | ||
| 59 | + | ||
| 60 | + if (err || !final || !stream.readable) { | ||
| 42 | 61 | destroyImpl.destroyer(stream, err); | |
| 43 | 62 | } | |
| 63 | + | ||
| 44 | 64 | callback(err); | |
| 45 | 65 | }); | |
| 46 | 66 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -995,3 +995,24 @@ const { promisify } = require('util'); | |||
| 995 | 995 | assert.strictEqual(res, ''); | |
| 996 | 996 | })); | |
| 997 | 997 | } | |
| 998 | + | ||
| 999 | + { | ||
| 1000 | + const server = http.createServer((req, res) => { | ||
| 1001 | + req.socket.on('error', common.mustNotCall()); | ||
| 1002 | + pipeline(req, new PassThrough(), (err) => { | ||
| 1003 | + assert.ifError(err); | ||
| 1004 | + res.end(); | ||
| 1005 | + server.close(); | ||
| 1006 | + }); | ||
| 1007 | + }); | ||
| 1008 | + | ||
| 1009 | + server.listen(0, () => { | ||
| 1010 | + const req = http.request({ | ||
| 1011 | + method: 'PUT', | ||
| 1012 | + port: server.address().port | ||
| 1013 | + }); | ||
| 1014 | + req.end('asd123'); | ||
| 1015 | + req.on('response', common.mustCall()); | ||
| 1016 | + req.on('error', common.mustNotCall()); | ||
| 1017 | + }); | ||
| 1018 | + } | ||
0 commit comments