Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

connect: regression testing bugfix #34112

Merged
merged 4 commits into from
May 16, 2024
Merged

connect: regression testing bugfix #34112

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
983d075
connect: working around balsa bug
alyssawilk May 13, 2024
65c5c49
comments
alyssawilk May 13, 2024
5a8e4f1
relying on bnc fix
alyssawilk May 14, 2024
e76725a
Merge branch 'main' into connect
alyssawilk May 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions test/extensions/transport_sockets/http_11_proxy/connect_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,17 @@ name: envoy.clusters.dynamic_forward_proxy
}
}

void stripConnectUpgradeAndRespond() {
void stripConnectUpgradeAndRespond(bool include_content_length = false) {
// Strip the CONNECT upgrade.
std::string prefix_data;
const std::string hostname(default_request_headers_.getHostValue());
ASSERT_TRUE(fake_upstream_connection_->waitForInexactRawData("\r\n\r\n", &prefix_data));
EXPECT_EQ(absl::StrCat("CONNECT ", hostname, ":443 HTTP/1.1\r\n\r\n"), prefix_data);

absl::string_view content_length = include_content_length ? "Content-Length: 0\r\n" : "";
// Ship the CONNECT response.
fake_upstream_connection_->writeRawData("HTTP/1.1 200 OK\r\n\r\n");
fake_upstream_connection_->writeRawData(
absl::StrCat("HTTP/1.1 200 OK\r\n", content_length, "\r\n"));
}
bool use_alpn_ = false;
bool try_http3_ = false;
Expand Down Expand Up @@ -450,7 +452,8 @@ TEST_P(Http11ConnectHttpIntegrationTest, DfpNoProxyAddress) {
EXPECT_THAT(waitForAccessLog(access_log_name_), testing::HasSubstr("dns_resolution_failure"));
}

TEST_P(Http11ConnectHttpIntegrationTest, DfpWithProxyAddress) {
// Regression tests https://github.com/envoyproxy/envoy/issues/34096
TEST_P(Http11ConnectHttpIntegrationTest, DfpWithProxyAddressAndContentLength) {
Comment on lines +455 to +456
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the test without content length also still exist?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't get a response on this comment. Seems like both paths should be getting tested.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah sorry we do that by default - all th eother tests test this with stripConnectUpgradeAndRespond()

addDfpConfig();

initialize();
Expand All @@ -470,7 +473,7 @@ TEST_P(Http11ConnectHttpIntegrationTest, DfpWithProxyAddress) {
// The request should be sent to fake upstream 1, as DNS lookup is bypassed due to the
// connect-proxy header.
ASSERT_TRUE(fake_upstreams_[1]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_));
stripConnectUpgradeAndRespond();
stripConnectUpgradeAndRespond(true);

ASSERT_TRUE(fake_upstream_connection_->readDisable(false));
ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_));
Expand Down
Loading