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);
}
