http: improve parser error messages

Include the library-provided reason in the Error’s `message`.

Fixes: https://github.com/nodejs/node/issues/28468

PR-URL: https://github.com/nodejs/node/pull/28487
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

8 files changed, 49 insertions, 6 deletions

diff --git a/lib/_http_client.js b/lib/_http_client.js
index 4af23fa8c5..a1750a1a00 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -33,6 +33,7 @@ const {
   httpSocketSetup,
   parsers,
   HTTPParser,
+  prepareError,
 } = require('_http_common');
 const { OutgoingMessage } = require('_http_outgoing');
 const Agent = require('_http_agent');
@@ -451,6 +452,7 @@ function socketOnData(d) {
 
   const ret = parser.execute(d);
   if (ret instanceof Error) {
+    prepareError(ret, parser, d);
     debug('parse error', ret);
     freeParser(parser, req, socket);
     socket.destroy();
diff --git a/lib/_http_common.js b/lib/_http_common.js
index 7599526012..b3aeb0721a 100644
--- a/lib/_http_common.js
+++ b/lib/_http_common.js
@@ -239,6 +239,12 @@ function cleanParser(parser) {
   parser._consumed = false;
 }
 
+function prepareError(err, parser, rawPacket) {
+  err.rawPacket = rawPacket || parser.getCurrentBuffer();
+  if (typeof err.reason === 'string')
+    err.message = `Parse Error: ${err.reason}`;
+}
+
 module.exports = {
   _checkInvalidHeaderChar: checkInvalidHeaderChar,
   _checkIsHttpToken: checkIsHttpToken,
@@ -251,5 +257,6 @@ module.exports = {
   methods,
   parsers,
   kIncomingMessage,
-  HTTPParser
+  HTTPParser,
+  prepareError,
 };
diff --git a/lib/_http_server.js b/lib/_http_server.js
index f82685e2be..941d571a67 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -35,7 +35,8 @@ const {
   httpSocketSetup,
   kIncomingMessage,
   HTTPParser,
-  _checkInvalidHeaderChar: checkInvalidHeaderChar
+  _checkInvalidHeaderChar: checkInvalidHeaderChar,
+  prepareError,
 } = require('_http_common');
 const { OutgoingMessage } = require('_http_outgoing');
 const { outHeadersKey, ondrain, nowDate } = require('internal/http');
@@ -550,6 +551,7 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
   resetSocketTimeout(server, socket, state);
 
   if (ret instanceof Error) {
+    prepareError(ret, parser, d);
     ret.rawPacket = d || parser.getCurrentBuffer();
     debug('parse error', ret);
     socketOnError.call(socket, ret);
diff --git a/test/parallel/test-http-client-error-rawbytes.js b/test/parallel/test-http-client-error-rawbytes.js
new file mode 100644
index 0000000000..909fcc796a
--- /dev/null
+++ b/test/parallel/test-http-client-error-rawbytes.js
@@ -0,0 +1,32 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+const net = require('net');
+
+const response = Buffer.from('HTTP/1.1 200 OK\r\n' +
+  'Content-Length: 6\r\n' +
+  'Transfer-Encoding: Chunked\r\n' +
+  '\r\n' +
+  '6\r\nfoobar' +
+  '0\r\n');
+
+const server = net.createServer(common.mustCall((conn) => {
+  conn.write(response);
+}));
+
+server.listen(0, common.mustCall(() => {
+  const req = http.get(`http://localhost:${server.address().port}/`);
+  req.end();
+  req.on('error', common.mustCall((err) => {
+    const reason = 'Content-Length can\'t be present with chunked encoding';
+    assert.strictEqual(err.message, `Parse Error: ${reason}`);
+    assert(err.bytesParsed < response.length);
+    assert(err.bytesParsed >= response.indexOf('Transfer-Encoding'));
+    assert.strictEqual(err.code, 'HPE_UNEXPECTED_CONTENT_LENGTH');
+    assert.strictEqual(err.reason, reason);
+    assert.deepStrictEqual(err.rawPacket, response);
+
+    server.close();
+  }));
+}));
diff --git a/test/parallel/test-http-client-parse-error.js b/test/parallel/test-http-client-parse-error.js
index cb4e3ff084..305a7bc170 100644
--- a/test/parallel/test-http-client-parse-error.js
+++ b/test/parallel/test-http-client-parse-error.js
@@ -44,7 +44,7 @@ server.listen(0, common.mustCall(() => {
     }).on('error', common.mustCall((e) => {
       common.expectsError({
         code: 'HPE_INVALID_CONSTANT',
-        message: 'Parse Error'
+        message: 'Parse Error: Expected HTTP/'
       })(e);
       countdown.dec();
     }));
diff --git a/test/parallel/test-http-header-overflow.js b/test/parallel/test-http-header-overflow.js
index 442776fab0..83a2d46910 100644
--- a/test/parallel/test-http-header-overflow.js
+++ b/test/parallel/test-http-header-overflow.js
@@ -19,7 +19,7 @@ const server = createServer();
 server.on('connection', mustCall((socket) => {
   socket.on('error', expectsError({
     type: Error,
-    message: 'Parse Error',
+    message: 'Parse Error: Header overflow',
     code: 'HPE_HEADER_OVERFLOW',
     bytesParsed: maxHeaderSize + PAYLOAD_GET.length,
     rawPacket: Buffer.from(PAYLOAD)
diff --git a/test/parallel/test-http-server-client-error.js b/test/parallel/test-http-server-client-error.js
index ffe7894155..d8a1633bbb 100644
--- a/test/parallel/test-http-server-client-error.js
+++ b/test/parallel/test-http-server-client-error.js
@@ -13,7 +13,7 @@ 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.message, 'Parse Error: Invalid method encountered');
   assert.strictEqual(err.rawPacket.toString(), 'Oopsie-doopsie\r\n');
 
   socket.end('HTTP/1.1 400 Bad Request\r\n\r\n');
diff --git a/test/parallel/test-http-server-destroy-socket-on-client-error.js b/test/parallel/test-http-server-destroy-socket-on-client-error.js
index 7085401883..75f795603d 100644
--- a/test/parallel/test-http-server-destroy-socket-on-client-error.js
+++ b/test/parallel/test-http-server-destroy-socket-on-client-error.js
@@ -14,7 +14,7 @@ const server = createServer();
 server.on('connection', mustCall((socket) => {
   socket.on('error', expectsError({
     type: Error,
-    message: 'Parse Error',
+    message: 'Parse Error: Invalid method encountered',
     code: 'HPE_INVALID_METHOD',
     bytesParsed: 0,
     rawPacket: Buffer.from('FOO /\r\n')