FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http: be more aggressive to reply 400, 408 and 431 · nodejs/node@eb19b1e · GitHub

/ node Public

Commit eb19b1e

Browse files
ywave620
authored and
danielleadams
committed
http: be more aggressive to reply 400, 408 and 431
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 <paolo@cowtech.it> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
1 parent 67fb765 commit eb19b1e

6 files changed

Lines changed: 12 additions & 15 deletions

‎doc/api/http.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1309,8 +1309,9 @@ type other than {net.Socket}.
13091309

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

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

‎lib/_http_server.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -819,7 +819,11 @@ function socketOnError(e) {
819819
}
820820

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

825829
switch (e.code) {

‎test/parallel/test-http-server-headers-timeout-keepalive.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,7 @@ server.listen(0, common.mustCall(() => {
8181
assert.strictEqual(second, true);
8282
assert.strictEqual(
8383
response,
84-
// Empty because of https://github.com/nodejs/node/commit/e8d7fedf7cad6e612e4f2e0456e359af57608ac7
85-
// 'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
86-
''
84+
'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
8785
);
8886
server.close();
8987
});

‎test/parallel/test-http-server-headers-timeout-pipelining.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,7 @@ server.listen(0, common.mustCall(() => {
5656

5757
assert.strictEqual(
5858
response,
59-
// Empty because of https://github.com/nodejs/node/commit/e8d7fedf7cad6e612e4f2e0456e359af57608ac7
60-
// 'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
61-
''
59+
'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
6260
);
6361
server.close();
6462
});

‎test/parallel/test-http-server-request-timeout-keepalive.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,7 @@ server.listen(0, common.mustCall(() => {
7979
assert.strictEqual(second, true);
8080
assert.strictEqual(
8181
response,
82-
// Empty because of https://github.com/nodejs/node/commit/e8d7fedf7cad6e612e4f2e0456e359af57608ac7
83-
// 'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
84-
''
82+
'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
8583
);
8684
server.close();
8785
});

‎test/parallel/test-http-server-request-timeout-pipelining.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,7 @@ server.listen(0, common.mustCall(() => {
5050

5151
assert.strictEqual(
5252
response,
53-
// Empty because of https://github.com/nodejs/node/commit/e8d7fedf7cad6e612e4f2e0456e359af57608ac7
54-
// 'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
55-
''
53+
'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
5654
);
5755
server.close();
5856
});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL