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>

Author: ywave620

doc/api/http.md

@@ -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.
 

lib/_http_server.js

@@ -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) {

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

@@ -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();
  });

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

@@ -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();
  });

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

@@ -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();
  });

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

@@ -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();
  });