From 867214b9260a29afd137fc9e876cdf10e8c8333d Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Sat, 24 Nov 2018 11:39:50 -0600 Subject: [PATCH] http: allow decoding/encoding a header only request/response (#4885) This adds a new return value to encodeHeaders that indicates that filter iteration should immediately stop, dropping any pending data/trailers. This results in a header only request/response, which can be used to terminate stream processing early, eg in case of invalid request or response. Signed-off-by: Snow Pettersen Signed-off-by: Fred Douglas --- include/envoy/http/filter.h | 5 +- source/common/http/conn_manager_impl.cc | 71 ++++++-- source/common/http/conn_manager_impl.h | 9 +- test/common/http/conn_manager_impl_test.cc | 119 ++++++++++++++ test/integration/filters/BUILD | 43 +++++ test/integration/filters/common.h | 25 +++ .../filters/headers_only_filter.cc | 42 +++++ .../integration/filters/passthrough_filter.cc | 22 +++ test/integration/http2_integration_test.cc | 6 + test/integration/http_integration.cc | 154 ++++++++++++++++++ test/integration/http_integration.h | 5 + test/integration/integration_test.cc | 14 ++ test/test_common/utility.h | 9 + 13 files changed, 511 insertions(+), 13 deletions(-) create mode 100644 test/integration/filters/common.h create mode 100644 test/integration/filters/headers_only_filter.cc create mode 100644 test/integration/filters/passthrough_filter.cc diff --git a/include/envoy/http/filter.h b/include/envoy/http/filter.h index 3b90655f1c90..7f851e9d9d6f 100644 --- a/include/envoy/http/filter.h +++ b/include/envoy/http/filter.h @@ -30,7 +30,10 @@ enum class FilterHeadersStatus { // Do not iterate to any of the remaining filters in the chain. Returning // FilterDataStatus::Continue from decodeData()/encodeData() or calling // continueDecoding()/continueEncoding() MUST be called if continued filter iteration is desired. - StopIteration + StopIteration, + // Continue iteration to remaining filters, but ignore any subsequent data or trailers. This + // results in creating a header only request/response. + ContinueAndEndStream }; /** diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index b7413b3cc4c9..4f259ea55e29 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -528,6 +528,9 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, ENVOY_STREAM_LOG(debug, "request headers complete (end_stream={}):\n{}", *this, end_stream, *request_headers_); + // We end the decode here only if the request is header only. If we convert the request to a + // header only, the stream will be marked as done once a subsequent decodeData/decodeTrailers is + // called with end_stream=true. maybeEndDecode(end_stream); // Drop new requests when overloaded as soon as we have decoded the headers. @@ -751,12 +754,16 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(ActiveStreamDecoderFilte for (; entry != decoder_filters_.end(); entry++) { ASSERT(!(state_.filter_call_state_ & FilterCallState::DecodeHeaders)); state_.filter_call_state_ |= FilterCallState::DecodeHeaders; - FilterHeadersStatus status = (*entry)->decodeHeaders( - headers, end_stream && continue_data_entry == decoder_filters_.end()); + const auto current_filter_end_stream = + decoding_headers_only_ || (end_stream && continue_data_entry == decoder_filters_.end()); + FilterHeadersStatus status = (*entry)->decodeHeaders(headers, current_filter_end_stream); + + ASSERT(!(status == FilterHeadersStatus::ContinueAndEndStream && current_filter_end_stream)); state_.filter_call_state_ &= ~FilterCallState::DecodeHeaders; ENVOY_STREAM_LOG(trace, "decode headers called: filter={} status={}", *this, static_cast((*entry).get()), static_cast(status)); - if (!(*entry)->commonHandleAfterHeadersCallback(status) && + + if (!(*entry)->commonHandleAfterHeadersCallback(status, decoding_headers_only_) && std::next(entry) != decoder_filters_.end()) { // Stop iteration IFF this is not the last filter. If it is the last filter, continue with // processing since we need to handle the case where a terminal filter wants to buffer, but @@ -795,6 +802,11 @@ void ConnectionManagerImpl::ActiveStream::decodeData(ActiveStreamDecoderFilter* Buffer::Instance& data, bool end_stream) { resetIdleTimer(); + // If we previously decided to decode only the headers, do nothing here. + if (decoding_headers_only_) { + return; + } + // If a response is complete or a reset has been sent, filters do not care about further body // data. Just drop it. if (state_.local_complete_) { @@ -894,6 +906,11 @@ void ConnectionManagerImpl::ActiveStream::decodeTrailers(HeaderMapPtr&& trailers void ConnectionManagerImpl::ActiveStream::decodeTrailers(ActiveStreamDecoderFilter* filter, HeaderMap& trailers) { + // If we previously decided to decode only the headers, do nothing here. + if (decoding_headers_only_) { + return; + } + // See decodeData() above for why we check local_complete_ here. if (state_.local_complete_) { return; @@ -1051,11 +1068,22 @@ void ConnectionManagerImpl::ActiveStream::encodeHeaders(ActiveStreamEncoderFilte ASSERT(!(state_.filter_call_state_ & FilterCallState::EncodeHeaders)); state_.filter_call_state_ |= FilterCallState::EncodeHeaders; FilterHeadersStatus status = (*entry)->handle_->encodeHeaders( - headers, end_stream && continue_data_entry == encoder_filters_.end()); + headers, + encoding_headers_only_ || (end_stream && continue_data_entry == encoder_filters_.end())); state_.filter_call_state_ &= ~FilterCallState::EncodeHeaders; ENVOY_STREAM_LOG(trace, "encode headers called: filter={} status={}", *this, static_cast((*entry).get()), static_cast(status)); - if (!(*entry)->commonHandleAfterHeadersCallback(status)) { + + const auto continue_iteration = + (*entry)->commonHandleAfterHeadersCallback(status, encoding_headers_only_); + + // If we're encoding a headers only response, then mark the local as complete. This ensures + // that we don't attempt to reset the downstream request in doEndStream. + if (encoding_headers_only_) { + state_.local_complete_ = true; + } + + if (!continue_iteration) { return; } @@ -1147,12 +1175,15 @@ void ConnectionManagerImpl::ActiveStream::encodeHeaders(ActiveStreamEncoderFilte chargeStats(headers); ENVOY_STREAM_LOG(debug, "encoding headers via codec (end_stream={}):\n{}", *this, - end_stream && continue_data_entry == encoder_filters_.end(), headers); + encoding_headers_only_ || + (end_stream && continue_data_entry == encoder_filters_.end()), + headers); // Now actually encode via the codec. stream_info_.onFirstDownstreamTxByteSent(); - response_encoder_->encodeHeaders(headers, - end_stream && continue_data_entry == encoder_filters_.end()); + response_encoder_->encodeHeaders( + headers, + encoding_headers_only_ || (end_stream && continue_data_entry == encoder_filters_.end())); if (continue_data_entry != encoder_filters_.end()) { // We use the continueEncoding() code since it will correctly handle not calling @@ -1161,7 +1192,9 @@ void ConnectionManagerImpl::ActiveStream::encodeHeaders(ActiveStreamEncoderFilte (*continue_data_entry)->stopped_ = true; (*continue_data_entry)->continueEncoding(); } else { - maybeEndEncode(end_stream); + // End encoding if this is a header only response, either due to a filter converting it to one + // or due to the upstream returning headers only. + maybeEndEncode(encoding_headers_only_ || end_stream); } } @@ -1200,6 +1233,12 @@ void ConnectionManagerImpl::ActiveStream::addEncodedData(ActiveStreamEncoderFilt void ConnectionManagerImpl::ActiveStream::encodeData(ActiveStreamEncoderFilter* filter, Buffer::Instance& data, bool end_stream) { resetIdleTimer(); + + // If we previously decided to encode only the headers, do nothing here. + if (encoding_headers_only_) { + return; + } + std::list::iterator entry = commonEncodePrefix(filter, end_stream); auto trailers_added_entry = encoder_filters_.end(); @@ -1252,6 +1291,12 @@ void ConnectionManagerImpl::ActiveStream::encodeData(ActiveStreamEncoderFilter* void ConnectionManagerImpl::ActiveStream::encodeTrailers(ActiveStreamEncoderFilter* filter, HeaderMap& trailers) { resetIdleTimer(); + + // If we previously decided to encode only the headers, do nothing here. + if (encoding_headers_only_) { + return; + } + std::list::iterator entry = commonEncodePrefix(filter, true); for (; entry != encoder_filters_.end(); entry++) { ASSERT(!(state_.filter_call_state_ & FilterCallState::EncodeTrailers)); @@ -1418,13 +1463,19 @@ bool ConnectionManagerImpl::ActiveStreamFilterBase::commonHandleAfter100Continue } bool ConnectionManagerImpl::ActiveStreamFilterBase::commonHandleAfterHeadersCallback( - FilterHeadersStatus status) { + FilterHeadersStatus status, bool& headers_only) { ASSERT(!headers_continued_); ASSERT(!stopped_); if (status == FilterHeadersStatus::StopIteration) { stopped_ = true; return false; + } else if (status == FilterHeadersStatus::ContinueAndEndStream) { + // Set headers_only to true so we know to end early if necessary, + // but continue filter iteration so we actually write the headers/run the cleanup code. + headers_only = true; + ENVOY_STREAM_LOG(debug, "converting to headers only", parent_); + return true; } else { ASSERT(status == FilterHeadersStatus::Continue); headers_continued_ = true; diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index 39fc522ac679..dbe0f7168de6 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -97,7 +97,7 @@ class ConnectionManagerImpl : Logger::Loggable, stopped_(false), dual_filter_(dual_filter) {} bool commonHandleAfter100ContinueHeadersCallback(FilterHeadersStatus status); - bool commonHandleAfterHeadersCallback(FilterHeadersStatus status); + bool commonHandleAfterHeadersCallback(FilterHeadersStatus status, bool& headers_only); void commonHandleBufferData(Buffer::Instance& provided_data); bool commonHandleAfterDataCallback(FilterDataStatus status, Buffer::Instance& provided_data, bool& buffer_was_streaming); @@ -417,7 +417,12 @@ class ConnectionManagerImpl : Logger::Loggable, // By default, we will assume there are no 100-Continue headers. If encode100ContinueHeaders // is ever called, this is set to true so commonContinue resumes processing the 100-Continue. bool has_continue_headers_{}; - bool is_head_request_{false}; + bool is_head_request_{}; + // Whether a filter has indicated that the request should be treated as a headers only request. + bool decoding_headers_only_{}; + // Whether a filter has indicated that the response should be treated as a headers only + // response. + bool encoding_headers_only_{}; }; typedef std::unique_ptr ActiveStreamPtr; diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index e7bd2a26276c..fcb493f1f9a2 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -2899,6 +2899,125 @@ TEST_F(HttpConnectionManagerImplTest, FilterHeadReply) { conn_manager_->onData(fake_input, false); } +TEST_F(HttpConnectionManagerImplTest, FilterContinueAndEndStreamHeaders) { + InSequence s; + setup(false, ""); + + EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> void { + StreamDecoder* decoder = &conn_manager_->newStream(response_encoder_); + auto headers = std::make_unique( + std::initializer_list>( + {{":authority", "host"}, {":path", "/"}, {":method", "GET"}})); + decoder->decodeHeaders(std::move(headers), false); + })); + + setupFilterChain(2, 2); + + EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, false)) + .WillOnce(Return(FilterHeadersStatus::ContinueAndEndStream)); + EXPECT_CALL(*decoder_filters_[1], decodeHeaders(_, true)) + .WillOnce(Return(FilterHeadersStatus::Continue)); + + // Kick off the incoming data. + Buffer::OwnedImpl fake_input("1234"); + conn_manager_->onData(fake_input, true); + + EXPECT_CALL(*encoder_filters_[1], encodeHeaders(_, true)) + .WillOnce(Return(FilterHeadersStatus::ContinueAndEndStream)); + EXPECT_CALL(*encoder_filters_[0], encodeHeaders(_, true)) + .WillOnce(Return(FilterHeadersStatus::Continue)); + EXPECT_CALL(response_encoder_, encodeHeaders(_, true)); + + expectOnDestroy(); + + decoder_filters_[1]->callbacks_->encodeHeaders(makeHeaderMap({{":status", "200"}}), true); + + Buffer::OwnedImpl response_body("response"); + decoder_filters_[1]->callbacks_->encodeData(response_body, true); +} + +TEST_F(HttpConnectionManagerImplTest, FilterContinueAndEndStreamData) { + InSequence s; + setup(false, ""); + + EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> void { + StreamDecoder* decoder = &conn_manager_->newStream(response_encoder_); + auto headers = makeHeaderMap({{":authority", "host"}, {":path", "/"}, {":method", "GET"}}); + decoder->decodeHeaders(std::move(headers), false); + + Buffer::OwnedImpl fake_data("hello"); + decoder->decodeData(fake_data, true); + })); + + setupFilterChain(2, 2); + + EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, false)) + .WillOnce(Return(FilterHeadersStatus::ContinueAndEndStream)); + EXPECT_CALL(*decoder_filters_[1], decodeHeaders(_, true)) + .WillOnce(Return(FilterHeadersStatus::Continue)); + + // Kick off the incoming data. + Buffer::OwnedImpl fake_input("1234"); + conn_manager_->onData(fake_input, false); + + EXPECT_CALL(*encoder_filters_[1], encodeHeaders(_, false)) + .WillOnce(Return(FilterHeadersStatus::ContinueAndEndStream)); + EXPECT_CALL(*encoder_filters_[0], encodeHeaders(_, true)) + .WillOnce(Return(FilterHeadersStatus::Continue)); + EXPECT_CALL(response_encoder_, encodeHeaders(_, true)); + + expectOnDestroy(); + + decoder_filters_[1]->callbacks_->encodeHeaders(makeHeaderMap({{":status", "200"}}), false); + + Buffer::OwnedImpl response_body("response"); + decoder_filters_[1]->callbacks_->encodeData(response_body, true); +} + +TEST_F(HttpConnectionManagerImplTest, FilterContinueAndEndStreamTrailers) { + InSequence s; + setup(false, ""); + + EXPECT_CALL(*codec_, dispatch(_)).WillOnce(Invoke([&](Buffer::Instance&) -> void { + StreamDecoder* decoder = &conn_manager_->newStream(response_encoder_); + auto headers = makeHeaderMap({{":authority", "host"}, {":path", "/"}, {":method", "GET"}}); + decoder->decodeHeaders(std::move(headers), false); + + Buffer::OwnedImpl fake_data("hello"); + decoder->decodeData(fake_data, false); + + auto trailers = makeHeaderMap({{"foo", "bar"}}); + decoder->decodeTrailers(std::move(trailers)); + })); + + setupFilterChain(2, 2); + + EXPECT_CALL(*decoder_filters_[0], decodeHeaders(_, false)) + .WillOnce(Return(FilterHeadersStatus::ContinueAndEndStream)); + EXPECT_CALL(*decoder_filters_[1], decodeHeaders(_, true)) + .WillOnce(Return(FilterHeadersStatus::Continue)); + + // Kick off the incoming data. + Buffer::OwnedImpl fake_input("1234"); + conn_manager_->onData(fake_input, false); + + EXPECT_CALL(*encoder_filters_[1], encodeHeaders(_, false)) + .WillOnce(Return(FilterHeadersStatus::ContinueAndEndStream)); + EXPECT_CALL(*encoder_filters_[0], encodeHeaders(_, true)) + .WillOnce(Return(FilterHeadersStatus::Continue)); + EXPECT_CALL(response_encoder_, encodeHeaders(_, true)); + + expectOnDestroy(); + + decoder_filters_[1]->callbacks_->encodeHeaders(makeHeaderMap({{":status", "200"}}), false); + + Buffer::OwnedImpl response_body("response"); + decoder_filters_[1]->callbacks_->encodeData(response_body, false); + + auto response_trailers = makeHeaderMap({{"x-trailer", "1"}}); + decoder_filters_[1]->callbacks_->encodeTrailers(std::move(response_trailers)); +} + TEST_F(HttpConnectionManagerImplTest, FilterAddBodyContinuation) { InSequence s; setup(false, ""); diff --git a/test/integration/filters/BUILD b/test/integration/filters/BUILD index b24dee0c0db4..1d21c996fdf2 100644 --- a/test/integration/filters/BUILD +++ b/test/integration/filters/BUILD @@ -22,6 +22,36 @@ envoy_cc_test_library( ], ) +envoy_cc_test_library( + name = "passthrough_filter_config_lib", + srcs = [ + "passthrough_filter.cc", + ], + deps = [ + ":common_lib", + "//include/envoy/http:filter_interface", + "//include/envoy/registry", + "//include/envoy/server:filter_config_interface", + "//source/extensions/filters/http/common:empty_http_filter_config_lib", + "//source/extensions/filters/http/common:pass_through_filter_lib", + ], +) + +envoy_cc_test_library( + name = "headers_only_filter_config_lib", + srcs = [ + "headers_only_filter.cc", + ], + deps = [ + ":common_lib", + "//include/envoy/http:filter_interface", + "//include/envoy/registry", + "//include/envoy/server:filter_config_interface", + "//source/extensions/filters/http/common:empty_http_filter_config_lib", + "//source/extensions/filters/http/common:pass_through_filter_lib", + ], +) + envoy_cc_test_library( name = "pause_filter_lib", srcs = [ @@ -36,6 +66,19 @@ envoy_cc_test_library( ], ) +envoy_cc_test_library( + name = "common_lib", + hdrs = [ + "common.h", + ], + deps = [ + "//include/envoy/http:filter_interface", + "//include/envoy/registry", + "//source/extensions/filters/http/common:empty_http_filter_config_lib", + "//test/test_common:utility_lib", + ], +) + envoy_cc_test_library( name = "random_pause_filter_lib", srcs = [ diff --git a/test/integration/filters/common.h b/test/integration/filters/common.h new file mode 100644 index 000000000000..524fbce49272 --- /dev/null +++ b/test/integration/filters/common.h @@ -0,0 +1,25 @@ +#pragma once + +#include + +#include "envoy/http/filter.h" +#include "envoy/server/filter_config.h" + +#include "extensions/filters/http/common/empty_http_filter_config.h" + +namespace Envoy { + +// DRYs up the creation of a simple filter config for a filter that requires no config. +template +class SimpleFilterConfig : public Extensions::HttpFilters::Common::EmptyHttpFilterConfig { +public: + SimpleFilterConfig() : EmptyHttpFilterConfig(T::name) {} + + Http::FilterFactoryCb createFilter(const std::string&, Server::Configuration::FactoryContext&) { + return [](Http::FilterChainFactoryCallbacks& callbacks) -> void { + callbacks.addStreamFilter(std::make_shared()); + }; + } +}; + +} // namespace Envoy diff --git a/test/integration/filters/headers_only_filter.cc b/test/integration/filters/headers_only_filter.cc new file mode 100644 index 000000000000..0006dd2fb338 --- /dev/null +++ b/test/integration/filters/headers_only_filter.cc @@ -0,0 +1,42 @@ +#include + +#include "envoy/http/filter.h" +#include "envoy/registry/registry.h" +#include "envoy/server/filter_config.h" + +#include "extensions/filters/http/common/empty_http_filter_config.h" +#include "extensions/filters/http/common/pass_through_filter.h" + +#include "test/integration/filters/common.h" + +namespace Envoy { + +class HeaderOnlyDecoderFilter : public Http::PassThroughFilter { +public: + constexpr static char name[] = "decode-headers-only"; + + Http::FilterHeadersStatus decodeHeaders(Http::HeaderMap&, bool) override { + return Http::FilterHeadersStatus::ContinueAndEndStream; + } +}; + +constexpr char HeaderOnlyDecoderFilter::name[]; +static Registry::RegisterFactory, + Server::Configuration::NamedHttpFilterConfigFactory> + decoder_register_; + +class HeaderOnlyEncoderFilter : public Http::PassThroughFilter { +public: + constexpr static char name[] = "encode-headers-only"; + + Http::FilterHeadersStatus encodeHeaders(Http::HeaderMap&, bool) override { + return Http::FilterHeadersStatus::ContinueAndEndStream; + } +}; + +constexpr char HeaderOnlyEncoderFilter::name[]; + +static Registry::RegisterFactory, + Server::Configuration::NamedHttpFilterConfigFactory> + encoder_register_; +} // namespace Envoy diff --git a/test/integration/filters/passthrough_filter.cc b/test/integration/filters/passthrough_filter.cc new file mode 100644 index 000000000000..bd89e806d651 --- /dev/null +++ b/test/integration/filters/passthrough_filter.cc @@ -0,0 +1,22 @@ +#include "envoy/registry/registry.h" +#include "envoy/server/filter_config.h" + +#include "extensions/filters/http/common/empty_http_filter_config.h" +#include "extensions/filters/http/common/pass_through_filter.h" + +#include "test/integration/filters/common.h" + +namespace Envoy { + +// Registers the passthrough filter for use in test. +class TestPassThroughFilter : public Http::PassThroughFilter { +public: + constexpr static char name[] = "passthrough-filter"; +}; + +constexpr char TestPassThroughFilter::name[]; +static Registry::RegisterFactory, + Server::Configuration::NamedHttpFilterConfigFactory> + register_; + +} // namespace Envoy diff --git a/test/integration/http2_integration_test.cc b/test/integration/http2_integration_test.cc index 3c4dd561b407..191ea86e4ed7 100644 --- a/test/integration/http2_integration_test.cc +++ b/test/integration/http2_integration_test.cc @@ -132,6 +132,12 @@ TEST_P(Http2IntegrationTest, MaxHeadersInCodec) { codec_client_->close(); } +TEST_P(Http2IntegrationTest, EncodingHeaderOnlyResponse) { testHeadersOnlyFilterEncoding(); } + +TEST_P(Http2IntegrationTest, DecodingHeaderOnlyResponse) { testHeadersOnlyFilterDecoding(); } + +TEST_P(Http2IntegrationTest, DecodingHeaderOnlyInterleaved) { testHeadersOnlyFilterInterleaved(); } + TEST_P(Http2IntegrationTest, DownstreamResetBeforeResponseComplete) { testDownstreamResetBeforeResponseComplete(); } diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 1d686a24f79f..4067d0178c65 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -1715,6 +1715,160 @@ void HttpIntegrationTest::testUpstreamProtocolError() { EXPECT_STREQ("503", response->headers().Status()->value().c_str()); } +void HttpIntegrationTest::testHeadersOnlyFilterEncoding() { + config_helper_.addFilter(R"EOF( +name: encode-headers-only +)EOF"); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + auto response = + codec_client_->makeRequestWithBody(Http::TestHeaderMapImpl{{":method", "GET"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}}, + 128); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "503"}}, false); + response->waitForEndStream(); + EXPECT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); + if (upstreamProtocol() == FakeHttpConnection::Type::HTTP1) { + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + } else { + ASSERT_TRUE(upstream_request_->waitForReset()); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + } + + EXPECT_TRUE(response->complete()); + EXPECT_STREQ("503", response->headers().Status()->value().c_str()); + EXPECT_EQ(0, response->body().size()); +} + +void HttpIntegrationTest::testHeadersOnlyFilterDecoding() { + config_helper_.addFilter(R"EOF( +name: decode-headers-only +)EOF"); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + auto response = + codec_client_->makeRequestWithBody(Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}}, + 128); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "503"}}, false); + upstream_request_->encodeData(128, true); + response->waitForEndStream(); + + EXPECT_TRUE(response->complete()); + EXPECT_STREQ("503", response->headers().Status()->value().c_str()); + EXPECT_EQ(128, response->body().size()); +} + +void HttpIntegrationTest::testHeadersOnlyFilterEncodingIntermediateFilters() { + config_helper_.addFilter(R"EOF( +name: passthrough-filter +)EOF"); + config_helper_.addFilter(R"EOF( +name: encode-headers-only +)EOF"); + config_helper_.addFilter(R"EOF( +name: passthrough-filter +)EOF"); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + auto response = + codec_client_->makeRequestWithBody(Http::TestHeaderMapImpl{{":method", "GET"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}}, + 128); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "503"}}, false); + response->waitForEndStream(); + if (upstreamProtocol() == FakeHttpConnection::Type::HTTP1) { + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + } else { + ASSERT_TRUE(upstream_request_->waitForReset()); + ASSERT_TRUE(fake_upstream_connection_->close()); + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + } + + EXPECT_TRUE(response->complete()); + EXPECT_STREQ("503", response->headers().Status()->value().c_str()); + EXPECT_EQ(0, response->body().size()); +} + +void HttpIntegrationTest::testHeadersOnlyFilterDecodingIntermediateFilters() { + config_helper_.addFilter(R"EOF( +name: passthrough-filter +)EOF"); + config_helper_.addFilter(R"EOF( +name: decode-headers-only +)EOF"); + config_helper_.addFilter(R"EOF( +name: passthrough-filter +)EOF"); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + auto response = + codec_client_->makeRequestWithBody(Http::TestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}}, + 128); + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "503"}}, false); + upstream_request_->encodeData(128, true); + response->waitForEndStream(); + + EXPECT_TRUE(response->complete()); + EXPECT_STREQ("503", response->headers().Status()->value().c_str()); + EXPECT_EQ(128, response->body().size()); +} + +// Verifies behavior when request data is encoded after the request has been +// turned into a headers-only request and the response has already begun. +void HttpIntegrationTest::testHeadersOnlyFilterInterleaved() { + config_helper_.addFilter(R"EOF( +name: decode-headers-only +)EOF"); + initialize(); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + // First send the request headers. The filter should turn this into a header-only + // request. + auto encoder_decoder = + codec_client_->startRequest(Http::TestHeaderMapImpl{{":method", "GET"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", "host"}}); + request_encoder_ = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + + // Wait for the upstream request and begin sending a response with end_stream = false. + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "503"}}, false); + + // Simulate additional data after the request has been turned into a headers only request. + Buffer::OwnedImpl data(std::string(128, 'a')); + request_encoder_->encodeData(data, false); + + // End the response. + upstream_request_->encodeData(128, true); + + response->waitForEndStream(); + EXPECT_TRUE(response->complete()); + EXPECT_STREQ("503", response->headers().Status()->value().c_str()); + EXPECT_EQ(0, upstream_request_->body().length()); +} + void HttpIntegrationTest::testDownstreamResetBeforeResponseComplete() { initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); diff --git a/test/integration/http_integration.h b/test/integration/http_integration.h index 10308c2c6de2..da740cd4c45c 100644 --- a/test/integration/http_integration.h +++ b/test/integration/http_integration.h @@ -140,6 +140,11 @@ class HttpIntegrationTest : public BaseIntegrationTest { void testIdleTimeoutWithTwoRequests(); void testIdleTimerDisabled(); void testUpstreamDisconnectWithTwoRequests(); + void testHeadersOnlyFilterEncoding(); + void testHeadersOnlyFilterDecoding(); + void testHeadersOnlyFilterEncodingIntermediateFilters(); + void testHeadersOnlyFilterDecodingIntermediateFilters(); + void testHeadersOnlyFilterInterleaved(); // HTTP/1 tests void testBadFirstline(); void testMissingDelimiter(); diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index fe0103724c72..e69daba653fa 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -139,6 +139,20 @@ TEST_P(IntegrationTest, UpstreamDisconnectWithTwoRequests) { testUpstreamDisconnectWithTwoRequests(); } +TEST_P(IntegrationTest, EncodingHeaderOnlyResponse) { testHeadersOnlyFilterEncoding(); } + +TEST_P(IntegrationTest, DecodingHeaderOnlyResponse) { testHeadersOnlyFilterDecoding(); } + +TEST_P(IntegrationTest, EncodingHeaderOnlyResponseIntermediateFilters) { + testHeadersOnlyFilterEncodingIntermediateFilters(); +} + +TEST_P(IntegrationTest, DecodingHeaderOnlyResponseIntermediateFilters) { + testHeadersOnlyFilterDecodingIntermediateFilters(); +} + +TEST_P(IntegrationTest, DecodingHeaderOnlyInterleaved) { testHeadersOnlyFilterInterleaved(); } + TEST_P(IntegrationTest, RetryHittingBufferLimit) { testRetryHittingBufferLimit(); } TEST_P(IntegrationTest, HittingDecoderFilterLimit) { testHittingDecoderFilterLimit(); } diff --git a/test/test_common/utility.h b/test/test_common/utility.h index a8e4cfb0d5c2..bba25fdcd82b 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -398,6 +398,15 @@ class TestHeaderMapImpl : public HeaderMapImpl { bool has(const LowerCaseString& key); }; +// Helper method to create a header map from an initializer list. Useful due to make_unique's +// inability to infer the initializer list type. +inline HeaderMapPtr +makeHeaderMap(const std::initializer_list>& values) { + return std::make_unique>&>( + values); +} + } // namespace Http namespace Stats {