HTTP1: Add DumpState support for HTTP1.

docs/root/version_history/current.rst

@@ -74,6 +74,7 @@ New Features
 * dispatcher: supports a stack of `Envoy::ScopeTrackedObject` instead of a single tracked object. This will allow Envoy to dump more debug information on crash.
 * grpc_json_transcoder: added option :ref:`strict_http_request_validation <envoy_v3_api_field_extensions.filters.http.grpc_json_transcoder.v3.GrpcJsonTranscoder.strict_http_request_validation>` to reject invalid requests early.
 * grpc_json_transcoder: filter can now be configured on per-route/per-vhost level as well. Leaving empty list of services in the filter configuration disables transcoding on the specific route.
+* http: added support for `Envoy::ScopeTrackedObject` for HTTP/1 dispatching. Crashes while inside the dispatching loop should dump debug information.
 * http: added support for :ref:`preconnecting <envoy_v3_api_msg_config.cluster.v3.Cluster.PreconnectPolicy>`. Preconnecting is off by default, but recommended for clusters serving latency-sensitive traffic, especially if using HTTP/1.1.
 * http: change frame flood and abuse checks to the upstream HTTP/2 codec to ON by default. It can be disabled by setting the `envoy.reloadable_features.upstream_http2_flood_checks` runtime key to false.
 * overload: add support for scaling :ref:`transport connection timeouts<envoy_v3_api_enum_value_config.overload.v3.ScaleTimersOverloadActionConfig.TimerType.TRANSPORT_SOCKET_CONNECT>`. This can be used to reduce the TLS handshake timeout in response to overload.

source/common/common/dump_state_utils.h

@@ -10,12 +10,13 @@ namespace Envoy {
 // memory would likely lead to the crash handler itself causing a subsequent OOM.
 
 #define DUMP_MEMBER(member) ", " #member ": " << (member)
+#define DUMP_MEMBER_AS(member, value) ", " #member ": " << (value)
 
 #define DUMP_OPTIONAL_MEMBER(member)                                                               \
   ", " #member ": " << ((member).has_value() ? absl::StrCat((member).value()) : "null")
 
 #define DUMP_NULLABLE_MEMBER(member, value)                                                        \
-  ", " #member ": " << ((member) != nullptr ? value : "null")
+  ", " #member ": " << ((member) != nullptr ? (value) : "null")
 
 // Macro assumes local member variables
 // os (ostream)

source/common/common/utility.cc

@@ -457,6 +457,35 @@ std::string StringUtil::escape(const std::string& source) {
   return ret;
 }
 
+// TODO(kbaichoo): If needed, add support for escaping chars < 32 and >= 127.
+void StringUtil::escapeToOstream(std::ostream& os, absl::string_view view) {
+  for (const char c : view) {
+    switch (c) {
+    case '\r':
+      os << "\\r";
+      break;
+    case '\n':
+      os << "\\n";
+      break;
+    case '\t':
+      os << "\\t";
+      break;
+    case '"':
+      os << "\\\"";
+      break;
+    case '\'':
+      os << "\\\'";
+      break;
+    case '\\':
+      os << "\\\\";
+      break;
+    default:
+      os << c;
+      break;
+    }
+  }
+}
+
 const std::string& getDefaultDateFormat() {
   CONSTRUCT_ON_FIRST_USE(std::string, "%Y-%m-%dT%H:%M:%E3SZ");
 }

source/common/common/utility.h

@@ -400,6 +400,16 @@ class StringUtil {
    */
   static std::string escape(const std::string& source);
 
+  /**
+   * Outputs the string to the provided ostream, while escaping \n, \r, \t, and "
+   * (double quote), ' (single quote), and \ (backslash) escaped.
+   * This may be particularly useful if you cannot allocate memory, and the
+   * ostream being written to is backed by an entity that won't allocate memory.
+   * @param os the ostream to output to.
+   * @param view a string view to output
+   */
+  static void escapeToOstream(std::ostream& os, absl::string_view view);
+
   /**
    * Provide a default value for a string if empty.
    * @param s string.

source/common/http/http1/BUILD

@@ -33,13 +33,15 @@ envoy_cc_library(
         ":codec_stats_lib",
         ":header_formatter_lib",
         "//include/envoy/buffer:buffer_interface",
+        "//include/envoy/common:scope_tracker_interface",
         "//include/envoy/http:codec_interface",
         "//include/envoy/http:header_map_interface",
         "//include/envoy/network:connection_interface",
         "//source/common/buffer:buffer_lib",
         "//source/common/buffer:watermark_buffer_lib",
         "//source/common/common:assert_lib",
         "//source/common/common:cleanup_lib",
+        "//source/common/common:dump_state_utils",
         "//source/common/common:statusor_lib",
         "//source/common/common:utility_lib",
         "//source/common/grpc:common_lib",

source/common/http/http1/codec_impl.cc

@@ -10,7 +10,9 @@
 #include "envoy/network/connection.h"
 
 #include "common/common/cleanup.h"
+#include "common/common/dump_state_utils.h"
 #include "common/common/enum_to_int.h"
+#include "common/common/scope_tracker.h"
 #include "common/common/statusor.h"
 #include "common/common/utility.h"
 #include "common/grpc/common.h"
@@ -67,7 +69,6 @@ HeaderKeyFormatterPtr formatter(const Http::Http1Settings& settings) {
 
   return nullptr;
 }
-
 } // namespace
 
 const std::string StreamEncoderImpl::CRLF = "\r\n";
@@ -569,6 +570,8 @@ Http::Status ClientConnectionImpl::dispatch(Buffer::Instance& data) {
 }
 
 Http::Status ConnectionImpl::innerDispatch(Buffer::Instance& data) {
+  // Add self to the Dispatcher's tracked object stack.
+  ScopeTrackerScopeState scope(this, connection_.dispatcher());
   ENVOY_CONN_LOG(trace, "parsing {} bytes", connection_, data.length());
   // Make sure that dispatching_ is set to false after dispatching, even when
   // http_parser exits early with an error code.
@@ -863,6 +866,73 @@ void ConnectionImpl::onResetStreamBase(StreamResetReason reason) {
   onResetStream(reason);
 }
 
+void ConnectionImpl::dumpState(std::ostream& os, int indent_level) const {
+  const char* spaces = spacesForLevel(indent_level);
+  os << spaces << "Http1::ConnectionImpl " << this << DUMP_MEMBER(dispatching_)
+     << DUMP_MEMBER(dispatching_slice_already_drained_) << DUMP_MEMBER(reset_stream_called_)
+     << DUMP_MEMBER(handling_upgrade_) << DUMP_MEMBER(deferred_end_stream_headers_)
+     << DUMP_MEMBER(strict_1xx_and_204_headers_) << DUMP_MEMBER(processing_trailers_)
+     << DUMP_MEMBER(buffered_body_.length());
+
+  // Dump header parsing state, and any progress on headers.
+  os << DUMP_MEMBER(header_parsing_state_);
+  os << DUMP_MEMBER_AS(current_header_field_, current_header_field_.getStringView());
+  os << DUMP_MEMBER_AS(current_header_value_, current_header_value_.getStringView());
+
+  // Dump Child
+  os << '\n';
+  dumpAdditionalState(os, indent_level);
+
+  // Dump the first slice of the dispatching buffer if not drained escaping
+  // certain characters. We do this last as the slice could be rather large.
+  if (current_dispatching_buffer_ == nullptr || dispatching_slice_already_drained_) {
+    // Buffer is either null or already drained (in the body).
+    // Use the macro for consistent formatting.
+    os << DUMP_NULLABLE_MEMBER(current_dispatching_buffer_, "drained");
+    return;
+  } else {
+    absl::string_view front_slice = [](Buffer::RawSlice slice) {
+      return absl::string_view(static_cast<const char*>(slice.mem_), slice.len_);
+    }(current_dispatching_buffer_->frontSlice());
+
+    // Dump buffer data escaping \r, \n, \t, ", ', and \.
+    // This is not the most performant implementation, but we're crashing and
+    // cannot allocate memory.
+    os << spaces << "current_dispatching_buffer_ front_slice length: " << front_slice.length()
+       << " contents: \"";
+    StringUtil::escapeToOstream(os, front_slice);
+    os << "\"\n";
+  }
+}
+
+void ServerConnectionImpl::dumpAdditionalState(std::ostream& os, int indent_level) const {
+  const char* spaces = spacesForLevel(indent_level);
+  os << DUMP_MEMBER_AS(active_request_.request_url_,
+                       active_request_.has_value() &&
+                               !active_request_.value().request_url_.getStringView().empty()
+                           ? active_request_.value().request_url_.getStringView()
+                           : "null");
+  os << '\n';
+
+  // Dump header map, it may be null if it was moved to the request, and
+  // request_url.
+  if (absl::holds_alternative<RequestHeaderMapPtr>(headers_or_trailers_)) {
+    DUMP_DETAILS(absl::get<RequestHeaderMapPtr>(headers_or_trailers_));
+  } else {
+    DUMP_DETAILS(absl::get<RequestTrailerMapPtr>(headers_or_trailers_));
+  }
+}
+
+void ClientConnectionImpl::dumpAdditionalState(std::ostream& os, int indent_level) const {
+  const char* spaces = spacesForLevel(indent_level);
+  // Dump header map, it may be null if it was moved to the request.
+  if (absl::holds_alternative<ResponseHeaderMapPtr>(headers_or_trailers_)) {
+    DUMP_DETAILS(absl::get<ResponseHeaderMapPtr>(headers_or_trailers_));
+  } else {
+    DUMP_DETAILS(absl::get<ResponseTrailerMapPtr>(headers_or_trailers_));
+  }
+}
+
 ServerConnectionImpl::ServerConnectionImpl(
     Network::Connection& connection, CodecStats& stats, ServerConnectionCallbacks& callbacks,
     const Http1Settings& settings, uint32_t max_request_headers_kb,

source/common/http/http1/codec_impl.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <string>
 
+#include "envoy/common/scope_tracker.h"
 #include "envoy/config/core/v3/protocol.pb.h"
 #include "envoy/http/codec.h"
 #include "envoy/network/connection.h"
@@ -170,7 +171,9 @@ class RequestEncoderImpl : public StreamEncoderImpl, public RequestEncoder {
  * Handles the callbacks of http_parser with its own base routine and then
  * virtual dispatches to its subclasses.
  */
-class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Logger::Id::http> {
+class ConnectionImpl : public virtual Connection,
+                       protected Logger::Loggable<Logger::Id::http>,
+                       public ScopeTrackedObject {
 public:
   /**
    * @return Network::Connection& the backing network connection.
@@ -228,6 +231,9 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
   // errors to propagate them through to dispatch() where we can handle the error.
   Envoy::Http::Status codec_status_;
 
+  // ScopeTrackedObject
+  void dumpState(std::ostream& os, int indent_level) const override;
+
 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,
@@ -292,6 +298,17 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
 
 private:
   enum class HeaderParsingState { Field, Value, Done };
+  friend std::ostream& operator<<(std::ostream& os, HeaderParsingState parsing_state) {
+    switch (parsing_state) {
+    case ConnectionImpl::HeaderParsingState::Field:
+      return os << "Field";
+    case ConnectionImpl::HeaderParsingState::Value:
+      return os << "Value";
+    case ConnectionImpl::HeaderParsingState::Done:
+      return os << "Done";
+    }
+    return os;
+  }
 
   virtual HeaderMap& headersOrTrailers() PURE;
   virtual RequestOrResponseHeaderMap& requestOrResponseHeaders() PURE;
@@ -440,6 +457,11 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
    */
   virtual Status checkHeaderNameForUnderscores() { return okStatus(); }
 
+  /**
+   * Additional state to dump on crash.
+   */
+  virtual void dumpAdditionalState(std::ostream& os, int indent_level) const PURE;
+
   static http_parser_settings settings_;
 
   HeaderParsingState header_parsing_state_{HeaderParsingState::Field};
@@ -532,6 +554,7 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl {
       headers_or_trailers_.emplace<RequestTrailerMapPtr>(RequestTrailerMapImpl::create());
     }
   }
+  void dumpAdditionalState(std::ostream& os, int indent_level) const override;
 
   void releaseOutboundResponse(const Buffer::OwnedBufferFragmentImpl* fragment);
   void maybeAddSentinelBufferFragment(Buffer::Instance& output_buffer) override;
@@ -614,6 +637,7 @@ class ClientConnectionImpl : public ClientConnection, public ConnectionImpl {
       headers_or_trailers_.emplace<ResponseTrailerMapPtr>(ResponseTrailerMapImpl::create());
     }
   }
+  void dumpAdditionalState(std::ostream& os, int indent_level) const override;
 
   absl::optional<PendingResponse> pending_response_;
   // TODO(mattklein123): The following bool tracks whether a pending response is complete before

test/common/common/utility_test.cc

@@ -282,6 +282,43 @@ TEST(StringUtil, escape) {
   EXPECT_EQ(StringUtil::escape("{\"linux\": \"penguin\"}"), "{\\\"linux\\\": \\\"penguin\\\"}");
 }
 
+TEST(StringUtil, escapeToOstream) {
+  {
+    std::array<char, 64> buffer;
+    OutputBufferStream ostream{buffer.data(), buffer.size()};
+    StringUtil::escapeToOstream(ostream, "hello world");
+    EXPECT_EQ(ostream.contents(), "hello world");
+  }
+
+  {
+    std::array<char, 64> buffer;
+    OutputBufferStream ostream{buffer.data(), buffer.size()};
+    StringUtil::escapeToOstream(ostream, "hello\nworld\n");
+    EXPECT_EQ(ostream.contents(), "hello\\nworld\\n");
+  }
+
+  {
+    std::array<char, 64> buffer;
+    OutputBufferStream ostream{buffer.data(), buffer.size()};
+    StringUtil::escapeToOstream(ostream, "\t\nworld\r\n");
+    EXPECT_EQ(ostream.contents(), "\\t\\nworld\\r\\n");
+  }
+
+  {
+    std::array<char, 64> buffer;
+    OutputBufferStream ostream{buffer.data(), buffer.size()};
+    StringUtil::escapeToOstream(ostream, "{'linux': \"penguin\"}");
+    EXPECT_EQ(ostream.contents(), "{\\'linux\\': \\\"penguin\\\"}");
+  }
+
+  {
+    std::array<char, 64> buffer;
+    OutputBufferStream ostream{buffer.data(), buffer.size()};
+    StringUtil::escapeToOstream(ostream, R"(\\)");
+    EXPECT_EQ(ostream.contents(), R"(\\\\)");
+  }
+}
+
 TEST(StringUtil, toUpper) {
   EXPECT_EQ(StringUtil::toUpper(""), "");
   EXPECT_EQ(StringUtil::toUpper("a"), "A");