Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit eb19b1e

Browse filesBrowse files
ywave620danielleadams
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
Copy full SHA for eb19b1e

File tree

Expand file treeCollapse file tree

6 files changed

+12
-15
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+12
-15
lines changed
Open diff view settings
Collapse file

‎doc/api/http.md‎

Copy file name to clipboardExpand all lines: doc/api/http.md
+3-2Lines changed: 3 additions & 2 deletions
  • Display the source diff
  • Display the rich diff
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

Collapse file

‎lib/_http_server.js‎

Copy file name to clipboardExpand all lines: lib/_http_server.js
+5-1Lines 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) {
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-http-server-headers-timeout-keepalive.js
+1-3Lines 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
});
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-http-server-headers-timeout-pipelining.js
+1-3Lines 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
});
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-http-server-request-timeout-keepalive.js
+1-3Lines 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
});
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-http-server-request-timeout-pipelining.js
+1-3Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.