Fix content-length validation for trailers.
This CL fixes a bug in content-length validation when trailers end the stream:
1. End of headers, stores content-length for validation
2. End of data, does not end stream and does not match content-length
3. End of trailers, ends stream and stores content-length for validation (!)
The re-storing of content-length at the end of trailers erases the previous
validation at the end of data. This CL fixes Step 3 to avoid storing
content-length for trailers.
MultiplexedIntegrationTest.InconsistentContentLength:
http://sponge2/e572060c-8ff8-45a8-b3b3-25994345b2bb (demonstrates RST_STREAM)
PiperOrigin-RevId: 422904538
diff --git a/http2/adapter/header_validator.cc b/http2/adapter/header_validator.cc
index d256156..d5ae7a2 100644
--- a/http2/adapter/header_validator.cc
+++ b/http2/adapter/header_validator.cc
@@ -136,6 +136,7 @@
return false;
}
+ // TODO(diannahu): Do the same for 1xx responses.
if (status_ == "204" && value != "0") {
// There should be no body in a "204 No Content" response.
return false;
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index 0bf7295..206d8e2 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -4402,6 +4402,16 @@
{":path", "/this/is/request/four"},
{"content-length", "2"}},
/*fin=*/true)
+ .Headers(7,
+ {{":method", "GET"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/four"},
+ {"content-length", "2"}},
+ /*fin=*/false)
+ .Data(7, "h", /*fin=*/false)
+ .Headers(7, {{"extra-info", "Trailers with content-length mismatch"}},
+ /*fin=*/true)
.Serialize();
// Client preface (empty SETTINGS)
@@ -4438,6 +4448,22 @@
OnInvalidFrame(
5, Http2VisitorInterface::InvalidFrameError::kHttpMessaging));
+ // Stream 7: content-length is invalid and trailers end the stream
+ // When the stream ends with trailers, nghttp2 invokes OnInvalidFrame().
+ EXPECT_CALL(visitor, OnFrameHeader(7, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(7));
+ EXPECT_CALL(visitor, OnHeaderForStream(7, _, _)).Times(5);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(7));
+ EXPECT_CALL(visitor, OnFrameHeader(7, _, DATA, 0));
+ EXPECT_CALL(visitor, OnBeginDataForStream(7, 1));
+ EXPECT_CALL(visitor, OnDataForStream(7, "h"));
+ EXPECT_CALL(visitor, OnFrameHeader(7, _, HEADERS, 5));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(7));
+ EXPECT_CALL(visitor, OnHeaderForStream(7, _, _));
+ EXPECT_CALL(visitor,
+ OnInvalidFrame(
+ 7, Http2VisitorInterface::InvalidFrameError::kHttpMessaging));
+
const int64_t stream_result = adapter->ProcessBytes(stream_frames);
EXPECT_EQ(stream_frames.size(), static_cast<size_t>(stream_result));
@@ -4458,6 +4484,11 @@
OnFrameSent(RST_STREAM, 5, _, 0x0,
static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR)));
EXPECT_CALL(visitor, OnCloseStream(5, Http2ErrorCode::PROTOCOL_ERROR));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 7, _, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(RST_STREAM, 7, _, 0x0,
+ static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR)));
+ EXPECT_CALL(visitor, OnCloseStream(7, Http2ErrorCode::PROTOCOL_ERROR));
EXPECT_TRUE(adapter->want_write());
int result = adapter->Send();
@@ -4465,6 +4496,7 @@
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
spdy::SpdyFrameType::RST_STREAM,
spdy::SpdyFrameType::RST_STREAM,
+ spdy::SpdyFrameType::RST_STREAM,
spdy::SpdyFrameType::RST_STREAM}));
}
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index 3660d20..d3f235d 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -4564,6 +4564,16 @@
{":path", "/this/is/request/four"},
{"content-length", "2"}},
/*fin=*/true)
+ .Headers(7,
+ {{":method", "GET"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/four"},
+ {"content-length", "2"}},
+ /*fin=*/false)
+ .Data(7, "h", /*fin=*/false)
+ .Headers(7, {{"extra-info", "Trailers with content-length mismatch"}},
+ /*fin=*/true)
.Serialize();
// Client preface (empty SETTINGS)
@@ -4600,6 +4610,21 @@
EXPECT_CALL(visitor, OnHeaderForStream(5, _, _)).Times(5);
EXPECT_CALL(visitor, OnEndHeadersForStream(5));
+ // Stream 7: content-length is invalid and trailers end the stream
+ // Only oghttp2 invokes OnEndHeadersForStream(). Only nghttp2 invokes
+ // OnInvalidFrame().
+ EXPECT_CALL(visitor, OnFrameHeader(7, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(7));
+ EXPECT_CALL(visitor, OnHeaderForStream(7, _, _)).Times(5);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(7));
+ EXPECT_CALL(visitor, OnFrameHeader(7, _, DATA, 0));
+ EXPECT_CALL(visitor, OnBeginDataForStream(7, 1));
+ EXPECT_CALL(visitor, OnDataForStream(7, "h"));
+ EXPECT_CALL(visitor, OnFrameHeader(7, _, HEADERS, 5));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(7));
+ EXPECT_CALL(visitor, OnHeaderForStream(7, _, _));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(7));
+
const int64_t stream_result = adapter->ProcessBytes(stream_frames);
EXPECT_EQ(stream_frames.size(), static_cast<size_t>(stream_result));
@@ -4622,15 +4647,21 @@
OnFrameSent(RST_STREAM, 5, _, 0x0,
static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR)));
EXPECT_CALL(visitor, OnCloseStream(5, Http2ErrorCode::HTTP2_NO_ERROR));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 7, _, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(RST_STREAM, 7, _, 0x0,
+ static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR)));
+ EXPECT_CALL(visitor, OnCloseStream(7, 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,
- spdy::SpdyFrameType::RST_STREAM,
- spdy::SpdyFrameType::RST_STREAM}));
+ EXPECT_THAT(
+ visitor.data(),
+ EqualsFrames(
+ {spdy::SpdyFrameType::SETTINGS, spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::RST_STREAM, spdy::SpdyFrameType::RST_STREAM,
+ spdy::SpdyFrameType::RST_STREAM, spdy::SpdyFrameType::RST_STREAM}));
}
} // namespace
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index 827e80b..4c14b1b 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -6,6 +6,7 @@
#include "absl/memory/memory.h"
#include "absl/strings/escaping.h"
+#include "http2/adapter/header_validator.h"
#include "http2/adapter/http2_protocol.h"
#include "http2/adapter/http2_util.h"
#include "http2/adapter/http2_visitor_interface.h"
@@ -1117,7 +1118,10 @@
} else {
it->second.received_header_type = headers_handler_.header_type();
}
- it->second.remaining_content_length = headers_handler_.content_length();
+ if (headers_handler_.header_type() == HeaderType::REQUEST ||
+ headers_handler_.header_type() == HeaderType::RESPONSE) {
+ it->second.remaining_content_length = headers_handler_.content_length();
+ }
headers_handler_.set_stream_id(0);
}
}
