diff --git a/lib/_http_common.js b/lib/_http_common.js index 019b73a1ff225e..3c389ba054decc 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -189,8 +189,8 @@ function freeParser(parser, req, socket) { if (parser) { if (parser._consumed) parser.unconsume(); - cleanParser(parser); parser.remove(); + cleanParser(parser); if (parsers.free(parser) === false) { // Make sure the parser's stack has unwound before deleting the // corresponding C++ object through .close(). diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 8edd9f9523fd6f..50d7f9e6916096 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -628,6 +628,8 @@ class Parser : public AsyncWrap, public StreamListener { Parser* parser; ASSIGN_OR_RETURN_UNWRAP(&parser, args.This()); + parser->is_being_freed_ = true; + if (parser->connectionsList_ != nullptr) { parser->connectionsList_->Pop(parser); parser->connectionsList_->PopActive(parser); @@ -1012,6 +1014,7 @@ class Parser : public AsyncWrap, public StreamListener { num_values_ = 0; have_flushed_ = false; got_exception_ = false; + is_being_freed_ = false; headers_completed_ = false; max_http_header_size_ = max_http_header_size; } @@ -1056,6 +1059,7 @@ class Parser : public AsyncWrap, public StreamListener { size_t num_values_; bool have_flushed_; bool got_exception_; + bool is_being_freed_ = false; size_t current_buffer_len_; const char* current_buffer_data_; bool headers_completed_ = false; @@ -1075,6 +1079,9 @@ class Parser : public AsyncWrap, public StreamListener { struct Proxy { static int Raw(llhttp_t* p, Args ... args) { Parser* parser = ContainerOf(&Parser::parser_, p); + if (parser->is_being_freed_) { + return 0; + } int rv = (parser->*Member)(std::forward(args)...); if (rv == 0) { rv = parser->MaybePause(); diff --git a/test/parallel/test-http-parser-freed-during-execute.js b/test/parallel/test-http-parser-freed-during-execute.js new file mode 100644 index 00000000000000..c7605b75d95aa5 --- /dev/null +++ b/test/parallel/test-http-parser-freed-during-execute.js @@ -0,0 +1,23 @@ +'use strict'; + +const common = require('../common'); +const { createServer } = require('http'); +const { connect } = require('net'); + +// Regression test: ensure llhttp_execute() is aborted when freeParser() is +// called synchronously during parsing of pipelined requests. +const server = createServer(common.mustCall((req, res) => { + req.socket.emit('close'); + res.end(); +}, 1)); + +server.unref(); + +server.listen(0, common.mustCall(() => { + // Two pipelined requests in one write, processed by a single llhttp_execute(). + const client = connect(server.address().port); + client.end( + 'GET / HTTP/1.1\r\nHost: localhost\r\nConnection: keep-alive\r\n\r\n' + + 'GET / HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n', + ); +}));