diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 56ea33449ca2c..61a7b9b7158b3 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -16,6 +16,7 @@ Minor Behavior Changes * build: run as non-root inside Docker containers. Existing behaviour can be restored by setting the environment variable `ENVOY_UID` to `0`. `ENVOY_UID` and `ENVOY_GID` can be used to set the envoy user's `uid` and `gid` respectively. * health check: in the health check filter the :ref:`percentage of healthy servers in upstream clusters ` is now interpreted as an integer. * hot restart: added the option :option:`--use-dynamic-base-id` to select an unused base ID at startup and the option :option:`--base-id-path` to write the base id to a file (for reuse with later hot restarts). +* http: changed early error path for HTTP/1.1 so that responses consistently flow through the http connection manager, and the http filter chains. This behavior may be temporarily reverted by setting runtime feature `envoy.reloadable_features.early_errors_via_hcm` to false. * http: fixed several bugs with applying correct connection close behavior across the http connection manager, health checker, and connection pool. This behavior may be temporarily reverted by setting runtime feature `envoy.reloadable_features.fix_connection_close` to false. * http: fixed a bug where the upgrade header was not cleared on responses to non-upgrade requests. Can be reverted temporarily by setting runtime feature `envoy.reloadable_features.fix_upgrade_response` to false. diff --git a/include/envoy/http/BUILD b/include/envoy/http/BUILD index 7ffc41f6372d5..f84ccbc9c60a5 100644 --- a/include/envoy/http/BUILD +++ b/include/envoy/http/BUILD @@ -35,6 +35,7 @@ envoy_cc_library( ":metadata_interface", ":protocol_interface", "//include/envoy/buffer:buffer_interface", + "//include/envoy/grpc:status", "//include/envoy/network:address_interface", "//source/common/http:status_lib", ], diff --git a/include/envoy/http/codec.h b/include/envoy/http/codec.h index bb19ce83bcab5..c8c458dbf9977 100644 --- a/include/envoy/http/codec.h +++ b/include/envoy/http/codec.h @@ -6,6 +6,7 @@ #include "envoy/buffer/buffer.h" #include "envoy/common/pure.h" +#include "envoy/grpc/status.h" #include "envoy/http/header_map.h" #include "envoy/http/metadata_interface.h" #include "envoy/http/protocol.h" @@ -177,6 +178,22 @@ class RequestDecoder : public virtual StreamDecoder { * @param trailers supplies the decoded trailers. */ virtual void decodeTrailers(RequestTrailerMapPtr&& trailers) PURE; + + /** + * Called if the codec needs to send a protocol error. + * @param is_grpc_request indicates if the request is a gRPC request + * @param code supplies the HTTP error code to send. + * @param body supplies an optional body to send with the local reply. + * @param modify_headers supplies a way to edit headers before they are sent downstream. + * @param is_head_request indicates if the request is a HEAD request + * @param grpc_status an optional gRPC status for gRPC requests + * @param details details about the source of the error, for debug purposes + */ + virtual void sendLocalReply(bool is_grpc_request, Code code, absl::string_view body, + const std::function& modify_headers, + bool is_head_request, + const absl::optional grpc_status, + absl::string_view details) PURE; }; /** diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index bf8f9d1530530..ba0d01e6c2bda 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -495,7 +495,7 @@ class ConnectionManagerImpl : Logger::Loggable, const std::function& modify_headers, bool is_head_request, const absl::optional grpc_status, - absl::string_view details); + absl::string_view details) override; void encode100ContinueHeaders(ActiveStreamEncoderFilter* filter, ResponseHeaderMap& headers); // As with most of the encode functions, this runs encodeHeaders on various // filters before calling encodeHeadersInternal which does final header munging and passes the diff --git a/source/common/http/http1/BUILD b/source/common/http/http1/BUILD index 47499a86bb0c1..bb7bdd0e8ba54 100644 --- a/source/common/http/http1/BUILD +++ b/source/common/http/http1/BUILD @@ -32,6 +32,7 @@ envoy_cc_library( "//source/common/common:statusor_lib", "//source/common/common:thread_lib", "//source/common/common:utility_lib", + "//source/common/grpc:common_lib", "//source/common/http:codec_helper_lib", "//source/common/http:codes_lib", "//source/common/http:exception_lib", diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index 7fe0a62590924..61f793b0aa387 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/source/common/http/http1/codec_impl.cc @@ -11,6 +11,7 @@ #include "common/common/enum_to_int.h" #include "common/common/utility.h" +#include "common/grpc/common.h" #include "common/http/exception.h" #include "common/http/header_utility.h" #include "common/http/headers.h" @@ -969,7 +970,7 @@ void ServerConnectionImpl::onResetStream(StreamResetReason reason) { active_request_.reset(); } -void ServerConnectionImpl::sendProtocolError(absl::string_view details) { +void ServerConnectionImpl::sendProtocolErrorOld(absl::string_view details) { if (active_request_.has_value()) { active_request_.value().response_encoder_.setDetails(details); } @@ -987,6 +988,35 @@ void ServerConnectionImpl::sendProtocolError(absl::string_view details) { } } +void ServerConnectionImpl::sendProtocolError(absl::string_view details) { + if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.early_errors_via_hcm")) { + sendProtocolErrorOld(details); + return; + } + // We do this here because we may get a protocol error before we have a logical stream. + if (!active_request_.has_value()) { + onMessageBeginBase(); + } + ASSERT(active_request_.has_value()); + + active_request_.value().response_encoder_.setDetails(details); + if (!active_request_.value().response_encoder_.startedResponse()) { + // Note that the correctness of is_grpc_request and is_head_request is best-effort. + // If headers have not been fully parsed they may not be inferred correctly. + bool is_grpc_request = false; + if (absl::holds_alternative(headers_or_trailers_) && + absl::get(headers_or_trailers_) != nullptr) { + is_grpc_request = + Grpc::Common::isGrpcRequestHeaders(*absl::get(headers_or_trailers_)); + } + const bool is_head_request = parser_.method == HTTP_HEAD; + active_request_->request_decoder_->sendLocalReply(is_grpc_request, error_code_, + CodeUtility::toString(error_code_), nullptr, + is_head_request, absl::nullopt, details); + return; + } +} + void ServerConnectionImpl::onAboveHighWatermark() { if (active_request_.has_value()) { active_request_.value().response_encoder_.runHighWatermarkCallbacks(); diff --git a/source/common/http/http1/codec_impl.h b/source/common/http/http1/codec_impl.h index d96a175722a82..36ef8e596ffdd 100644 --- a/source/common/http/http1/codec_impl.h +++ b/source/common/http/http1/codec_impl.h @@ -246,6 +246,7 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable( @@ -144,9 +141,9 @@ void Http1ServerConnectionImplTest::expect400(Protocol p, bool allow_absolute_ur return decoder; })); + EXPECT_CALL(decoder, sendLocalReply(_, Http::Code::BadRequest, "Bad Request", _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); - EXPECT_EQ("HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", output); EXPECT_EQ(p, codec_->protocol()); if (!details.empty()) { EXPECT_EQ(details, response_encoder->getStream().responseDetails()); @@ -243,6 +240,7 @@ void Http1ServerConnectionImplTest::testTrailersExceedLimit(std::string trailer_ EXPECT_TRUE(status.ok()); buffer = Buffer::OwnedImpl(trailer_string + "\r\n\r\n"); if (enable_trailers) { + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "trailers size exceeds limit"); @@ -270,6 +268,7 @@ void Http1ServerConnectionImplTest::testRequestHeadersExceedLimit(std::string he auto status = codec_->dispatch(buffer); EXPECT_TRUE(status.ok()); buffer = Buffer::OwnedImpl(header_string + "\r\n"); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "headers size exceeds limit"); @@ -329,6 +328,7 @@ TEST_F(Http1ServerConnectionImplTest, IdentityEncodingNoChunked) { EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder)); Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\ntransfer-encoding: identity\r\n\r\n"); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "http/1.1 protocol error: unsupported transfer encoding"); @@ -343,6 +343,7 @@ TEST_F(Http1ServerConnectionImplTest, UnsupportedEncoding) { EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder)); Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\ntransfer-encoding: gzip\r\n\r\n"); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "http/1.1 protocol error: unsupported transfer encoding"); @@ -494,6 +495,7 @@ TEST_F(Http1ServerConnectionImplTest, InvalidChunkHeader) { "6\r\nHello \r\n" "invalid\r\nWorl"); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "http/1.1 protocol error: HPE_INVALID_CHUNK_SIZE"); @@ -510,6 +512,7 @@ TEST_F(Http1ServerConnectionImplTest, IdentityAndChunkedBody) { Buffer::OwnedImpl buffer("POST / HTTP/1.1\r\ntransfer-encoding: " "identity,chunked\r\n\r\nb\r\nHello World\r\n0\r\n\r\n"); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "http/1.1 protocol error: unsupported transfer encoding"); @@ -698,9 +701,6 @@ TEST_F(Http1ServerConnectionImplTest, Http11InvalidRequest) { TEST_F(Http1ServerConnectionImplTest, Http11InvalidTrailerPost) { initialize(); - std::string output; - ON_CALL(connection_, write(_, _)).WillByDefault(AddBufferToString(&output)); - MockRequestDecoder decoder; EXPECT_CALL(callbacks_, newStream(_, _)) .WillOnce(Invoke([&](ResponseEncoder&, bool) -> RequestDecoder& { return decoder; })); @@ -718,9 +718,9 @@ TEST_F(Http1ServerConnectionImplTest, Http11InvalidTrailerPost) { "body\r\n0\r\n" "badtrailer\r\n\r\n"); + EXPECT_CALL(decoder, sendLocalReply(_, Http::Code::BadRequest, "Bad Request", _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); - EXPECT_EQ("HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", output); } TEST_F(Http1ServerConnectionImplTest, Http11AbsolutePathNoSlash) { @@ -789,16 +789,82 @@ TEST_F(Http1ServerConnectionImplTest, SimpleGet) { EXPECT_EQ(0U, buffer.length()); } -TEST_F(Http1ServerConnectionImplTest, BadRequestNoStream) { +TEST_F(Http1ServerConnectionImplTest, BadRequestNoStreamLegacy) { + TestScopedRuntime scoped_runtime; + Runtime::LoaderSingleton::getExisting()->mergeValues( + {{"envoy.reloadable_features.early_errors_via_hcm", "false"}}); initialize(); std::string output; ON_CALL(connection_, write(_, _)).WillByDefault(AddBufferToString(&output)); + MockRequestDecoder decoder; + EXPECT_CALL(callbacks_, newStream(_, _)).Times(0); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)).Times(0); + + Buffer::OwnedImpl buffer("bad"); + auto status = codec_->dispatch(buffer); + EXPECT_TRUE(isCodecProtocolError(status)); +} + +// Test that if the stream is not created at the time an error is detected, it +// is created as part of sending the protocol error. +TEST_F(Http1ServerConnectionImplTest, BadRequestNoStream) { + initialize(); + + MockRequestDecoder decoder; + Http::ResponseEncoder* response_encoder = nullptr; + EXPECT_CALL(callbacks_, newStream(_, _)) + .WillOnce(Invoke([&](ResponseEncoder& encoder, bool) -> RequestDecoder& { + response_encoder = &encoder; + return decoder; + })); + // Check that before any headers are parsed, requests do not look like HEAD or gRPC requests. + EXPECT_CALL(decoder, sendLocalReply(false, _, _, _, false, _, _)); + Buffer::OwnedImpl buffer("bad"); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); - EXPECT_EQ("HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", output); +} + +// Make sure that if the first line is parsed, that sendLocalReply tracks HEAD requests correctly. +TEST_F(Http1ServerConnectionImplTest, BadHeadRequest) { + initialize(); + + MockRequestDecoder decoder; + Http::ResponseEncoder* response_encoder = nullptr; + EXPECT_CALL(callbacks_, newStream(_, _)) + .WillOnce(Invoke([&](ResponseEncoder& encoder, bool) -> RequestDecoder& { + response_encoder = &encoder; + return decoder; + })); + // Make sure sendLocalReply picks up the head request. + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, true, _, _)); + + // Send invalid characters + Buffer::OwnedImpl buffer("HEAD / HTTP/1.1\r\nHOST: h.com\r\r\r\r"); + auto status = codec_->dispatch(buffer); + EXPECT_TRUE(isCodecProtocolError(status)); +} + +// Make sure that if gRPC headers are parsed, they are tracked by sendLocalReply. +TEST_F(Http1ServerConnectionImplTest, BadGrpcRequest) { + initialize(); + + MockRequestDecoder decoder; + Http::ResponseEncoder* response_encoder = nullptr; + EXPECT_CALL(callbacks_, newStream(_, _)) + .WillOnce(Invoke([&](ResponseEncoder& encoder, bool) -> RequestDecoder& { + response_encoder = &encoder; + return decoder; + })); + // Make sure sendLocalReply picks up the head request. + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, true, _, _)); + + // Send invalid characters + Buffer::OwnedImpl buffer("HEAD / HTTP/1.1\r\ncontent-type: application/grpc\r\nHOST: ###\r\r"); + auto status = codec_->dispatch(buffer); + EXPECT_TRUE(isCodecProtocolError(status)); } // This behavior was observed during CVE-2019-18801 and helped to limit the @@ -809,21 +875,15 @@ TEST_F(Http1ServerConnectionImplTest, RejectInvalidMethod) { MockRequestDecoder decoder; EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder)); - std::string output; - ON_CALL(connection_, write(_, _)).WillByDefault(AddBufferToString(&output)); - Buffer::OwnedImpl buffer("BAD / HTTP/1.1\r\nHost: foo\r\n"); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); - EXPECT_EQ("HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", output); } TEST_F(Http1ServerConnectionImplTest, BadRequestStartedStream) { initialize(); - std::string output; - ON_CALL(connection_, write(_, _)).WillByDefault(AddBufferToString(&output)); - MockRequestDecoder decoder; EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder)); @@ -832,9 +892,9 @@ TEST_F(Http1ServerConnectionImplTest, BadRequestStartedStream) { EXPECT_TRUE(status.ok()); Buffer::OwnedImpl buffer2("g"); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); - EXPECT_EQ("HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", output); } TEST_F(Http1ServerConnectionImplTest, FloodProtection) { @@ -982,6 +1042,7 @@ TEST_F(Http1ServerConnectionImplTest, HeaderInvalidCharsRejection) { })); Buffer::OwnedImpl buffer( absl::StrCat("GET / HTTP/1.1\r\nHOST: h.com\r\nfoo: ", std::string(1, 3), "\r\n")); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "http/1.1 protocol error: header value contains invalid chars"); @@ -1050,6 +1111,7 @@ TEST_F(Http1ServerConnectionImplTest, HeaderNameWithUnderscoreCauseRequestReject })); Buffer::OwnedImpl buffer(absl::StrCat("GET / HTTP/1.1\r\nHOST: h.com\r\nfoo_bar: bar\r\n\r\n")); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "http/1.1 protocol error: header name contains underscores"); @@ -1070,6 +1132,7 @@ TEST_F(Http1ServerConnectionImplTest, HeaderInvalidAuthority) { return decoder; })); Buffer::OwnedImpl buffer(absl::StrCat("GET / HTTP/1.1\r\nHOST: h.\"com\r\n\r\n")); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), @@ -1092,6 +1155,7 @@ TEST_F(Http1ServerConnectionImplTest, HeaderEmbeddedNulRejection) { Buffer::OwnedImpl buffer( absl::StrCat("GET / HTTP/1.1\r\nHOST: h.com\r\nfoo: bar", std::string(1, '\0'), "baz\r\n")); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "http/1.1 protocol error: HPE_INVALID_HEADER_TOKEN"); @@ -1112,6 +1176,7 @@ TEST_F(Http1ServerConnectionImplTest, HeaderMutateEmbeddedNul) { Buffer::OwnedImpl buffer( absl::StrCat(example_input.substr(0, n), std::string(1, '\0'), example_input.substr(n))); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_FALSE(status.ok()); EXPECT_TRUE(isCodecProtocolError(status)); @@ -1716,6 +1781,7 @@ TEST_F(Http1ServerConnectionImplTest, ConnectRequestAbsoluteURLNotallowed) { EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder)); Buffer::OwnedImpl buffer("CONNECT http://host:80 HTTP/1.1\r\n\r\n"); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); } @@ -1744,6 +1810,7 @@ TEST_F(Http1ServerConnectionImplTest, ConnectRequestWithTEChunked) { // Per https://tools.ietf.org/html/rfc7231#section-4.3.6 CONNECT with body has no defined // semantics: Envoy will reject chunked CONNECT requests. + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); Buffer::OwnedImpl buffer( "CONNECT host:80 HTTP/1.1\r\ntransfer-encoding: chunked\r\n\r\n12345abcd"); auto status = codec_->dispatch(buffer); @@ -1761,6 +1828,7 @@ TEST_F(Http1ServerConnectionImplTest, ConnectRequestWithNonZeroContentLength) { // Make sure we avoid the deferred_end_stream_headers_ optimization for // requests-with-no-body. Buffer::OwnedImpl buffer("CONNECT host:80 HTTP/1.1\r\ncontent-length: 1\r\n\r\nabcd"); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); auto status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "http/1.1 protocol error: unsupported content length"); @@ -2508,6 +2576,7 @@ TEST_F(Http1ServerConnectionImplTest, LargeRequestHeadersSplitRejected) { } // the 60th 1kb header should induce overflow buffer = Buffer::OwnedImpl(fmt::format("big: {}\r\n", long_string)); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "headers size exceeds limit"); @@ -2537,6 +2606,7 @@ TEST_F(Http1ServerConnectionImplTest, ManyRequestHeadersSplitRejected) { // The final 101th header should induce overflow. buffer = Buffer::OwnedImpl("header101:\r\n\r\n"); + EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _, _, _)); status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "headers size exceeds limit"); @@ -2565,6 +2635,7 @@ TEST_F(Http1ServerConnectionImplTest, ManyRequestHeadersAccepted) { TEST_F(Http1ClientConnectionImplTest, LargeResponseHeadersRejected) { initialize(); + NiceMock decoder; NiceMock response_decoder; Http::RequestEncoder& request_encoder = codec_->newStream(response_decoder); TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/"}, {":authority", "host"}}; @@ -2661,6 +2732,7 @@ TEST_F(Http1ClientConnectionImplTest, ManyResponseHeadersRejected) { Buffer::OwnedImpl buffer("HTTP/1.1 200 OK\r\nContent-Length: 0\r\n"); auto status = codec_->dispatch(buffer); buffer = Buffer::OwnedImpl(createHeaderFragment(101) + "\r\n"); + status = codec_->dispatch(buffer); EXPECT_TRUE(isCodecProtocolError(status)); EXPECT_EQ(status.message(), "headers size exceeds limit"); diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index d31866c894213..d87549663e61c 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -81,6 +81,23 @@ class FakeStream : public Http::RequestDecoder, Http::Http1StreamEncoderOptionsOptRef http1StreamEncoderOptions() { return encoder_.http1StreamEncoderOptions(); } + void + sendLocalReply(bool is_grpc_request, Http::Code code, absl::string_view body, + const std::function& /*modify_headers*/, + bool is_head_request, const absl::optional grpc_status, + absl::string_view /*details*/) override { + Http::Utility::sendLocalReply( + false, + Http::Utility::EncodeFunctions( + {nullptr, + [&](Http::ResponseHeaderMapPtr&& headers, bool end_stream) -> void { + encoder_.encodeHeaders(*headers, end_stream); + }, + [&](Buffer::Instance& data, bool end_stream) -> void { + encoder_.encodeData(data, end_stream); + }}), + Http::Utility::LocalReplyData({is_grpc_request, code, body, grpc_status, is_head_request})); + } ABSL_MUST_USE_RESULT testing::AssertionResult diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index 987eb03bedf12..07d7a6a2a2a73 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -365,8 +365,7 @@ TEST_P(IntegrationTest, TestSmuggling) { "GET / HTTP/1.1\r\nHost: host\r\ncontent-length: 36\r\ntransfer-encoding: chunked\r\n\r\n" + smuggled_request; sendRawHttpAndWaitForResponse(lookupPort("http"), full_request.c_str(), &response, false); - EXPECT_EQ("HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", - response); + EXPECT_THAT(response, HasSubstr("HTTP/1.1 400 Bad Request\r\n")); } { std::string response; @@ -374,8 +373,7 @@ TEST_P(IntegrationTest, TestSmuggling) { "\r\ncontent-length: 36\r\n\r\n" + smuggled_request; sendRawHttpAndWaitForResponse(lookupPort("http"), request.c_str(), &response, false); - EXPECT_EQ("HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", - response); + EXPECT_THAT(response, HasSubstr("HTTP/1.1 400 Bad Request\r\n")); } { std::string response; @@ -383,8 +381,7 @@ TEST_P(IntegrationTest, TestSmuggling) { "identity,chunked \r\ncontent-length: 36\r\n\r\n" + smuggled_request; sendRawHttpAndWaitForResponse(lookupPort("http"), request.c_str(), &response, false); - EXPECT_EQ("HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", - response); + EXPECT_THAT(response, HasSubstr("HTTP/1.1 400 Bad Request\r\n")); } } @@ -392,7 +389,7 @@ TEST_P(IntegrationTest, BadFirstline) { initialize(); std::string response; sendRawHttpAndWaitForResponse(lookupPort("http"), "hello", &response); - EXPECT_EQ("HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", response); + EXPECT_THAT(response, HasSubstr("HTTP/1.1 400 Bad Request\r\n")); } TEST_P(IntegrationTest, MissingDelimiter) { @@ -401,7 +398,7 @@ TEST_P(IntegrationTest, MissingDelimiter) { std::string response; sendRawHttpAndWaitForResponse(lookupPort("http"), "GET / HTTP/1.1\r\nHost: host\r\nfoo bar\r\n\r\n", &response); - EXPECT_EQ("HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", response); + EXPECT_THAT(response, HasSubstr("HTTP/1.1 400 Bad Request\r\n")); EXPECT_THAT(waitForAccessLog(access_log_name_), HasSubstr("http1.codec_error")); } @@ -410,7 +407,7 @@ TEST_P(IntegrationTest, InvalidCharacterInFirstline) { std::string response; sendRawHttpAndWaitForResponse(lookupPort("http"), "GE(T / HTTP/1.1\r\nHost: host\r\n\r\n", &response); - EXPECT_EQ("HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", response); + EXPECT_THAT(response, HasSubstr("HTTP/1.1 400 Bad Request\r\n")); } TEST_P(IntegrationTest, InvalidVersion) { @@ -418,7 +415,7 @@ TEST_P(IntegrationTest, InvalidVersion) { std::string response; sendRawHttpAndWaitForResponse(lookupPort("http"), "GET / HTTP/1.01\r\nHost: host\r\n\r\n", &response); - EXPECT_EQ("HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", response); + EXPECT_THAT(response, HasSubstr("HTTP/1.1 400 Bad Request\r\n")); } // Expect that malformed trailers to break the connection @@ -435,7 +432,7 @@ TEST_P(IntegrationTest, BadTrailer) { "badtrailer\r\n\r\n", &response); - EXPECT_EQ("HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", response); + EXPECT_THAT(response, HasSubstr("HTTP/1.1 400 Bad Request\r\n")); } // Expect malformed headers to break the connection @@ -452,7 +449,7 @@ TEST_P(IntegrationTest, BadHeader) { "body\r\n0\r\n\r\n", &response); - EXPECT_EQ("HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\nconnection: close\r\n\r\n", response); + EXPECT_THAT(response, HasSubstr("HTTP/1.1 400 Bad Request\r\n")); } TEST_P(IntegrationTest, Http10Disabled) { diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index be6d5b5e48e65..7532f44a94dbd 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -1098,6 +1098,7 @@ TEST_P(DownstreamProtocolIntegrationTest, InvalidContentLength) { if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { ASSERT_TRUE(response->complete()); EXPECT_EQ("400", response->headers().getStatusValue()); + test_server_->waitForCounterGe("http.config_test.downstream_rq_4xx", 1); } else { ASSERT_TRUE(response->reset()); EXPECT_EQ(Http::StreamResetReason::ConnectionTermination, response->reset_reason()); diff --git a/test/mocks/http/mocks.h b/test/mocks/http/mocks.h index 0f7a4063d4192..5108639847651 100644 --- a/test/mocks/http/mocks.h +++ b/test/mocks/http/mocks.h @@ -258,6 +258,11 @@ class MockStreamDecoderFilter : public StreamDecoderFilter { MOCK_METHOD(FilterMetadataStatus, decodeMetadata, (Http::MetadataMap & metadata_map)); MOCK_METHOD(void, setDecoderFilterCallbacks, (StreamDecoderFilterCallbacks & callbacks)); MOCK_METHOD(void, decodeComplete, ()); + MOCK_METHOD(void, sendLocalReply, + (bool is_grpc_request, Code code, absl::string_view body, + const std::function& modify_headers, + bool is_head_request, const absl::optional grpc_status, + absl::string_view details)); Http::StreamDecoderFilterCallbacks* callbacks_{}; }; diff --git a/test/mocks/http/stream_decoder.h b/test/mocks/http/stream_decoder.h index 1238c55f91b54..b822de460b9d8 100644 --- a/test/mocks/http/stream_decoder.h +++ b/test/mocks/http/stream_decoder.h @@ -16,6 +16,11 @@ class MockRequestDecoder : public RequestDecoder { // Http::StreamDecoder MOCK_METHOD(void, decodeData, (Buffer::Instance & data, bool end_stream)); MOCK_METHOD(void, decodeMetadata_, (MetadataMapPtr & metadata_map)); + MOCK_METHOD(void, sendLocalReply, + (bool is_grpc_request, Code code, absl::string_view body, + const std::function& modify_headers, + bool is_head_request, const absl::optional grpc_status, + absl::string_view details)); void decodeHeaders(RequestHeaderMapPtr&& headers, bool end_stream) override { decodeHeaders_(headers, end_stream);