Internal QUICHE change
PiperOrigin-RevId: 260509821
Change-Id: I493ff904b175d015825232cd3cccf5e58c3f1300
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index c9fa852..5d58c75 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -152,18 +152,16 @@
return stream_->OnPushPromiseFrameEnd();
}
- bool OnUnknownFrameStart(uint64_t /* frame_type */,
- Http3FrameLengths /* frame_length */) override {
- // TODO(b/137554973): Consume frame header.
- return true;
+ bool OnUnknownFrameStart(uint64_t frame_type,
+ Http3FrameLengths frame_length) override {
+ return stream_->OnUnknownFrameStart(frame_type, frame_length);
}
- bool OnUnknownFramePayload(QuicStringPiece /* payload */) override {
- // TODO(b/137554973): Consume frame payload.
- return true;
+ bool OnUnknownFramePayload(QuicStringPiece payload) override {
+ return stream_->OnUnknownFramePayload(payload);
}
- bool OnUnknownFrameEnd() override { return true; }
+ bool OnUnknownFrameEnd() override { return stream_->OnUnknownFrameEnd(); }
private:
void CloseConnectionOnWrongFrame(std::string frame_type) {
@@ -967,6 +965,30 @@
return !sequencer()->IsClosed() && !reading_stopped();
}
+bool QuicSpdyStream::OnUnknownFrameStart(uint64_t frame_type,
+ Http3FrameLengths frame_length) {
+ // Ignore unknown frames, but consume frame header.
+ QUIC_DVLOG(1) << "Discarding " << frame_length.header_length
+ << " byte long frame header of frame of unknown type "
+ << frame_type << ".";
+ QUIC_DVLOG(1) << "Frame total payload length is "
+ << frame_length.payload_length << ".";
+ sequencer()->MarkConsumed(body_buffer_.OnNonBody(frame_length.header_length));
+ return true;
+}
+
+bool QuicSpdyStream::OnUnknownFramePayload(QuicStringPiece payload) {
+ // Ignore unknown frames, but consume frame payload.
+ QUIC_DVLOG(1) << "Discarding " << payload.size()
+ << " bytes of payload of frame of unknown type.";
+ sequencer()->MarkConsumed(body_buffer_.OnNonBody(payload.size()));
+ return true;
+}
+
+bool QuicSpdyStream::OnUnknownFrameEnd() {
+ return true;
+}
+
void QuicSpdyStream::ProcessDecodedHeaders(const QuicHeaderList& headers) {
if (spdy_session_->promised_stream_id() ==
QuicUtils::GetInvalidStreamId(
diff --git a/quic/core/http/quic_spdy_stream.h b/quic/core/http/quic_spdy_stream.h
index b2e9c84..48cb948 100644
--- a/quic/core/http/quic_spdy_stream.h
+++ b/quic/core/http/quic_spdy_stream.h
@@ -263,6 +263,9 @@
QuicByteCount push_id_length);
bool OnPushPromiseFramePayload(QuicStringPiece payload);
bool OnPushPromiseFrameEnd();
+ bool OnUnknownFrameStart(uint64_t frame_type, Http3FrameLengths frame_length);
+ bool OnUnknownFramePayload(QuicStringPiece payload);
+ bool OnUnknownFrameEnd();
// Called internally when headers are decoded.
void ProcessDecodedHeaders(const QuicHeaderList& headers);
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc
index f641dce..1defc5a 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -228,6 +228,24 @@
return QuicStrCat(data_frame_header, payload);
}
+ std::string UnknownFrame(uint64_t frame_type, QuicStringPiece payload) {
+ std::string frame;
+ const size_t length = QuicDataWriter::GetVarInt62Len(frame_type) +
+ QuicDataWriter::GetVarInt62Len(payload.size()) +
+ payload.size();
+ frame.resize(length);
+
+ QuicDataWriter writer(length, frame.data());
+ writer.WriteVarInt62(frame_type);
+ writer.WriteStringPieceVarInt62(payload);
+ // Even though integers can be encoded with different lengths,
+ // QuicDataWriter is expected to produce an encoding in Write*() of length
+ // promised in GetVarInt62Len().
+ DCHECK_EQ(length, writer.length());
+
+ return frame;
+ }
+
MockQuicConnectionHelper helper_;
MockAlarmFactory alarm_factory_;
MockQuicConnection* connection_;
@@ -2086,7 +2104,7 @@
// Test that stream bytes are consumed (by calling
// sequencer()->MarkConsumed()) incrementally, as soon as possible.
-TEST_P(QuicSpdyStreamIncrementalConsumptionTest, IncrementalConsumptionTest) {
+TEST_P(QuicSpdyStreamIncrementalConsumptionTest, OnlyKnownFrames) {
if (!VersionUsesQpack(GetParam().transport_version)) {
return;
}
@@ -2149,6 +2167,92 @@
ElementsAre(Pair("custom-key", "custom-value")));
}
+TEST_P(QuicSpdyStreamIncrementalConsumptionTest, UnknownFramesInterleaved) {
+ if (!VersionUsesQpack(GetParam().transport_version)) {
+ return;
+ }
+
+ Initialize(!kShouldProcessData);
+
+ // Unknown frame of reserved type before HEADERS is consumed immediately.
+ std::string unknown_frame1 = UnknownFrame(0x21, "foo");
+ OnStreamFrame(unknown_frame1);
+ EXPECT_EQ(unknown_frame1.size(), NewlyConsumedBytes());
+
+ // HEADERS frame with QPACK encoded single header field "foo: bar".
+ std::string headers =
+ HeadersFrame(QuicTextUtils::HexDecode("00002a94e703626172"));
+
+ // All HEADERS frame bytes are consumed even if the frame is not received
+ // completely.
+ OnStreamFrame(QuicStringPiece(headers).substr(0, headers.size() - 1));
+ EXPECT_EQ(headers.size() - 1, NewlyConsumedBytes());
+
+ // The rest of the HEADERS frame is also consumed immediately.
+ OnStreamFrame(QuicStringPiece(headers).substr(headers.size() - 1));
+ EXPECT_EQ(1u, NewlyConsumedBytes());
+
+ // Verify headers.
+ EXPECT_THAT(stream_->header_list(), ElementsAre(Pair("foo", "bar")));
+ stream_->ConsumeHeaderList();
+
+ // Frame of unknown, not reserved type between HEADERS and DATA is consumed
+ // immediately.
+ std::string unknown_frame2 = UnknownFrame(0x3a, "");
+ OnStreamFrame(unknown_frame2);
+ EXPECT_EQ(unknown_frame2.size(), NewlyConsumedBytes());
+
+ // DATA frame.
+ QuicStringPiece data_payload(kDataFramePayload);
+ std::string data_frame = DataFrame(data_payload);
+ QuicByteCount data_frame_header_length =
+ data_frame.size() - data_payload.size();
+
+ // DATA frame header is consumed.
+ // DATA frame payload is not consumed because payload has to be buffered.
+ OnStreamFrame(data_frame);
+ EXPECT_EQ(data_frame_header_length, NewlyConsumedBytes());
+
+ // Frame of unknown, not reserved type is not consumed because DATA payload is
+ // still buffered.
+ std::string unknown_frame3 = UnknownFrame(0x39, "bar");
+ OnStreamFrame(unknown_frame3);
+ EXPECT_EQ(0u, NewlyConsumedBytes());
+
+ // Consume all but last byte of data.
+ EXPECT_EQ(data_payload.substr(0, data_payload.size() - 1),
+ ReadFromStream(data_payload.size() - 1));
+ EXPECT_EQ(data_payload.size() - 1, NewlyConsumedBytes());
+
+ // Trailing HEADERS frame with QPACK encoded
+ // single header field "custom-key: custom-value".
+ std::string trailers = HeadersFrame(
+ QuicTextUtils::HexDecode("00002f0125a849e95ba97d7f8925a849e95bb8e8b4bf"));
+
+ // No bytes are consumed, because last byte of DATA payload is still buffered.
+ OnStreamFrame(QuicStringPiece(trailers).substr(0, trailers.size() - 1));
+ EXPECT_EQ(0u, NewlyConsumedBytes());
+
+ // Reading last byte of DATA payload triggers consumption of all data received
+ // so far, even though last HEADERS frame has not been received completely.
+ EXPECT_EQ(data_payload.substr(data_payload.size() - 1), ReadFromStream(1));
+ EXPECT_EQ(1 + unknown_frame3.size() + trailers.size() - 1,
+ NewlyConsumedBytes());
+
+ // Last byte of trailers is immediately consumed.
+ OnStreamFrame(QuicStringPiece(trailers).substr(trailers.size() - 1));
+ EXPECT_EQ(1u, NewlyConsumedBytes());
+
+ // Verify trailers.
+ EXPECT_THAT(stream_->received_trailers(),
+ ElementsAre(Pair("custom-key", "custom-value")));
+
+ // Unknown frame of reserved type after trailers is consumed immediately.
+ std::string unknown_frame4 = UnknownFrame(0x40, "");
+ OnStreamFrame(unknown_frame4);
+ EXPECT_EQ(unknown_frame4.size(), NewlyConsumedBytes());
+}
+
TEST_P(QuicSpdyStreamTest, PushPromiseOnDataStream) {
Initialize(kShouldProcessData);
if (!HasFrameHeader()) {
