diff --git a/source/common/grpc/common.cc b/source/common/grpc/common.cc index 72abbbefcaa0..3cbe02aabffb 100644 --- a/source/common/grpc/common.cc +++ b/source/common/grpc/common.cc @@ -284,44 +284,6 @@ Common::prepareHeaders(const std::string& host_name, const std::string& service_ return message; } -void Common::checkForHeaderOnlyError(Http::ResponseMessage& http_response) { - // First check for grpc-status in headers. If it is here, we have an error. - absl::optional grpc_status_code = - Common::getGrpcStatus(http_response.headers()); - if (!grpc_status_code) { - return; - } - - if (grpc_status_code.value() == Status::WellKnownGrpcStatus::InvalidCode) { - throw Exception(absl::optional(), "bad grpc-status header"); - } - - throw Exception(grpc_status_code.value(), Common::getGrpcMessage(http_response.headers())); -} - -void Common::validateResponse(Http::ResponseMessage& http_response) { - if (Http::Utility::getResponseStatus(http_response.headers()) != enumToInt(Http::Code::OK)) { - throw Exception(absl::optional(), "non-200 response code"); - } - - checkForHeaderOnlyError(http_response); - - // Check for existence of trailers. - if (!http_response.trailers()) { - throw Exception(absl::optional(), "no response trailers"); - } - - absl::optional grpc_status_code = - Common::getGrpcStatus(*http_response.trailers()); - if (!grpc_status_code || grpc_status_code.value() < 0) { - throw Exception(absl::optional(), "bad grpc-status trailer"); - } - - if (grpc_status_code.value() != 0) { - throw Exception(grpc_status_code.value(), Common::getGrpcMessage(*http_response.trailers())); - } -} - const std::string& Common::typeUrlPrefix() { CONSTRUCT_ON_FIRST_USE(std::string, "type.googleapis.com"); } diff --git a/source/common/grpc/common.h b/source/common/grpc/common.h index 0868433e7623..e9f7117eec2e 100644 --- a/source/common/grpc/common.h +++ b/source/common/grpc/common.h @@ -177,11 +177,6 @@ class Common { const std::string& method_name, const absl::optional& timeout); - /** - * Basic validation of gRPC response, @throws Grpc::Exception in case of non successful response. - */ - static void validateResponse(Http::ResponseMessage& http_response); - /** * @return const std::string& type URL prefix. */ @@ -223,8 +218,6 @@ class Common { static absl::optional resolveServiceAndMethod(const Http::HeaderEntry* path); private: - static void checkForHeaderOnlyError(Http::ResponseMessage& http_response); - static constexpr size_t MAX_GRPC_TIMEOUT_VALUE = 99999999; }; diff --git a/test/common/grpc/common_test.cc b/test/common/grpc/common_test.cc index 69176d635efe..209ba10a342a 100644 --- a/test/common/grpc/common_test.cc +++ b/test/common/grpc/common_test.cc @@ -337,61 +337,6 @@ TEST(GrpcContextTest, IsProtobufRequestHeader) { EXPECT_FALSE(Common::isProtobufRequestHeaders(is_not)); } -TEST(GrpcContextTest, ValidateResponse) { - { - Http::ResponseMessageImpl response( - Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "200"}}}); - response.trailers( - Http::ResponseTrailerMapPtr{new Http::TestResponseTrailerMapImpl{{"grpc-status", "0"}}}); - EXPECT_NO_THROW(Common::validateResponse(response)); - } - { - Http::ResponseMessageImpl response( - Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "503"}}}); - EXPECT_THROW_WITH_MESSAGE(Common::validateResponse(response), Exception, - "non-200 response code"); - } - { - Http::ResponseMessageImpl response( - Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "200"}}}); - response.trailers( - Http::ResponseTrailerMapPtr{new Http::TestResponseTrailerMapImpl{{"grpc-status", "100"}}}); - EXPECT_THROW_WITH_MESSAGE(Common::validateResponse(response), Exception, - "bad grpc-status trailer"); - } - { - Http::ResponseMessageImpl response( - Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "200"}}}); - response.trailers( - Http::ResponseTrailerMapPtr{new Http::TestResponseTrailerMapImpl{{"grpc-status", "4"}}}); - EXPECT_THROW_WITH_MESSAGE(Common::validateResponse(response), Exception, ""); - } - { - Http::ResponseMessageImpl response( - Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "200"}}}); - response.trailers(Http::ResponseTrailerMapPtr{new Http::TestResponseTrailerMapImpl{ - {"grpc-status", "4"}, {"grpc-message", "custom error"}}}); - EXPECT_THROW_WITH_MESSAGE(Common::validateResponse(response), Exception, "custom error"); - } - { - Http::ResponseMessageImpl response(Http::ResponseHeaderMapPtr{ - new Http::TestResponseHeaderMapImpl{{":status", "200"}, {"grpc-status", "100"}}}); - EXPECT_THROW_WITH_MESSAGE(Common::validateResponse(response), Exception, - "bad grpc-status header"); - } - { - Http::ResponseMessageImpl response(Http::ResponseHeaderMapPtr{ - new Http::TestResponseHeaderMapImpl{{":status", "200"}, {"grpc-status", "4"}}}); - EXPECT_THROW_WITH_MESSAGE(Common::validateResponse(response), Exception, ""); - } - { - Http::ResponseMessageImpl response( - Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{ - {":status", "200"}, {"grpc-status", "4"}, {"grpc-message", "custom error"}}}); - EXPECT_THROW_WITH_MESSAGE(Common::validateResponse(response), Exception, "custom error"); - } -} - // Ensure that the correct gPRC header is constructed for a Buffer::Instance. TEST(GrpcContextTest, PrependGrpcFrameHeader) { auto buffer = std::make_unique(); diff --git a/tools/code_format/config.yaml b/tools/code_format/config.yaml index 24305c115832..615aca0ccd41 100644 --- a/tools/code_format/config.yaml +++ b/tools/code_format/config.yaml @@ -135,7 +135,6 @@ paths: - source/common/secret/secret_manager_impl.cc - source/common/secret/sds_api.cc - source/common/grpc/async_client_manager_impl.cc - - source/common/grpc/common.cc - source/common/grpc/google_grpc_utils.cc - source/common/tcp_proxy/tcp_proxy.cc - source/common/config/subscription_factory_impl.cc