From 38f8cd5ba100a3922d84ae5bc6c3828559bbc693 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 1 Jul 2019 00:34:55 +0200 Subject: [PATCH] http: improve parser error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-By: Daniel Bevenius Reviewed-By: Fedor Indutny Reviewed-By: Rich Trott Reviewed-By: Ruben Bridgewater Reviewed-By: Colin Ihrig Reviewed-By: Trivikram Kamat --- lib/_http_client.js | 2 ++ lib/_http_common.js | 9 +++++- lib/_http_server.js | 4 ++- .../test-http-client-error-rawbytes.js | 32 +++++++++++++++++++ test/parallel/test-http-client-parse-error.js | 2 +- test/parallel/test-http-header-overflow.js | 2 +- .../parallel/test-http-server-client-error.js | 2 +- ...p-server-destroy-socket-on-client-error.js | 2 +- 8 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 test/parallel/test-http-client-error-rawbytes.js diff --git a/lib/_http_client.js b/lib/_http_client.js index 5555db13623553..fcd7ca90aca7ed 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 75995260128862..b3aeb0721ae58e 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 ff69c70c9ca353..c109fa6b9b7ad4 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'); @@ -553,6 +554,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 00000000000000..909fcc796ad915 --- /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 cb4e3ff08434dd..305a7bc170b693 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 442776fab08909..83a2d469103bae 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 ffe78941553367..d8a1633bbb7af4 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 7085401883933b..75f795603d598a 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')