Deprecate --gfe2_reloadable_flag_quic_parse_accept_ch_frame.
PiperOrigin-RevId: 359827183
Change-Id: I91f8989e4943055d1aed7c96baca8e2dd8086a54
diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc
index fe33a81..95f5079 100644
--- a/quic/core/http/http_decoder.cc
+++ b/quic/core/http/http_decoder.cc
@@ -238,13 +238,7 @@
continue_processing = visitor_->OnPriorityUpdateFrameStart(header_length);
break;
case static_cast<uint64_t>(HttpFrameType::ACCEPT_CH):
- if (GetQuicReloadableFlag(quic_parse_accept_ch_frame)) {
- QUIC_RELOADABLE_FLAG_COUNT(quic_parse_accept_ch_frame);
- continue_processing = visitor_->OnAcceptChFrameStart(header_length);
- } else {
- continue_processing = visitor_->OnUnknownFrameStart(
- current_frame_type_, header_length, current_frame_length_);
- }
+ continue_processing = visitor_->OnAcceptChFrameStart(header_length);
break;
default:
continue_processing = visitor_->OnUnknownFrameStart(
@@ -381,13 +375,9 @@
break;
}
case static_cast<uint64_t>(HttpFrameType::ACCEPT_CH): {
- if (GetQuicReloadableFlag(quic_parse_accept_ch_frame)) {
- // TODO(bnc): Avoid buffering if the entire frame is present, and
- // instead parse directly out of |reader|.
- BufferFramePayload(reader);
- } else {
- continue_processing = HandleUnknownFramePayload(reader);
- }
+ // TODO(bnc): Avoid buffering if the entire frame is present, and
+ // instead parse directly out of |reader|.
+ BufferFramePayload(reader);
break;
}
default: {
@@ -499,18 +489,14 @@
break;
}
case static_cast<uint64_t>(HttpFrameType::ACCEPT_CH): {
- if (GetQuicReloadableFlag(quic_parse_accept_ch_frame)) {
- // TODO(bnc): Avoid buffering if the entire frame is present, and
- // instead parse directly out of |reader|.
- AcceptChFrame frame;
- QuicDataReader reader(buffer_.data(), current_frame_length_);
- if (!ParseAcceptChFrame(&reader, &frame)) {
- return false;
- }
- continue_processing = visitor_->OnAcceptChFrame(frame);
- } else {
- continue_processing = visitor_->OnUnknownFrameEnd();
+ // TODO(bnc): Avoid buffering if the entire frame is present, and
+ // instead parse directly out of |reader|.
+ AcceptChFrame frame;
+ QuicDataReader reader(buffer_.data(), current_frame_length_);
+ if (!ParseAcceptChFrame(&reader, &frame)) {
+ return false;
}
+ continue_processing = visitor_->OnAcceptChFrame(frame);
break;
}
default:
diff --git a/quic/core/http/http_decoder_test.cc b/quic/core/http/http_decoder_test.cc
index 1be5084..4f518a4 100644
--- a/quic/core/http/http_decoder_test.cc
+++ b/quic/core/http/http_decoder_test.cc
@@ -867,11 +867,43 @@
"\x04" // length
"\x05" // valid stream id
"foo", // superfluous data
- "Superfluous data in GOAWAY frame."}};
+ "Superfluous data in GOAWAY frame."},
+ {"\x40\x89" // type (ACCEPT_CH)
+ "\x01" // length
+ "\x40", // first byte of two-byte varint origin length
+ "Unable to read ACCEPT_CH origin."},
+ {"\x40\x89" // type (ACCEPT_CH)
+ "\x01" // length
+ "\x05", // valid origin length but no origin string
+ "Unable to read ACCEPT_CH origin."},
+ {"\x40\x89" // type (ACCEPT_CH)
+ "\x04" // length
+ "\x05" // valid origin length
+ "foo", // payload ends before origin ends
+ "Unable to read ACCEPT_CH origin."},
+ {"\x40\x89" // type (ACCEPT_CH)
+ "\x04" // length
+ "\x03" // valid origin length
+ "foo", // payload ends at end of origin: no value
+ "Unable to read ACCEPT_CH value."},
+ {"\x40\x89" // type (ACCEPT_CH)
+ "\x05" // length
+ "\x03" // valid origin length
+ "foo" // payload ends at end of origin: no value
+ "\x40", // first byte of two-byte varint value length
+ "Unable to read ACCEPT_CH value."},
+ {"\x40\x89" // type (ACCEPT_CH)
+ "\x08" // length
+ "\x03" // valid origin length
+ "foo" // origin
+ "\x05" // valid value length
+ "bar", // payload ends before value ends
+ "Unable to read ACCEPT_CH value."}};
for (const auto& test_data : kTestData) {
{
HttpDecoder decoder(&visitor_);
+ EXPECT_CALL(visitor_, OnAcceptChFrameStart(_)).Times(AnyNumber());
EXPECT_CALL(visitor_, OnError(&decoder));
absl::string_view input(test_data.input);
@@ -881,6 +913,7 @@
}
{
HttpDecoder decoder(&visitor_);
+ EXPECT_CALL(visitor_, OnAcceptChFrameStart(_)).Times(AnyNumber());
EXPECT_CALL(visitor_, OnError(&decoder));
absl::string_view input(test_data.input);
@@ -1194,10 +1227,6 @@
}
TEST_F(HttpDecoderTest, AcceptChFrame) {
- if (!GetQuicReloadableFlag(quic_parse_accept_ch_frame)) {
- return;
- }
-
InSequence s;
std::string input1 = absl::HexStringToBytes(
"4089" // type (ACCEPT_CH)
@@ -1272,74 +1301,6 @@
EXPECT_EQ("", decoder_.error_detail());
}
-TEST_F(HttpDecoderTest, CorruptAcceptChFrame) {
- if (!GetQuicReloadableFlag(quic_parse_accept_ch_frame)) {
- // TODO(bnc): merge this test into HttpDecoderTest.CorruptFrame when
- // deprecating flag.
- return;
- }
-
- EXPECT_CALL(visitor_, OnAcceptChFrameStart(_)).Times(AnyNumber());
-
- struct {
- const char* const input;
- const char* const error_message;
- } kTestData[] = {{"\x40\x89" // type (ACCEPT_CH)
- "\x01" // length
- "\x40", // first byte of two-byte varint origin length
- "Unable to read ACCEPT_CH origin."},
- {"\x40\x89" // type (ACCEPT_CH)
- "\x01" // length
- "\x05", // valid origin length but no origin string
- "Unable to read ACCEPT_CH origin."},
- {"\x40\x89" // type (ACCEPT_CH)
- "\x04" // length
- "\x05" // valid origin length
- "foo", // payload ends before origin ends
- "Unable to read ACCEPT_CH origin."},
- {"\x40\x89" // type (ACCEPT_CH)
- "\x04" // length
- "\x03" // valid origin length
- "foo", // payload ends at end of origin: no value
- "Unable to read ACCEPT_CH value."},
- {"\x40\x89" // type (ACCEPT_CH)
- "\x05" // length
- "\x03" // valid origin length
- "foo" // payload ends at end of origin: no value
- "\x40", // first byte of two-byte varint value length
- "Unable to read ACCEPT_CH value."},
- {"\x40\x89" // type (ACCEPT_CH)
- "\x08" // length
- "\x03" // valid origin length
- "foo" // origin
- "\x05" // valid value length
- "bar", // payload ends before value ends
- "Unable to read ACCEPT_CH value."}};
-
- for (const auto& test_data : kTestData) {
- {
- HttpDecoder decoder(&visitor_);
- EXPECT_CALL(visitor_, OnError(&decoder));
-
- absl::string_view input(test_data.input);
- decoder.ProcessInput(input.data(), input.size());
- EXPECT_THAT(decoder.error(), IsError(QUIC_HTTP_FRAME_ERROR));
- EXPECT_EQ(test_data.error_message, decoder.error_detail());
- }
- {
- HttpDecoder decoder(&visitor_);
- EXPECT_CALL(visitor_, OnError(&decoder));
-
- absl::string_view input(test_data.input);
- for (auto c : input) {
- decoder.ProcessInput(&c, 1);
- }
- EXPECT_THAT(decoder.error(), IsError(QUIC_HTTP_FRAME_ERROR));
- EXPECT_EQ(test_data.error_message, decoder.error_detail());
- }
- }
-}
-
TEST_F(HttpDecoderTest, DecodeSettings) {
std::string input = absl::HexStringToBytes(
"04" // type (SETTINGS)
diff --git a/quic/core/http/quic_receive_control_stream.cc b/quic/core/http/quic_receive_control_stream.cc
index 35d5902..ffc38a0 100644
--- a/quic/core/http/quic_receive_control_stream.cc
+++ b/quic/core/http/quic_receive_control_stream.cc
@@ -263,8 +263,7 @@
frame_type == HttpFrameType::PUSH_PROMISE) ||
(spdy_session()->perspective() == Perspective::IS_CLIENT &&
frame_type == HttpFrameType::MAX_PUSH_ID) ||
- (GetQuicReloadableFlag(quic_parse_accept_ch_frame) &&
- spdy_session()->perspective() == Perspective::IS_SERVER &&
+ (spdy_session()->perspective() == Perspective::IS_SERVER &&
frame_type == HttpFrameType::ACCEPT_CH)) {
stream_delegate()->OnStreamError(
QUIC_HTTP_FRAME_UNEXPECTED_ON_CONTROL_STREAM,
diff --git a/quic/core/http/quic_receive_control_stream_test.cc b/quic/core/http/quic_receive_control_stream_test.cc
index 3e4532f..491255d 100644
--- a/quic/core/http/quic_receive_control_stream_test.cc
+++ b/quic/core/http/quic_receive_control_stream_test.cc
@@ -404,8 +404,7 @@
"4089" // type (ACCEPT_CH)
"00"); // length
- if (GetQuicReloadableFlag(quic_parse_accept_ch_frame) &&
- perspective() == Perspective::IS_SERVER) {
+ if (perspective() == Perspective::IS_SERVER) {
EXPECT_CALL(*connection_,
CloseConnection(
QUIC_HTTP_FRAME_UNEXPECTED_ON_CONTROL_STREAM,
@@ -449,23 +448,17 @@
"4089" // type (ACCEPT_CH)
"00"); // length
- if (GetQuicReloadableFlag(quic_parse_accept_ch_frame)) {
- if (perspective() == Perspective::IS_CLIENT) {
- EXPECT_CALL(debug_visitor, OnAcceptChFrameReceived(_));
- } else {
- EXPECT_CALL(*connection_,
- CloseConnection(
- QUIC_HTTP_FRAME_UNEXPECTED_ON_CONTROL_STREAM,
- "Invalid frame type 137 received on control stream.", _))
- .WillOnce(
- Invoke(connection_, &MockQuicConnection::ReallyCloseConnection));
- EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _, _));
- EXPECT_CALL(session_, OnConnectionClosed(_, _));
- }
+ if (perspective() == Perspective::IS_CLIENT) {
+ EXPECT_CALL(debug_visitor, OnAcceptChFrameReceived(_));
} else {
- EXPECT_CALL(debug_visitor,
- OnUnknownFrameReceived(id, /* frame_type = */ 0x89,
- /* payload_length = */ 0));
+ EXPECT_CALL(*connection_,
+ CloseConnection(
+ QUIC_HTTP_FRAME_UNEXPECTED_ON_CONTROL_STREAM,
+ "Invalid frame type 137 received on control stream.", _))
+ .WillOnce(
+ Invoke(connection_, &MockQuicConnection::ReallyCloseConnection));
+ EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _, _));
+ EXPECT_CALL(session_, OnConnectionClosed(_, _));
}
receive_control_stream_->OnStreamFrame(
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index b266633..3985769 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -3203,10 +3203,6 @@
return;
}
- if (!GetQuicReloadableFlag(quic_parse_accept_ch_frame)) {
- return;
- }
-
StrictMock<MockHttp3DebugVisitor> debug_visitor;
session_.set_debug_visitor(&debug_visitor);
@@ -3265,11 +3261,9 @@
"03" // length of value
"626172"); // value "bar"
- if (GetQuicReloadableFlag(quic_parse_accept_ch_frame)) {
- AcceptChFrame expected_accept_ch_frame{{{"foo", "bar"}}};
- EXPECT_CALL(debug_visitor,
- OnAcceptChFrameReceivedViaAlps(expected_accept_ch_frame));
- }
+ AcceptChFrame expected_accept_ch_frame{{{"foo", "bar"}}};
+ EXPECT_CALL(debug_visitor,
+ OnAcceptChFrameReceivedViaAlps(expected_accept_ch_frame));
auto error = session_.OnAlpsData(
reinterpret_cast<const uint8_t*>(serialized_accept_ch_frame.data()),
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index 40f8502..d2a66e0 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -46,7 +46,6 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_on_stream_reset, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_willing_and_able_to_write2, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_h3_datagram, false)
-QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_parse_accept_ch_frame, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_pass_path_response_to_validator, false)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_require_handshake_confirmation, false)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_send_goaway_with_connection_close, true)
