From 6657553dc81febba9c6509be6d00e0bb823eee39 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Tue, 21 Jul 2020 20:53:07 +0200
Subject: [PATCH 1/5] http: don't write error to socket

The state of the connection is unknown at this point and
writing to it can corrupt client state before it is aware
of an error.
---
 lib/_http_server.js | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/lib/_http_server.js b/lib/_http_server.js
index 88ae7e823f09a6..39df2f3575cc7e 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -604,25 +604,12 @@ function onParserTimeout(server, socket) {
 }
 
 const noop = () => {};
-const badRequestResponse = Buffer.from(
-  `HTTP/1.1 400 ${STATUS_CODES[400]}${CRLF}` +
-  `Connection: close${CRLF}${CRLF}`, 'ascii'
-);
-const requestHeaderFieldsTooLargeResponse = Buffer.from(
-  `HTTP/1.1 431 ${STATUS_CODES[431]}${CRLF}` +
-  `Connection: close${CRLF}${CRLF}`, 'ascii'
-);
 function socketOnError(e) {
   // Ignore further errors
   this.removeListener('error', socketOnError);
   this.on('error', noop);
 
   if (!this.server.emit('clientError', e, this)) {
-    if (this.writable) {
-      const response = e.code === 'HPE_HEADER_OVERFLOW' ?
-        requestHeaderFieldsTooLargeResponse : badRequestResponse;
-      this.write(response);
-    }
     this.destroy(e);
   }
 }

From 40f7def75c5d3f81afd0dc1baf239f710dad492a Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Tue, 21 Jul 2020 21:25:40 +0200
Subject: [PATCH 2/5] fixup: linting

---
 lib/_http_server.js | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/_http_server.js b/lib/_http_server.js
index 39df2f3575cc7e..85f71842271b92 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -71,7 +71,6 @@ const {
   validateInteger,
   validateBoolean
 } = require('internal/validators');
-const Buffer = require('buffer').Buffer;
 const {
   DTRACE_HTTP_SERVER_REQUEST,
   DTRACE_HTTP_SERVER_RESPONSE

From 56a12f335efc587384c5dcc0d9a71fb774e21cf5 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Tue, 21 Jul 2020 21:51:10 +0200
Subject: [PATCH 3/5] fixup: altenrative

---
 lib/_http_server.js | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/lib/_http_server.js b/lib/_http_server.js
index 85f71842271b92..084b67b6b3a6fe 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -71,6 +71,7 @@ const {
   validateInteger,
   validateBoolean
 } = require('internal/validators');
+const Buffer = require('buffer').Buffer;
 const {
   DTRACE_HTTP_SERVER_REQUEST,
   DTRACE_HTTP_SERVER_RESPONSE
@@ -603,12 +604,25 @@ function onParserTimeout(server, socket) {
 }
 
 const noop = () => {};
+const badRequestResponse = Buffer.from(
+  `HTTP/1.1 400 ${STATUS_CODES[400]}${CRLF}` +
+  `Connection: close${CRLF}${CRLF}`, 'ascii'
+);
+const requestHeaderFieldsTooLargeResponse = Buffer.from(
+  `HTTP/1.1 431 ${STATUS_CODES[431]}${CRLF}` +
+  `Connection: close${CRLF}${CRLF}`, 'ascii'
+);
 function socketOnError(e) {
   // Ignore further errors
   this.removeListener('error', socketOnError);
   this.on('error', noop);
 
   if (!this.server.emit('clientError', e, this)) {
+    if (this.writable && this.bytesWritten === 0) {
+      const response = e.code === 'HPE_HEADER_OVERFLOW' ?
+        requestHeaderFieldsTooLargeResponse : badRequestResponse;
+      this.write(response);
+    }
     this.destroy(e);
   }
 }

From 9b702946a52f66ad52c06cddf45573ff2e84762c Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Tue, 21 Jul 2020 22:40:03 +0200
Subject: [PATCH 4/5] fixup: add test

---
 test/parallel/test-http-header-badrequest.js | 31 ++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 test/parallel/test-http-header-badrequest.js

diff --git a/test/parallel/test-http-header-badrequest.js b/test/parallel/test-http-header-badrequest.js
new file mode 100644
index 00000000000000..8294ca9fe208ec
--- /dev/null
+++ b/test/parallel/test-http-header-badrequest.js
@@ -0,0 +1,31 @@
+'use strict';
+const { mustCall } = require('../common');
+const assert = require('assert');
+const { createServer } = require('http');
+const { createConnection } = require('net');
+
+const server = createServer();
+
+server.on('request', mustCall((req, res) => {
+  res.write('asd', () => {
+    res.socket.emit('error', new Error('kaboom'));
+  });
+}));
+
+server.listen(0, mustCall(() => {
+  const c = createConnection(server.address().port);
+  let received = '';
+
+  c.on('connect', mustCall(() => {
+    c.write('GET /blah HTTP/1.1\r\n\r\n');
+  }));
+  c.on('data', mustCall((data) => {
+    received += data.toString();
+  }));
+  c.on('end', mustCall(() => {
+    // Should not include anything else after asd.
+    assert.strictEqual(received.indexOf('asd\r\n'), received.length - 5);
+    c.end();
+  }));
+  c.on('close', mustCall(() => server.close()));
+}));

From 08a3fa9090d12ed92cabead631a7c94bfdde43e3 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Tue, 21 Jul 2020 23:56:33 +0200
Subject: [PATCH 5/5] fixup: update docs

---
 doc/api/http.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/api/http.md b/doc/api/http.md
index f90918590541a7..98bb57b69dd8d3 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -1074,8 +1074,8 @@ 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 it is
-immediately destroyed.
+[`HPE_HEADER_OVERFLOW`][] error. If the socket is not writable or has already
+written data it is immediately destroyed.
 
 `socket` is the [`net.Socket`][] object that the error originated from.