Skip to content

Commit 3a43d39

Browse files
authored
h/2: remove an assertion and add test for malformed frame (envoyproxy#32369)
A malformed frame with 1xx and end_stream should not crash Envoy. The assertion should not hold based on the runtime data.
1 parent ea10ba4 commit 3a43d39

File tree

4 files changed

+37
-2
lines changed

4 files changed

+37
-2
lines changed

source/common/http/conn_manager_impl.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1735,7 +1735,7 @@ void ConnectionManagerImpl::ActiveStream::encode1xxHeaders(ResponseHeaderMap& re
17351735
// Count both the 1xx and follow-up response code in stats.
17361736
chargeStats(response_headers);
17371737

1738-
ENVOY_STREAM_LOG(debug, "encoding 100 continue headers via codec:\n{}", *this, response_headers);
1738+
ENVOY_STREAM_LOG(debug, "encoding 1xx continue headers via codec:\n{}", *this, response_headers);
17391739

17401740
// Now actually encode via the codec.
17411741
response_encoder_->encode1xxHeaders(response_headers);

source/common/http/http2/codec_impl.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,6 @@ void ConnectionImpl::ClientStreamImpl::decodeHeaders() {
532532
!CodeUtility::is1xx(status) || status == enumToInt(Http::Code::SwitchingProtocols);
533533

534534
if (HeaderUtility::isSpecial1xx(*headers)) {
535-
ASSERT(!remote_end_stream_);
536535
response_decoder_.decode1xxHeaders(std::move(headers));
537536
} else {
538537
response_decoder_.decodeHeaders(std::move(headers), sendEndStream());

test/integration/multiplexed_integration_test.cc

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1944,6 +1944,42 @@ class Http2FrameIntegrationTest : public testing::TestWithParam<FrameIntegration
19441944
}
19451945
};
19461946

1947+
TEST_P(Http2FrameIntegrationTest, UpstreamRemoteMalformedFrameEndstreamWith1xxHeader) {
1948+
config_helper_.addConfigModifier(
1949+
[&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
1950+
hcm) -> void { hcm.set_proxy_100_continue(true); });
1951+
beginSession();
1952+
FakeRawConnectionPtr fake_upstream_connection;
1953+
1954+
// Start a request and wait for it to reach the upstream.
1955+
sendFrame(Http2Frame::makeRequest(1, "host", "/path/to/long/url"));
1956+
ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(fake_upstream_connection));
1957+
const Http2Frame settings_frame = Http2Frame::makeEmptySettingsFrame();
1958+
ASSERT_TRUE(fake_upstream_connection->write(std::string(settings_frame)));
1959+
1960+
test_server_->waitForGaugeEq("cluster.cluster_0.upstream_rq_active", 1);
1961+
1962+
// A malformed frame is translated to 103 header with END_STREAM by the underlying codec.
1963+
// Typically we should get a protocol error, but this should not crash Envoy.
1964+
// PAYLOAD_LENGTH: \x05
1965+
// FRAME_TYPE: \x01
1966+
// FLAGS: \x32
1967+
// STREAM_ID: \x01
1968+
// ASCII: \x31, \x30, \x33 for 1, 0, 3 respectively
1969+
const std::vector<uint8_t> header_frame = {
1970+
0x00, 0x00, 0x05, 0x01, 0x32, 0x00, 0x00, 0x00, 0x01, 0x2d, 0xfe, 0xff, 0x01, 0x10,
1971+
0x00, 0x00, 0x05, 0x09, 0x0d, 0x00, 0x00, 0x00, 0x01, 0x09, 0x03, 0x31, 0x30, 0x33};
1972+
const std::string header_frame_str(reinterpret_cast<const char*>(header_frame.data()),
1973+
header_frame.size());
1974+
ASSERT_TRUE(fake_upstream_connection->write(header_frame_str));
1975+
1976+
const Http2Frame response = readFrame();
1977+
EXPECT_EQ(Http2Frame::Type::Headers, response.type());
1978+
1979+
tcp_client_->close();
1980+
test_server_->waitForGaugeEq("http.config_test.downstream_rq_active", 0);
1981+
}
1982+
19471983
TEST_P(Http2FrameIntegrationTest, MaxConcurrentStreamsIsRespected) {
19481984
const int kTotalRequests = 101;
19491985
config_helper_.addConfigModifier(

0 commit comments

Comments
 (0)