gfe-relnote: Close connection with H3_CLOSED_CRITICAL_STREAM if peer closes a critical stream. Protected by gfe2_reloadable_flag_quic_enable_version_draft_25_v3 and gfe2_reloadable_flag_quic_enable_version_draft_27.
Note that write (send) unidirectional streams can only receive STOP_SENDING, and
read (received) unidirectional streams can only receive RESET_STREAM. The wrong
kind of frame would not reach the stream object from the transport layer.
PiperOrigin-RevId: 300733944
Change-Id: I2780dc928b6f9ed093854329378c12dcd94ecb0a
diff --git a/quic/core/http/quic_receive_control_stream.cc b/quic/core/http/quic_receive_control_stream.cc
index d87ec83..f77bdea 100644
--- a/quic/core/http/quic_receive_control_stream.cc
+++ b/quic/core/http/quic_receive_control_stream.cc
@@ -160,10 +160,9 @@
void QuicReceiveControlStream::OnStreamReset(
const QuicRstStreamFrame& /*frame*/) {
- // TODO(renjietang) Change the error code to H/3 specific
- // HTTP_CLOSED_CRITICAL_STREAM.
- stream_delegate()->OnStreamError(QUIC_INVALID_STREAM_ID,
- "Attempt to reset receive control stream");
+ stream_delegate()->OnStreamError(
+ QUIC_HTTP_CLOSED_CRITICAL_STREAM,
+ "RESET_STREAM received for receive control stream");
}
void QuicReceiveControlStream::OnDataAvailable() {
diff --git a/quic/core/http/quic_receive_control_stream_test.cc b/quic/core/http/quic_receive_control_stream_test.cc
index 0997fc0..8bbf789 100644
--- a/quic/core/http/quic_receive_control_stream_test.cc
+++ b/quic/core/http/quic_receive_control_stream_test.cc
@@ -147,7 +147,8 @@
QuicRstStreamFrame rst_frame(kInvalidControlFrameId,
receive_control_stream_->id(),
QUIC_STREAM_CANCELLED, 1234);
- EXPECT_CALL(*connection_, CloseConnection(QUIC_INVALID_STREAM_ID, _, _));
+ EXPECT_CALL(*connection_,
+ CloseConnection(QUIC_HTTP_CLOSED_CRITICAL_STREAM, _, _));
receive_control_stream_->OnStreamReset(rst_frame);
}
diff --git a/quic/core/http/quic_send_control_stream.cc b/quic/core/http/quic_send_control_stream.cc
index 0d8616c..01c03dd 100644
--- a/quic/core/http/quic_send_control_stream.cc
+++ b/quic/core/http/quic_send_control_stream.cc
@@ -32,10 +32,14 @@
max_inbound_header_list_size_(max_inbound_header_list_size) {}
void QuicSendControlStream::OnStreamReset(const QuicRstStreamFrame& /*frame*/) {
- // TODO(renjietang) Change the error code to H/3 specific
- // HTTP_CLOSED_CRITICAL_STREAM.
- stream_delegate()->OnStreamError(QUIC_INVALID_STREAM_ID,
- "Attempt to reset send control stream");
+ QUIC_BUG << "OnStreamReset() called for write unidirectional stream.";
+}
+
+bool QuicSendControlStream::OnStopSending(uint16_t /* code */) {
+ stream_delegate()->OnStreamError(
+ QUIC_HTTP_CLOSED_CRITICAL_STREAM,
+ "STOP_SENDING received for send control stream");
+ return false;
}
void QuicSendControlStream::MaybeSendSettingsFrame() {
diff --git a/quic/core/http/quic_send_control_stream.h b/quic/core/http/quic_send_control_stream.h
index 0095317..899abb9 100644
--- a/quic/core/http/quic_send_control_stream.h
+++ b/quic/core/http/quic_send_control_stream.h
@@ -30,9 +30,10 @@
QuicSendControlStream& operator=(const QuicSendControlStream&) = delete;
~QuicSendControlStream() override = default;
- // Overriding QuicStream::OnStreamReset to make sure control stream is never
+ // Overriding QuicStream::OnStopSending() to make sure control stream is never
// closed before connection.
void OnStreamReset(const QuicRstStreamFrame& frame) override;
+ bool OnStopSending(uint16_t code) override;
// Send SETTINGS frame if it hasn't been sent yet. Settings frame must be the
// first frame sent on this stream.
diff --git a/quic/core/http/quic_send_control_stream_test.cc b/quic/core/http/quic_send_control_stream_test.cc
index a5ff591..8332cec 100644
--- a/quic/core/http/quic_send_control_stream_test.cc
+++ b/quic/core/http/quic_send_control_stream_test.cc
@@ -185,13 +185,11 @@
send_control_stream_->WritePriorityUpdate(frame);
}
-TEST_P(QuicSendControlStreamTest, ResetControlStream) {
+TEST_P(QuicSendControlStreamTest, CloseControlStream) {
Initialize();
- QuicRstStreamFrame rst_frame(kInvalidControlFrameId,
- send_control_stream_->id(),
- QUIC_STREAM_CANCELLED, 1234);
- EXPECT_CALL(*connection_, CloseConnection(QUIC_INVALID_STREAM_ID, _, _));
- send_control_stream_->OnStreamReset(rst_frame);
+ EXPECT_CALL(*connection_,
+ CloseConnection(QUIC_HTTP_CLOSED_CRITICAL_STREAM, _, _));
+ send_control_stream_->OnStopSending(QUIC_STREAM_CANCELLED);
}
TEST_P(QuicSendControlStreamTest, ReceiveDataOnSendControlStream) {
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index 0412a76..408b0fa 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -1187,6 +1187,7 @@
TEST_P(QuicSpdySessionTestServer, OnRstStreamStaticStreamId) {
QuicStreamId id;
+ QuicErrorCode expected_error;
std::string error_message;
// Initialize HTTP/3 control stream.
if (VersionUsesHttp3(transport_version())) {
@@ -1195,9 +1196,11 @@
QuicStreamFrame data1(id, false, 0, quiche::QuicheStringPiece(type, 1));
session_.OnStreamFrame(data1);
- error_message = "Attempt to reset receive control stream";
+ expected_error = QUIC_HTTP_CLOSED_CRITICAL_STREAM;
+ error_message = "RESET_STREAM received for receive control stream";
} else {
id = QuicUtils::GetHeadersStreamId(transport_version());
+ expected_error = QUIC_INVALID_STREAM_ID;
error_message = "Attempt to reset headers stream";
}
@@ -1206,7 +1209,7 @@
QUIC_ERROR_PROCESSING_STREAM, 0);
EXPECT_CALL(
*connection_,
- CloseConnection(QUIC_INVALID_STREAM_ID, error_message,
+ CloseConnection(expected_error, error_message,
ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET));
session_.OnRstStream(rst1);
}
@@ -2879,6 +2882,82 @@
session_.OnStreamFrame(data);
}
+TEST_P(QuicSpdySessionTestServer, PeerClosesCriticalReceiveStream) {
+ if (!VersionUsesHttp3(transport_version())) {
+ return;
+ }
+
+ struct {
+ char type;
+ const char* error_details;
+ } kTestData[] = {
+ {kControlStream, "RESET_STREAM received for receive control stream"},
+ {kQpackEncoderStream, "RESET_STREAM received for QPACK receive stream"},
+ {kQpackDecoderStream, "RESET_STREAM received for QPACK receive stream"},
+ };
+ for (size_t i = 0; i < QUICHE_ARRAYSIZE(kTestData); ++i) {
+ QuicStreamId stream_id =
+ GetNthClientInitiatedUnidirectionalStreamId(transport_version(), i + 1);
+ const QuicByteCount data_length = 1;
+ QuicStreamFrame data(
+ stream_id, false, 0,
+ quiche::QuicheStringPiece(&kTestData[i].type, data_length));
+ session_.OnStreamFrame(data);
+
+ EXPECT_CALL(*connection_, CloseConnection(QUIC_HTTP_CLOSED_CRITICAL_STREAM,
+ kTestData[i].error_details, _));
+
+ QuicRstStreamFrame rst(kInvalidControlFrameId, stream_id,
+ QUIC_STREAM_CANCELLED, data_length);
+ session_.OnRstStream(rst);
+ }
+}
+
+TEST_P(QuicSpdySessionTestServer, PeerClosesCriticalSendStream) {
+ if (!VersionUsesHttp3(transport_version())) {
+ return;
+ }
+
+ QuicSendControlStream* control_stream =
+ QuicSpdySessionPeer::GetSendControlStream(&session_);
+ ASSERT_TRUE(control_stream);
+
+ QuicStopSendingFrame stop_sending_control_stream(
+ kInvalidControlFrameId, control_stream->id(),
+ static_cast<QuicApplicationErrorCode>(QUIC_STREAM_CANCELLED));
+ EXPECT_CALL(
+ *connection_,
+ CloseConnection(QUIC_HTTP_CLOSED_CRITICAL_STREAM,
+ "STOP_SENDING received for send control stream", _));
+ session_.OnStopSendingFrame(stop_sending_control_stream);
+
+ QpackSendStream* decoder_stream =
+ QuicSpdySessionPeer::GetQpackDecoderSendStream(&session_);
+ ASSERT_TRUE(decoder_stream);
+
+ QuicStopSendingFrame stop_sending_decoder_stream(
+ kInvalidControlFrameId, decoder_stream->id(),
+ static_cast<QuicApplicationErrorCode>(QUIC_STREAM_CANCELLED));
+ EXPECT_CALL(
+ *connection_,
+ CloseConnection(QUIC_HTTP_CLOSED_CRITICAL_STREAM,
+ "STOP_SENDING received for QPACK send stream", _));
+ session_.OnStopSendingFrame(stop_sending_decoder_stream);
+
+ QpackSendStream* encoder_stream =
+ QuicSpdySessionPeer::GetQpackEncoderSendStream(&session_);
+ ASSERT_TRUE(encoder_stream);
+
+ QuicStopSendingFrame stop_sending_encoder_stream(
+ kInvalidControlFrameId, encoder_stream->id(),
+ static_cast<QuicApplicationErrorCode>(QUIC_STREAM_CANCELLED));
+ EXPECT_CALL(
+ *connection_,
+ CloseConnection(QUIC_HTTP_CLOSED_CRITICAL_STREAM,
+ "STOP_SENDING received for QPACK send stream", _));
+ session_.OnStopSendingFrame(stop_sending_encoder_stream);
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/core/qpack/qpack_receive_stream.cc b/quic/core/qpack/qpack_receive_stream.cc
index f896ca4..d2fc88b 100644
--- a/quic/core/qpack/qpack_receive_stream.cc
+++ b/quic/core/qpack/qpack_receive_stream.cc
@@ -14,10 +14,9 @@
receiver_(receiver) {}
void QpackReceiveStream::OnStreamReset(const QuicRstStreamFrame& /*frame*/) {
- // TODO(renjietang) Change the error code to H/3 specific
- // HTTP_CLOSED_CRITICAL_STREAM.
- stream_delegate()->OnStreamError(QUIC_INVALID_STREAM_ID,
- "Attempt to reset Qpack receive stream");
+ stream_delegate()->OnStreamError(
+ QUIC_HTTP_CLOSED_CRITICAL_STREAM,
+ "RESET_STREAM received for QPACK receive stream");
}
void QpackReceiveStream::OnDataAvailable() {
diff --git a/quic/core/qpack/qpack_receive_stream_test.cc b/quic/core/qpack/qpack_receive_stream_test.cc
index 6aeeeae..bc47d9d 100644
--- a/quic/core/qpack/qpack_receive_stream_test.cc
+++ b/quic/core/qpack/qpack_receive_stream_test.cc
@@ -86,7 +86,8 @@
QuicRstStreamFrame rst_frame(kInvalidControlFrameId,
qpack_receive_stream_->id(),
QUIC_STREAM_CANCELLED, 1234);
- EXPECT_CALL(*connection_, CloseConnection(QUIC_INVALID_STREAM_ID, _, _));
+ EXPECT_CALL(*connection_,
+ CloseConnection(QUIC_HTTP_CLOSED_CRITICAL_STREAM, _, _));
qpack_receive_stream_->OnStreamReset(rst_frame);
}
diff --git a/quic/core/qpack/qpack_send_stream.cc b/quic/core/qpack/qpack_send_stream.cc
index 57493e3..15525b5 100644
--- a/quic/core/qpack/qpack_send_stream.cc
+++ b/quic/core/qpack/qpack_send_stream.cc
@@ -17,10 +17,14 @@
stream_type_sent_(false) {}
void QpackSendStream::OnStreamReset(const QuicRstStreamFrame& /*frame*/) {
- // TODO(renjietang) Change the error code to H/3 specific
- // HTTP_CLOSED_CRITICAL_STREAM.
- stream_delegate()->OnStreamError(QUIC_INVALID_STREAM_ID,
- "Attempt to reset qpack send stream");
+ QUIC_BUG << "OnStreamReset() called for write unidirectional stream.";
+}
+
+bool QpackSendStream::OnStopSending(uint16_t /* code */) {
+ stream_delegate()->OnStreamError(
+ QUIC_HTTP_CLOSED_CRITICAL_STREAM,
+ "STOP_SENDING received for QPACK send stream");
+ return false;
}
void QpackSendStream::WriteStreamData(quiche::QuicheStringPiece data) {
diff --git a/quic/core/qpack/qpack_send_stream.h b/quic/core/qpack/qpack_send_stream.h
index bc2d9c0..50d2808 100644
--- a/quic/core/qpack/qpack_send_stream.h
+++ b/quic/core/qpack/qpack_send_stream.h
@@ -30,9 +30,10 @@
QpackSendStream& operator=(const QpackSendStream&) = delete;
~QpackSendStream() override = default;
- // Overriding QuicStream::OnStreamReset to make sure QPACK stream is
- // never closed before connection.
+ // Overriding QuicStream::OnStopSending() to make sure QPACK stream is never
+ // closed before connection.
void OnStreamReset(const QuicRstStreamFrame& frame) override;
+ bool OnStopSending(uint16_t code) override;
// The send QPACK stream is write unidirectional, so this method
// should never be called.
diff --git a/quic/core/qpack/qpack_send_stream_test.cc b/quic/core/qpack/qpack_send_stream_test.cc
index d6a4df7..c77a621 100644
--- a/quic/core/qpack/qpack_send_stream_test.cc
+++ b/quic/core/qpack/qpack_send_stream_test.cc
@@ -106,11 +106,10 @@
qpack_send_stream_->MaybeSendStreamType();
}
-TEST_P(QpackSendStreamTest, ResetQpackStream) {
- QuicRstStreamFrame rst_frame(kInvalidControlFrameId, qpack_send_stream_->id(),
- QUIC_STREAM_CANCELLED, 1234);
- EXPECT_CALL(*connection_, CloseConnection(QUIC_INVALID_STREAM_ID, _, _));
- qpack_send_stream_->OnStreamReset(rst_frame);
+TEST_P(QpackSendStreamTest, StopSendingQpackStream) {
+ EXPECT_CALL(*connection_,
+ CloseConnection(QUIC_HTTP_CLOSED_CRITICAL_STREAM, _, _));
+ qpack_send_stream_->OnStopSending(QUIC_STREAM_CANCELLED);
}
TEST_P(QpackSendStreamTest, ReceiveDataOnSendStream) {
diff --git a/quic/core/quic_error_codes.cc b/quic/core/quic_error_codes.cc
index e24e839..d897aac 100644
--- a/quic/core/quic_error_codes.cc
+++ b/quic/core/quic_error_codes.cc
@@ -174,6 +174,7 @@
RETURN_STRING_LITERAL(QUIC_HTTP_DUPLICATE_UNIDIRECTIONAL_STREAM);
RETURN_STRING_LITERAL(QUIC_HTTP_SERVER_INITIATED_BIDIRECTIONAL_STREAM);
RETURN_STRING_LITERAL(QUIC_HTTP_STREAM_WRONG_DIRECTION);
+ RETURN_STRING_LITERAL(QUIC_HTTP_CLOSED_CRITICAL_STREAM);
RETURN_STRING_LITERAL(QUIC_HPACK_INDEX_VARINT_ERROR);
RETURN_STRING_LITERAL(QUIC_HPACK_NAME_LENGTH_VARINT_ERROR);
RETURN_STRING_LITERAL(QUIC_HPACK_VALUE_LENGTH_VARINT_ERROR);
diff --git a/quic/core/quic_error_codes.h b/quic/core/quic_error_codes.h
index 3ef34a5..dc575d3 100644
--- a/quic/core/quic_error_codes.h
+++ b/quic/core/quic_error_codes.h
@@ -373,6 +373,9 @@
// Server opens stream with stream ID corresponding to client-initiated
// stream or vice versa.
QUIC_HTTP_STREAM_WRONG_DIRECTION = 155,
+ // Peer closes one of the six critical unidirectional streams (control, QPACK
+ // encoder or decoder, in either direction).
+ QUIC_HTTP_CLOSED_CRITICAL_STREAM = 156,
// HPACK header block decoding errors.
// Index varint beyond implementation limit.
@@ -409,7 +412,7 @@
QUIC_HPACK_COMPRESSED_HEADER_SIZE_EXCEEDS_LIMIT = 150,
// No error. Used as bound while iterating.
- QUIC_LAST_ERROR = 156,
+ QUIC_LAST_ERROR = 157,
};
// QuicErrorCodes is encoded as four octets on-the-wire when doing Google QUIC,
// or a varint62 when doing IETF QUIC. Ensure that its value does not exceed
diff --git a/quic/core/quic_types.cc b/quic/core/quic_types.cc
index 5198eec..6282358 100644
--- a/quic/core/quic_types.cc
+++ b/quic/core/quic_types.cc
@@ -489,6 +489,10 @@
QuicHttp3ErrorCode::IETF_QUIC_HTTP3_STREAM_CREATION_ERROR)}};
case QUIC_HTTP_STREAM_WRONG_DIRECTION:
return {true, {static_cast<uint64_t>(STREAM_STATE_ERROR)}};
+ case QUIC_HTTP_CLOSED_CRITICAL_STREAM:
+ return {false,
+ {static_cast<uint64_t>(
+ QuicHttp3ErrorCode::IETF_QUIC_HTTP3_CLOSED_CRITICAL_STREAM)}};
case QUIC_HPACK_INDEX_VARINT_ERROR:
return {false, {static_cast<uint64_t>(QUIC_HPACK_INDEX_VARINT_ERROR)}};
case QUIC_HPACK_NAME_LENGTH_VARINT_ERROR:
