Skip to content

Commit

Permalink
test: fix flake golang test. (envoyproxy#26288)
Browse files Browse the repository at this point in the history 
fixes envoyproxy#26182
it's due to sendData on terminated request could cause segfault.

Signed-off-by: doujiang24 <doujiang24@gmail.com>
Signed-off-by: Ashish Banerjee <ashish.banerjee@solo.io>
  • Loading branch information
@doujiang24 @ashishb-solo
doujiang24 authored and ashishb-solo committed Mar 24, 2023
1 parent df1d6a8 commit fcfe3af
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 48 deletions.
39 changes: 19 additions & 20 deletions contrib/golang/filters/http/test/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
65 changes: 37 additions & 28 deletions contrib/golang/filters/http/test/golang_integration_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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")));
Expand All @@ -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());
Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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

0 comments on commit fcfe3af

Please sign in to comment.