Adds validation for 204 responses, which should carry no body.
PiperOrigin-RevId: 416414380
diff --git a/http2/adapter/header_validator.cc b/http2/adapter/header_validator.cc
index 321e958..0b042b1 100644
--- a/http2/adapter/header_validator.cc
+++ b/http2/adapter/header_validator.cc
@@ -92,6 +92,9 @@
method_ = std::string(value);
}
pseudo_headers_.push_back(std::string(key));
+ } else if (key == "content-length" && status_ == "204" && value != "0") {
+ // There should be no body in a "204 No Content" response.
+ return HEADER_FIELD_INVALID;
}
return HEADER_OK;
}
diff --git a/http2/adapter/header_validator_test.cc b/http2/adapter/header_validator_test.cc
index 35750d3..1f202d9 100644
--- a/http2/adapter/header_validator_test.cc
+++ b/http2/adapter/header_validator_test.cc
@@ -222,6 +222,42 @@
}
}
+TEST(HeaderValidatorTest, Response204) {
+ HeaderValidator v;
+
+ v.StartHeaderBlock();
+ EXPECT_EQ(HeaderValidator::HEADER_OK,
+ v.ValidateSingleHeader(":status", "204"));
+ EXPECT_EQ(HeaderValidator::HEADER_OK,
+ v.ValidateSingleHeader("x-content", "is not present"));
+ EXPECT_TRUE(v.FinishHeaderBlock(HeaderType::RESPONSE));
+}
+
+TEST(HeaderValidatorTest, Response204WithContentLengthZero) {
+ HeaderValidator v;
+
+ v.StartHeaderBlock();
+ EXPECT_EQ(HeaderValidator::HEADER_OK,
+ v.ValidateSingleHeader(":status", "204"));
+ 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, Response204WithContentLength) {
+ HeaderValidator v;
+
+ v.StartHeaderBlock();
+ EXPECT_EQ(HeaderValidator::HEADER_OK,
+ v.ValidateSingleHeader(":status", "204"));
+ 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/http2_visitor_interface.h b/http2/adapter/http2_visitor_interface.h
index 6d843a3..8e39ef4 100644
--- a/http2/adapter/http2_visitor_interface.h
+++ b/http2/adapter/http2_visitor_interface.h
@@ -121,7 +121,10 @@
HEADER_CONNECTION_ERROR,
// The application rejects the header and requests the stream be reset.
HEADER_RST_STREAM,
- // The header is a violation of HTTP messaging semantics and will be reset
+ // The header field is invalid and will be reset with error code
+ // PROTOCOL_ERROR.
+ HEADER_FIELD_INVALID,
+ // The headers are a violation of HTTP messaging semantics and will be reset
// with error code PROTOCOL_ERROR.
HEADER_HTTP_MESSAGING,
};
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index 147d991..6410b97 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -326,6 +326,70 @@
spdy::SpdyFrameType::RST_STREAM}));
}
+TEST(OgHttp2AdapterClientTest, ClientHandles204WithContent) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
+
+ testing::InSequence s;
+
+ const std::vector<const 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", "204"}, {"content-length", "2"}},
+ /*fin=*/false)
+ .Data(1, "hi")
+ .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", "204"));
+ EXPECT_CALL(
+ visitor,
+ OnErrorDebug("Invalid HTTP header field was received: frame type: 1, "
+ "stream: 1, name: [content-length], value: [2]"));
+ 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::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, ClientHandlesTrailers) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
diff --git a/http2/adapter/nghttp2_callbacks.cc b/http2/adapter/nghttp2_callbacks.cc
index cb15a21..dc6df1d 100644
--- a/http2/adapter/nghttp2_callbacks.cc
+++ b/http2/adapter/nghttp2_callbacks.cc
@@ -176,6 +176,7 @@
case Http2VisitorInterface::HEADER_CONNECTION_ERROR:
return NGHTTP2_ERR_CALLBACK_FAILURE;
case Http2VisitorInterface::HEADER_RST_STREAM:
+ case Http2VisitorInterface::HEADER_FIELD_INVALID:
case Http2VisitorInterface::HEADER_HTTP_MESSAGING:
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
}
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index fccdf1d..50dce2c 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -340,6 +340,73 @@
spdy::SpdyFrameType::RST_STREAM}));
}
+TEST(OgHttp2AdapterClientTest, ClientHandles204WithContent) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+ testing::InSequence s;
+
+ const std::vector<const 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", "204"}, {"content-length", "2"}},
+ /*fin=*/false)
+ .Data(1, "hi")
+ .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", "204"));
+ EXPECT_CALL(
+ visitor,
+ OnInvalidFrame(1, Http2VisitorInterface::InvalidFrameError::kHttpHeader));
+ // TODO(b/210728715): Stop delivering events for streams with errors.
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 0));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, 2));
+ EXPECT_CALL(visitor, OnDataForStream(1, "hi"));
+
+ 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, ClientHandlesTrailers) {
DataSavingVisitor visitor;
OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
@@ -1354,9 +1421,9 @@
EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
- EXPECT_CALL(visitor,
- OnInvalidFrame(
- 1, Http2VisitorInterface::InvalidFrameError::kHttpMessaging));
+ EXPECT_CALL(
+ visitor,
+ OnInvalidFrame(1, Http2VisitorInterface::InvalidFrameError::kHttpHeader));
const int64_t stream_result = adapter->ProcessBytes(stream_frames);
EXPECT_EQ(static_cast<int64_t>(stream_frames.size()), stream_result);
@@ -2854,9 +2921,9 @@
EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https"));
EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com"));
EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/this/is/request/one"));
- EXPECT_CALL(visitor,
- OnInvalidFrame(
- 1, Http2VisitorInterface::InvalidFrameError::kHttpMessaging))
+ EXPECT_CALL(
+ visitor,
+ OnInvalidFrame(1, Http2VisitorInterface::InvalidFrameError::kHttpHeader))
.WillOnce(testing::Return(false));
EXPECT_CALL(visitor, OnConnectionError(ConnectionError::kHeaderError));
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index 09bbe7e..346035a 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -229,9 +229,9 @@
}
const auto validation_result = validator_.ValidateSingleHeader(key, value);
if (validation_result != HeaderValidator::HEADER_OK) {
- QUICHE_VLOG(2) << "RST_STREAM HEADER_HTTP_MESSAGING with result "
+ QUICHE_VLOG(2) << "RST_STREAM with result "
<< static_cast<int>(validation_result);
- result_ = Http2VisitorInterface::HEADER_HTTP_MESSAGING;
+ result_ = Http2VisitorInterface::HEADER_FIELD_INVALID;
return;
}
result_ = visitor_.OnHeaderForStream(stream_id_, key, value);
@@ -1159,6 +1159,7 @@
QUICHE_DCHECK_NE(result, Http2VisitorInterface::HEADER_OK);
const bool should_reset_stream =
result == Http2VisitorInterface::HEADER_RST_STREAM ||
+ result == Http2VisitorInterface::HEADER_FIELD_INVALID ||
result == Http2VisitorInterface::HEADER_HTTP_MESSAGING;
if (should_reset_stream) {
const Http2ErrorCode error_code =
@@ -1167,7 +1168,8 @@
: Http2ErrorCode::PROTOCOL_ERROR;
const spdy::SpdyErrorCode spdy_error_code = TranslateErrorCode(error_code);
const Http2VisitorInterface::InvalidFrameError frame_error =
- (result == Http2VisitorInterface::HEADER_RST_STREAM)
+ (result == Http2VisitorInterface::HEADER_RST_STREAM ||
+ result == Http2VisitorInterface::HEADER_FIELD_INVALID)
? Http2VisitorInterface::InvalidFrameError::kHttpHeader
: Http2VisitorInterface::InvalidFrameError::kHttpMessaging;
auto it = streams_reset_.find(stream_id);
@@ -1175,7 +1177,8 @@
EnqueueFrame(
absl::make_unique<spdy::SpdyRstStreamIR>(stream_id, spdy_error_code));
- if (result == Http2VisitorInterface::HEADER_HTTP_MESSAGING) {
+ if (result == Http2VisitorInterface::HEADER_FIELD_INVALID ||
+ result == Http2VisitorInterface::HEADER_HTTP_MESSAGING) {
const bool ok = visitor_.OnInvalidFrame(stream_id, frame_error);
if (!ok) {
LatchErrorAndNotify(error_code, ConnectionError::kHeaderError);
