gfe-relnote: Remove DUPLICATE_PUSH frame. Protected by gfe2_reloadable_flag_quic_enable_version_draft_25_v3 and gfe2_reloadable_flag_quic_enable_version_draft_27. DUPLICATE_PUSH frame has been removed from HTTP/3 specs at https://github.com/quicwg/base-drafts/pull/3309. PiperOrigin-RevId: 300117789 Change-Id: I39aef6d20829fca9185de9aa1272e4cd707782c7
diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc index c5aa08c..4e3734a 100644 --- a/quic/core/http/http_decoder.cc +++ b/quic/core/http/http_decoder.cc
@@ -164,8 +164,6 @@ break; case static_cast<uint64_t>(HttpFrameType::MAX_PUSH_ID): break; - case static_cast<uint64_t>(HttpFrameType::DUPLICATE_PUSH): - break; case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE): continue_processing = visitor_->OnPriorityUpdateFrameStart(header_length); break; @@ -289,10 +287,6 @@ BufferFramePayload(reader); break; } - case static_cast<uint64_t>(HttpFrameType::DUPLICATE_PUSH): { - BufferFramePayload(reader); - break; - } case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE): { // TODO(bnc): Avoid buffering if the entire frame is present, and // instead parse directly out of |reader|. @@ -398,22 +392,6 @@ continue_processing = visitor_->OnMaxPushIdFrame(frame); break; } - case static_cast<uint64_t>(HttpFrameType::DUPLICATE_PUSH): { - QuicDataReader reader(buffer_.data(), current_frame_length_); - DuplicatePushFrame frame; - if (!reader.ReadVarInt62(&frame.push_id)) { - RaiseError(QUIC_HTTP_FRAME_ERROR, - "Unable to read DUPLICATE_PUSH push_id."); - return false; - } - if (!reader.IsDoneReading()) { - RaiseError(QUIC_HTTP_FRAME_ERROR, - "Superfluous data in DUPLICATE_PUSH frame."); - return false; - } - continue_processing = visitor_->OnDuplicatePushFrame(frame); - break; - } case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE): { // TODO(bnc): Avoid buffering if the entire frame is present, and // instead parse directly out of |reader|. @@ -571,8 +549,6 @@ return VARIABLE_LENGTH_INTEGER_LENGTH_8; case static_cast<uint64_t>(HttpFrameType::MAX_PUSH_ID): return sizeof(PushId); - case static_cast<uint64_t>(HttpFrameType::DUPLICATE_PUSH): - return sizeof(PushId); case static_cast<uint64_t>(HttpFrameType::PRIORITY_UPDATE): // This limit is arbitrary. return 1024 * 1024;
diff --git a/quic/core/http/http_decoder.h b/quic/core/http/http_decoder.h index 426a86e..2558805 100644 --- a/quic/core/http/http_decoder.h +++ b/quic/core/http/http_decoder.h
@@ -54,9 +54,6 @@ // Called when a SETTINGS frame has been successfully parsed. virtual bool OnSettingsFrame(const SettingsFrame& frame) = 0; - // Called when a DUPLICATE_PUSH frame has been successfully parsed. - virtual bool OnDuplicatePushFrame(const DuplicatePushFrame& frame) = 0; - // Called when a DATA frame has been received. // |header_length| contains DATA frame length and payload length. virtual bool OnDataFrameStart(QuicByteCount header_length) = 0;
diff --git a/quic/core/http/http_decoder_test.cc b/quic/core/http/http_decoder_test.cc index 7cd0853..1dba599 100644 --- a/quic/core/http/http_decoder_test.cc +++ b/quic/core/http/http_decoder_test.cc
@@ -46,7 +46,6 @@ MOCK_METHOD1(OnGoAwayFrame, bool(const GoAwayFrame& frame)); MOCK_METHOD1(OnSettingsFrameStart, bool(QuicByteCount header_length)); MOCK_METHOD1(OnSettingsFrame, bool(const SettingsFrame& frame)); - MOCK_METHOD1(OnDuplicatePushFrame, bool(const DuplicatePushFrame& frame)); MOCK_METHOD1(OnDataFrameStart, bool(QuicByteCount header_length)); MOCK_METHOD1(OnDataFramePayload, bool(quiche::QuicheStringPiece payload)); @@ -79,7 +78,6 @@ ON_CALL(visitor_, OnGoAwayFrame(_)).WillByDefault(Return(true)); ON_CALL(visitor_, OnSettingsFrameStart(_)).WillByDefault(Return(true)); ON_CALL(visitor_, OnSettingsFrame(_)).WillByDefault(Return(true)); - ON_CALL(visitor_, OnDuplicatePushFrame(_)).WillByDefault(Return(true)); ON_CALL(visitor_, OnDataFrameStart(_)).WillByDefault(Return(true)); ON_CALL(visitor_, OnDataFramePayload(_)).WillByDefault(Return(true)); ON_CALL(visitor_, OnDataFrameEnd()).WillByDefault(Return(true)); @@ -347,33 +345,6 @@ EXPECT_EQ("", decoder_.error_detail()); } -TEST_F(HttpDecoderTest, DuplicatePush) { - InSequence s; - std::string input = quiche::QuicheTextUtils::HexDecode( - "0E" // type (DUPLICATE_PUSH) - "01" // length - "01"); // Push Id - - // Visitor pauses processing. - EXPECT_CALL(visitor_, OnDuplicatePushFrame(DuplicatePushFrame({1}))) - .WillOnce(Return(false)); - EXPECT_EQ(input.size(), ProcessInputWithGarbageAppended(input)); - EXPECT_THAT(decoder_.error(), IsQuicNoError()); - EXPECT_EQ("", decoder_.error_detail()); - - // Process the full frame. - EXPECT_CALL(visitor_, OnDuplicatePushFrame(DuplicatePushFrame({1}))); - EXPECT_EQ(input.size(), ProcessInput(input)); - EXPECT_THAT(decoder_.error(), IsQuicNoError()); - EXPECT_EQ("", decoder_.error_detail()); - - // Process the frame incrementally. - EXPECT_CALL(visitor_, OnDuplicatePushFrame(DuplicatePushFrame({1}))); - ProcessInputCharByChar(input); - EXPECT_THAT(decoder_.error(), IsQuicNoError()); - EXPECT_EQ("", decoder_.error_detail()); -} - TEST_F(HttpDecoderTest, SettingsFrame) { InSequence s; std::string input = quiche::QuicheTextUtils::HexDecode( @@ -829,15 +800,6 @@ "\x05" // valid push id "foo", // superfluous data "Superfluous data in MAX_PUSH_ID frame."}, - {"\x0E" // type (DUPLICATE_PUSH) - "\x01" // length - "\x40", // first byte of two-byte varint push id - "Unable to read DUPLICATE_PUSH push_id."}, - {"\x0E" // type (DUPLICATE_PUSH) - "\x04" // length - "\x05" // valid push id - "foo", // superfluous data - "Superfluous data in DUPLICATE_PUSH frame."}, {"\x07" // type (GOAWAY) "\x01" // length "\x40", // first byte of two-byte varint stream id @@ -932,17 +894,6 @@ EXPECT_EQ("Unable to read MAX_PUSH_ID push_id.", decoder_.error_detail()); } -TEST_F(HttpDecoderTest, EmptyDuplicatePushFrame) { - std::string input = quiche::QuicheTextUtils::HexDecode( - "0e" // type (DUPLICATE_PUSH) - "00"); // frame length - - EXPECT_CALL(visitor_, OnError(&decoder_)); - EXPECT_EQ(input.size(), ProcessInput(input)); - EXPECT_THAT(decoder_.error(), IsError(QUIC_HTTP_FRAME_ERROR)); - EXPECT_EQ("Unable to read DUPLICATE_PUSH push_id.", decoder_.error_detail()); -} - TEST_F(HttpDecoderTest, LargeStreamIdInGoAway) { GoAwayFrame frame; frame.stream_id = 1 << 30;
diff --git a/quic/core/http/http_encoder.cc b/quic/core/http/http_encoder.cc index dfc7946..6066b26 100644 --- a/quic/core/http/http_encoder.cc +++ b/quic/core/http/http_encoder.cc
@@ -197,28 +197,6 @@ } // static -QuicByteCount HttpEncoder::SerializeDuplicatePushFrame( - const DuplicatePushFrame& duplicate_push, - std::unique_ptr<char[]>* output) { - QuicByteCount payload_length = - QuicDataWriter::GetVarInt62Len(duplicate_push.push_id); - QuicByteCount total_length = - GetTotalLength(payload_length, HttpFrameType::DUPLICATE_PUSH); - - output->reset(new char[total_length]); - QuicDataWriter writer(total_length, output->get()); - - if (WriteFrameHeader(payload_length, HttpFrameType::DUPLICATE_PUSH, - &writer) && - writer.WriteVarInt62(duplicate_push.push_id)) { - return total_length; - } - QUIC_DLOG(ERROR) << "Http encoder failed when attempting to serialize " - "duplicate push frame."; - return 0; -} - -// static QuicByteCount HttpEncoder::SerializePriorityUpdateFrame( const PriorityUpdateFrame& priority_update, std::unique_ptr<char[]>* output) {
diff --git a/quic/core/http/http_encoder.h b/quic/core/http/http_encoder.h index 5a835c2..4d6a54a 100644 --- a/quic/core/http/http_encoder.h +++ b/quic/core/http/http_encoder.h
@@ -61,12 +61,6 @@ const MaxPushIdFrame& max_push_id, std::unique_ptr<char[]>* output); - // Serialize a DUPLICATE_PUSH frame into a new buffer stored in |output|. - // Returns the length of the buffer on success, or 0 otherwise. - static QuicByteCount SerializeDuplicatePushFrame( - const DuplicatePushFrame& duplicate_push, - std::unique_ptr<char[]>* output); - // Serializes a PRIORITY_UPDATE frame into a new buffer stored in |output|. // Returns the length of the buffer on success, or 0 otherwise. static QuicByteCount SerializePriorityUpdateFrame(
diff --git a/quic/core/http/http_encoder_test.cc b/quic/core/http/http_encoder_test.cc index 316d6be..aad20e5 100644 --- a/quic/core/http/http_encoder_test.cc +++ b/quic/core/http/http_encoder_test.cc
@@ -132,23 +132,6 @@ "MAX_PUSH_ID", buffer.get(), length, output, QUICHE_ARRAYSIZE(output)); } -TEST(HttpEncoderTest, SerializeDuplicatePushFrame) { - DuplicatePushFrame duplicate_push; - duplicate_push.push_id = 0x1; - char output[] = {// type (DUPLICATE_PUSH) - 0x0E, - // length - 0x1, - // Push Id - 0x01}; - std::unique_ptr<char[]> buffer; - uint64_t length = - HttpEncoder::SerializeDuplicatePushFrame(duplicate_push, &buffer); - EXPECT_EQ(QUICHE_ARRAYSIZE(output), length); - quiche::test::CompareCharArraysWithHexError( - "DUPLICATE_PUSH", buffer.get(), length, output, QUICHE_ARRAYSIZE(output)); -} - TEST(HttpEncoderTest, SerializePriorityUpdateFrame) { PriorityUpdateFrame priority_update1; priority_update1.prioritized_element_type = REQUEST_STREAM;
diff --git a/quic/core/http/http_frames.h b/quic/core/http/http_frames.h index e1e184f..d7d875a 100644 --- a/quic/core/http/http_frames.h +++ b/quic/core/http/http_frames.h
@@ -26,7 +26,6 @@ PUSH_PROMISE = 0x5, GOAWAY = 0x7, MAX_PUSH_ID = 0xD, - DUPLICATE_PUSH = 0xE, PRIORITY_UPDATE = 0XF, }; @@ -130,19 +129,6 @@ } }; -// 7.2.8. DUPLICATE_PUSH -// -// The DUPLICATE_PUSH frame (type=0xE) is used by servers to indicate -// that an existing pushed resource is related to multiple client -// requests. -struct QUIC_EXPORT_PRIVATE DuplicatePushFrame { - PushId push_id; - - bool operator==(const DuplicatePushFrame& rhs) const { - return push_id == rhs.push_id; - } -}; - // https://httpwg.org/http-extensions/draft-ietf-httpbis-priority.html // // The PRIORITY_UPDATE (type=0x0f) frame specifies the sender-advised priority
diff --git a/quic/core/http/quic_receive_control_stream.cc b/quic/core/http/quic_receive_control_stream.cc index 6e7389b..4f21498 100644 --- a/quic/core/http/quic_receive_control_stream.cc +++ b/quic/core/http/quic_receive_control_stream.cc
@@ -60,11 +60,6 @@ return stream_->OnSettingsFrame(frame); } - bool OnDuplicatePushFrame(const DuplicatePushFrame& /*frame*/) override { - OnWrongFrame("Duplicate Push"); - return false; - } - bool OnDataFrameStart(QuicByteCount /*header_length*/) override { OnWrongFrame("Data"); 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 84cc8ed..67a9a17 100644 --- a/quic/core/http/quic_receive_control_stream_test.cc +++ b/quic/core/http/quic_receive_control_stream_test.cc
@@ -233,11 +233,11 @@ } TEST_P(QuicReceiveControlStreamTest, ReceiveWrongFrame) { - DuplicatePushFrame dup; - dup.push_id = 0x1; + CancelPushFrame cancel; + cancel.push_id = 0x1; std::unique_ptr<char[]> buffer; QuicByteCount header_length = - HttpEncoder::SerializeDuplicatePushFrame(dup, &buffer); + HttpEncoder::SerializeCancelPushFrame(cancel, &buffer); std::string data = std::string(buffer.get(), header_length); QuicStreamFrame frame(receive_control_stream_->id(), false, 1, data);
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc index 67cc6b4..b019b1e 100644 --- a/quic/core/http/quic_spdy_stream.cc +++ b/quic/core/http/quic_spdy_stream.cc
@@ -69,12 +69,6 @@ return false; } - bool OnDuplicatePushFrame(const DuplicatePushFrame& /*frame*/) override { - // TODO(b/137554973): Consume frame. - CloseConnectionOnWrongFrame("Duplicate Push"); - return false; - } - bool OnDataFrameStart(QuicByteCount header_length) override { return stream_->OnDataFrameStart(header_length); }