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.
nodejs · nodejs-github-bot · Oct 6, 2022 · Sep 29, 2022 · Sep 29, 2022 · Sep 29, 2022
commit 7945879939f31428b10b38ef0266a1df97ceb093
diff --git a/doc/api/http.md b/doc/api/http.md
@@ -1311,8 +1311,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.
 

diff --git a/lib/_http_server.js b/lib/_http_server.js
@@ -823,7 +823,13 @@ function socketOnError(e) {
   }
 
   if (!this.server.emit('clientError', e, this)) {
-    if (this.writable && this.bytesWritten === 0) {
+    if (this.writable && 
+      // Cautions must be taken to avoid corrupting the remote peer,
+      // reply an error segment if
+        // no in-flight ServerResponse
+        (!this._httpMessage ||
+        // or data of the in-flight one is not yet written to this socket
+        !this._httpMessage._headerSent)) {
       let response;
 
       switch (e.code) {

diff --git a/test/parallel/test-http-server-headers-timeout-keepalive.js b/test/parallel/test-http-server-headers-timeout-keepalive.js
@@ -82,8 +82,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();
   });

diff --git a/test/parallel/test-http-server-headers-timeout-pipelining.js b/test/parallel/test-http-server-headers-timeout-pipelining.js
@@ -57,8 +57,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();
   });

diff --git a/test/parallel/test-http-server-request-timeout-keepalive.js b/test/parallel/test-http-server-request-timeout-keepalive.js
@@ -80,8 +80,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();
   });

diff --git a/test/parallel/test-http-server-request-timeout-pipelining.js b/test/parallel/test-http-server-request-timeout-pipelining.js
@@ -51,8 +51,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();
   });