From 95bd7b2910a1733c170e702ae87e4129a9c5e549 Mon Sep 17 00:00:00 2001
From: Fedor Indutny <fedor@indutny.com>
Date: Thu, 29 Nov 2018 22:01:33 -0500
Subject: [PATCH] http: fix error return in `Finish()`

`http_parser_execute(..., nullptr, 0)` returns either `0` or `1`. The
expectation is that no error must be returned if it is `0`, and if
it is `1` - a `Error` object must be returned back to user.

The introduction of `llhttp` and the refactor that happened during it
accidentally removed the error-returning code. This commit reverts it
back to its original state.

Fix: #24585
---
 src/node_http_parser.cc | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
index 18ac6599d80919..4d683b4db2cfaa 100644
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser.cc
@@ -491,7 +491,10 @@ class Parser : public AsyncWrap, public StreamListener {
     ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder());
 
     CHECK(parser->current_buffer_.IsEmpty());
-    parser->Execute(nullptr, 0);
+    Local<Value> ret = parser->Execute(nullptr, 0);
+
+    if (!ret.IsEmpty())
+      args.GetReturnValue().Set(ret);
   }
 
 
@@ -684,11 +687,28 @@ class Parser : public AsyncWrap, public StreamListener {
     }
 #else  /* !NODE_EXPERIMENTAL_HTTP */
     size_t nread = http_parser_execute(&parser_, &settings, data, len);
-    if (data != nullptr) {
+    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 (nread) {
+        case 0:
+          err = HPE_OK;
+          break;
+        case 1:
+          nread = 0;
+          break;
+        default:
+          UNREACHABLE();
+      }
+
+    // Regular Execute()
+    } else {
       Save();
     }
-
-    err = HTTP_PARSER_ERRNO(&parser_);
 #endif  /* NODE_EXPERIMENTAL_HTTP */
 
     // Unassign the 'buffer_' variable
@@ -738,6 +758,10 @@ 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(nread_obj);
   }