Reject 1xx responses with nonzero content-length in oghttp2.
This CL updates oghttp2 to:
- Discard incoming HEADERS (e.g., trailers or final headers after 1xx
headers) for streams that have been locally marked for reset.
- Reject 1xx responses with nonzero content-length header values in the same
way as 204 responses are rejected.
This change increases parity with nghttp2.
PiperOrigin-RevId: 423338319
diff --git a/http2/adapter/header_validator.cc b/http2/adapter/header_validator.cc
index d4c5ee4..4111652 100644
--- a/http2/adapter/header_validator.cc
+++ b/http2/adapter/header_validator.cc
@@ -162,11 +162,14 @@
return false;
}
- // TODO(diannahu): Do the same for 1xx responses.
if (status_ == "204" && value != "0") {
// There should be no body in a "204 No Content" response.
return false;
}
+ if (!status_.empty() && status_[0] == '1' && value != "0") {
+ // There should also be no body in a 1xx response.
+ return false;
+ }
size_t content_length = 0;
const bool valid = absl::SimpleAtoi(value, &content_length);
diff --git a/http2/adapter/header_validator_test.cc b/http2/adapter/header_validator_test.cc
index addb35a..ba6d06a 100644
--- a/http2/adapter/header_validator_test.cc
+++ b/http2/adapter/header_validator_test.cc
@@ -307,6 +307,42 @@
v.ValidateSingleHeader("content-length", "1"));
}
+TEST(HeaderValidatorTest, Response100) {
+ HeaderValidator v;
+
+ v.StartHeaderBlock();
+ EXPECT_EQ(HeaderValidator::HEADER_OK,
+ v.ValidateSingleHeader(":status", "100"));
+ EXPECT_EQ(HeaderValidator::HEADER_OK,
+ v.ValidateSingleHeader("x-content", "is not present"));
+ EXPECT_TRUE(v.FinishHeaderBlock(HeaderType::RESPONSE));
+}
+
+TEST(HeaderValidatorTest, Response100WithContentLengthZero) {
+ HeaderValidator v;
+
+ v.StartHeaderBlock();
+ EXPECT_EQ(HeaderValidator::HEADER_OK,
+ v.ValidateSingleHeader(":status", "100"));
+ EXPECT_EQ(HeaderValidator::HEADER_OK,
+ v.ValidateSingleHeader("x-content", "is not present"));
+ EXPECT_EQ(HeaderValidator::HEADER_OK,
+ v.ValidateSingleHeader("content-length", "0"));
+ EXPECT_TRUE(v.FinishHeaderBlock(HeaderType::RESPONSE));
+}
+
+TEST(HeaderValidatorTest, Response100WithContentLength) {
+ HeaderValidator v;
+
+ v.StartHeaderBlock();
+ EXPECT_EQ(HeaderValidator::HEADER_OK,
+ v.ValidateSingleHeader(":status", "100"));
+ EXPECT_EQ(HeaderValidator::HEADER_OK,
+ v.ValidateSingleHeader("x-content", "is not present"));
+ EXPECT_EQ(HeaderValidator::HEADER_FIELD_INVALID,
+ v.ValidateSingleHeader("content-length", "1"));
+}
+
TEST(HeaderValidatorTest, ResponseTrailerPseudoHeaders) {
HeaderValidator v;
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index c68dad5..b16241b 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -386,6 +386,75 @@
spdy::SpdyFrameType::RST_STREAM}));
}
+TEST(NgHttp2AdapterTest, ClientRejects100HeadersWithContentLength) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
+
+ testing::InSequence s;
+
+ const std::vector<Header> headers1 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}});
+
+ const int32_t stream_id1 = adapter->SubmitRequest(headers1, nullptr, nullptr);
+ ASSERT_GT(stream_id1, 0);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id1, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id1, _, 0x5, 0));
+
+ int result = adapter->Send();
+ EXPECT_EQ(0, result);
+ visitor.Clear();
+
+ const std::string stream_frames =
+ TestFrameSequence()
+ .ServerPreface()
+ .Headers(1, {{":status", "100"}, {"content-length", "42"}},
+ /*fin=*/false)
+ .Headers(1,
+ {{":status", "200"},
+ {"server", "my-fake-server"},
+ {"date", "Tue, 6 Apr 2021 12:54:01 GMT"}},
+ /*fin=*/true)
+ .Serialize();
+
+ // Server preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":status", "100"));
+ EXPECT_CALL(
+ visitor,
+ OnErrorDebug("Invalid HTTP header field was received: frame type: 1, "
+ "stream: 1, name: [content-length], value: [42]"));
+ EXPECT_CALL(
+ visitor,
+ OnInvalidFrame(1, Http2VisitorInterface::InvalidFrameError::kHttpHeader));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5));
+
+ const int64_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_EQ(stream_frames.size(), static_cast<size_t>(stream_result));
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 1, _, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(RST_STREAM, 1, _, 0x0,
+ static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR)));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::PROTOCOL_ERROR));
+
+ EXPECT_TRUE(adapter->want_write());
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::RST_STREAM}));
+}
+
TEST(NgHttp2AdapterTest, ClientHandles204WithContent) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index 910fbc5..2d0adb7 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -469,6 +469,73 @@
spdy::SpdyFrameType::RST_STREAM}));
}
+TEST(OgHttp2AdapterClientTest, ClientRejects100HeadersWithContentLength) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+ testing::InSequence s;
+
+ const std::vector<Header> headers1 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}});
+
+ const int32_t stream_id1 = adapter->SubmitRequest(headers1, nullptr, nullptr);
+ ASSERT_GT(stream_id1, 0);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id1, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id1, _, 0x5, 0));
+
+ int result = adapter->Send();
+ EXPECT_EQ(0, result);
+ visitor.Clear();
+
+ const std::string stream_frames =
+ TestFrameSequence()
+ .ServerPreface()
+ .Headers(1, {{":status", "100"}, {"content-length", "42"}},
+ /*fin=*/false)
+ .Headers(1,
+ {{":status", "200"},
+ {"server", "my-fake-server"},
+ {"date", "Tue, 6 Apr 2021 12:54:01 GMT"}},
+ /*fin=*/true)
+ .Serialize();
+
+ // Server preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":status", "100"));
+ EXPECT_CALL(
+ visitor,
+ OnInvalidFrame(1, Http2VisitorInterface::InvalidFrameError::kHttpHeader));
+
+ const int64_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_EQ(stream_frames.size(), static_cast<size_t>(stream_result));
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 1, _, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(RST_STREAM, 1, _, 0x0,
+ static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR)));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR));
+
+ EXPECT_TRUE(adapter->want_write());
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::RST_STREAM}));
+}
+
TEST(OgHttp2AdapterClientTest, ClientHandles204WithContent) {
DataSavingVisitor visitor;
OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index 256e23d..2467009 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -1119,7 +1119,7 @@
spdy::SpdyHeadersHandlerInterface* OgHttp2Session::OnHeaderFrameStart(
spdy::SpdyStreamId stream_id) {
auto it = stream_map_.find(stream_id);
- if (it != stream_map_.end()) {
+ if (it != stream_map_.end() && !streams_reset_.contains(stream_id)) {
headers_handler_.set_stream_id(stream_id);
headers_handler_.set_header_type(
NextHeaderType(it->second.received_header_type));
