refactor http parser implementation in http_inspector (envoyproxy#36430)

<!--
!!!ATTENTION!!!

If you are fixing *any* crash or *any* potential security issue, *do
not*
open a pull request in this repo. Please report the issue via emailing
envoy-security@googlegroups.com where the issue will be triaged
appropriately.
Thank you in advance for helping to keep Envoy secure.

!!!ATTENTION!!!

For an explanation of how to fill out the fields, please see the
relevant section
in
[PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/main/PULL_REQUESTS.md)
-->

Commit Message: refactor http parser implementation in http_inspector
Additional Description:
This update replaces the usage of the legacy http_parser in
http_inspector with the `LegacyHttpParserImpl` as the first step in
transitioning to the Balsa parser (part of envoyproxy#36433). The Parser interface
is used with LegacyHttpParserImpl to preserve the current behaviour of
http_parser.

Risk Level: Low (no behavioural change)
Testing: All the http_inspector tests have passed (`bazel test
//test/extensions/filters/listener/http_inspector/...`)

Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

---------

Signed-off-by: G Vamshi Krishna Reddy <vamshikrishna.gunreddy@gmail.com>
Signed-off-by: Gustavo <grnmeira@gmail.com>

source/extensions/filters/listener/http_inspector/BUILD

@@ -16,7 +16,6 @@ envoy_cc_library(
     srcs = ["http_inspector.cc"],
     hdrs = ["http_inspector.h"],
     deps = [
-        "//bazel/external/http_parser",
         "//envoy/event:dispatcher_interface",
         "//envoy/event:timer_interface",
         "//envoy/network:filter_interface",
@@ -25,6 +24,8 @@ envoy_cc_library(
         "//source/common/common:minimal_logger_lib",
         "//source/common/http:headers_lib",
         "//source/common/http:utility_lib",
+        "//source/common/http/http1:legacy_parser_lib",
+        "//source/common/http/http1:parser_interface",
     ],
 )
 

source/extensions/filters/listener/http_inspector/http_inspector.cc

@@ -23,14 +23,12 @@ Config::Config(Stats::Scope& scope)
 
 const absl::string_view Filter::HTTP2_CONNECTION_PREFACE = "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n";
 
-Filter::Filter(const ConfigSharedPtr config) : config_(config) {
-  http_parser_init(&parser_, HTTP_REQUEST);
+Filter::Filter(const ConfigSharedPtr config) : config_(config), no_op_callbacks_() {
+  // Filter for only Request Message types with NoOp Parser callbacks.
+  parser_ = std::make_unique<Http::Http1::LegacyHttpParserImpl>(Http::Http1::MessageType::Request,
+                                                                &no_op_callbacks_);
 }
 
-http_parser_settings Filter::settings_{
-    nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
-};
-
 Network::FilterStatus Filter::onData(Network::ListenerFilterBuffer& buffer) {
   auto raw_slice = buffer.rawSlice();
   const char* buf = static_cast<const char*>(raw_slice.mem_);
@@ -81,35 +79,40 @@ ParseState Filter::parseHttpHeader(absl::string_view data) {
       return ParseState::Error;
     }
 
-    absl::string_view new_data = data.substr(parser_.nread);
+    absl::string_view new_data = data.substr(nread_);
     const size_t pos = new_data.find_first_of("\r\n");
 
     if (pos != absl::string_view::npos) {
       // Include \r or \n
       new_data = new_data.substr(0, pos + 1);
-      ssize_t rc = http_parser_execute(&parser_, &settings_, new_data.data(), new_data.length());
+      ssize_t rc = parser_->execute(new_data.data(), new_data.length());
+      nread_ += rc;
       ENVOY_LOG(trace, "http inspector: http_parser parsed {} chars, error code: {}", rc,
-                static_cast<int>(HTTP_PARSER_ERRNO(&parser_)));
+                parser_->errorMessage());
 
       // Errors in parsing HTTP.
-      if (HTTP_PARSER_ERRNO(&parser_) != HPE_OK && HTTP_PARSER_ERRNO(&parser_) != HPE_PAUSED) {
+      if (parser_->getStatus() != Http::Http1::ParserStatus::Ok &&
+          parser_->getStatus() != Http::Http1::ParserStatus::Paused) {
         return ParseState::Error;
       }
 
-      if (parser_.http_major == 1 && parser_.http_minor == 1) {
+      if (parser_->isHttp11()) {
         protocol_ = Http::Headers::get().ProtocolStrings.Http11String;
       } else {
         // Set other HTTP protocols to HTTP/1.0
         protocol_ = Http::Headers::get().ProtocolStrings.Http10String;
       }
+
       return ParseState::Done;
     } else {
-      ssize_t rc = http_parser_execute(&parser_, &settings_, new_data.data(), new_data.length());
+      ssize_t rc = parser_->execute(new_data.data(), new_data.length());
+      nread_ += rc;
       ENVOY_LOG(trace, "http inspector: http_parser parsed {} chars, error code: {}", rc,
-                static_cast<int>(HTTP_PARSER_ERRNO(&parser_)));
+                parser_->errorMessage());
 
       // Errors in parsing HTTP.
-      if (HTTP_PARSER_ERRNO(&parser_) != HPE_OK && HTTP_PARSER_ERRNO(&parser_) != HPE_PAUSED) {
+      if (parser_->getStatus() != Http::Http1::ParserStatus::Ok &&
+          parser_->getStatus() != Http::Http1::ParserStatus::Paused) {
         return ParseState::Error;
       } else {
         return ParseState::Continue;

source/extensions/filters/listener/http_inspector/http_inspector.h

@@ -1,14 +1,14 @@
 #pragma once
 
-#include <http_parser.h>
-
 #include "envoy/event/file_event.h"
 #include "envoy/event/timer.h"
 #include "envoy/network/filter.h"
 #include "envoy/stats/scope.h"
 #include "envoy/stats/stats_macros.h"
 
 #include "source/common/common/logger.h"
+#include "source/common/http/http1/legacy_parser_impl.h"
+#include "source/common/http/http1/parser.h"
 
 #include "absl/container/flat_hash_set.h"
 
@@ -58,6 +58,41 @@ class Config {
   HttpInspectorStats stats_;
 };
 
+class NoOpParserCallbacks : public Http::Http1::ParserCallbacks {
+public:
+  Http::Http1::CallbackResult onMessageBegin() override {
+    return Http::Http1::CallbackResult::Success;
+  }
+
+  Http::Http1::CallbackResult onUrl(const char* /*data*/, size_t /*length*/) override {
+    return Http::Http1::CallbackResult::Success;
+  }
+
+  Http::Http1::CallbackResult onStatus(const char* /*data*/, size_t /*length*/) override {
+    return Http::Http1::CallbackResult::Success;
+  }
+
+  Http::Http1::CallbackResult onHeaderField(const char* /*data*/, size_t /*length*/) override {
+    return Http::Http1::CallbackResult::Success;
+  }
+
+  Http::Http1::CallbackResult onHeaderValue(const char* /*data*/, size_t /*length*/) override {
+    return Http::Http1::CallbackResult::Success;
+  }
+
+  Http::Http1::CallbackResult onHeadersComplete() override {
+    return Http::Http1::CallbackResult::Success;
+  }
+
+  void bufferBody(const char* /*data*/, size_t /*length*/) override {}
+
+  Http::Http1::CallbackResult onMessageComplete() override {
+    return Http::Http1::CallbackResult::Success;
+  }
+
+  void onChunkHeader(bool /*is_final_chunk*/) override {}
+};
+
 using ConfigSharedPtr = std::shared_ptr<Config>;
 
 /**
@@ -85,8 +120,10 @@ class Filter : public Network::ListenerFilter, Logger::Loggable<Logger::Id::filt
   ConfigSharedPtr config_;
   Network::ListenerFilterCallbacks* cb_{nullptr};
   absl::string_view protocol_;
-  http_parser parser_;
-  static http_parser_settings settings_;
+
+  std::unique_ptr<Http::Http1::Parser> parser_;
+  NoOpParserCallbacks no_op_callbacks_;
+  ssize_t nread_ = 0;
 };
 
 } // namespace HttpInspector

test/extensions/filters/listener/http_inspector/BUILD

@@ -50,6 +50,19 @@ envoy_extension_cc_test(
     ],
 )
 
+envoy_extension_cc_test(
+    name = "http_inspector_parser_callbacks_test",
+    srcs = ["http_inspector_parser_callbacks_test.cc"],
+    extension_names = ["envoy.filters.listener.http_inspector"],
+    rbe_pool = "2core",
+    deps = [
+        "//source/extensions/filters/listener/http_inspector:http_inspector_lib",
+        "//test/mocks/api:api_mocks",
+        "//test/mocks/stats:stats_mocks",
+        "//test/test_common:threadsafe_singleton_injector_lib",
+    ],
+)
+
 envoy_cc_fuzz_test(
     name = "http_inspector_fuzz_test",
     srcs = ["http_inspector_fuzz_test.cc"],

test/extensions/filters/listener/http_inspector/http_inspector_parser_callbacks_test.cc

@@ -0,0 +1,72 @@
+#include "source/extensions/filters/listener/http_inspector/http_inspector.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace ListenerFilters {
+namespace HttpInspector {
+namespace {
+
+/**
+ * These tests are written specifically for the NoOpParserCallbacks class to verify
+ * that it's callback methods work as expected. This is necessary because the
+ * HttpInspector filter only parses the request up to the first line break to determine the HTTP
+ * version. As a result, not all callback methods are triggered in the HttpInspector tests.
+ */
+class NoOpParserCallbacksTest : public ::testing::Test {
+protected:
+  NoOpParserCallbacks callbacks_;
+};
+
+TEST_F(NoOpParserCallbacksTest, OnMessageBeginTest) {
+  EXPECT_EQ(callbacks_.onMessageBegin(), Http::Http1::CallbackResult::Success);
+}
+
+TEST_F(NoOpParserCallbacksTest, OnUrlTest) {
+  const absl::string_view url = "/index.html";
+  EXPECT_EQ(callbacks_.onUrl(url.data(), url.size()), Http::Http1::CallbackResult::Success);
+}
+
+TEST_F(NoOpParserCallbacksTest, OnStatusTest) {
+  const absl::string_view status = "200 OK";
+  EXPECT_EQ(callbacks_.onStatus(status.data(), status.size()),
+            Http::Http1::CallbackResult::Success);
+}
+
+TEST_F(NoOpParserCallbacksTest, OnHeaderFieldTest) {
+  const absl::string_view header_field = "Content-Type";
+  EXPECT_EQ(callbacks_.onHeaderField(header_field.data(), header_field.size()),
+            Http::Http1::CallbackResult::Success);
+}
+
+TEST_F(NoOpParserCallbacksTest, OnHeaderValueTest) {
+  const absl::string_view header_value = "text/html";
+  EXPECT_EQ(callbacks_.onHeaderValue(header_value.data(), header_value.size()),
+            Http::Http1::CallbackResult::Success);
+}
+
+TEST_F(NoOpParserCallbacksTest, OnHeadersCompleteTest) {
+  EXPECT_EQ(callbacks_.onHeadersComplete(), Http::Http1::CallbackResult::Success);
+}
+
+TEST_F(NoOpParserCallbacksTest, BufferBodyTest) {
+  const absl::string_view buffer_body = "buffer_body";
+  callbacks_.bufferBody(buffer_body.data(), buffer_body.size());
+}
+
+TEST_F(NoOpParserCallbacksTest, OnMessageCompleteTest) {
+  EXPECT_EQ(callbacks_.onMessageComplete(), Http::Http1::CallbackResult::Success);
+}
+
+TEST_F(NoOpParserCallbacksTest, OnChunkHeaderTest) {
+  callbacks_.onChunkHeader(true);
+  callbacks_.onChunkHeader(false);
+}
+
+} // namespace
+} // namespace HttpInspector
+} // namespace ListenerFilters
+} // namespace Extensions
+} // namespace Envoy