Reject TE headers with values other than "trailers" in oghttp2. This CL improves oghttp2 spec compliance: https://httpwg.org/specs/rfc7540.html#n-connection-specific-header-fields In particular, the TE header field may only have the "trailers" value; otherwise the headers will be rejected as invalid. Http2RequestsTest.InvalidTeHeader with oghttp2: http://sponge2/6330792e-5128-4114-ab14-b99eeb73e17b PiperOrigin-RevId: 424381258
diff --git a/http2/adapter/header_validator.cc b/http2/adapter/header_validator.cc index d018991..dd605f4 100644 --- a/http2/adapter/header_validator.cc +++ b/http2/adapter/header_validator.cc
@@ -155,6 +155,8 @@ if (!success) { return HEADER_FIELD_INVALID; } + } else if (key == "te" && value != "trailers") { + return HEADER_FIELD_INVALID; } return HEADER_OK; }
diff --git a/http2/adapter/header_validator_test.cc b/http2/adapter/header_validator_test.cc index c76a770..52148df 100644 --- a/http2/adapter/header_validator_test.cc +++ b/http2/adapter/header_validator_test.cc
@@ -460,6 +460,18 @@ EXPECT_THAT(v.content_length(), Optional(42)); } +TEST(HeaderValidatorTest, TeHeader) { + HeaderValidator v; + + v.StartHeaderBlock(); + EXPECT_EQ(HeaderValidator::HEADER_OK, + v.ValidateSingleHeader("te", "trailers")); + + v.StartHeaderBlock(); + EXPECT_EQ(HeaderValidator::HEADER_FIELD_INVALID, + v.ValidateSingleHeader("te", "trailers, deflate")); +} + } // namespace test } // namespace adapter } // namespace http2
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc index dbab8c6..a2fd68f 100644 --- a/http2/adapter/nghttp2_adapter_test.cc +++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -5038,6 +5038,72 @@ spdy::SpdyFrameType::RST_STREAM})); } +TEST(NgHttp2AdapterTest, ServerHandlesTeHeader) { + DataSavingVisitor visitor; + auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor); + + testing::InSequence s; + + const std::string stream_frames = TestFrameSequence() + .ClientPreface() + .Headers(1, + {{":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/"}, + {":method", "GET"}, + {"te", "trailers"}}, + /*fin=*/true) + .Headers(3, + {{":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/"}, + {":method", "GET"}, + {"te", "trailers, deflate"}}, + /*fin=*/true) + .Serialize(); + + // Client preface (empty SETTINGS) + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, OnSettingsEnd()); + + // Stream 1: TE: trailers should be allowed. + EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); + EXPECT_CALL(visitor, OnHeaderForStream(1, _, _)).Times(5); + EXPECT_CALL(visitor, OnEndHeadersForStream(1)); + EXPECT_CALL(visitor, OnEndStream(1)); + + // Stream 3: TE: <non-trailers> should be rejected. + EXPECT_CALL(visitor, OnFrameHeader(3, _, HEADERS, 5)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(3)); + EXPECT_CALL(visitor, OnHeaderForStream(3, _, _)).Times(4); + EXPECT_CALL( + visitor, + OnErrorDebug("Invalid HTTP header field was received: frame type: 1, " + "stream: 3, name: [te], value: [trailers, deflate]")); + EXPECT_CALL( + visitor, + OnInvalidFrame(3, 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, 3, _, 0x0)); + EXPECT_CALL(visitor, + OnFrameSent(RST_STREAM, 3, _, 0x0, + static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR))); + EXPECT_CALL(visitor, OnCloseStream(3, Http2ErrorCode::PROTOCOL_ERROR)); + + EXPECT_TRUE(adapter->want_write()); + int result = adapter->Send(); + EXPECT_EQ(0, result); + EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS, + spdy::SpdyFrameType::RST_STREAM})); +} + } // namespace } // namespace test } // namespace adapter
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc index 672f6fb..7917bf2 100644 --- a/http2/adapter/oghttp2_adapter_test.cc +++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -5103,7 +5103,7 @@ spdy::SpdyFrameType::RST_STREAM, spdy::SpdyFrameType::RST_STREAM})); } -TEST(NgHttp2AdapterTest, ServerHandlesAsteriskPathForOptions) { +TEST(OgHttp2AdapterServerTest, ServerHandlesAsteriskPathForOptions) { DataSavingVisitor visitor; OgHttp2Adapter::Options options{.perspective = Perspective::kServer}; auto adapter = OgHttp2Adapter::Create(visitor, options); @@ -5146,7 +5146,7 @@ spdy::SpdyFrameType::SETTINGS})); } -TEST(NgHttp2AdapterTest, ServerHandlesInvalidPath) { +TEST(OgHttp2AdapterServerTest, ServerHandlesInvalidPath) { DataSavingVisitor visitor; OgHttp2Adapter::Options options{.perspective = Perspective::kServer}; auto adapter = OgHttp2Adapter::Create(visitor, options); @@ -5235,6 +5235,72 @@ spdy::SpdyFrameType::RST_STREAM})); } +TEST(OgHttp2AdapterServerTest, ServerHandlesTeHeader) { + DataSavingVisitor visitor; + OgHttp2Adapter::Options options{.perspective = Perspective::kServer}; + auto adapter = OgHttp2Adapter::Create(visitor, options); + + testing::InSequence s; + + const std::string stream_frames = TestFrameSequence() + .ClientPreface() + .Headers(1, + {{":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/"}, + {":method", "GET"}, + {"te", "trailers"}}, + /*fin=*/true) + .Headers(3, + {{":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/"}, + {":method", "GET"}, + {"te", "trailers, deflate"}}, + /*fin=*/true) + .Serialize(); + + // Client preface (empty SETTINGS) + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, OnSettingsEnd()); + + // Stream 1: TE: trailers should be allowed. + EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); + EXPECT_CALL(visitor, OnHeaderForStream(1, _, _)).Times(5); + EXPECT_CALL(visitor, OnEndHeadersForStream(1)); + EXPECT_CALL(visitor, OnEndStream(1)); + + // Stream 3: TE: <non-trailers> should be rejected. + EXPECT_CALL(visitor, OnFrameHeader(3, _, HEADERS, 5)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(3)); + EXPECT_CALL(visitor, OnHeaderForStream(3, _, _)).Times(4); + EXPECT_CALL( + visitor, + OnInvalidFrame(3, 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, _, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 3, _, 0x0)); + EXPECT_CALL(visitor, + OnFrameSent(RST_STREAM, 3, _, 0x0, + static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR))); + EXPECT_CALL(visitor, OnCloseStream(3, Http2ErrorCode::HTTP2_NO_ERROR)); + + EXPECT_TRUE(adapter->want_write()); + int result = adapter->Send(); + EXPECT_EQ(0, result); + EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS, + spdy::SpdyFrameType::SETTINGS, + spdy::SpdyFrameType::RST_STREAM})); +} + } // namespace } // namespace test } // namespace adapter