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: #25863
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
mscdex authored and rvagg committed Feb 28, 2019
1 parent 30d04fc commit b9204db
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 @@ -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();
Expand All @@ -619,7 +640,7 @@ class Parser : public AsyncWrap, public StreamListener {
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 @@ -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<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.mustCall((res) => {
assert.strictEqual(res.statusCode, 400);
server.close();
}));
}));
}

0 comments on commit b9204db

Please sign in to comment.