diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 62ab94f0ce40a..3e9cded86722b 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -23,6 +23,7 @@ Bug Fixes * active http health checks: properly handles HTTP/2 GOAWAY frames from the upstream. Previously a GOAWAY frame due to a graceful listener drain could cause improper failed health checks due to streams being refused by the upstream on a connection that is going away. To revert to old GOAWAY handling behavior, set the runtime feature `envoy.reloadable_features.health_check.graceful_goaway_handling` to false. * buffer: tighten network connection read and write buffer high watermarks in preparation to more careful enforcement of read limits. Buffer high-watermark is now set to the exact configured value; previously it was set to value + 1. +* grpc-web: fix local reply and non-proto-encoded gRPC response handling for small response bodies. This fix can be temporarily reverted by setting `envoy.reloadable_features.grpc_web_fix_non_proto_encoded_response_handling` to false. * http: disallowing "host:" in request_headers_to_add for behavioral consistency with rejecting :authority header. This behavior can be temporarily reverted by setting `envoy.reloadable_features.treat_host_like_authority` to false. * http: reverting a behavioral change where upstream connect timeouts were temporarily treated differently from other connection failures. The change back to the original behavior can be temporarily reverted by setting `envoy.reloadable_features.treat_upstream_connect_timeout_as_connect_failure` to false. * upstream: fix handling of moving endpoints between priorities when active health checks are enabled. Previously moving to a higher numbered priority was a NOOP, and moving to a lower numbered priority caused an abort. diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 116ed1b2fb2d8..6fb54e3b2ba3f 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -64,6 +64,7 @@ constexpr const char* runtime_features[] = { "envoy.reloadable_features.always_nodelay", "envoy.reloadable_features.check_ocsp_policy", "envoy.reloadable_features.disable_tls_inspector_injection", + "envoy.reloadable_features.grpc_web_fix_non_proto_encoded_response_handling", "envoy.reloadable_features.hcm_stream_error_on_invalid_message", "envoy.reloadable_features.health_check.graceful_goaway_handling", "envoy.reloadable_features.http_default_alpn", diff --git a/source/extensions/filters/http/grpc_web/grpc_web_filter.cc b/source/extensions/filters/http/grpc_web/grpc_web_filter.cc index 62d7eda354f9e..93953fe44efd7 100644 --- a/source/extensions/filters/http/grpc_web/grpc_web_filter.cc +++ b/source/extensions/filters/http/grpc_web/grpc_web_filter.cc @@ -7,16 +7,40 @@ #include "common/common/assert.h" #include "common/common/base64.h" #include "common/common/empty_string.h" +#include "common/common/enum_to_int.h" #include "common/common/utility.h" +#include "common/grpc/common.h" #include "common/grpc/context_impl.h" #include "common/http/headers.h" #include "common/http/utility.h" +#include "common/runtime/runtime_features.h" namespace Envoy { namespace Extensions { namespace HttpFilters { namespace GrpcWeb { +namespace { + +// This is the maximum buffered plaintext data length when we have buffered data in the encoding +// buffer. This is effectively used (to limit the length of grpc-message) only when we have encoding +// buffer filled with data. The value is arbitrarily chosen. This can be made configurable when it +// is required. +constexpr uint64_t MAX_BUFFERED_PLAINTEXT_LENGTH = 16384; + +// This builds grpc-message header value from body data. +std::string buildGrpcMessage(Buffer::Instance& body_data) { + const uint64_t message_length = body_data.length(); + std::string message; + message.reserve(message_length); + message.resize(message_length); + body_data.copyOut(0, message_length, message.data()); + + return Http::Utility::PercentEncoding::encode(message); +} + +} // namespace + Http::RegisterCustomInlineHeader accept_handle(Http::CustomHeaders::get().Accept); Http::RegisterCustomInlineHeader @@ -54,6 +78,81 @@ bool GrpcWebFilter::isGrpcWebRequest(const Http::RequestHeaderMap& headers) { return false; } +bool GrpcWebFilter::isProtoEncodedGrpcWebResponseHeaders( + const Http::ResponseHeaderMap& headers) const { + // We expect the response headers to have 200 OK status (a valid gRPC, also gRPC-Web, response + // needs to have 200 OK status + // https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses) and contain + // proto-encoded gRPC-Web content-type. + return Http::Utility::getResponseStatus(headers) == enumToInt(Http::Code::OK) && + hasProtoEncodedGrpcWebContentType(headers); +} + +// TODO(dio): Move this as a shared utility function. +bool GrpcWebFilter::hasProtoEncodedGrpcWebContentType( + const Http::RequestOrResponseHeaderMap& headers) const { + const Http::HeaderEntry* content_type = headers.ContentType(); + if (content_type != nullptr) { + absl::string_view content_type_value = content_type->value().getStringView(); + // We ignore "parameter" value. Note that "*( ";" parameter )" indicates that there can be + // multiple parameters. + absl::string_view current_content_type = + StringUtil::rtrim(content_type_value.substr(0, content_type_value.find_first_of(';'))); + // We expect only proto encoding response + // https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-WEB.md. And the value of media-type is + // case-sensitive https://tools.ietf.org/html/rfc2616#section-3.7. + return StringUtil::CaseInsensitiveCompare()( + current_content_type, Http::Headers::get().ContentTypeValues.GrpcWebProto) || + StringUtil::CaseInsensitiveCompare()(current_content_type, + Http::Headers::get().ContentTypeValues.GrpcWeb); + } + return false; +} + +// If response headers do not contain valid response headers, it needs transformation. +bool GrpcWebFilter::needsTransformationForNonProtoEncodedResponse(Http::ResponseHeaderMap& headers, + bool end_stream) const { + return Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.grpc_web_fix_non_proto_encoded_response_handling") && + // We transform the response unless it is already a gRPC or proto-encoded gRPC-Web + // response. + !Grpc::Common::isGrpcResponseHeaders(headers, end_stream) && + !isProtoEncodedGrpcWebResponseHeaders(headers); +} + +void GrpcWebFilter::mergeAndLimitNonProtoEncodedResponseData(Buffer::OwnedImpl& output, + Buffer::Instance* last_data) { + const auto* encoding_buffer = encoder_callbacks_->encodingBuffer(); + if (encoding_buffer != nullptr) { + if (last_data != nullptr) { + encoder_callbacks_->addEncodedData(*last_data, false); + } + encoder_callbacks_->modifyEncodingBuffer([&output](Buffer::Instance& buffered) { + // When we have buffered data (encoding buffer is filled), we limit the final buffer length. + output.move(buffered, MAX_BUFFERED_PLAINTEXT_LENGTH); + buffered.drain(buffered.length()); + }); + } else if (last_data != nullptr) { + // In the case of local reply and when the response only contains a single data chunk, + // "encoding_buffer" is nullptr and we only have filled "last_data". + output.move(*last_data); + last_data->drain(last_data->length()); + } +} + +void GrpcWebFilter::setTransformedNonProtoEncodedResponseHeaders(Buffer::Instance* data) { + Buffer::OwnedImpl merged_data; + // When we have buffered data in encoding buffer, we limit the length of the output to be smaller + // than MAX_BUFFERED_PLAINTEXT_LENGTH. However, when we only have "last" data, we send it all. + mergeAndLimitNonProtoEncodedResponseData(merged_data, data); + + const std::string grpc_message = buildGrpcMessage(merged_data); + response_headers_->setGrpcMessage(grpc_message); + response_headers_->setGrpcStatus(Grpc::Utility::httpToGrpcStatus( + enumToInt(Http::Utility::getResponseStatus(*response_headers_)))); + response_headers_->setContentLength(0); +} + // Implements StreamDecoderFilter. // TODO(fengli): Implements the subtypes of gRPC-Web content-type other than proto, like +json, etc. Http::FilterHeadersStatus GrpcWebFilter::decodeHeaders(Http::RequestHeaderMap& headers, bool) { @@ -143,7 +242,8 @@ Http::FilterDataStatus GrpcWebFilter::decodeData(Buffer::Instance& data, bool en } // Implements StreamEncoderFilter. -Http::FilterHeadersStatus GrpcWebFilter::encodeHeaders(Http::ResponseHeaderMap& headers, bool) { +Http::FilterHeadersStatus GrpcWebFilter::encodeHeaders(Http::ResponseHeaderMap& headers, + bool end_stream) { if (!is_grpc_web_request_) { return Http::FilterHeadersStatus::Continue; } @@ -151,19 +251,51 @@ Http::FilterHeadersStatus GrpcWebFilter::encodeHeaders(Http::ResponseHeaderMap& if (doStatTracking()) { chargeStat(headers); } + + needs_transformation_for_non_proto_encoded_response_ = + needsTransformationForNonProtoEncodedResponse(headers, end_stream); + if (is_text_response_) { headers.setReferenceContentType(Http::Headers::get().ContentTypeValues.GrpcWebTextProto); } else { headers.setReferenceContentType(Http::Headers::get().ContentTypeValues.GrpcWebProto); } - return Http::FilterHeadersStatus::Continue; + + if (end_stream || !needs_transformation_for_non_proto_encoded_response_) { + return Http::FilterHeadersStatus::Continue; + } + + response_headers_ = &headers; + return Http::FilterHeadersStatus::StopIteration; } -Http::FilterDataStatus GrpcWebFilter::encodeData(Buffer::Instance& data, bool) { +Http::FilterDataStatus GrpcWebFilter::encodeData(Buffer::Instance& data, bool end_stream) { if (!is_grpc_web_request_) { return Http::FilterDataStatus::Continue; } + // When the upstream response (this is also relevant for local reply, since gRPC-Web request is + // not a gRPC request which makes the local reply's is_grpc_request set to false) is not a gRPC + // response, we set the "grpc-message" header with the upstream body content. + if (needs_transformation_for_non_proto_encoded_response_) { + const auto* encoding_buffer = encoder_callbacks_->encodingBuffer(); + if (!end_stream) { + // We limit the buffered data in encoding buffer here to eliminate the possibility of + // buffering too large data from upstream. Note that the buffered data here will be + // transformed as grpc-message later. + if (encoding_buffer != nullptr && + encoding_buffer->length() >= MAX_BUFFERED_PLAINTEXT_LENGTH) { + return Http::FilterDataStatus::StopIterationNoBuffer; + } + return Http::FilterDataStatus::StopIterationAndBuffer; + } + + ASSERT(response_headers_ != nullptr); + needs_transformation_for_non_proto_encoded_response_ = false; + setTransformedNonProtoEncodedResponseHeaders(&data); + return Http::FilterDataStatus::Continue; + } + if (!is_text_response_) { // No additional transcoding required if gRPC-Web client asked for binary response. return Http::FilterDataStatus::Continue; @@ -202,6 +334,11 @@ Http::FilterTrailersStatus GrpcWebFilter::encodeTrailers(Http::ResponseTrailerMa chargeStat(trailers); } + if (needs_transformation_for_non_proto_encoded_response_) { + setTransformedNonProtoEncodedResponseHeaders(nullptr); + return Http::FilterTrailersStatus::Continue; + } + // Trailers are expected to come all in once, and will be encoded into one single trailers frame. // Trailers in the trailers frame are separated by `CRLFs`. Buffer::OwnedImpl temp; diff --git a/source/extensions/filters/http/grpc_web/grpc_web_filter.h b/source/extensions/filters/http/grpc_web/grpc_web_filter.h index 2ae3d2381fbf2..01f4d59699b26 100644 --- a/source/extensions/filters/http/grpc_web/grpc_web_filter.h +++ b/source/extensions/filters/http/grpc_web/grpc_web_filter.h @@ -56,6 +56,13 @@ class GrpcWebFilter : public Http::StreamFilter, NonCopyable { void chargeStat(const Http::ResponseHeaderOrTrailerMap& headers); void setupStatTracking(const Http::RequestHeaderMap& headers); bool isGrpcWebRequest(const Http::RequestHeaderMap& headers); + bool isProtoEncodedGrpcWebResponseHeaders(const Http::ResponseHeaderMap& headers) const; + bool hasProtoEncodedGrpcWebContentType(const Http::RequestOrResponseHeaderMap& headers) const; + bool needsTransformationForNonProtoEncodedResponse(Http::ResponseHeaderMap& headers, + bool end_stream) const; + void mergeAndLimitNonProtoEncodedResponseData(Buffer::OwnedImpl& output, + Buffer::Instance* last_data); + void setTransformedNonProtoEncodedResponseHeaders(Buffer::Instance* data); static const uint8_t GRPC_WEB_TRAILER; const absl::flat_hash_set& gRpcWebContentTypes() const; @@ -65,11 +72,13 @@ class GrpcWebFilter : public Http::StreamFilter, NonCopyable { Http::StreamEncoderFilterCallbacks* encoder_callbacks_{}; bool is_text_request_{}; bool is_text_response_{}; + bool needs_transformation_for_non_proto_encoded_response_{}; Buffer::OwnedImpl decoding_buffer_; Grpc::Decoder decoder_; absl::optional request_stat_names_; bool is_grpc_web_request_{}; Grpc::Context& context_; + Http::ResponseHeaderMap* response_headers_{nullptr}; }; } // namespace GrpcWeb diff --git a/test/extensions/filters/http/grpc_web/BUILD b/test/extensions/filters/http/grpc_web/BUILD index 831e596ffb149..118bb57ab794c 100644 --- a/test/extensions/filters/http/grpc_web/BUILD +++ b/test/extensions/filters/http/grpc_web/BUILD @@ -19,6 +19,7 @@ envoy_extension_cc_test( "//source/extensions/filters/http/grpc_web:grpc_web_filter_lib", "//test/mocks/http:http_mocks", "//test/test_common:global_lib", + "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", ], ) diff --git a/test/extensions/filters/http/grpc_web/grpc_web_filter_integration_test.cc b/test/extensions/filters/http/grpc_web/grpc_web_filter_integration_test.cc index 5a8831476b532..6f0925196dfd6 100644 --- a/test/extensions/filters/http/grpc_web/grpc_web_filter_integration_test.cc +++ b/test/extensions/filters/http/grpc_web/grpc_web_filter_integration_test.cc @@ -1,5 +1,7 @@ #include +#include "common/common/base64.h" + #include "extensions/filters/http/well_known_names.h" #include "test/integration/http_integration.h" @@ -9,9 +11,15 @@ namespace Envoy { namespace { +constexpr absl::string_view text{"application/grpc-web-text"}; +constexpr absl::string_view binary{"application/grpc-web"}; +constexpr uint64_t MAX_BUFFERED_PLAINTEXT_LENGTH = 16384; + using SkipEncodingEmptyTrailers = bool; -using TestParams = - std::tuple; +using ContentType = std::string; +using Accept = std::string; +using TestParams = std::tuple; class GrpcWebFilterIntegrationTest : public testing::TestWithParam, public HttpIntegrationTest { @@ -24,20 +32,123 @@ class GrpcWebFilterIntegrationTest : public testing::TestWithParam, config_helper_.addFilter("name: envoy.filters.http.grpc_web"); } + void initialize() override { + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); + } else { + skipEncodingEmptyTrailers(http2_skip_encoding_empty_trailers_); + } + + setUpstreamProtocol(FakeHttpConnection::Type::HTTP2); + + HttpIntegrationTest::initialize(); + } + void skipEncodingEmptyTrailers(SkipEncodingEmptyTrailers http2_skip_encoding_empty_trailers) { config_helper_.addRuntimeOverride( "envoy.reloadable_features.http2_skip_encoding_empty_trailers", http2_skip_encoding_empty_trailers ? "true" : "false"); } + void setLocalReplyConfig(const std::string& yaml) { + envoy::extensions::filters::network::http_connection_manager::v3::LocalReplyConfig + local_reply_config; + TestUtility::loadFromYaml(yaml, local_reply_config); + config_helper_.setLocalReply(local_reply_config); + } + + void testUpstreamDisconnect(const std::string& expected_grpc_message_value, + const std::string& local_reply_config_yaml = EMPTY_STRING) { + if (!local_reply_config_yaml.empty()) { + setLocalReplyConfig(local_reply_config_yaml); + } + + initialize(); + + Http::TestRequestTrailerMapImpl request_trailers{{"request1", "trailer1"}, + {"request2", "trailer2"}}; + + codec_client_ = makeHttpConnection(lookupPort("http")); + auto encoder_decoder = + codec_client_->startRequest(Http::TestRequestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {"content-type", content_type_}, + {"accept", accept_}, + {":authority", "host"}}); + request_encoder_ = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + codec_client_->sendData(*request_encoder_, 1, false); + codec_client_->sendTrailers(*request_encoder_, request_trailers); + waitForNextUpstreamRequest(); + + ASSERT_TRUE(fake_upstream_connection_->close()); + response->waitForEndStream(); + EXPECT_TRUE(response->complete()); + + EXPECT_EQ("503", response->headers().getStatusValue()); + EXPECT_EQ(absl::StrCat(accept_, "+proto"), response->headers().getContentTypeValue()); + EXPECT_EQ(expected_grpc_message_value, response->headers().getGrpcMessageValue()); + EXPECT_EQ(0U, response->body().length()); + + codec_client_->close(); + } + + void testBadUpstreamResponse(const std::string& start, const std::string& end, + const std::string& expected, bool remove_content_type = false) { + initialize(); + Http::TestRequestTrailerMapImpl request_trailers{{"request1", "trailer1"}, + {"request2", "trailer2"}}; + + codec_client_ = makeHttpConnection(lookupPort("http")); + auto encoder_decoder = + codec_client_->startRequest(Http::TestRequestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {"content-type", content_type_}, + {"accept", accept_}, + {":authority", "host"}}); + request_encoder_ = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + codec_client_->sendData(*request_encoder_, 1, false); + codec_client_->sendTrailers(*request_encoder_, request_trailers); + waitForNextUpstreamRequest(); + + // Sending back non gRPC-Web response. + if (remove_content_type) { + default_response_headers_.removeContentType(); + } else { + default_response_headers_.setReferenceContentType( + Http::Headers::get().ContentTypeValues.Json); + } + upstream_request_->encodeHeaders(default_response_headers_, /*end_stream=*/false); + upstream_request_->encodeData(start, /*end_stream=*/false); + upstream_request_->encodeData(end, /*end_stream=*/true); + response->waitForEndStream(); + + EXPECT_TRUE(response->complete()); + EXPECT_EQ(expected, response->headers().getGrpcMessageValue()); + EXPECT_EQ("200", response->headers().getStatusValue()); + EXPECT_EQ(absl::StrCat(accept_, "+proto"), response->headers().getContentTypeValue()); + EXPECT_EQ(0U, response->body().length()); + codec_client_->close(); + } + static std::string testParamsToString(const testing::TestParamInfo params) { return fmt::format( - "{}_{}_{}", + "{}_{}_{}_{}_{}", TestUtility::ipTestParamsToString(testing::TestParamInfo( std::get<0>(params.param), params.index)), std::get<1>(params.param) == Http::CodecClient::Type::HTTP2 ? "Http2" : "Http", - std::get<2>(params.param) ? "SkipEncodingEmptyTrailers" : "SubmitEncodingEmptyTrailers"); + std::get<2>(params.param) ? "SkipEncodingEmptyTrailers" : "SubmitEncodingEmptyTrailers", + std::get<3>(params.param) == text ? "SendText" : "SendBinary", + std::get<4>(params.param) == text ? "AcceptText" : "AcceptBinary"); } + + const Envoy::Http::CodecClient::Type downstream_protocol_{std::get<1>(GetParam())}; + const bool http2_skip_encoding_empty_trailers_{std::get<2>(GetParam())}; + const ContentType content_type_{std::get<3>(GetParam())}; + const Accept accept_{std::get<4>(GetParam())}; }; INSTANTIATE_TEST_SUITE_P( @@ -45,60 +156,59 @@ INSTANTIATE_TEST_SUITE_P( testing::Combine( testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), testing::Values(Http::CodecClient::Type::HTTP1, Http::CodecClient::Type::HTTP2), - testing::Values(SkipEncodingEmptyTrailers{true}, SkipEncodingEmptyTrailers{false})), + testing::Values(SkipEncodingEmptyTrailers{true}, SkipEncodingEmptyTrailers{false}), + testing::Values(ContentType{text}, ContentType{binary}), + testing::Values(Accept{text}, Accept{binary})), GrpcWebFilterIntegrationTest::testParamsToString); TEST_P(GrpcWebFilterIntegrationTest, GrpcWebTrailersNotDuplicated) { - const auto downstream_protocol = std::get<1>(GetParam()); - const bool http2_skip_encoding_empty_trailers = std::get<2>(GetParam()); - - if (downstream_protocol == Http::CodecClient::Type::HTTP1) { - config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); - } else { - skipEncodingEmptyTrailers(http2_skip_encoding_empty_trailers); - } - - setUpstreamProtocol(FakeHttpConnection::Type::HTTP2); + initialize(); Http::TestRequestTrailerMapImpl request_trailers{{"request1", "trailer1"}, {"request2", "trailer2"}}; Http::TestResponseTrailerMapImpl response_trailers{{"response1", "trailer1"}, {"response2", "trailer2"}}; - initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); - auto encoder_decoder = codec_client_->startRequest( - Http::TestRequestHeaderMapImpl{{":method", "POST"}, - {":path", "/test/long/url"}, - {":scheme", "http"}, - {"content-type", "application/grpc-web"}, - {":authority", "host"}}); + auto encoder_decoder = + codec_client_->startRequest(Http::TestRequestHeaderMapImpl{{":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {"content-type", content_type_}, + {"accept", accept_}, + {":authority", "host"}}); request_encoder_ = &encoder_decoder.first; auto response = std::move(encoder_decoder.second); - codec_client_->sendData(*request_encoder_, 1, false); + + const std::string body = "hello"; + const std::string encoded_body = + content_type_ == text ? Base64::encode(body.data(), body.length()) : body; + codec_client_->sendData(*request_encoder_, encoded_body, false); codec_client_->sendTrailers(*request_encoder_, request_trailers); waitForNextUpstreamRequest(); + default_response_headers_.setReferenceContentType(Http::Headers::get().ContentTypeValues.Grpc); upstream_request_->encodeHeaders(default_response_headers_, false); upstream_request_->encodeData(1, false); upstream_request_->encodeTrailers(response_trailers); response->waitForEndStream(); EXPECT_TRUE(upstream_request_->complete()); - EXPECT_EQ(1, upstream_request_->bodyLength()); + EXPECT_EQ(body.length(), upstream_request_->bodyLength()); EXPECT_THAT(*upstream_request_->trailers(), HeaderMapEqualRef(&request_trailers)); EXPECT_TRUE(response->complete()); EXPECT_EQ("200", response->headers().getStatusValue()); - EXPECT_TRUE(absl::StrContains(response->body(), "response1:trailer1")); - EXPECT_TRUE(absl::StrContains(response->body(), "response2:trailer2")); + const auto response_body = accept_ == text ? Base64::decode(response->body()) : response->body(); + EXPECT_TRUE(absl::StrContains(response_body, "response1:trailer1")); + EXPECT_TRUE(absl::StrContains(response_body, "response2:trailer2")); - if (downstream_protocol == Http::CodecClient::Type::HTTP1) { + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { // When the downstream protocol is HTTP/1.1 we expect the trailers to be in the response-body. EXPECT_EQ(nullptr, response->trailers()); } - if (downstream_protocol == Http::CodecClient::Type::HTTP2) { - if (http2_skip_encoding_empty_trailers) { + if (downstream_protocol_ == Http::CodecClient::Type::HTTP2) { + if (http2_skip_encoding_empty_trailers_) { // When the downstream protocol is HTTP/2 and the feature-flag to skip encoding empty trailers // is turned on, expect that the trailers are included in the response-body. EXPECT_EQ(nullptr, response->trailers()); @@ -111,39 +221,46 @@ TEST_P(GrpcWebFilterIntegrationTest, GrpcWebTrailersNotDuplicated) { } TEST_P(GrpcWebFilterIntegrationTest, UpstreamDisconnect) { - const auto downstream_protocol = std::get<1>(GetParam()); - const bool http2_skip_encoding_empty_trailers = std::get<2>(GetParam()); - - if (downstream_protocol == Http::CodecClient::Type::HTTP1) { - config_helper_.addConfigModifier(setEnableDownstreamTrailersHttp1()); - } else { - skipEncodingEmptyTrailers(http2_skip_encoding_empty_trailers); - } + testUpstreamDisconnect("upstream connect error or disconnect/reset before headers. reset reason: " + "connection termination"); +} - setUpstreamProtocol(FakeHttpConnection::Type::HTTP2); +TEST_P(GrpcWebFilterIntegrationTest, UpstreamDisconnectWithLargeBody) { + // The local reply is configured to send a large (its length is greater than + // MAX_BUFFERED_PLAINTEXT_LENGTH) body. + const std::string local_reply_body = std::string(2 * MAX_BUFFERED_PLAINTEXT_LENGTH, 'a'); + const std::string local_reply_config_yaml = fmt::format(R"EOF( +body_format: + text_format_source: + inline_string: "{}" +)EOF", + local_reply_body); + testUpstreamDisconnect(local_reply_body, local_reply_config_yaml); +} - Http::TestRequestTrailerMapImpl request_trailers{{"request1", "trailer1"}, - {"request2", "trailer2"}}; +TEST_P(GrpcWebFilterIntegrationTest, BadUpstreamResponse) { + testBadUpstreamResponse("{", "\"percentage\": \"100%\"}", + /*expected=*/"{\"percentage\": \"100%25\"}"); +} - initialize(); - codec_client_ = makeHttpConnection(lookupPort("http")); - auto encoder_decoder = codec_client_->startRequest( - Http::TestRequestHeaderMapImpl{{":method", "POST"}, - {":path", "/test/long/url"}, - {":scheme", "http"}, - {"content-type", "application/grpc-web"}, - {":authority", "host"}}); - request_encoder_ = &encoder_decoder.first; - auto response = std::move(encoder_decoder.second); - codec_client_->sendData(*request_encoder_, 1, false); - codec_client_->sendTrailers(*request_encoder_, request_trailers); - waitForNextUpstreamRequest(); +TEST_P(GrpcWebFilterIntegrationTest, BadUpstreamResponseWithoutContentType) { + testBadUpstreamResponse("{", "\"percentage\": \"100%\"}", + /*expected=*/"{\"percentage\": \"100%25\"}", + /*remove_content_type=*/true); +} - ASSERT_TRUE(fake_upstream_connection_->close()); - response->waitForEndStream(); - EXPECT_EQ("503", response->headers().getStatusValue()); +TEST_P(GrpcWebFilterIntegrationTest, BadUpstreamResponseLargeEnd) { + // When we have buffered data in encoding buffer, we limit the length to + // MAX_BUFFERED_PLAINTEXT_LENGTH. + const std::string start(MAX_BUFFERED_PLAINTEXT_LENGTH, 'a'); + const std::string end(MAX_BUFFERED_PLAINTEXT_LENGTH, 'b'); + testBadUpstreamResponse(start, end, /*expected=*/start); +} - codec_client_->close(); +TEST_P(GrpcWebFilterIntegrationTest, BadUpstreamResponseLargeFirst) { + const std::string start(2 * MAX_BUFFERED_PLAINTEXT_LENGTH, 'a'); + const std::string end(MAX_BUFFERED_PLAINTEXT_LENGTH, 'b'); + testBadUpstreamResponse(start, end, /*expected=*/start.substr(0, MAX_BUFFERED_PLAINTEXT_LENGTH)); } } // namespace diff --git a/test/extensions/filters/http/grpc_web/grpc_web_filter_test.cc b/test/extensions/filters/http/grpc_web/grpc_web_filter_test.cc index 6221286157781..bbc401405cf97 100644 --- a/test/extensions/filters/http/grpc_web/grpc_web_filter_test.cc +++ b/test/extensions/filters/http/grpc_web/grpc_web_filter_test.cc @@ -2,6 +2,7 @@ #include "common/buffer/buffer_impl.h" #include "common/common/base64.h" +#include "common/common/empty_string.h" #include "common/common/utility.h" #include "common/grpc/common.h" #include "common/http/codes.h" @@ -14,6 +15,7 @@ #include "test/mocks/http/mocks.h" #include "test/test_common/global.h" #include "test/test_common/printers.h" +#include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" @@ -44,6 +46,7 @@ const char INVALID_B64_MESSAGE[] = "****"; const size_t INVALID_B64_MESSAGE_SIZE = sizeof(INVALID_B64_MESSAGE) - 1; const char TRAILERS[] = "\x80\x00\x00\x00\x20grpc-status:0\r\ngrpc-message:ok\r\n"; const size_t TRAILERS_SIZE = sizeof(TRAILERS) - 1; +constexpr uint64_t MAX_BUFFERED_PLAINTEXT_LENGTH = 16384; } // namespace @@ -103,6 +106,44 @@ class GrpcWebFilterTest : public testing::TestWithParam cb) { + cb(*encoded_buffer); + }; + if (last_data != nullptr) { + EXPECT_CALL(encoder_callbacks_, addEncodedData(_, false)) + .WillOnce(Invoke([&](Buffer::Instance& data, bool) { encoded_buffer->move(data); })); + } + EXPECT_CALL(encoder_callbacks_, encodingBuffer).WillRepeatedly(Return(encoded_buffer)); + EXPECT_CALL(encoder_callbacks_, modifyEncodingBuffer) + .WillRepeatedly(Invoke(on_modify_encoding_buffer)); + } + Buffer::OwnedImpl output; + filter_.mergeAndLimitNonProtoEncodedResponseData(output, last_data); + EXPECT_EQ(expected_merged_length, output.length()); + if (encoded_buffer != nullptr) { + EXPECT_EQ(0U, encoded_buffer->length()); + } + if (last_data != nullptr) { + EXPECT_EQ(0U, last_data->length()); + } + } + Stats::TestUtil::TestSymbolTable symbol_table_; Grpc::ContextImpl grpc_context_; GrpcWebFilter filter_; @@ -130,6 +171,80 @@ TEST_F(GrpcWebFilterTest, SupportedContentTypes) { } } +TEST_F(GrpcWebFilterTest, ExpectedGrpcWebProtoContentType) { + EXPECT_TRUE(isProtoEncodedGrpcWebContentType(Http::Headers::get().ContentTypeValues.GrpcWeb)); + EXPECT_TRUE( + isProtoEncodedGrpcWebContentType(Http::Headers::get().ContentTypeValues.GrpcWebProto)); + EXPECT_TRUE(isProtoEncodedGrpcWebContentType(Http::Headers::get().ContentTypeValues.GrpcWeb + + "; version=1; action=urn:CreateCredential")); + EXPECT_TRUE(isProtoEncodedGrpcWebContentType(Http::Headers::get().ContentTypeValues.GrpcWeb + + " ; version=1; action=urn:CreateCredential")); + EXPECT_TRUE(isProtoEncodedGrpcWebContentType(Http::Headers::get().ContentTypeValues.GrpcWebProto + + "; version=1")); + EXPECT_TRUE(isProtoEncodedGrpcWebContentType("Application/Grpc-Web")); + EXPECT_TRUE(isProtoEncodedGrpcWebContentType("Application/Grpc-Web+Proto")); + EXPECT_TRUE(isProtoEncodedGrpcWebContentType("APPLICATION/GRPC-WEB+PROTO; ok=1; great=1")); +} + +TEST_F(GrpcWebFilterTest, UnexpectedGrpcWebProtoContentType) { + EXPECT_FALSE(isProtoEncodedGrpcWebContentType(EMPTY_STRING)); + EXPECT_FALSE(isProtoEncodedGrpcWebContentType("Invalid; ok=1")); + EXPECT_FALSE(isProtoEncodedGrpcWebContentType("Invalid; ok=1; nok=2")); + EXPECT_FALSE( + isProtoEncodedGrpcWebContentType(Http::Headers::get().ContentTypeValues.GrpcWeb + "+thrift")); + EXPECT_FALSE( + isProtoEncodedGrpcWebContentType(Http::Headers::get().ContentTypeValues.GrpcWeb + "+json")); + EXPECT_FALSE( + isProtoEncodedGrpcWebContentType(Http::Headers::get().ContentTypeValues.GrpcWebText)); + EXPECT_FALSE( + isProtoEncodedGrpcWebContentType(Http::Headers::get().ContentTypeValues.GrpcWebTextProto)); +} + +TEST_F(GrpcWebFilterTest, ExpectedGrpcWebProtoResponseHeaders) { + EXPECT_TRUE(isProtoEncodedGrpcWebResponseHeaders(Http::TestResponseHeaderMapImpl{ + {":status", "200"}, {"content-type", "application/grpc-web"}})); + EXPECT_TRUE(isProtoEncodedGrpcWebResponseHeaders(Http::TestResponseHeaderMapImpl{ + {":status", "200"}, {"content-type", "application/grpc-web+proto"}})); +} + +TEST_F(GrpcWebFilterTest, UnexpectedGrpcWebProtoResponseHeaders) { + EXPECT_FALSE(isProtoEncodedGrpcWebResponseHeaders(Http::TestResponseHeaderMapImpl{ + {":status", "500"}, {"content-type", "application/grpc-web+proto"}})); + EXPECT_FALSE(isProtoEncodedGrpcWebResponseHeaders(Http::TestResponseHeaderMapImpl{ + {":status", "200"}, {"content-type", "application/grpc-web+json"}})); +} + +TEST_F(GrpcWebFilterTest, MergeAndLimitNonProtoEncodedResponseData) { + Buffer::OwnedImpl encoded_buffer(std::string(100, 'a')); + Buffer::OwnedImpl last_data(std::string(100, 'a')); + expectMergedAndLimitedResponseData(&encoded_buffer, &last_data, + /*expected_merged_length=*/encoded_buffer.length() + + last_data.length()); +} + +TEST_F(GrpcWebFilterTest, MergeAndLimitNonProtoEncodedResponseDataWithLargeEncodingBuffer) { + Buffer::OwnedImpl encoded_buffer(std::string(2 * MAX_BUFFERED_PLAINTEXT_LENGTH, 'a')); + Buffer::OwnedImpl last_data(std::string(2 * MAX_BUFFERED_PLAINTEXT_LENGTH, 'a')); + // Since the buffered data in encoding buffer is larger than MAX_BUFFERED_PLAINTEXT_LENGTH, the + // output length is limited to MAX_BUFFERED_PLAINTEXT_LENGTH. + expectMergedAndLimitedResponseData(&encoded_buffer, &last_data, + /*expected_merged_length=*/MAX_BUFFERED_PLAINTEXT_LENGTH); +} + +TEST_F(GrpcWebFilterTest, MergeAndLimitNonProtoEncodedResponseDataWithNullEncodingBuffer) { + Buffer::OwnedImpl last_data(std::string(2 * MAX_BUFFERED_PLAINTEXT_LENGTH, 'a')); + // If we don't have buffered data in encoding buffer, the merged data will be the same as last + // data. + expectMergedAndLimitedResponseData(nullptr, &last_data, + /*expected_merged_length=*/last_data.length()); +} + +TEST_F(GrpcWebFilterTest, MergeAndLimitNonProtoEncodedResponseDataWithNoEncodingBufferAndLastData) { + // If we don't have both buffered data in encoding buffer and last data, the output length is + // zero. + expectMergedAndLimitedResponseData(nullptr, nullptr, /*expected_merged_length=*/0U); +} + TEST_F(GrpcWebFilterTest, UnsupportedContentType) { Buffer::OwnedImpl data; request_headers_.addCopy(Http::Headers::get().ContentType, "unsupported"); @@ -195,6 +310,135 @@ TEST_F(GrpcWebFilterTest, Base64NoPadding) { EXPECT_EQ(decoder_callbacks_.details(), "grpc_base_64_decode_failed_bad_size"); } +TEST_F(GrpcWebFilterTest, InvalidUpstreamResponseForText) { + Http::TestRequestHeaderMapImpl request_headers{ + {"content-type", Http::Headers::get().ContentTypeValues.GrpcWebText}, {":path", "/"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers, false)); + + Http::TestResponseHeaderMapImpl response_headers{{":status", "400"}}; + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_.encodeHeaders(response_headers, false)); + Buffer::OwnedImpl data("hello"); + Buffer::InstancePtr buffer(new Buffer::OwnedImpl("hello")); + EXPECT_CALL(encoder_callbacks_, encodingBuffer()).WillRepeatedly(Return(buffer.get())); + auto on_modify_encoding_buffer = [encoded_buffer = + buffer.get()](std::function cb) { + cb(*encoded_buffer); + }; + EXPECT_CALL(encoder_callbacks_, modifyEncodingBuffer) + .WillRepeatedly(Invoke(on_modify_encoding_buffer)); + + EXPECT_CALL(encoder_callbacks_, addEncodedData(_, false)) + .WillOnce(Invoke([&](Buffer::Instance& data, bool) { buffer->move(data); })); + + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_.encodeData(data, false)); + + buffer->add(data); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_.encodeData(data, false)); + + TestUtility::feedBufferWithRandomCharacters(data, MAX_BUFFERED_PLAINTEXT_LENGTH); + const std::string expected_grpc_message = + absl::StrCat("hellohello", data.toString()).substr(0, MAX_BUFFERED_PLAINTEXT_LENGTH); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.encodeData(data, true)); + EXPECT_EQ(MAX_BUFFERED_PLAINTEXT_LENGTH, + response_headers.get_(Http::Headers::get().GrpcMessage).length()); + EXPECT_EQ(expected_grpc_message, response_headers.get_(Http::Headers::get().GrpcMessage)); +} + +TEST_F(GrpcWebFilterTest, InvalidUpstreamResponseForTextWithLargeEncodingBuffer) { + Http::TestRequestHeaderMapImpl request_headers{ + {"content-type", Http::Headers::get().ContentTypeValues.GrpcWebText}, {":path", "/"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers, false)); + + Http::TestResponseHeaderMapImpl response_headers{{":status", "400"}}; + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_.encodeHeaders(response_headers, false)); + Buffer::OwnedImpl encoded_buffer; + encoded_buffer.add(std::string(2 * MAX_BUFFERED_PLAINTEXT_LENGTH, 'a')); + // The encoding buffer is filled with data more than MAX_BUFFERED_PLAINTEXT_LENGTH. + auto on_modify_encoding_buffer = [&encoded_buffer](std::function cb) { + cb(encoded_buffer); + }; + EXPECT_CALL(encoder_callbacks_, encodingBuffer).WillRepeatedly(Return(&encoded_buffer)); + EXPECT_CALL(encoder_callbacks_, modifyEncodingBuffer) + .WillRepeatedly(Invoke(on_modify_encoding_buffer)); + + EXPECT_EQ(Http::FilterDataStatus::StopIterationNoBuffer, + filter_.encodeData(encoded_buffer, false)); + EXPECT_EQ(Http::FilterDataStatus::StopIterationNoBuffer, + filter_.encodeData(encoded_buffer, false)); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.encodeData(encoded_buffer, true)); + EXPECT_EQ(MAX_BUFFERED_PLAINTEXT_LENGTH, + response_headers.get_(Http::Headers::get().GrpcMessage).length()); +} + +TEST_F(GrpcWebFilterTest, InvalidUpstreamResponseForTextWithLargeLastData) { + Http::TestRequestHeaderMapImpl request_headers{ + {"content-type", Http::Headers::get().ContentTypeValues.GrpcWebText}, {":path", "/"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers, false)); + + Http::TestResponseHeaderMapImpl response_headers{{":status", "400"}}; + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_.encodeHeaders(response_headers, false)); + Buffer::OwnedImpl data; + // The last data length is set to be bigger than "MAX_BUFFERED_PLAINTEXT_LENGTH". + const std::string expected_grpc_message = std::string(MAX_BUFFERED_PLAINTEXT_LENGTH + 1, 'a'); + data.add(expected_grpc_message); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.encodeData(data, true)); + // The grpc-message value length is the same as the last sent buffer. + EXPECT_EQ(MAX_BUFFERED_PLAINTEXT_LENGTH + 1, + response_headers.get_(Http::Headers::get().GrpcMessage).length()); + EXPECT_EQ(expected_grpc_message, response_headers.get_(Http::Headers::get().GrpcMessage)); +} + +TEST_F(GrpcWebFilterTest, InvalidUpstreamResponseForTextWithTrailers) { + Http::TestRequestHeaderMapImpl request_headers{ + {"content-type", Http::Headers::get().ContentTypeValues.GrpcWebText}, {":path", "/"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers, false)); + + Http::TestResponseHeaderMapImpl response_headers{{":status", "400"}}; + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_.encodeHeaders(response_headers, false)); + Buffer::OwnedImpl data("hello"); + Buffer::InstancePtr buffer(new Buffer::OwnedImpl("hello")); + EXPECT_CALL(encoder_callbacks_, encodingBuffer()).WillRepeatedly(Return(buffer.get())); + auto on_modify_encoding_buffer = [encoded_buffer = + buffer.get()](std::function cb) { + cb(*encoded_buffer); + }; + EXPECT_CALL(encoder_callbacks_, modifyEncodingBuffer) + .WillRepeatedly(Invoke(on_modify_encoding_buffer)); + + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_.encodeData(data, false)); + + buffer->add(data); + EXPECT_EQ(Http::FilterDataStatus::StopIterationAndBuffer, filter_.encodeData(data, false)); + + Http::TestResponseTrailerMapImpl response_trailers{{"grpc-status", "0"}}; + EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_.encodeTrailers(response_trailers)); + + EXPECT_EQ("hellohello", response_headers.get_(Http::Headers::get().GrpcMessage)); +} + +TEST_F(GrpcWebFilterTest, InvalidUpstreamResponseForTextSkipTransformation) { + TestScopedRuntime scoped_runtime; + Runtime::LoaderSingleton::getExisting()->mergeValues( + {{"envoy.reloadable_features.grpc_web_fix_non_proto_encoded_response_handling", "false"}}); + + Http::TestRequestHeaderMapImpl request_headers{ + {"content-type", Http::Headers::get().ContentTypeValues.GrpcWebText}, + {"accept", Http::Headers::get().ContentTypeValues.GrpcWebText}, + {":path", "/"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers, false)); + + Http::TestResponseHeaderMapImpl response_headers{{":status", "400"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.encodeHeaders(response_headers, false)); + Buffer::OwnedImpl data("hello"); + // Since the client expects grpc-web-text and the upstream response does not contain gRPC frames, + // the iteration is paused. + EXPECT_EQ(Http::FilterDataStatus::StopIterationNoBuffer, filter_.encodeData(data, false)); +} + TEST_P(GrpcWebFilterTest, StatsNoCluster) { Http::TestRequestHeaderMapImpl request_headers{ {"content-type", request_content_type()}, @@ -218,7 +462,8 @@ TEST_P(GrpcWebFilterTest, StatsNormalResponse) { Http::MetadataMap metadata_map{{"metadata", "metadata"}}; EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_.encodeMetadata(metadata_map)); - Http::TestResponseHeaderMapImpl response_headers{{":status", "200"}}; + Http::TestResponseHeaderMapImpl response_headers{ + {":status", "200"}, {"content-type", Http::Headers::get().ContentTypeValues.GrpcWebProto}}; EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.encodeHeaders(response_headers, false)); Buffer::OwnedImpl data("hello"); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.encodeData(data, false)); @@ -240,7 +485,8 @@ TEST_P(GrpcWebFilterTest, StatsErrorResponse) { {"content-type", request_content_type()}, {":path", "/lyft.users.BadCompanions/GetBadCompanions"}}; EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers, false)); - Http::TestResponseHeaderMapImpl response_headers{{":status", "200"}}; + Http::TestResponseHeaderMapImpl response_headers{ + {":status", "200"}, {"content-type", Http::Headers::get().ContentTypeValues.GrpcWebProto}}; EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.encodeHeaders(response_headers, false)); Buffer::OwnedImpl data("hello"); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.encodeData(data, false)); @@ -258,12 +504,26 @@ TEST_P(GrpcWebFilterTest, StatsErrorResponse) { } TEST_P(GrpcWebFilterTest, ExternallyProvidedEncodingHeader) { - Http::TestRequestHeaderMapImpl request_headers{{"grpc-accept-encoding", "foo"}, {":path", "/"}}; + Http::TestRequestHeaderMapImpl request_headers{ + {"grpc-accept-encoding", "foo"}, {":path", "/"}, {"content-type", request_accept()}}; EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers_, false)); EXPECT_EQ("foo", request_headers.get_(Http::CustomHeaders::get().GrpcAcceptEncoding)); } +TEST_P(GrpcWebFilterTest, MediaTypeWithParameter) { + Http::TestRequestHeaderMapImpl request_headers{{"content-type", request_content_type()}, + {":path", "/test.MediaTypes/GetParameter"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers, false)); + Http::TestResponseHeaderMapImpl response_headers{ + {":status", "200"}, + // Set a valid media-type with a specified parameter value. + {"content-type", Http::Headers::get().ContentTypeValues.GrpcWebProto + "; version=1"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.encodeHeaders(response_headers, false)); + Buffer::OwnedImpl data("hello"); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.encodeData(data, false)); +} + TEST_P(GrpcWebFilterTest, Unary) { // Tests request headers. request_headers_.addCopy(Http::Headers::get().ContentType, request_content_type()); @@ -317,6 +577,8 @@ TEST_P(GrpcWebFilterTest, Unary) { // Tests response headers. Http::TestResponseHeaderMapImpl response_headers; response_headers.addCopy(Http::Headers::get().Status, "200"); + response_headers.addCopy(Http::Headers::get().ContentType, + Http::Headers::get().ContentTypeValues.GrpcWebProto); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.encodeHeaders(response_headers, false)); EXPECT_EQ("200", response_headers.get_(Http::Headers::get().Status.get())); if (accept_binary_response()) {