From e9eb79283dd866cf69f46c6849ae631c63a76322 Mon Sep 17 00:00:00 2001
From: XadillaX <i@2333.moe>
Date: Thu, 14 Dec 2017 12:40:32 +0800
Subject: [PATCH 1/6] http: add rawPacket in err of `clientError` event

The `rawPacket` is the current buffer that just parsed. Adding this
buffer to the error object of `clientError` event is to make it possible
that developers can log the broken packet.
---
 doc/api/http.md                                | 5 +++++
 lib/_http_server.js                            | 1 +
 test/parallel/test-http-server-client-error.js | 6 ++++++
 3 files changed, 12 insertions(+)

diff --git a/doc/api/http.md b/doc/api/http.md
index 9f06296e6e2584..aba0b9fc5de083 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -765,6 +765,11 @@ object, so any HTTP response sent, including response headers and payload,
 *must* be written directly to the `socket` object. Care must be taken to
 ensure the response is a properly formatted HTTP response message.
 
+> `err` is an instance of `Error` with two extra columns:
+>
+> + `bytesParsed`: the bytes count of request packet that Node.js may parse correctly;
+> + `rawPacket`: the raw packet of current request.
+
 ### Event: 'close'
 <!-- YAML
 added: v0.1.4
diff --git a/lib/_http_server.js b/lib/_http_server.js
index 7315a266b890f0..cb1c7bad464deb 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -476,6 +476,7 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
   resetSocketTimeout(server, socket, state);
 
   if (ret instanceof Error) {
+    ret.rawPacket = d || parser.getCurrentBuffer();
     debug('parse error', ret);
     socketOnError.call(socket, ret);
   } else if (parser.incoming && parser.incoming.upgrade) {
diff --git a/test/parallel/test-http-server-client-error.js b/test/parallel/test-http-server-client-error.js
index 88c224f8374145..ffe78941553367 100644
--- a/test/parallel/test-http-server-client-error.js
+++ b/test/parallel/test-http-server-client-error.js
@@ -10,6 +10,12 @@ const server = http.createServer(common.mustCall(function(req, res) {
 }));
 
 server.on('clientError', common.mustCall(function(err, socket) {
+  assert.strictEqual(err instanceof Error, true);
+  assert.strictEqual(err.code, 'HPE_INVALID_METHOD');
+  assert.strictEqual(err.bytesParsed, 1);
+  assert.strictEqual(err.message, 'Parse Error');
+  assert.strictEqual(err.rawPacket.toString(), 'Oopsie-doopsie\r\n');
+
   socket.end('HTTP/1.1 400 Bad Request\r\n\r\n');
 
   server.close();

From 5e1369f0584c305e315943a64dc995c57aba2ffc Mon Sep 17 00:00:00 2001
From: XadillaX <i@2333.moe>
Date: Fri, 15 Dec 2017 10:26:48 +0800
Subject: [PATCH 2/6] update after addaleax' review

---
 doc/api/http.md     | 8 ++++----
 lib/_http_server.js | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/doc/api/http.md b/doc/api/http.md
index aba0b9fc5de083..a22e37e7e6bc7e 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -765,10 +765,10 @@ object, so any HTTP response sent, including response headers and payload,
 *must* be written directly to the `socket` object. Care must be taken to
 ensure the response is a properly formatted HTTP response message.
 
-> `err` is an instance of `Error` with two extra columns:
->
-> + `bytesParsed`: the bytes count of request packet that Node.js may parse correctly;
-> + `rawPacket`: the raw packet of current request.
+`err` is an instance of `Error` with two extra columns:
+
++ `bytesParsed`: the bytes count of request packet that Node.js may have parsed correctly;
++ `rawPacket`: the raw packet of current request.
 
 ### Event: 'close'
 <!-- YAML
diff --git a/lib/_http_server.js b/lib/_http_server.js
index cb1c7bad464deb..6cac701dafbffe 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -475,8 +475,11 @@ function socketOnError(e) {
 function onParserExecuteCommon(server, socket, parser, state, ret, d) {
   resetSocketTimeout(server, socket, state);
 
+  if (!d)
+    d = parser.getCurrentBuffer();
+
   if (ret instanceof Error) {
-    ret.rawPacket = d || parser.getCurrentBuffer();
+    ret.rawPacket = d;
     debug('parse error', ret);
     socketOnError.call(socket, ret);
   } else if (parser.incoming && parser.incoming.upgrade) {
@@ -485,9 +488,6 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
     var req = parser.incoming;
     debug('SERVER upgrade or connect', req.method);
 
-    if (!d)
-      d = parser.getCurrentBuffer();
-
     socket.removeListener('data', state.onData);
     socket.removeListener('end', state.onEnd);
     socket.removeListener('close', state.onClose);

From 0e73e2c9fd62f0c2ed19b753e997f6fac3ff59b1 Mon Sep 17 00:00:00 2001
From: XadillaX <i@2333.moe>
Date: Fri, 15 Dec 2017 13:48:29 +0800
Subject: [PATCH 3/6] update after addaleax' review

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

diff --git a/doc/api/http.md b/doc/api/http.md
index a22e37e7e6bc7e..b6afcd581f84f6 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -734,6 +734,11 @@ changes:
     description: The default action of calling `.destroy()` on the `socket`
                  will no longer take place if there are listeners attached
                  for `clientError`.
+  - version: VERSION
+    pr-url: https://github.com/nodejs/node/pull/17672
+    description: The rawPacket is the current buffer that just parsed. Adding
+                 this buffer to the error object of clientError event is to make
+                 it possible that developers can log the broken packet.
 -->
 
 * `exception` {Error}

From 9758fb1fea43ca02ef819a27cf55782f4a2a9b69 Mon Sep 17 00:00:00 2001
From: XadillaX <i@2333.moe>
Date: Mon, 18 Dec 2017 12:29:12 +0800
Subject: [PATCH 4/6] update after bnoordhuis' review

---
 lib/_http_server.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/_http_server.js b/lib/_http_server.js
index 6cac701dafbffe..2840cd65e3b71c 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -475,8 +475,9 @@ function socketOnError(e) {
 function onParserExecuteCommon(server, socket, parser, state, ret, d) {
   resetSocketTimeout(server, socket, state);
 
-  if (!d)
+  if (!d) {
     d = parser.getCurrentBuffer();
+  }
 
   if (ret instanceof Error) {
     ret.rawPacket = d;

From 008e5a8929ad53452f29ef2847adad1315a38704 Mon Sep 17 00:00:00 2001
From: XadillaX <i@2333.moe>
Date: Mon, 18 Dec 2017 13:55:52 +0800
Subject: [PATCH 5/6] update

---
 doc/api/http.md     | 2 +-
 lib/_http_server.js | 9 ++++-----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/doc/api/http.md b/doc/api/http.md
index b6afcd581f84f6..9b09093b4c2b7d 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -734,7 +734,7 @@ changes:
     description: The default action of calling `.destroy()` on the `socket`
                  will no longer take place if there are listeners attached
                  for `clientError`.
-  - version: VERSION
+  - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/17672
     description: The rawPacket is the current buffer that just parsed. Adding
                  this buffer to the error object of clientError event is to make
diff --git a/lib/_http_server.js b/lib/_http_server.js
index 2840cd65e3b71c..cb1c7bad464deb 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -475,12 +475,8 @@ function socketOnError(e) {
 function onParserExecuteCommon(server, socket, parser, state, ret, d) {
   resetSocketTimeout(server, socket, state);
 
-  if (!d) {
-    d = parser.getCurrentBuffer();
-  }
-
   if (ret instanceof Error) {
-    ret.rawPacket = d;
+    ret.rawPacket = d || parser.getCurrentBuffer();
     debug('parse error', ret);
     socketOnError.call(socket, ret);
   } else if (parser.incoming && parser.incoming.upgrade) {
@@ -489,6 +485,9 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
     var req = parser.incoming;
     debug('SERVER upgrade or connect', req.method);
 
+    if (!d)
+      d = parser.getCurrentBuffer();
+
     socket.removeListener('data', state.onData);
     socket.removeListener('end', state.onEnd);
     socket.removeListener('close', state.onClose);

From 7fb6c6220eb05a8dbca50d7c33b8c2a87ae27616 Mon Sep 17 00:00:00 2001
From: XadillaX <i@2333.moe>
Date: Thu, 21 Dec 2017 10:07:15 +0800
Subject: [PATCH 6/6] long line

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

diff --git a/doc/api/http.md b/doc/api/http.md
index 9b09093b4c2b7d..928ed0d972b472 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -772,7 +772,8 @@ ensure the response is a properly formatted HTTP response message.
 
 `err` is an instance of `Error` with two extra columns:
 
-+ `bytesParsed`: the bytes count of request packet that Node.js may have parsed correctly;
++ `bytesParsed`: the bytes count of request packet that Node.js may have parsed
+  correctly;
 + `rawPacket`: the raw packet of current request.
 
 ### Event: 'close'