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
