gfe-relnote: Close connection if incoming MAX_PUSH_ID frame tries to reduce maximum push ID. Behavior change in IETF QUIC only, protected by gfe2_reloadable_flag_quic_enable_version_draft_25_v3 and gfe2_reloadable_flag_quic_enable_version_draft_27.
As Renjie suggested at cl/302687788, this is done in QuicSpdySession instead of
QuicReceiveControlStream.
PiperOrigin-RevId: 306424146
Change-Id: I51ee39b6ac9c8ead8616009d60f1dd5c933c25ab
diff --git a/quic/core/http/quic_receive_control_stream.cc b/quic/core/http/quic_receive_control_stream.cc
index 7b54756..d24e3dd 100644
--- a/quic/core/http/quic_receive_control_stream.cc
+++ b/quic/core/http/quic_receive_control_stream.cc
@@ -91,10 +91,7 @@
return false;
}
- // TODO(b/124216424): Signal error if received push ID is smaller than a
- // previously received value.
- spdy_session()->OnMaxPushIdFrame(frame.push_id);
- return true;
+ return spdy_session()->OnMaxPushIdFrame(frame.push_id);
}
bool QuicReceiveControlStream::OnGoAwayFrame(const GoAwayFrame& frame) {
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index c7bb953..f6dd0a7 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -1255,14 +1255,23 @@
quiche::QuicheOptional<PushId> old_max_push_id = max_push_id_;
max_push_id_ = max_push_id;
- if (!old_max_push_id.has_value() ||
- max_push_id_.value() > old_max_push_id.value()) {
+ if (!old_max_push_id.has_value() || max_push_id > old_max_push_id.value()) {
OnCanCreateNewOutgoingStream(true);
return true;
}
// Equal value is not considered an error.
- return max_push_id_.value() >= old_max_push_id.value();
+ if (max_push_id < old_max_push_id.value()) {
+ CloseConnectionWithDetails(
+ QUIC_HTTP_INVALID_MAX_PUSH_ID,
+ quiche::QuicheStrCat(
+ "MAX_PUSH_ID received with value ", max_push_id,
+ " which is smaller that previously received value ",
+ old_max_push_id.value()));
+ return false;
+ }
+
+ return true;
}
void QuicSpdySession::SendMaxPushId() {
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index 307dabb..63fb18d 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -424,6 +424,15 @@
return std::string(priority_buffer.get(), priority_frame_length);
}
+ std::string SerializeMaxPushIdFrame(PushId push_id) {
+ MaxPushIdFrame max_push_id_frame;
+ max_push_id_frame.push_id = push_id;
+ std::unique_ptr<char[]> buffer;
+ QuicByteCount frame_length =
+ HttpEncoder::SerializeMaxPushIdFrame(max_push_id_frame, &buffer);
+ return std::string(buffer.get(), frame_length);
+ }
+
QuicStreamId StreamCountToId(QuicStreamCount stream_count,
Perspective perspective,
bool bidirectional) {
@@ -1770,6 +1779,55 @@
}
}
+TEST_P(QuicSpdySessionTestServer, ReduceMaxPushId) {
+ if (!VersionUsesHttp3(transport_version())) {
+ return;
+ }
+
+ StrictMock<MockHttp3DebugVisitor> debug_visitor;
+ session_.set_debug_visitor(&debug_visitor);
+
+ // Use an arbitrary stream id for incoming control stream.
+ QuicStreamId stream_id =
+ GetNthClientInitiatedUnidirectionalStreamId(transport_version(), 3);
+ char type[] = {kControlStream};
+ quiche::QuicheStringPiece stream_type(type, 1);
+
+ QuicStreamOffset offset = 0;
+ QuicStreamFrame data1(stream_id, false, offset, stream_type);
+ offset += stream_type.length();
+ EXPECT_CALL(debug_visitor, OnPeerControlStreamCreated(stream_id));
+ session_.OnStreamFrame(data1);
+ EXPECT_EQ(stream_id,
+ QuicSpdySessionPeer::GetReceiveControlStream(&session_)->id());
+
+ SettingsFrame settings;
+ std::string settings_frame = EncodeSettings(settings);
+ QuicStreamFrame data2(stream_id, false, offset, settings_frame);
+ offset += settings_frame.length();
+
+ EXPECT_CALL(debug_visitor, OnSettingsFrameReceived(settings));
+ session_.OnStreamFrame(data2);
+
+ std::string max_push_id_frame1 = SerializeMaxPushIdFrame(/* push_id = */ 3);
+ QuicStreamFrame data3(stream_id, false, offset, max_push_id_frame1);
+ offset += max_push_id_frame1.length();
+
+ EXPECT_CALL(debug_visitor, OnMaxPushIdFrameReceived(_));
+ session_.OnStreamFrame(data3);
+
+ std::string max_push_id_frame2 = SerializeMaxPushIdFrame(/* push_id = */ 1);
+ QuicStreamFrame data4(stream_id, false, offset, max_push_id_frame2);
+
+ EXPECT_CALL(debug_visitor, OnMaxPushIdFrameReceived(_));
+ EXPECT_CALL(*connection_,
+ CloseConnection(QUIC_HTTP_INVALID_MAX_PUSH_ID,
+ "MAX_PUSH_ID received with value 1 which is "
+ "smaller that previously received value 3",
+ _));
+ session_.OnStreamFrame(data4);
+}
+
class QuicSpdySessionTestClient : public QuicSpdySessionTestBase {
protected:
QuicSpdySessionTestClient()
diff --git a/quic/core/quic_error_codes.cc b/quic/core/quic_error_codes.cc
index efbdc69..776ae8c 100644
--- a/quic/core/quic_error_codes.cc
+++ b/quic/core/quic_error_codes.cc
@@ -198,6 +198,7 @@
RETURN_STRING_LITERAL(QUIC_HTTP_CLOSED_CRITICAL_STREAM);
RETURN_STRING_LITERAL(QUIC_HTTP_MISSING_SETTINGS_FRAME);
RETURN_STRING_LITERAL(QUIC_HTTP_DUPLICATE_SETTING_IDENTIFIER);
+ RETURN_STRING_LITERAL(QUIC_HTTP_INVALID_MAX_PUSH_ID);
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);
@@ -557,6 +558,8 @@
static_cast<uint64_t>(QuicHttp3ErrorCode::MISSING_SETTINGS)};
case QUIC_HTTP_DUPLICATE_SETTING_IDENTIFIER:
return {false, static_cast<uint64_t>(QuicHttp3ErrorCode::SETTINGS_ERROR)};
+ case QUIC_HTTP_INVALID_MAX_PUSH_ID:
+ return {false, static_cast<uint64_t>(QuicHttp3ErrorCode::ID_ERROR)};
case QUIC_HPACK_INDEX_VARINT_ERROR:
return {true, static_cast<uint64_t>(INTERNAL_ERROR)};
case QUIC_HPACK_NAME_LENGTH_VARINT_ERROR:
diff --git a/quic/core/quic_error_codes.h b/quic/core/quic_error_codes.h
index b878de7..46e71b9 100644
--- a/quic/core/quic_error_codes.h
+++ b/quic/core/quic_error_codes.h
@@ -426,6 +426,9 @@
QUIC_HTTP_MISSING_SETTINGS_FRAME = 157,
// The received SETTINGS frame contains duplicate setting identifiers.
QUIC_HTTP_DUPLICATE_SETTING_IDENTIFIER = 158,
+ // MAX_PUSH_ID frame received with push ID value smaller than a previously
+ // received value.
+ QUIC_HTTP_INVALID_MAX_PUSH_ID = 159,
// HPACK header block decoding errors.
// Index varint beyond implementation limit.
@@ -462,7 +465,7 @@
QUIC_HPACK_COMPRESSED_HEADER_SIZE_EXCEEDS_LIMIT = 150,
// No error. Used as bound while iterating.
- QUIC_LAST_ERROR = 159,
+ QUIC_LAST_ERROR = 160,
};
// 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
