From 1a2c04c6598f20bb0edd3d204f87b598bdff158e Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Thu, 15 Jul 2021 18:14:31 +0000 Subject: [PATCH 1/2] Add local_end_stream_ to crash dump for H1 Signed-off-by: Pradeep Rao --- source/common/http/http1/codec_impl.cc | 5 +++++ test/common/http/http1/codec_impl_test.cc | 8 +++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index d26328488f4d5..6eb839f263bcd 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/source/common/http/http1/codec_impl.cc @@ -904,6 +904,11 @@ void ServerConnectionImpl::dumpAdditionalState(std::ostream& os, int indent_leve !active_request_.value().request_url_.getStringView().empty() ? active_request_.value().request_url_.getStringView() : "null"); + os << DUMP_MEMBER_AS( + active_request_.response_encoder_.local_end_stream_, + active_request_.has_value() + ? absl::StrCat(active_request_.value().response_encoder_.local_end_stream_) + : "null"); os << '\n'; // Dump header map, it may be null if it was moved to the request, and diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index 59f8e1dc0dd1e..2bd8073ba85ce 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -2157,9 +2157,11 @@ TEST_F(Http1ServerConnectionImplTest, ShouldDumpDispatchBufferWithoutAllocatingM EXPECT_TRUE(status.ok()); // Check dump contents - EXPECT_THAT(ostream.contents(), HasSubstr("buffered_body_.length(): 5, header_parsing_state_: " - "Done, current_header_field_: , current_header_value_: " - "\n, active_request_.request_url_: null")); + EXPECT_THAT(ostream.contents(), + HasSubstr("buffered_body_.length(): 5, header_parsing_state_: " + "Done, current_header_field_: , current_header_value_: " + "\n, active_request_.request_url_: null" + ", active_request_.response_encoder_.local_end_stream_: 0")); EXPECT_THAT(ostream.contents(), HasSubstr("current_dispatching_buffer_ front_slice length: 43 contents: \"POST / " "HTTP/1.1\\r\\nContent-Length: 5\\r\\n\\r\\nHello\"\n")); From 2b7861a75c470b1ea309b7a9132d200403966bd4 Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Fri, 16 Jul 2021 18:09:02 +0000 Subject: [PATCH 2/2] Implement dumpState for ServerConnectionImpl::AcitveRequest Signed-off-by: Pradeep Rao --- source/common/http/http1/codec_impl.cc | 19 +++++++++---------- source/common/http/http1/codec_impl.h | 1 + test/common/http/http1/codec_impl_test.cc | 9 ++++----- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index 6eb839f263bcd..cb0af92e83bc9 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/source/common/http/http1/codec_impl.cc @@ -899,16 +899,8 @@ void ConnectionImpl::dumpState(std::ostream& os, int indent_level) const { 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 << DUMP_MEMBER_AS( - active_request_.response_encoder_.local_end_stream_, - active_request_.has_value() - ? absl::StrCat(active_request_.value().response_encoder_.local_end_stream_) - : "null"); + + DUMP_DETAILS(active_request_); os << '\n'; // Dump header map, it may be null if it was moved to the request, and @@ -1233,6 +1225,13 @@ Status ServerConnectionImpl::checkHeaderNameForUnderscores() { return okStatus(); } +void ServerConnectionImpl::ActiveRequest::dumpState(std::ostream& os, int indent_level) const { + (void)indent_level; + os << DUMP_MEMBER_AS( + request_url_, !request_url_.getStringView().empty() ? request_url_.getStringView() : "null"); + os << DUMP_MEMBER(response_encoder_.local_end_stream_); +} + ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, CodecStats& stats, ConnectionCallbacks&, const Http1Settings& settings, const uint32_t max_response_headers_count) diff --git a/source/common/http/http1/codec_impl.h b/source/common/http/http1/codec_impl.h index aa8125b440d26..d67412706a712 100644 --- a/source/common/http/http1/codec_impl.h +++ b/source/common/http/http1/codec_impl.h @@ -440,6 +440,7 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl { : response_encoder_(connection, connection.codec_settings_.stream_error_on_invalid_http_message_) {} + void dumpState(std::ostream& os, int indent_level) const; HeaderString request_url_; RequestDecoder* request_decoder_{}; ResponseEncoderImpl response_encoder_; diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index 2bd8073ba85ce..433acb4473a11 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -2157,11 +2157,10 @@ TEST_F(Http1ServerConnectionImplTest, ShouldDumpDispatchBufferWithoutAllocatingM EXPECT_TRUE(status.ok()); // Check dump contents - EXPECT_THAT(ostream.contents(), - HasSubstr("buffered_body_.length(): 5, header_parsing_state_: " - "Done, current_header_field_: , current_header_value_: " - "\n, active_request_.request_url_: null" - ", active_request_.response_encoder_.local_end_stream_: 0")); + EXPECT_THAT(ostream.contents(), HasSubstr("buffered_body_.length(): 5, header_parsing_state_: " + "Done, current_header_field_: , current_header_value_: " + "\nactive_request_: \n, request_url_: null" + ", response_encoder_.local_end_stream_: 0")); EXPECT_THAT(ostream.contents(), HasSubstr("current_dispatching_buffer_ front_slice length: 43 contents: \"POST / " "HTTP/1.1\\r\\nContent-Length: 5\\r\\n\\r\\nHello\"\n"));