Skip to content

Commit

Permalink
http: fix error check in Execute()
Browse files Browse the repository at this point in the history
Refs: #24738
Fixes: #25858

PR-URL: #25939
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
  • Loading branch information
mscdex authored and BethGriggs committed Mar 11, 2019
1 parent b5d4649 commit 8080a9b
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 3 deletions.
30 changes: 28 additions & 2 deletions src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,28 @@ class Parser : public AsyncWrap {
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();
Expand All @@ -624,7 +645,7 @@ class Parser : public AsyncWrap {
Local<Integer> 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<Value> e = Exception::Error(env()->parse_error_string());
Expand All @@ -635,6 +656,11 @@ class Parser : public AsyncWrap {

return scope.Escape(e);
}

// No return value is needed for `Finish()`
if (data == nullptr) {
return scope.Escape(Local<Value>());
}
return scope.Escape(nparsed_obj);
}

Expand Down
15 changes: 14 additions & 1 deletion test/parallel/test-http-max-header-size.js
Original file line number Diff line number Diff line change
@@ -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');
Expand All @@ -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.mustNotCall()).once('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'ECONNRESET');
server.close();
}));
}));
}

0 comments on commit 8080a9b

Please sign in to comment.