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

Do not 503 on Upgrade: h2c instead remove the header and ignore. #7981

Merged
merged 38 commits into from
Sep 4, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
a27ef3f
Do not 503 on Upgrade: h2c instead remove the header and ignore.
jplevyak Aug 20, 2019
36cd5ca
Add release note.
jplevyak Aug 20, 2019
66243c3
Address comments.
jplevyak Aug 20, 2019
b516bd7
Address comments.
jplevyak Aug 21, 2019
215e3aa
Address comments.
jplevyak Aug 21, 2019
b563518
Address comments.
jplevyak Aug 21, 2019
b5ea6c6
Address comments.
jplevyak Aug 21, 2019
34632d5
Address comments.
jplevyak Aug 21, 2019
92726a5
Address comments.
jplevyak Aug 21, 2019
a82cc9d
Address comments.
jplevyak Aug 21, 2019
30ef294
Address comments.
jplevyak Aug 21, 2019
9d3a216
Address comments.
jplevyak Aug 21, 2019
1e1ab46
Address comments.
jplevyak Aug 21, 2019
11dbe74
Merge remote-tracking branch 'envoyproxy/master' into ignore-upgrade-h2c
jplevyak Aug 21, 2019
ad6c698
Address comments.
jplevyak Aug 22, 2019
51e0372
Address comments.
jplevyak Aug 22, 2019
069b642
Address comments.
jplevyak Aug 22, 2019
a2ac07f
Address comments.
jplevyak Aug 22, 2019
756224b
Address comments.
jplevyak Aug 22, 2019
5d0a1a6
Address comments.
jplevyak Aug 22, 2019
561650a
Fix clang-tidy error.
jplevyak Aug 22, 2019
427b0aa
Merge remote-tracking branch 'envoyproxy/master' into ignore-upgrade-h2c
jplevyak Aug 26, 2019
cecc97f
Address comments.
jplevyak Aug 26, 2019
53134ef
Rewritten to use <algorithm> as requested.
jplevyak Aug 27, 2019
7378c2e
Merge remote-tracking branch 'envoyproxy/master' into ignore-upgrade-h2c
jplevyak Aug 27, 2019
150f6fd
Fix test.
jplevyak Aug 27, 2019
bba47af
Address comments.
jplevyak Aug 27, 2019
f6c0797
Address comments.
jplevyak Aug 27, 2019
84a2f7b
Address comments.
jplevyak Aug 27, 2019
b79eb9c
Address comments.
jplevyak Aug 27, 2019
46b6993
Remove unnecessary include.
jplevyak Aug 27, 2019
973aa7f
Merge remote-tracking branch 'envoyproxy/master' into ignore-upgrade-h2c
jplevyak Aug 28, 2019
a2e7210
Merge remote-tracking branch 'envoyproxy/master' into ignore-upgrade-h2c
jplevyak Aug 28, 2019
e230448
Address comments.
jplevyak Aug 29, 2019
f97827f
Format
jplevyak Aug 29, 2019
9663c31
Merge remote-tracking branch 'envoyproxy/master' into ignore-upgrade-h2c
jplevyak Aug 30, 2019
73a2b9c
Merge remote-tracking branch 'envoyproxy/master' into ignore-upgrade-h2c
jplevyak Sep 4, 2019
af9d672
Merge remote-tracking branch 'envoyproxy/master' into ignore-upgrade-h2c
jplevyak Sep 4, 2019
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
Prev Previous commit
Next Next commit
Address comments.
Signed-off-by: John Plevyak <[email protected]>
  • Loading branch information
jplevyak committed Aug 21, 2019
commit 1e1ab469fd6512e252b5fff66f0009b02201bd3e
1 change: 1 addition & 0 deletions source/common/http/headers.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ class HeaderValues {

struct {
const std::string Close{"close"};
const std::string Http2Settings{"http2-settings"};
const std::string KeepAlive{"keep-alive"};
const std::string Upgrade{"upgrade"};
} ConnectionValues;
Expand Down
27 changes: 22 additions & 5 deletions source/common/http/http1/codec_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -476,11 +476,28 @@ int ConnectionImpl::onHeadersCompleteBase() {
Http::Headers::get().UpgradeValues.H2c)) {
ENVOY_CONN_LOG(trace, "removing unsupported h2c upgrade headers.", connection_);
current_header_map_->removeUpgrade();
// TODO(alyssawilk): remove the individual value "upgrade".
if (current_header_map_->Connection() &&
absl::EqualsIgnoreCase(current_header_map_->Connection()->value().getStringView(),
Http::Headers::get().ConnectionValues.Upgrade)) {
current_header_map_->removeConnection();
if (current_header_map_->Connection()) {
auto values = Envoy::StringUtil::splitToken(
current_header_map_->Connection()->value().getStringView(), ",");
std::string new_values;
for (auto& v : values) {
auto value = StringUtil::toLower(StringUtil::trim(v));
if (value == Http::Headers::get().ConnectionValues.Upgrade) {
continue;
}
if (value == Http::Headers::get().ConnectionValues.Http2Settings) {
continue;
}
if (!new_values.empty()) {
new_values += ", ";
}
new_values.append(value.data(), value.size());
}
if (new_values.empty()) {
current_header_map_->removeConnecton();
} else {
current_header_map_->Connection()->value(new_values);
}
}
current_header_map_->remove(Headers::get().Http2Settings);
} else {
Expand Down
28 changes: 27 additions & 1 deletion test/common/http/http1/codec_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,33 @@ TEST_F(Http1ServerConnectionImplTest, IgnoreUpgradeH2c) {
{":authority", "www.somewhere.com"}, {":path", "/"}, {":method", "GET"}};
Buffer::OwnedImpl buffer(
"GET http://www.somewhere.com/ HTTP/1.1\r\nConnection: "
"upgrade\r\nUpgrade: h2c\r\nHTTP2-Settings: token64\r\nHost: bah\r\n\r\n");
"Upgrade, HTTP2-Settings\r\nUpgrade: h2c\r\nHTTP2-Settings: token64\r\nHost: bah\r\n\r\n");
expectHeadersTest(Protocol::Http11, true, buffer, expected_headers);
}

TEST_F(Http1ServerConnectionImplTest, IgnoreUpgradeH2cClose) {
initialize();

TestHeaderMapImpl expected_headers{{":authority", "www.somewhere.com"},
{":path", "/"},
{":method", "GET"},
{"connection", "close"}};
Buffer::OwnedImpl buffer("GET http://www.somewhere.com/ HTTP/1.1\r\nConnection: "
"Upgrade, Close, HTTP2-Settings\r\nUpgrade: h2c\r\nHTTP2-Settings: "
"token64\r\nHost: bah\r\n\r\n");
expectHeadersTest(Protocol::Http11, true, buffer, expected_headers);
}

TEST_F(Http1ServerConnectionImplTest, IgnoreUpgradeH2cCloseEtc) {
initialize();

TestHeaderMapImpl expected_headers{{":authority", "www.somewhere.com"},
{":path", "/"},
{":method", "GET"},
{"connection", "close, etc"}};
Buffer::OwnedImpl buffer("GET http://www.somewhere.com/ HTTP/1.1\r\nConnection: "
"Upgrade, Close, HTTP2-Settings, Etc\r\nUpgrade: h2c\r\nHTTP2-Settings: "
"token64\r\nHost: bah\r\n\r\n");
expectHeadersTest(Protocol::Http11, true, buffer, expected_headers);
}

Expand Down