Update goaway behavior. protected by gfe2_reloadable_flag_quic_http3_goaway_new_behavior.
Updating behavior to https://github.com/quicwg/base-drafts/pull/3129.
If client receives GOAWAY with Stream ID that is not client-initiated
bidirectional stream ID, it now closes connection with application error
H3_ID_ERROR instead of transport error PROTOCOL_VIOLATION (draft-04 behavior).
If server receives GOAWAY, it ignores it instead of closing connection with
H3_FRAME_UNEXPECTED (draft-27 behavior). Stream ID/push ID carried by GOAWAY
frame is ignored in all cases.
This updates behavior to draft-28. While we are still supporting draft-27,
this should be okay as the new behavior is more permissive.
PiperOrigin-RevId: 323358652
Change-Id: I17919364a259a38a110284bab0023779b194094e
diff --git a/quic/core/http/quic_receive_control_stream.cc b/quic/core/http/quic_receive_control_stream.cc
index e0e51a7..82d1fe1 100644
--- a/quic/core/http/quic_receive_control_stream.cc
+++ b/quic/core/http/quic_receive_control_stream.cc
@@ -10,6 +10,8 @@
#include "net/third_party/quiche/src/quic/core/http/http_decoder.h"
#include "net/third_party/quiche/src/quic/core/http/quic_spdy_session.h"
#include "net/third_party/quiche/src/quic/core/quic_types.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_flag_utils.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_flags.h"
#include "net/third_party/quiche/src/common/platform/api/quiche_string_piece.h"
#include "net/third_party/quiche/src/common/platform/api/quiche_text_utils.h"
@@ -105,7 +107,9 @@
return false;
}
- if (spdy_session()->perspective() == Perspective::IS_SERVER) {
+ if (GetQuicReloadableFlag(quic_http3_goaway_new_behavior)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_http3_goaway_new_behavior);
+ } else if (spdy_session()->perspective() == Perspective::IS_SERVER) {
OnWrongFrame("Go Away");
return false;
}
diff --git a/quic/core/http/quic_receive_control_stream_test.cc b/quic/core/http/quic_receive_control_stream_test.cc
index 91e09f2..6e677d7 100644
--- a/quic/core/http/quic_receive_control_stream_test.cc
+++ b/quic/core/http/quic_receive_control_stream_test.cc
@@ -294,14 +294,16 @@
EXPECT_CALL(debug_visitor, OnGoAwayFrameReceived(goaway));
- if (perspective() == Perspective::IS_SERVER) {
+ if (!GetQuicReloadableFlag(quic_http3_goaway_new_behavior) &&
+ perspective() == Perspective::IS_SERVER) {
EXPECT_CALL(
*connection_,
CloseConnection(QUIC_HTTP_FRAME_UNEXPECTED_ON_CONTROL_STREAM, _, _));
}
receive_control_stream_->OnStreamFrame(frame);
- if (perspective() == Perspective::IS_CLIENT) {
+ if (GetQuicReloadableFlag(quic_http3_goaway_new_behavior) ||
+ perspective() == Perspective::IS_CLIENT) {
EXPECT_TRUE(session_.http3_goaway_received());
}
}
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index 63b6b87..e3f6dcf 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -404,7 +404,6 @@
server_push_enabled_(true),
ietf_server_push_enabled_(
GetQuicFlag(FLAGS_quic_enable_http3_server_push)),
- http3_goaway_received_(false),
http3_goaway_sent_(false),
http3_max_push_id_sent_(false) {
h2_deframer_.set_visitor(spdy_framer_visitor_.get());
@@ -657,6 +656,41 @@
}
void QuicSpdySession::OnHttp3GoAway(uint64_t id) {
+ if (GetQuicReloadableFlag(quic_http3_goaway_new_behavior)) {
+ if (last_received_http3_goaway_id_.has_value() &&
+ id > last_received_http3_goaway_id_.value()) {
+ CloseConnectionWithDetails(
+ QUIC_HTTP_GOAWAY_ID_LARGER_THAN_PREVIOUS,
+ quiche::QuicheStrCat("GOAWAY received with ID ", id,
+ " greater than previously received ID ",
+ last_received_http3_goaway_id_.value()));
+ return;
+ }
+ last_received_http3_goaway_id_ = id;
+
+ if (perspective() == Perspective::IS_SERVER) {
+ // TODO(b/151749109): Cancel server pushes with push ID larger than |id|.
+ return;
+ }
+
+ // QuicStreamId is uint32_t. Casting to this narrower type is well-defined
+ // and preserves the lower 32 bits. Both IsBidirectionalStreamId() and
+ // IsIncomingStream() give correct results, because their return value is
+ // determined by the least significant two bits.
+ QuicStreamId stream_id = static_cast<QuicStreamId>(id);
+ if (!QuicUtils::IsBidirectionalStreamId(stream_id, version()) ||
+ IsIncomingStream(stream_id)) {
+ CloseConnectionWithDetails(QUIC_HTTP_GOAWAY_INVALID_STREAM_ID,
+ "GOAWAY with invalid stream ID");
+ return;
+ }
+
+ // TODO(b/161252736): Cancel client requests with ID larger than |id|.
+ // If |id| is larger than numeric_limits<QuicStreamId>::max(), then use
+ // max() instead of downcast value.
+ return;
+ }
+
DCHECK_EQ(perspective(), Perspective::IS_CLIENT);
// QuicStreamId is uint32_t. Casting to this narrower type is well-defined
@@ -671,7 +705,7 @@
"GOAWAY's last stream id has to point to a request stream");
return;
}
- http3_goaway_received_ = true;
+ last_received_http3_goaway_id_ = id;
}
bool QuicSpdySession::OnStreamsBlockedFrame(
diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h
index 9342da2..6dc65f3 100644
--- a/quic/core/http/quic_spdy_session.h
+++ b/quic/core/http/quic_spdy_session.h
@@ -351,7 +351,9 @@
Http3DebugVisitor* debug_visitor() { return debug_visitor_; }
- bool http3_goaway_received() const { return http3_goaway_received_; }
+ bool http3_goaway_received() const {
+ return last_received_http3_goaway_id_.has_value();
+ }
bool http3_goaway_sent() const { return http3_goaway_sent_; }
@@ -577,8 +579,9 @@
// Server push is enabled for a client by calling SetMaxPushId().
bool ietf_server_push_enabled_;
- // If the endpoint has received HTTP/3 GOAWAY frame.
- bool http3_goaway_received_;
+ // The identifier in the most recently received GOAWAY frame. Unset if no
+ // GOAWAY frame has been received yet.
+ quiche::QuicheOptional<uint64_t> last_received_http3_goaway_id_;
// If the endpoint has sent HTTP/3 GOAWAY frame.
bool http3_goaway_sent_;
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index df83890..18b3951 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -1114,6 +1114,29 @@
session_.OnGoAway(go_away);
}
+TEST_P(QuicSpdySessionTestServer, Http3GoAwayLargerIdThanBefore) {
+ if (!VersionUsesHttp3(transport_version())) {
+ return;
+ }
+ if (!GetQuicReloadableFlag(quic_http3_goaway_new_behavior)) {
+ return;
+ }
+
+ EXPECT_FALSE(session_.http3_goaway_received());
+ PushId push_id1 = 0;
+ session_.OnHttp3GoAway(push_id1);
+ EXPECT_TRUE(session_.http3_goaway_received());
+
+ EXPECT_CALL(
+ *connection_,
+ CloseConnection(
+ QUIC_HTTP_GOAWAY_ID_LARGER_THAN_PREVIOUS,
+ "GOAWAY received with ID 1 greater than previously received ID 0",
+ _));
+ PushId push_id2 = 1;
+ session_.OnHttp3GoAway(push_id2);
+}
+
// Test that server session will send a connectivity probe in response to a
// connectivity probe on the same path.
TEST_P(QuicSpdySessionTestServer, ServerReplyToConnecitivityProbe) {
@@ -2831,16 +2854,47 @@
if (!VersionUsesHttp3(transport_version())) {
return;
}
- EXPECT_CALL(
- *connection_,
- CloseConnection(
- QUIC_INVALID_STREAM_ID,
- "GOAWAY's last stream id has to point to a request stream", _));
+ if (GetQuicReloadableFlag(quic_http3_goaway_new_behavior)) {
+ EXPECT_CALL(*connection_,
+ CloseConnection(QUIC_HTTP_GOAWAY_INVALID_STREAM_ID,
+ "GOAWAY with invalid stream ID", _));
+ } else {
+ EXPECT_CALL(
+ *connection_,
+ CloseConnection(
+ QUIC_INVALID_STREAM_ID,
+ "GOAWAY's last stream id has to point to a request stream", _));
+ }
QuicStreamId stream_id =
GetNthServerInitiatedUnidirectionalStreamId(transport_version(), 0);
session_.OnHttp3GoAway(stream_id);
}
+TEST_P(QuicSpdySessionTestClient, Http3GoAwayLargerIdThanBefore) {
+ if (!VersionUsesHttp3(transport_version())) {
+ return;
+ }
+ if (!GetQuicReloadableFlag(quic_http3_goaway_new_behavior)) {
+ return;
+ }
+
+ EXPECT_FALSE(session_.http3_goaway_received());
+ QuicStreamId stream_id1 =
+ GetNthClientInitiatedBidirectionalStreamId(transport_version(), 0);
+ session_.OnHttp3GoAway(stream_id1);
+ EXPECT_TRUE(session_.http3_goaway_received());
+
+ EXPECT_CALL(
+ *connection_,
+ CloseConnection(
+ QUIC_HTTP_GOAWAY_ID_LARGER_THAN_PREVIOUS,
+ "GOAWAY received with ID 4 greater than previously received ID 0",
+ _));
+ QuicStreamId stream_id2 =
+ GetNthClientInitiatedBidirectionalStreamId(transport_version(), 1);
+ session_.OnHttp3GoAway(stream_id2);
+}
+
// Test that receipt of CANCEL_PUSH frame does not result in closing the
// connection.
// TODO(b/151841240): Handle CANCEL_PUSH frames instead of ignoring them.
diff --git a/quic/core/quic_error_codes.cc b/quic/core/quic_error_codes.cc
index 1c0984a..7065363 100644
--- a/quic/core/quic_error_codes.cc
+++ b/quic/core/quic_error_codes.cc
@@ -205,6 +205,8 @@
RETURN_STRING_LITERAL(QUIC_HTTP_STREAM_LIMIT_TOO_LOW);
RETURN_STRING_LITERAL(QUIC_HTTP_ZERO_RTT_RESUMPTION_SETTINGS_MISMATCH);
RETURN_STRING_LITERAL(QUIC_HTTP_ZERO_RTT_REJECTION_SETTINGS_MISMATCH);
+ RETURN_STRING_LITERAL(QUIC_HTTP_GOAWAY_INVALID_STREAM_ID);
+ RETURN_STRING_LITERAL(QUIC_HTTP_GOAWAY_ID_LARGER_THAN_PREVIOUS);
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);
@@ -576,6 +578,10 @@
return {false, static_cast<uint64_t>(QuicHttp3ErrorCode::SETTINGS_ERROR)};
case QUIC_HTTP_ZERO_RTT_REJECTION_SETTINGS_MISMATCH:
return {true, static_cast<uint64_t>(INTERNAL_ERROR)};
+ case QUIC_HTTP_GOAWAY_INVALID_STREAM_ID:
+ return {false, static_cast<uint64_t>(QuicHttp3ErrorCode::ID_ERROR)};
+ case QUIC_HTTP_GOAWAY_ID_LARGER_THAN_PREVIOUS:
+ 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 4c13f2c..0c990c7 100644
--- a/quic/core/quic_error_codes.h
+++ b/quic/core/quic_error_codes.h
@@ -437,6 +437,11 @@
// Received mismatched SETTINGS frame from HTTP/3 connection where early data
// is rejected. Our implementation currently doesn't support it.
QUIC_HTTP_ZERO_RTT_REJECTION_SETTINGS_MISMATCH = 165,
+ // Client received GOAWAY frame with stream ID that is not for a
+ // client-initiated bidirectional stream.
+ QUIC_HTTP_GOAWAY_INVALID_STREAM_ID = 166,
+ // Received GOAWAY frame with ID that is greater than previously received ID.
+ QUIC_HTTP_GOAWAY_ID_LARGER_THAN_PREVIOUS = 167,
// HPACK header block decoding errors.
// Index varint beyond implementation limit.
@@ -485,7 +490,7 @@
QUIC_ZERO_RTT_RESUMPTION_LIMIT_REDUCED = 163,
// No error. Used as bound while iterating.
- QUIC_LAST_ERROR = 166,
+ QUIC_LAST_ERROR = 168,
};
// 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
