gfe-relnote: Enforce HTTP/3 frame ordering requirements on request and control streams. Protected by gfe2_reloadable_flag_quic_enable_version_draft_25_v3 and gfe2_reloadable_flag_quic_enable_version_draft_27.
Add new error codes and use them in request and control streams. Add new code
to control stream to enforce that first frame must be SETTINGS.
PiperOrigin-RevId: 300217136
Change-Id: Ib3ced012961f34c912ccd2061a3dc913f7bb96e6
diff --git a/quic/core/http/quic_receive_control_stream.cc b/quic/core/http/quic_receive_control_stream.cc
index 4f21498..d87ec83 100644
--- a/quic/core/http/quic_receive_control_stream.cc
+++ b/quic/core/http/quic_receive_control_stream.cc
@@ -122,8 +122,7 @@
bool OnUnknownFrameStart(uint64_t /* frame_type */,
QuicByteCount /* header_length */) override {
- // Ignore unknown frame types.
- return true;
+ return stream_->OnUnknownFrameStart();
}
bool OnUnknownFramePayload(quiche::QuicheStringPiece /* payload */) override {
@@ -190,9 +189,9 @@
bool QuicReceiveControlStream::OnSettingsFrameStart(
QuicByteCount /* header_length */) {
if (settings_frame_received_) {
- // TODO(renjietang): Change error code to HTTP_UNEXPECTED_FRAME.
- stream_delegate()->OnStreamError(QUIC_INVALID_STREAM_ID,
- "Settings frames are received twice.");
+ stream_delegate()->OnStreamError(
+ QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_CONTROL_STREAM,
+ "Settings frames are received twice.");
return false;
}
@@ -217,7 +216,7 @@
QuicByteCount /* header_length */) {
if (!settings_frame_received_) {
stream_delegate()->OnStreamError(
- QUIC_INVALID_STREAM_ID,
+ QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_CONTROL_STREAM,
"PRIORITY_UPDATE frame received before SETTINGS.");
return false;
}
@@ -263,4 +262,15 @@
return true;
}
+bool QuicReceiveControlStream::OnUnknownFrameStart() {
+ if (!settings_frame_received_) {
+ stream_delegate()->OnStreamError(
+ QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_CONTROL_STREAM,
+ "Unknown frame received before SETTINGS.");
+ return false;
+ }
+
+ return true;
+}
+
} // namespace quic
diff --git a/quic/core/http/quic_receive_control_stream.h b/quic/core/http/quic_receive_control_stream.h
index e77ccf9..d24bcd0 100644
--- a/quic/core/http/quic_receive_control_stream.h
+++ b/quic/core/http/quic_receive_control_stream.h
@@ -43,6 +43,7 @@
bool OnSettingsFrame(const SettingsFrame& settings);
bool OnPriorityUpdateFrameStart(QuicByteCount header_length);
bool OnPriorityUpdateFrame(const PriorityUpdateFrame& priority);
+ bool OnUnknownFrameStart();
// False until a SETTINGS frame is received.
bool settings_frame_received_;
diff --git a/quic/core/http/quic_receive_control_stream_test.cc b/quic/core/http/quic_receive_control_stream_test.cc
index 67a9a17..0997fc0 100644
--- a/quic/core/http/quic_receive_control_stream_test.cc
+++ b/quic/core/http/quic_receive_control_stream_test.cc
@@ -197,9 +197,10 @@
EXPECT_EQ(settings_frame.size() + 1, NumBytesConsumed());
// Second SETTINGS frame causes the connection to be closed.
- EXPECT_CALL(*connection_,
- CloseConnection(QUIC_INVALID_STREAM_ID,
- "Settings frames are received twice.", _))
+ EXPECT_CALL(
+ *connection_,
+ CloseConnection(QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_CONTROL_STREAM,
+ "Settings frames are received twice.", _))
.WillOnce(
Invoke(connection_, &MockQuicConnection::ReallyCloseConnection));
EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _));
@@ -255,7 +256,7 @@
EXPECT_CALL(
*connection_,
- CloseConnection(QUIC_INVALID_STREAM_ID,
+ CloseConnection(QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_CONTROL_STREAM,
"PRIORITY_UPDATE frame received before SETTINGS.", _))
.WillOnce(
Invoke(connection_, &MockQuicConnection::ReallyCloseConnection));
@@ -310,18 +311,51 @@
// Regression test for b/137554973: unknown frames should be consumed.
TEST_P(QuicReceiveControlStreamTest, ConsumeUnknownFrame) {
+ EXPECT_EQ(1u, NumBytesConsumed());
+
+ // Receive SETTINGS frame.
+ SettingsFrame settings;
+ std::string settings_frame = EncodeSettings(settings);
+ receive_control_stream_->OnStreamFrame(
+ QuicStreamFrame(receive_control_stream_->id(), /* fin = */ false,
+ /* offset = */ 1, settings_frame));
+
+ // SETTINGS frame is consumed.
+ EXPECT_EQ(1 + settings_frame.size(), NumBytesConsumed());
+
+ // Receive unknown frame.
std::string unknown_frame = quiche::QuicheTextUtils::HexDecode(
"21" // reserved frame type
"03" // payload length
"666f6f"); // payload "foo"
- EXPECT_EQ(1u, NumBytesConsumed());
+ receive_control_stream_->OnStreamFrame(
+ QuicStreamFrame(receive_control_stream_->id(), /* fin = */ false,
+ /* offset = */ 1 + settings_frame.size(), unknown_frame));
+
+ // Unknown frame is consumed.
+ EXPECT_EQ(1 + settings_frame.size() + unknown_frame.size(),
+ NumBytesConsumed());
+}
+
+TEST_P(QuicReceiveControlStreamTest, UnknownFrameBeforeSettings) {
+ std::string unknown_frame = quiche::QuicheTextUtils::HexDecode(
+ "21" // reserved frame type
+ "03" // payload length
+ "666f6f"); // payload "foo"
+
+ EXPECT_CALL(
+ *connection_,
+ CloseConnection(QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_CONTROL_STREAM,
+ "Unknown frame received before SETTINGS.", _))
+ .WillOnce(
+ Invoke(connection_, &MockQuicConnection::ReallyCloseConnection));
+ EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _));
+ EXPECT_CALL(session_, OnConnectionClosed(_, _));
receive_control_stream_->OnStreamFrame(
QuicStreamFrame(receive_control_stream_->id(), /* fin = */ false,
/* offset = */ 1, unknown_frame));
-
- EXPECT_EQ(unknown_frame.size() + 1, NumBytesConsumed());
}
} // namespace
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index b019b1e..9d2772a 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -844,9 +844,9 @@
bool QuicSpdyStream::OnDataFrameStart(QuicByteCount header_length) {
DCHECK(VersionUsesHttp3(transport_version()));
if (!headers_decompressed_ || trailers_decompressed_) {
- // TODO(b/124216424): Change error code to HTTP_UNEXPECTED_FRAME.
- stream_delegate()->OnStreamError(QUIC_INVALID_HEADERS_STREAM_DATA,
- "Unexpected DATA frame received.");
+ stream_delegate()->OnStreamError(
+ QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_SPDY_STREAM,
+ "Unexpected DATA frame received.");
return false;
}
@@ -925,9 +925,8 @@
DCHECK(!qpack_decoded_headers_accumulator_);
if (trailers_decompressed_) {
- // TODO(b/124216424): Change error code to HTTP_UNEXPECTED_FRAME.
stream_delegate()->OnStreamError(
- QUIC_INVALID_HEADERS_STREAM_DATA,
+ QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_SPDY_STREAM,
"HEADERS frame received after trailing HEADERS.");
return false;
}
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc
index 1d5305b..f1cb316 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -2473,10 +2473,9 @@
// Closing the connection is mocked out in tests. Instead, simply stop
// reading data at the stream level to prevent QuicSpdyStream from blowing up.
- // TODO(b/124216424): Change error code to HTTP_UNEXPECTED_FRAME.
EXPECT_CALL(
*connection_,
- CloseConnection(QUIC_INVALID_HEADERS_STREAM_DATA,
+ CloseConnection(QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_SPDY_STREAM,
"Unexpected DATA frame received.",
ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET))
.WillOnce(InvokeWithoutArgs([this]() { stream_->StopReading(); }));
@@ -2523,10 +2522,9 @@
// Closing the connection is mocked out in tests. Instead, simply stop
// reading data at the stream level to prevent QuicSpdyStream from blowing up.
- // TODO(b/124216424): Change error code to HTTP_UNEXPECTED_FRAME.
EXPECT_CALL(
*connection_,
- CloseConnection(QUIC_INVALID_HEADERS_STREAM_DATA,
+ CloseConnection(QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_SPDY_STREAM,
"HEADERS frame received after trailing HEADERS.",
ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET))
.WillOnce(InvokeWithoutArgs([this]() { stream_->StopReading(); }));
@@ -2574,10 +2572,9 @@
// Closing the connection is mocked out in tests. Instead, simply stop
// reading data at the stream level to prevent QuicSpdyStream from blowing up.
- // TODO(b/124216424): Change error code to HTTP_UNEXPECTED_FRAME.
EXPECT_CALL(
*connection_,
- CloseConnection(QUIC_INVALID_HEADERS_STREAM_DATA,
+ CloseConnection(QUIC_HTTP_INVALID_FRAME_SEQUENCE_ON_SPDY_STREAM,
"Unexpected DATA frame received.",
ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET))
.WillOnce(InvokeWithoutArgs([this]() { stream_->StopReading(); }));
