Skip to content

Commit

Permalink
http: be more aggressive to reply 400, 408 and 431
Browse files Browse the repository at this point in the history
As long as data of the in-flight response is not yet written
to the socket, we can reply an error response without corrupting
the client.

PR-URL: #44818
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ricky Zhou <[email protected]>
  • Loading branch information
ywave620 authored and danielleadams committed Oct 11, 2022
1 parent 67fb765 commit eb19b1e
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 15 deletions.
5 changes: 3 additions & 2 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -1309,8 +1309,9 @@ type other than {net.Socket}.

Default behavior is to try close the socket with a HTTP '400 Bad Request',
or a HTTP '431 Request Header Fields Too Large' in the case of a
[`HPE_HEADER_OVERFLOW`][] error. If the socket is not writable or has already
written data it is immediately destroyed.
[`HPE_HEADER_OVERFLOW`][] error. If the socket is not writable or headers
of the current attached [`http.ServerResponse`][] has been sent, it is
immediately destroyed.

`socket` is the [`net.Socket`][] object that the error originated from.

Expand Down
6 changes: 5 additions & 1 deletion lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,11 @@ function socketOnError(e) {
}

if (!this.server.emit('clientError', e, this)) {
if (this.writable && this.bytesWritten === 0) {
// Caution must be taken to avoid corrupting the remote peer.
// Reply an error segment if there is no in-flight `ServerResponse`,
// or no data of the in-flight one has been written yet to this socket.
if (this.writable &&
(!this._httpMessage || !this._httpMessage._headerSent)) {
let response;

switch (e.code) {
Expand Down
4 changes: 1 addition & 3 deletions test/parallel/test-http-server-headers-timeout-keepalive.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,7 @@ server.listen(0, common.mustCall(() => {
assert.strictEqual(second, true);
assert.strictEqual(
response,
// Empty because of https://github.com/nodejs/node/commit/e8d7fedf7cad6e612e4f2e0456e359af57608ac7
// 'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
''
'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
);
server.close();
});
Expand Down
4 changes: 1 addition & 3 deletions test/parallel/test-http-server-headers-timeout-pipelining.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,7 @@ server.listen(0, common.mustCall(() => {

assert.strictEqual(
response,
// Empty because of https://github.com/nodejs/node/commit/e8d7fedf7cad6e612e4f2e0456e359af57608ac7
// 'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
''
'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
);
server.close();
});
Expand Down
4 changes: 1 addition & 3 deletions test/parallel/test-http-server-request-timeout-keepalive.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@ server.listen(0, common.mustCall(() => {
assert.strictEqual(second, true);
assert.strictEqual(
response,
// Empty because of https://github.com/nodejs/node/commit/e8d7fedf7cad6e612e4f2e0456e359af57608ac7
// 'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
''
'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
);
server.close();
});
Expand Down
4 changes: 1 addition & 3 deletions test/parallel/test-http-server-request-timeout-pipelining.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ server.listen(0, common.mustCall(() => {

assert.strictEqual(
response,
// Empty because of https://github.com/nodejs/node/commit/e8d7fedf7cad6e612e4f2e0456e359af57608ac7
// 'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
''
'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
);
server.close();
});
Expand Down

0 comments on commit eb19b1e

Please sign in to comment.