Validates that a HEADERS frame with a 100-199 status code does not contain a fin.
This is one small piece of HTTP messaging validation that is expected by Envoy's codec implementation.
PiperOrigin-RevId: 411087924
diff --git a/http2/adapter/http2_visitor_interface.h b/http2/adapter/http2_visitor_interface.h
index 5622452..08b9ba5 100644
--- a/http2/adapter/http2_visitor_interface.h
+++ b/http2/adapter/http2_visitor_interface.h
@@ -113,9 +113,15 @@
// HEADER_RST_STREAM. Returning HEADER_CONNECTION_ERROR will lead to a
// non-recoverable error on the connection.
enum OnHeaderResult {
+ // The header was accepted.
HEADER_OK,
+ // The application considers the header a connection error.
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
+ // with error code PROTOCOL_ERROR.
+ HEADER_HTTP_MESSAGING,
};
virtual OnHeaderResult OnHeaderForStream(Http2StreamId stream_id,
absl::string_view key,
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index cae2cec..c49f1f1 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -263,6 +263,69 @@
EXPECT_THAT(visitor.data(), testing::IsEmpty());
}
+TEST(NgHttp2AdapterTest, ClientRejects100HeadersWithFin) {
+ 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);
+ QUICHE_LOG(INFO) << "Created stream: " << stream_id1;
+
+ 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"}}, /*fin=*/false)
+ .Headers(1, {{":status", "100"}}, /*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, OnEndHeadersForStream(1));
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":status", "100"));
+ EXPECT_CALL(visitor,
+ OnInvalidFrame(
+ 1, Http2VisitorInterface::InvalidFrameError::kHttpMessaging));
+
+ 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, 1));
+ 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 2ef6633..cb15a21 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_HTTP_MESSAGING:
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
}
// Unexpected value.
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index 765fe22..122cb07 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -193,6 +193,151 @@
spdy::SpdyFrameType::SETTINGS}));
}
+TEST(OgHttp2AdapterClientTest, ClientHandles100Headers) {
+ 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);
+ QUICHE_LOG(INFO) << "Created stream: " << stream_id1;
+
+ 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"}},
+ /*fin=*/false)
+ .Ping(101)
+ .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, OnEndHeadersForStream(1));
+
+ EXPECT_CALL(visitor, OnFrameHeader(0, 8, PING, 0));
+ EXPECT_CALL(visitor, OnPing(101, false));
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":status", "200"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, "server", "my-fake-server"));
+ EXPECT_CALL(visitor,
+ OnHeaderForStream(1, "date", "Tue, 6 Apr 2021 12:54:01 GMT"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnEndStream(1));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR));
+
+ 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(PING, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(PING, 0, _, 0x1, 0));
+
+ EXPECT_TRUE(adapter->want_write());
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::PING}));
+}
+
+TEST(OgHttp2AdapterClientTest, ClientRejects100HeadersWithFin) {
+ 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);
+ QUICHE_LOG(INFO) << "Created stream: " << stream_id1;
+
+ 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"}}, /*fin=*/false)
+ .Headers(1, {{":status", "100"}}, /*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, OnEndHeadersForStream(1));
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":status", "100"));
+ EXPECT_CALL(visitor,
+ OnInvalidFrame(
+ 1, Http2VisitorInterface::InvalidFrameError::kHttpMessaging));
+ // NOTE: nghttp2 does not deliver the OnEndStream event.
+ EXPECT_CALL(visitor, OnEndStream(1));
+
+ 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, 1));
+ 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};
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index 8ca1267..0f1eb0d 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -181,6 +181,14 @@
return Http2ErrorCode::INTERNAL_ERROR;
}
+bool IsResponse(HeaderType type) {
+ return type == HeaderType::RESPONSE_100 || type == HeaderType::RESPONSE;
+}
+
+bool StatusIs1xx(absl::string_view status) {
+ return status.size() == 3 && status[0] == '1';
+}
+
} // namespace
void OgHttp2Session::PassthroughHeadersHandler::OnHeaderBlockStart() {
@@ -202,6 +210,7 @@
const auto validation_result = validator_.ValidateSingleHeader(key, value);
if (validation_result != HeaderValidator::HEADER_OK) {
QUICHE_VLOG(2) << "RST_STREAM: invalid header found";
+ // TODO(birenroy): consider updating this to return HEADER_HTTP_MESSAGING.
result_ = Http2VisitorInterface::HEADER_RST_STREAM;
return;
}
@@ -216,6 +225,11 @@
result_ = Http2VisitorInterface::HEADER_RST_STREAM;
}
}
+ if (frame_contains_fin_ && IsResponse(type_) &&
+ StatusIs1xx(status_header())) {
+ // Unexpected end of stream without final headers.
+ result_ = Http2VisitorInterface::HEADER_HTTP_MESSAGING;
+ }
if (result_ == Http2VisitorInterface::HEADER_OK) {
const bool result = visitor_.OnEndHeadersForStream(stream_id_);
if (!result) {
@@ -224,6 +238,7 @@
} else {
session_.OnHeaderStatus(stream_id_, result_);
}
+ frame_contains_fin_ = false;
}
OgHttp2Session::OgHttp2Session(Http2VisitorInterface& visitor, Options options)
@@ -793,12 +808,13 @@
iter->second.half_closed_remote = true;
visitor_.OnEndStream(stream_id);
}
+ auto queued_frames_iter = queued_frames_.find(stream_id);
+ const bool no_queued_frames = queued_frames_iter == queued_frames_.end() ||
+ queued_frames_iter->second == 0;
if (iter != stream_map_.end() && iter->second.half_closed_local &&
- options_.perspective == Perspective::kClient) {
+ options_.perspective == Perspective::kClient && no_queued_frames) {
// From the client's perspective, the stream can be closed if it's already
// half_closed_local.
- // TODO(birenroy): consider whether there are outbound frames queued for the
- // stream.
CloseStream(stream_id, Http2ErrorCode::HTTP2_NO_ERROR);
}
}
@@ -917,13 +933,16 @@
void OgHttp2Session::OnHeaders(spdy::SpdyStreamId stream_id,
bool /*has_priority*/, int /*weight*/,
spdy::SpdyStreamId /*parent_stream_id*/,
- bool /*exclusive*/, bool /*fin*/, bool /*end*/) {
+ bool /*exclusive*/, bool fin, bool /*end*/) {
if (stream_id % 2 == 0) {
// Server push is disabled; receiving push HEADERS is a connection error.
LatchErrorAndNotify(Http2ErrorCode::PROTOCOL_ERROR,
ConnectionError::kInvalidNewStreamId);
return;
}
+ if (fin) {
+ headers_handler_.set_frame_contains_fin();
+ }
if (options_.perspective == Perspective::kServer) {
const auto new_stream_id = static_cast<Http2StreamId>(stream_id);
if (new_stream_id <= highest_processed_stream_id_) {
@@ -995,17 +1014,27 @@
void OgHttp2Session::OnHeaderStatus(
Http2StreamId stream_id, Http2VisitorInterface::OnHeaderResult result) {
QUICHE_DCHECK_NE(result, Http2VisitorInterface::HEADER_OK);
- if (result == Http2VisitorInterface::HEADER_RST_STREAM) {
+ const bool should_reset_stream =
+ result == Http2VisitorInterface::HEADER_RST_STREAM ||
+ result == Http2VisitorInterface::HEADER_HTTP_MESSAGING;
+ if (should_reset_stream) {
+ const Http2ErrorCode error_code =
+ (result == Http2VisitorInterface::HEADER_RST_STREAM)
+ ? Http2ErrorCode::INTERNAL_ERROR
+ : Http2ErrorCode::PROTOCOL_ERROR;
+ const spdy::SpdyErrorCode spdy_error_code = TranslateErrorCode(error_code);
+ const Http2VisitorInterface::InvalidFrameError frame_error =
+ (result == Http2VisitorInterface::HEADER_RST_STREAM)
+ ? Http2VisitorInterface::InvalidFrameError::kHttpHeader
+ : Http2VisitorInterface::InvalidFrameError::kHttpMessaging;
auto it = streams_reset_.find(stream_id);
if (it == streams_reset_.end()) {
- EnqueueFrame(absl::make_unique<spdy::SpdyRstStreamIR>(
- stream_id, spdy::ERROR_CODE_INTERNAL_ERROR));
+ EnqueueFrame(
+ absl::make_unique<spdy::SpdyRstStreamIR>(stream_id, spdy_error_code));
- const bool ok = visitor_.OnInvalidFrame(
- stream_id, Http2VisitorInterface::InvalidFrameError::kHttpHeader);
+ const bool ok = visitor_.OnInvalidFrame(stream_id, frame_error);
if (!ok) {
- LatchErrorAndNotify(Http2ErrorCode::INTERNAL_ERROR,
- ConnectionError::kHeaderError);
+ LatchErrorAndNotify(error_code, ConnectionError::kHeaderError);
}
}
} else if (result == Http2VisitorInterface::HEADER_CONNECTION_ERROR) {
diff --git a/http2/adapter/oghttp2_session.h b/http2/adapter/oghttp2_session.h
index c6a3e0f..564ea76 100644
--- a/http2/adapter/oghttp2_session.h
+++ b/http2/adapter/oghttp2_session.h
@@ -218,6 +218,7 @@
result_ = Http2VisitorInterface::HEADER_OK;
}
+ void set_frame_contains_fin() { frame_contains_fin_ = true; }
void set_header_type(HeaderType type) { type_ = type; }
HeaderType header_type() const { return type_; }
@@ -240,6 +241,7 @@
// Validates header blocks according to the HTTP/2 specification.
HeaderValidator validator_;
HeaderType type_ = HeaderType::RESPONSE;
+ bool frame_contains_fin_ = false;
};
// Queues the connection preface, if not already done.
