[http1] Refactor H/1 codec with a parser interface

source/common/http/http1/BUILD

@@ -28,10 +28,11 @@ envoy_cc_library(
     name = "codec_lib",
     srcs = ["codec_impl.cc"],
     hdrs = ["codec_impl.h"],
-    external_deps = ["http_parser"],
     deps = [
         ":codec_stats_lib",
         ":header_formatter_lib",
+        ":legacy_parser_lib",
+        ":parser_interface",
         "//include/envoy/buffer:buffer_interface",
         "//include/envoy/common:scope_tracker_interface",
         "//include/envoy/http:codec_interface",
@@ -79,3 +80,23 @@ envoy_cc_library(
         "//source/common/upstream:upstream_lib",
     ],
 )
+
+envoy_cc_library(
+    name = "parser_interface",
+    hdrs = ["parser.h"],
+    deps = [
+        "//source/common/common:statusor_lib",
+        "//source/common/http:status_lib",
+    ],
+)
+
+envoy_cc_library(
+    name = "legacy_parser_lib",
+    srcs = ["legacy_parser_impl.cc"],
+    hdrs = ["legacy_parser_impl.h"],
+    external_deps = ["http_parser"],
+    deps = [
+        ":parser_interface",
+        "//source/common/common:assert_lib",
+    ],
+)

source/common/http/http1/codec_impl.h

@@ -1,7 +1,5 @@
 #pragma once
 
-#include <http_parser.h>
-
 #include <array>
 #include <cstdint>
 #include <list>
@@ -21,6 +19,7 @@
 #include "common/http/header_map_impl.h"
 #include "common/http/http1/codec_stats.h"
 #include "common/http/http1/header_formatter.h"
+#include "common/http/http1/parser.h"
 #include "common/http/status.h"
 
 namespace Envoy {
@@ -175,6 +174,7 @@ class RequestEncoderImpl : public StreamEncoderImpl, public RequestEncoder {
  */
 class ConnectionImpl : public virtual Connection,
                        protected Logger::Loggable<Logger::Id::http>,
+                       public ParserCallbacks,
                        public ScopeTrackedObject {
 public:
   /**
@@ -238,31 +238,18 @@ class ConnectionImpl : public virtual Connection,
 
 protected:
   ConnectionImpl(Network::Connection& connection, CodecStats& stats, const Http1Settings& settings,
-                 http_parser_type type, uint32_t max_headers_kb, const uint32_t max_headers_count,
+                 MessageType type, uint32_t max_headers_kb, const uint32_t max_headers_count,
                  HeaderKeyFormatterPtr&& header_key_formatter);
 
-  // The following define special return values for http_parser callbacks. See:
-  // https://github.com/nodejs/http-parser/blob/5c5b3ac62662736de9e71640a8dc16da45b32503/http_parser.h#L72
-  // These codes do not overlap with standard HTTP Status codes. They are only used for user
-  // callbacks.
-  enum class HttpParserCode {
-    // Callbacks other than on_headers_complete should return a non-zero int to indicate an error
-    // and
-    // halt execution.
-    Error = -1,
-    Success = 0,
-    // Returning '1' from on_headers_complete will tell http_parser that it should not expect a
-    // body.
-    NoBody = 1,
-    // Returning '2' from on_headers_complete will tell http_parser that it should not expect a body
-    // nor any further data on the connection.
-    NoBodyData = 2,
-  };
-  int setAndCheckCallbackStatus(Status&& status);
-  int setAndCheckCallbackStatusOr(Envoy::StatusOr<HttpParserCode>&& statusor);
+  int setAndCheckCallbackStatus(Http::Status&& status) override;
+  int setAndCheckCallbackStatusOr(Envoy::StatusOr<ParserStatus>&& statusor) override;
 
   bool resetStreamCalled() { return reset_stream_called_; }
-  Status onMessageBeginBase();
+
+  Status onMessageBegin() override;
+  virtual Status onMessageBeginBase() PURE;
+
+  StatusOr<ParserStatus> onHeadersCompleteStatus();
 
   /**
    * Get memory used to represent HTTP headers or trailers currently being parsed.
@@ -272,16 +259,16 @@ class ConnectionImpl : public virtual Connection,
   virtual uint32_t getHeadersSize();
 
   /**
-   * Called from onUrl, onHeaderField and onHeaderValue to verify that the headers do not exceed the
-   * configured max header size limit.
+   * Called from onUrl, onHeaderFields and onHeaderValue to verify that the headers do not exceed
+   * the configured max header size limit.
    * @return A codecProtocolError status if headers exceed the size limit.
    */
   Status checkMaxHeadersSize();
 
   Network::Connection& connection_;
   CodecStats& stats_;
   const Http1Settings codec_settings_;
-  http_parser parser_;
+  std::unique_ptr<Parser> parser_;
   Buffer::Instance* current_dispatching_buffer_{};
   Http::Code error_code_{Http::Code::BadRequest};
   const HeaderKeyFormatterPtr header_key_formatter_;
@@ -351,55 +338,19 @@ class ConnectionImpl : public virtual Connection,
    */
   Envoy::StatusOr<size_t> dispatchSlice(const char* slice, size_t len);
 
-  /**
-   * Called by the http_parser when body data is received.
-   * @param data supplies the start address.
-   * @param length supplies the length.
-   */
-  void bufferBody(const char* data, size_t length);
+  void bufferBody(const char* data, size_t length) override;
 
   /**
    * Push the accumulated body through the filter pipeline.
    */
   void dispatchBufferedBody();
 
-  /**
-   * Called when a request/response is beginning. A base routine happens first then a virtual
-   * dispatch is invoked.
-   */
-  virtual Status onMessageBegin() PURE;
-
-  /**
-   * Called when URL data is received.
-   * @param data supplies the start address.
-   * @param length supplies the length.
-   */
-  virtual Status onUrl(const char* data, size_t length) PURE;
-
-  /**
-   * Called when header field data is received.
-   * @param data supplies the start address.
-   * @param length supplies the length.
-   * @return A status representing success.
-   */
-  Status onHeaderField(const char* data, size_t length);
+  Status onHeaderField(const char* data, size_t length) override;
 
-  /**
-   * Called when header value data is received.
-   * @param data supplies the start address.
-   * @param length supplies the length.
-   * @return A status representing success.
-   */
-  Status onHeaderValue(const char* data, size_t length);
+  Status onHeaderValue(const char* data, size_t length) override;
 
-  /**
-   * Called when headers are complete. A base routine happens first then a virtual dispatch is
-   * invoked. Note that this only applies to headers and NOT trailers. End of
-   * trailers are signaled via onMessageCompleteBase().
-   * @return An error status or a HttpParserCode.
-   */
-  Envoy::StatusOr<HttpParserCode> onHeadersCompleteBase();
-  virtual Envoy::StatusOr<HttpParserCode> onHeadersComplete() PURE;
+  Envoy::StatusOr<ParserStatus> onHeadersComplete() override;
+  virtual Envoy::StatusOr<ParserStatus> onHeadersCompleteBase() PURE;
 
   /**
    * Called to see if upgrade transition is allowed.
@@ -418,17 +369,13 @@ class ConnectionImpl : public virtual Connection,
    */
   virtual void onBody(Buffer::Instance& data) PURE;
 
-  /**
-   * Called when the request/response is complete.
-   * @return A status representing success.
-   */
-  Status onMessageCompleteBase();
-  virtual void onMessageComplete() PURE;
+  StatusOr<ParserStatus> onMessageComplete() override;
+  virtual ParserStatus onMessageCompleteBase() PURE;
 
   /**
    * Called when accepting a chunk header.
    */
-  void onChunkHeader(bool is_final_chunk);
+  void onChunkHeader(bool is_final_chunk) override;
 
   /**
    * @see onResetStreamBase().
@@ -464,8 +411,6 @@ class ConnectionImpl : public virtual Connection,
    */
   virtual void dumpAdditionalState(std::ostream& os, int indent_level) const PURE;
 
-  static http_parser_settings settings_;
-
   HeaderParsingState header_parsing_state_{HeaderParsingState::Field};
   // Used to accumulate the HTTP message body during the current dispatch call. The accumulated body
   // is pushed through the filter pipeline either at the end of the current dispatch call, or when
@@ -507,7 +452,7 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl {
   };
   absl::optional<ActiveRequest>& activeRequest() { return active_request_; }
   // ConnectionImpl
-  void onMessageComplete() override;
+  ParserStatus onMessageCompleteBase() override;
   // Add the size of the request_url to the reported header size when processing request headers.
   uint32_t getHeadersSize() override;
 
@@ -525,9 +470,9 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl {
 
   // ConnectionImpl
   void onEncodeComplete() override;
-  Status onMessageBegin() override;
+  Status onMessageBeginBase() override;
   Status onUrl(const char* data, size_t length) override;
-  Envoy::StatusOr<HttpParserCode> onHeadersComplete() override;
+  Envoy::StatusOr<ParserStatus> onHeadersCompleteBase() override;
   // If upgrade behavior is not allowed, the HCM will have sanitized the headers out.
   bool upgradeAllowed() const override { return true; }
   void onBody(Buffer::Instance& data) override;
@@ -608,12 +553,12 @@ class ClientConnectionImpl : public ClientConnection, public ConnectionImpl {
   // ConnectionImpl
   Http::Status dispatch(Buffer::Instance& data) override;
   void onEncodeComplete() override {}
-  Status onMessageBegin() override { return okStatus(); }
+  Status onMessageBeginBase() override { return okStatus(); }
   Status onUrl(const char*, size_t) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
-  Envoy::StatusOr<HttpParserCode> onHeadersComplete() override;
+  Envoy::StatusOr<ParserStatus> onHeadersCompleteBase() override;
   bool upgradeAllowed() const override;
   void onBody(Buffer::Instance& data) override;
-  void onMessageComplete() override;
+  ParserStatus onMessageCompleteBase() override;
   void onResetStream(StreamResetReason reason) override;
   Status sendProtocolError(absl::string_view details) override;
   void onAboveHighWatermark() override;