Skip to content

Commit 56d3349

Browse files
committed
Fix HTTP/2 race condition with 204/304 responses + abort
In some cases (only sporadically with real traffic) it was possible to see spurious aborts & inconsistent upstream/downstream data because the HTTP/2 response would come in, we'd still be waiting for the stream to close, but Node would have already successfully closed the downstream stream for us. That resulted in a close event suggesting the client was aborting before the server response completed, which wasn't true. This isn't correct: if the writable has already ended then either it's closed successfully (and we don't need the server response) or its already aborted (and we still don't need the server response). Best to close preemptively.
1 parent c93d31f commit 56d3349

File tree

1 file changed

+14
-0
lines changed

1 file changed

+14
-0
lines changed

src/rules/requests/request-step-impls.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,6 +1018,20 @@ export class PassThroughStepImpl extends PassThroughStep {
10181018
// onward directly:
10191019
clientRes.end(originalBody);
10201020
resolve();
1021+
} else if (isH2Downstream && (
1022+
serverRes.statusCode === 204 ||
1023+
serverRes.statusCode === 205 ||
1024+
serverRes.statusCode === 304 ||
1025+
method === 'HEAD'
1026+
)) {
1027+
// Here, Node's HTTP/2 implementation auto-ends the downstream request knowing
1028+
// that it can't have a body. We need to mirror this, or we end up with a confusing
1029+
// race condition where the client is done (and can even close the connection) while
1030+
// the server response is still technically pending.
1031+
// https://github.com/nodejs/node/blob/f6f8eb7c/lib/internal/http2/core.js#L2976-L2985
1032+
clientRes.end();
1033+
serverRes.destroy();
1034+
resolve();
10211035
} else {
10221036
// Otherwise the body hasn't been read - stream it live:
10231037
serverRes.pipe(clientRes);

0 commit comments

Comments
 (0)