From b9204dbc0eedcb3c21b5e80a9dac63301de2c729 Mon Sep 17 00:00:00 2001 From: Brian White Date: Thu, 31 Jan 2019 20:44:33 -0500 Subject: [PATCH] http: fix error check in `Execute()` Refs: https://github.com/nodejs/node/pull/24738 Fixes: https://github.com/nodejs/node/issues/25858 PR-URL: https://github.com/nodejs/node/pull/25863 Reviewed-By: Fedor Indutny Reviewed-By: Matteo Collina --- src/node_http_parser.cc | 30 ++++++++++++++++++++-- test/parallel/test-http-max-header-size.js | 15 ++++++++++- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 4411b6b32f8e2b..b29cdf4b28b9be 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -605,7 +605,28 @@ class Parser : public AsyncWrap, public StreamListener { size_t nparsed = http_parser_execute(&parser_, &settings, data, len); - Save(); + enum http_errno err = HTTP_PARSER_ERRNO(&parser_); + + // Finish() + if (data == nullptr) { + // `http_parser_execute()` returns either `0` or `1` when `len` is 0 + // (part of the finishing sequence). + CHECK_EQ(len, 0); + switch (nparsed) { + case 0: + err = HPE_OK; + break; + case 1: + nparsed = 0; + break; + default: + UNREACHABLE(); + } + + // Regular Execute() + } else { + Save(); + } // Unassign the 'buffer_' variable current_buffer_.Clear(); @@ -619,7 +640,7 @@ class Parser : public AsyncWrap, public StreamListener { Local nparsed_obj = Integer::New(env()->isolate(), nparsed); // If there was a parse error in one of the callbacks // TODO(bnoordhuis) What if there is an error on EOF? - if (!parser_.upgrade && nparsed != len) { + if (!parser_.upgrade && err != HPE_OK) { enum http_errno err = HTTP_PARSER_ERRNO(&parser_); Local e = Exception::Error(env()->parse_error_string()); @@ -631,6 +652,11 @@ class Parser : public AsyncWrap, public StreamListener { return scope.Escape(e); } + + // No return value is needed for `Finish()` + if (data == nullptr) { + return scope.Escape(Local()); + } return scope.Escape(nparsed_obj); } diff --git a/test/parallel/test-http-max-header-size.js b/test/parallel/test-http-max-header-size.js index 07fbe902963525..b70deff764dc5d 100644 --- a/test/parallel/test-http-max-header-size.js +++ b/test/parallel/test-http-max-header-size.js @@ -1,6 +1,6 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const { spawnSync } = require('child_process'); const http = require('http'); @@ -9,3 +9,16 @@ assert.strictEqual(http.maxHeaderSize, 8 * 1024); const child = spawnSync(process.execPath, ['--max-http-header-size=10', '-p', 'http.maxHeaderSize']); assert.strictEqual(+child.stdout.toString().trim(), 10); + +{ + const server = http.createServer(common.mustNotCall()); + server.listen(0, common.mustCall(() => { + http.get({ + port: server.address().port, + headers: { foo: 'x'.repeat(http.maxHeaderSize + 1) } + }, common.mustCall((res) => { + assert.strictEqual(res.statusCode, 400); + server.close(); + })); + })); +}