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.

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

http: keep trailers TE header instead of removing it #32255

Merged
merged 23 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
56 changes: 32 additions & 24 deletions source/common/http/conn_manager_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,30 +93,7 @@ ConnectionManagerUtility::MutateRequestHeadersResult ConnectionManagerUtility::m
request_headers.removeConnection();
request_headers.removeUpgrade();

if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.sanitize_te")) {
std::string te_header = request_headers.getTEValue();

if (!te_header.empty()) {
bool has_trailers_te = false;

std::vector<std::string> te_values = absl::StrSplit(, ",");
for (const auto& teValue : te_values) {
std::vector<std::string> parts = absl::StrSplit(teValue, ";"); // Handles cases like "chunked, trailers;q=0.5"
std::string value = absl::StripAsciiWhitespace(parts[0]);

if (value == Http::Headers::get().TEValues.Trailers) {
has_trailers_te = true;
break;
}
}

if (has_trailers_te) {
request_headers.setTE(Http::Headers::get().TEValues.Trailers);
} else {
request_headers.removeTE();
}
}
}
sanitizeTEHeader(request_headers);
}

// Clean proxy headers.
Expand Down Expand Up @@ -314,6 +291,37 @@ ConnectionManagerUtility::MutateRequestHeadersResult ConnectionManagerUtility::m
return {final_remote_address, absl::nullopt};
}

void ConnectionManagerUtility::sanitizeTEHeader(RequestHeaderMap& request_headers) {
if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.sanitize_te")) {
return;
}

std::string te_header = request_headers.getTEValue();
if (te_header.empty()) {
return;
}

bool has_trailers_te = false;

std::vector<std::string> te_values = absl::StrSplit(, ",");
for (const auto& teValue : te_values) {
quantumsheep marked this conversation as resolved.
Show resolved Hide resolved
std::vector<std::string> parts =
absl::StrSplit(teValue, ";"); // Handles cases like "chunked, trailers;q=0.5"
std::string value = absl::StripAsciiWhitespace(parts[0]);

if (value == Http::Headers::get().TEValues.Trailers) {
has_trailers_te = true;
quantumsheep marked this conversation as resolved.
Show resolved Hide resolved
break;
}
}

if (has_trailers_te) {
request_headers.setTE(Http::Headers::get().TEValues.Trailers);
} else {
request_headers.removeTE();
}
}

void ConnectionManagerUtility::cleanInternalHeaders(
RequestHeaderMap& request_headers, bool edge_request,
const std::list<Http::LowerCaseString>& internal_only_headers) {
Expand Down
1 change: 1 addition & 0 deletions source/common/http/conn_manager_utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ class ConnectionManagerUtility {
static void mutateXfccRequestHeader(RequestHeaderMap& request_headers,
Network::Connection& connection,
ConnectionManagerConfig& config);
static void sanitizeTEHeader(RequestHeaderMap& request_headers);
static void cleanInternalHeaders(RequestHeaderMap& request_headers, bool edge_request,
const std::list<Http::LowerCaseString>& internal_only_headers);
};
Expand Down
34 changes: 34 additions & 0 deletions test/common/http/conn_manager_utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2237,5 +2237,39 @@ TEST_F(ConnectionManagerUtilityTest, DoNotOverwriteXForwardedPortFromUntrustedHo
EXPECT_EQ("80", headers.getForwardedPortValue());
}

// Verify when TE header is present, the value should be preserved only if it's equal to "trailers".
TEST_F(ConnectionManagerUtilityTest, KeepTrailersTEHeaderSimple) {
TestScopedRuntime scoped_runtime;
scoped_runtime.mergeValues({{"envoy.reloadable_features.sanitize_te", "true"}});

TestRequestHeaderMapImpl headers{{"te", "trailers"}};
callMutateRequestHeaders(headers, Protocol::Http2);

EXPECT_EQ("trailers", headers.getTEValue());
}

// Verify when TE header is present, the value should be preserved only if it contains "trailers".
TEST_F(ConnectionManagerUtilityTest, KeepTrailersTEHeaderMultipleValuesAndWeigthted) {
TestScopedRuntime scoped_runtime;
scoped_runtime.mergeValues({{"envoy.reloadable_features.sanitize_te", "true"}});

TestRequestHeaderMapImpl headers{{"te", "chunked;q=0.8 , trailers ;q=0.5,deflate "}};
callMutateRequestHeaders(headers, Protocol::Http2);

EXPECT_EQ("trailers", headers.getTEValue());
}

// Verify when TE header is present, the value should be discarded if it doesn't contains
// "trailers".
TEST_F(ConnectionManagerUtilityTest, DiscardTEHeaderWithoutTrailers) {
TestScopedRuntime scoped_runtime;
scoped_runtime.mergeValues({{"envoy.reloadable_features.sanitize_te", "true"}});

TestRequestHeaderMapImpl headers{{"te", "gzip"}};
callMutateRequestHeaders(headers, Protocol::Http2);

EXPECT_EQ("", headers.getTEValue());
}

} // namespace Http
} // namespace Envoy