From ff521e07d1781b401b5415c606c435da0ea86633 Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Thu, 23 Mar 2023 13:29:33 -0700 Subject: [PATCH] test: fix flake golang test. (#26288) fixes #26182 it's due to sendData on terminated request could cause segfault. Signed-off-by: doujiang24 --- contrib/golang/filters/http/test/BUILD | 39 ++++++----- .../http/test/golang_integration_test.cc | 65 +++++++++++-------- 2 files changed, 56 insertions(+), 48 deletions(-) diff --git a/contrib/golang/filters/http/test/BUILD b/contrib/golang/filters/http/test/BUILD index 641f0b63732a..300833ccd7a4 100644 --- a/contrib/golang/filters/http/test/BUILD +++ b/contrib/golang/filters/http/test/BUILD @@ -48,26 +48,25 @@ envoy_cc_test( ], ) -#disabled: https://github.com/envoyproxy/envoy/issues/26182 -#envoy_cc_test( -# name = "golang_integration_test", -# srcs = ["golang_integration_test.cc"], -# data = [ -# "//contrib/golang/filters/http/test/test_data/basic:filter.so", -# "//contrib/golang/filters/http/test/test_data/echo:filter.so", -# "//contrib/golang/filters/http/test/test_data/passthrough:filter.so", -# "//contrib/golang/filters/http/test/test_data/routeconfig:filter.so", -# ], -# env = {"GODEBUG": "cgocheck=0"}, -# deps = [ -# "//contrib/golang/filters/http/source:config", -# "//source/exe:main_common_lib", -# "//test/config:v2_link_hacks", -# "//test/integration:http_integration_lib", -# "//test/test_common:utility_lib", -# "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", -# ], -#) +envoy_cc_test( + name = "golang_integration_test", + srcs = ["golang_integration_test.cc"], + data = [ + "//contrib/golang/filters/http/test/test_data/basic:filter.so", + "//contrib/golang/filters/http/test/test_data/echo:filter.so", + "//contrib/golang/filters/http/test/test_data/passthrough:filter.so", + "//contrib/golang/filters/http/test/test_data/routeconfig:filter.so", + ], + env = {"GODEBUG": "cgocheck=0"}, + deps = [ + "//contrib/golang/filters/http/source:config", + "//source/exe:main_common_lib", + "//test/config:v2_link_hacks", + "//test/integration:http_integration_lib", + "//test/test_common:utility_lib", + "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", + ], +) envoy_proto_library( name = "golang_filter_fuzz_proto", diff --git a/contrib/golang/filters/http/test/golang_integration_test.cc b/contrib/golang/filters/http/test/golang_integration_test.cc index bc3d23f981a2..f8eb2e2ae120 100644 --- a/contrib/golang/filters/http/test/golang_integration_test.cc +++ b/contrib/golang/filters/http/test/golang_integration_test.cc @@ -303,19 +303,24 @@ name: golang auto encoder_decoder = codec_client_->startRequest(request_headers); Http::RequestEncoder& request_encoder = encoder_decoder.first; auto response = std::move(encoder_decoder.second); - codec_client_->sendData(request_encoder, "hello", false); - codec_client_->sendData(request_encoder, "world", true); - // need upstream request then when not seen decode-, which means send local reply in encode - // phases. - if (path.find("decode-") == std::string::npos) { + // do not sendData when phase is decode-header, + // since the request may be terminated before sendData. + if (phase != "decode-header") { + codec_client_->sendData(request_encoder, "hello", true); + } + + // need upstream request when send local reply in encode phases. + if (phase == "encode-header" || phase == "encode-data") { waitForNextUpstreamRequest(); Http::TestResponseHeaderMapImpl response_headers{{":status", "200"}}; upstream_request_->encodeHeaders(response_headers, false); - Buffer::OwnedImpl response_data1("good"); - upstream_request_->encodeData(response_data1, false); - Buffer::OwnedImpl response_data2("bye"); - upstream_request_->encodeData(response_data2, true); + + // do not sendData when phase is encode-header + if (phase == "encode-data") { + Buffer::OwnedImpl response_data("bye"); + upstream_request_->encodeData(response_data, true); + } } ASSERT_TRUE(response->waitForEndStream()); @@ -356,7 +361,7 @@ name: golang cleanup(); } - void testPanicRecover(std::string path) { + void testPanicRecover(std::string path, std::string phase) { initializeBasicFilter(BASIC); codec_client_ = makeHttpConnection(makeClientConnection(lookupPort("http"))); @@ -366,18 +371,24 @@ name: golang auto encoder_decoder = codec_client_->startRequest(request_headers); Http::RequestEncoder& request_encoder = encoder_decoder.first; auto response = std::move(encoder_decoder.second); - codec_client_->sendData(request_encoder, "hello", false); - codec_client_->sendData(request_encoder, "world", true); - // need upstream request then when not seen decode- - if (path.find("decode-") == std::string::npos) { + // do not sendData when phase is decode-header, + // since the request may be terminated before sendData. + if (phase != "decode-header") { + codec_client_->sendData(request_encoder, "hello", true); + } + + // need upstream request when send local reply in encode phases. + if (phase == "encode-header" || phase == "encode-data") { waitForNextUpstreamRequest(); Http::TestResponseHeaderMapImpl response_headers{{":status", "200"}}; upstream_request_->encodeHeaders(response_headers, false); - Buffer::OwnedImpl response_data1("good"); - upstream_request_->encodeData(response_data1, false); - Buffer::OwnedImpl response_data2("bye"); - upstream_request_->encodeData(response_data2, true); + + // do not sendData when phase is encode-header + if (phase == "encode-data") { + Buffer::OwnedImpl response_data("bye"); + upstream_request_->encodeData(response_data, true); + } } ASSERT_TRUE(response->waitForEndStream()); @@ -425,9 +436,7 @@ TEST_P(GolangIntegrationTest, Echo) { {":method", "POST"}, {":path", path}, {":scheme", "http"}, {":authority", "test.com"}}; auto encoder_decoder = codec_client_->startRequest(request_headers); - Http::RequestEncoder& request_encoder = encoder_decoder.first; auto response = std::move(encoder_decoder.second); - codec_client_->sendData(request_encoder, "helloworld", true); ASSERT_TRUE(response->waitForEndStream()); @@ -568,9 +577,7 @@ name: envoy.filters.http.lua {"x-test-header-0", "foo"}}; auto encoder_decoder = codec_client_->startRequest(request_headers); - Http::RequestEncoder& request_encoder = encoder_decoder.first; auto response = std::move(encoder_decoder.second); - codec_client_->sendData(request_encoder, "hello", true); ASSERT_TRUE(response->waitForEndStream()); @@ -612,30 +619,32 @@ TEST_P(GolangIntegrationTest, RouteConfig_Route) { // Out of range in decode header phase TEST_P(GolangIntegrationTest, PanicRecover_DecodeHeader) { - testPanicRecover("/test?panic=decode-header"); + testPanicRecover("/test?panic=decode-header", "decode-header"); } // Out of range in decode header phase with async mode TEST_P(GolangIntegrationTest, PanicRecover_DecodeHeader_Async) { - testPanicRecover("/test?async=1&panic=decode-header"); + testPanicRecover("/test?async=1&panic=decode-header", "decode-header"); } // Out of range in decode data phase TEST_P(GolangIntegrationTest, PanicRecover_DecodeData) { - testPanicRecover("/test?panic=decode-data"); + testPanicRecover("/test?panic=decode-data", "decode-data"); } // Out of range in decode data phase with async mode & sleep TEST_P(GolangIntegrationTest, PanicRecover_DecodeData_Async) { - testPanicRecover("/test?async=1&sleep=1&panic=decode-data"); + testPanicRecover("/test?async=1&sleep=1&panic=decode-data", "decode-data"); } // Out of range in encode data phase with async mode & sleep TEST_P(GolangIntegrationTest, PanicRecover_EncodeData_Async) { - testPanicRecover("/test?async=1&sleep=1&panic=encode-data"); + testPanicRecover("/test?async=1&sleep=1&panic=encode-data", "encode-data"); } // Panic ErrInvalidPhase -TEST_P(GolangIntegrationTest, PanicRecover_BadAPI) { testPanicRecover("/test?badapi=decode-data"); } +TEST_P(GolangIntegrationTest, PanicRecover_BadAPI) { + testPanicRecover("/test?badapi=decode-data", "decode-data"); +} } // namespace Envoy