diff --git a/lib/_http_common.js b/lib/_http_common.js index 019b73a1ff225e..40a410062b0642 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -189,6 +189,7 @@ function freeParser(parser, req, socket) { if (parser) { if (parser._consumed) parser.unconsume(); + parser.markFreed(); cleanParser(parser); parser.remove(); if (parsers.free(parser) === false) { diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 8edd9f9523fd6f..21d907a7543965 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -623,6 +623,12 @@ class Parser : public AsyncWrap, public StreamListener { parser->EmitDestroy(); } + static void MarkFreed(const FunctionCallbackInfo& args) { + Parser* parser; + ASSIGN_OR_RETURN_UNWRAP(&parser, args.This()); + parser->is_being_freed_ = true; + } + // TODO(@anonrig): Add V8 Fast API static void Remove(const FunctionCallbackInfo& args) { Parser* parser; @@ -1012,6 +1018,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 +1063,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 +1083,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(); @@ -1332,6 +1343,7 @@ void CreatePerIsolateProperties(IsolateData* isolate_data, t->Inherit(AsyncWrap::GetConstructorTemplate(isolate_data)); SetProtoMethod(isolate, t, "close", Parser::Close); SetProtoMethod(isolate, t, "free", Parser::Free); + SetProtoMethod(isolate, t, "markFreed", Parser::MarkFreed); SetProtoMethod(isolate, t, "remove", Parser::Remove); SetProtoMethod(isolate, t, "execute", Parser::Execute); SetProtoMethod(isolate, t, "finish", Parser::Finish); @@ -1401,6 +1413,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(Parser::New); registry->Register(Parser::Close); registry->Register(Parser::Free); + registry->Register(Parser::MarkFreed); registry->Register(Parser::Remove); registry->Register(Parser::Execute); registry->Register(Parser::Finish); 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', + ); +}));