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.