Enable partial delivery of Push Promise Frame's push id.
gfe-relnote: v99 only, not protected.
PiperOrigin-RevId: 262578050
Change-Id: I738b7d5be49dfa07eeb5201eca21a6e144d5a3ca
diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc
index 227a839..3a13300 100644
--- a/quic/core/http/http_decoder.cc
+++ b/quic/core/http/http_decoder.cc
@@ -23,6 +23,8 @@
remaining_frame_length_(0),
current_type_field_length_(0),
remaining_type_field_length_(0),
+ current_push_id_length_(0),
+ remaining_push_id_length_(0),
error_(QUIC_NO_ERROR),
error_detail_("") {
DCHECK(visitor_);
@@ -214,25 +216,57 @@
break;
}
case static_cast<uint64_t>(HttpFrameType::PUSH_PROMISE): {
+ PushId push_id;
if (current_frame_length_ == remaining_frame_length_) {
- QuicByteCount bytes_remaining = reader->BytesRemaining();
- PushId push_id;
- QuicByteCount push_id_length = reader->PeekVarInt62Length();
- // TODO(rch): Handle partial delivery of this field.
- if (!reader->ReadVarInt62(&push_id)) {
- // TODO(b/124216424): Use HTTP_MALFORMED_FRAME.
- RaiseError(QUIC_INVALID_FRAME_DATA, "Unable to read push_id");
+ // A new Push Promise frame just arrived.
+ DCHECK_EQ(0u, current_push_id_length_);
+ current_push_id_length_ = reader->PeekVarInt62Length();
+ if (current_push_id_length_ > remaining_frame_length_) {
+ RaiseError(QUIC_INVALID_FRAME_DATA, "PUSH_PROMISE frame malformed.");
return false;
}
- remaining_frame_length_ -= bytes_remaining - reader->BytesRemaining();
- if (!visitor_->OnPushPromiseFrameStart(
- push_id,
- current_length_field_length_ + current_type_field_length_,
- push_id_length)) {
- continue_processing = false;
+ if (current_push_id_length_ > reader->BytesRemaining()) {
+ // Not all bytes of push id is present yet, buffer push id.
+ DCHECK_EQ(0u, remaining_push_id_length_);
+ remaining_push_id_length_ = current_push_id_length_;
+ BufferPushId(reader);
break;
}
+ bool success = reader->ReadVarInt62(&push_id);
+ DCHECK(success);
+ remaining_frame_length_ -= current_push_id_length_;
+ if (!visitor_->OnPushPromiseFrameStart(
+ push_id,
+ current_length_field_length_ + current_type_field_length_,
+ current_push_id_length_)) {
+ continue_processing = false;
+ current_push_id_length_ = 0;
+ break;
+ }
+ current_push_id_length_ = 0;
+ } else if (remaining_push_id_length_ > 0) {
+ // Waiting for more bytes on push id.
+ BufferPushId(reader);
+ if (remaining_push_id_length_ != 0) {
+ break;
+ }
+ QuicDataReader push_id_reader(push_id_buffer_.data(),
+ current_push_id_length_);
+
+ bool success = push_id_reader.ReadVarInt62(&push_id);
+ DCHECK(success);
+ if (!visitor_->OnPushPromiseFrameStart(
+ push_id,
+ current_length_field_length_ + current_type_field_length_,
+ current_push_id_length_)) {
+ continue_processing = false;
+ current_push_id_length_ = 0;
+ break;
+ }
+ current_push_id_length_ = 0;
}
+
+ // Read Push Promise headers.
DCHECK_LT(remaining_frame_length_, current_frame_length_);
QuicByteCount bytes_to_read = std::min<QuicByteCount>(
remaining_frame_length_, reader->BytesRemaining());
@@ -252,7 +286,6 @@
break;
}
case static_cast<uint64_t>(HttpFrameType::MAX_PUSH_ID): {
- // TODO(rch): Handle partial delivery.
BufferFramePayload(reader);
break;
}
@@ -306,7 +339,6 @@
break;
}
case static_cast<uint64_t>(HttpFrameType::CANCEL_PUSH): {
- // TODO(rch): Handle partial delivery.
CancelPushFrame frame;
QuicDataReader reader(buffer_.data(), current_frame_length_);
if (!reader.ReadVarInt62(&frame.push_id)) {
@@ -336,7 +368,6 @@
break;
}
case static_cast<uint64_t>(HttpFrameType::GOAWAY): {
- // TODO(bnc): Handle partial delivery.
QuicDataReader reader(buffer_.data(), current_frame_length_);
GoAwayFrame frame;
static_assert(!std::is_same<decltype(frame.stream_id), uint64_t>::value,
@@ -359,7 +390,6 @@
break;
}
case static_cast<uint64_t>(HttpFrameType::MAX_PUSH_ID): {
- // TODO(bnc): Handle partial delivery.
QuicDataReader reader(buffer_.data(), current_frame_length_);
MaxPushIdFrame frame;
if (!reader.ReadVarInt62(&frame.push_id)) {
@@ -376,7 +406,6 @@
break;
}
case static_cast<uint64_t>(HttpFrameType::DUPLICATE_PUSH): {
- // TODO(bnc): Handle partial delivery.
QuicDataReader reader(buffer_.data(), current_frame_length_);
DuplicatePushFrame frame;
if (!reader.ReadVarInt62(&frame.push_id)) {
@@ -433,9 +462,6 @@
}
void HttpDecoder::BufferFrameLength(QuicDataReader* reader) {
- if (current_length_field_length_ == remaining_length_field_length_) {
- length_buffer_.fill(0);
- }
QuicByteCount bytes_to_read = std::min<QuicByteCount>(
remaining_length_field_length_, reader->BytesRemaining());
bool success =
@@ -447,9 +473,6 @@
}
void HttpDecoder::BufferFrameType(QuicDataReader* reader) {
- if (current_type_field_length_ == remaining_type_field_length_) {
- type_buffer_.fill(0);
- }
QuicByteCount bytes_to_read = std::min<QuicByteCount>(
remaining_type_field_length_, reader->BytesRemaining());
bool success =
@@ -460,6 +483,19 @@
remaining_type_field_length_ -= bytes_to_read;
}
+void HttpDecoder::BufferPushId(QuicDataReader* reader) {
+ DCHECK_LE(remaining_push_id_length_, current_frame_length_);
+ QuicByteCount bytes_to_read = std::min<QuicByteCount>(
+ reader->BytesRemaining(), remaining_push_id_length_);
+ bool success =
+ reader->ReadBytes(push_id_buffer_.data() + current_push_id_length_ -
+ remaining_push_id_length_,
+ bytes_to_read);
+ DCHECK(success);
+ remaining_push_id_length_ -= bytes_to_read;
+ remaining_frame_length_ -= bytes_to_read;
+}
+
void HttpDecoder::RaiseError(QuicErrorCode error, std::string error_detail) {
state_ = STATE_ERROR;
error_ = error;
diff --git a/quic/core/http/http_decoder.h b/quic/core/http/http_decoder.h
index 21561b9..30ebadc 100644
--- a/quic/core/http/http_decoder.h
+++ b/quic/core/http/http_decoder.h
@@ -5,8 +5,11 @@
#ifndef QUICHE_QUIC_CORE_HTTP_HTTP_DECODER_H_
#define QUICHE_QUIC_CORE_HTTP_HTTP_DECODER_H_
+#include <cstdint>
+
#include "net/third_party/quiche/src/quic/core/http/http_frames.h"
#include "net/third_party/quiche/src/quic/core/quic_error_codes.h"
+#include "net/third_party/quiche/src/quic/core/quic_types.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_export.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_string_piece.h"
@@ -169,6 +172,10 @@
// Buffers any remaining frame type field from |reader| into |type_buffer_|.
void BufferFrameType(QuicDataReader* reader);
+ // Buffers at most |remaining_push_id_length_| from |reader| to
+ // |push_id_buffer_|.
+ void BufferPushId(QuicDataReader* reader);
+
// Sets |error_| and |error_detail_| accordingly.
void RaiseError(QuicErrorCode error, std::string error_detail);
@@ -199,6 +206,10 @@
QuicByteCount current_type_field_length_;
// Remaining length that's needed for the frame's type field.
QuicByteCount remaining_type_field_length_;
+ // Length of PUSH_PROMISE frame's push id.
+ QuicByteCount current_push_id_length_;
+ // Remaining length that's needed for PUSH_PROMISE frame's push id field.
+ QuicByteCount remaining_push_id_length_;
// Last error.
QuicErrorCode error_;
// The issue which caused |error_|
@@ -209,6 +220,8 @@
std::array<char, sizeof(uint64_t)> length_buffer_;
// Remaining unparsed type field data.
std::array<char, sizeof(uint64_t)> type_buffer_;
+ // Remaining unparsed push id data.
+ std::array<char, sizeof(uint64_t)> push_id_buffer_;
};
} // namespace quic
diff --git a/quic/core/http/http_decoder_test.cc b/quic/core/http/http_decoder_test.cc
index cbadf3b..12d96e8 100644
--- a/quic/core/http/http_decoder_test.cc
+++ b/quic/core/http/http_decoder_test.cc
@@ -202,19 +202,27 @@
TEST_F(HttpDecoderTest, PushPromiseFrame) {
InSequence s;
- std::string input =
- "\x05" // type (PUSH_PROMISE)
- "\x08" // length
- "\x01" // Push Id
- "Headers"; // Header Block
+ std::string input(
+ "\x05" // type (PUSH PROMISE)
+ "\x0f" // length
+ "\xC0"
+ "\x00"
+ "\x00"
+ "\x00"
+ "\x00"
+ "\x00"
+ "\x01"
+ "\x01" // push id 257.
+ "Headers",
+ 17);
// Visitor pauses processing.
- EXPECT_CALL(visitor_, OnPushPromiseFrameStart(1, 2, 1))
+ EXPECT_CALL(visitor_, OnPushPromiseFrameStart(257, 2, 8))
.WillOnce(Return(false));
QuicStringPiece remaining_input(input);
QuicByteCount processed_bytes =
ProcessInputWithGarbageAppended(remaining_input);
- EXPECT_EQ(3u, processed_bytes);
+ EXPECT_EQ(10u, processed_bytes);
remaining_input = remaining_input.substr(processed_bytes);
EXPECT_CALL(visitor_, OnPushPromiseFramePayload(QuicStringPiece("Headers")))
@@ -228,7 +236,7 @@
EXPECT_EQ("", decoder_.error_detail());
// Process the full frame.
- EXPECT_CALL(visitor_, OnPushPromiseFrameStart(1, 2, 1));
+ EXPECT_CALL(visitor_, OnPushPromiseFrameStart(257, 2, 8));
EXPECT_CALL(visitor_, OnPushPromiseFramePayload(QuicStringPiece("Headers")));
EXPECT_CALL(visitor_, OnPushPromiseFrameEnd());
EXPECT_EQ(input.size(), ProcessInput(input));
@@ -236,7 +244,7 @@
EXPECT_EQ("", decoder_.error_detail());
// Process the frame incrementally.
- EXPECT_CALL(visitor_, OnPushPromiseFrameStart(1, 2, 1));
+ EXPECT_CALL(visitor_, OnPushPromiseFrameStart(257, 2, 8));
EXPECT_CALL(visitor_, OnPushPromiseFramePayload(QuicStringPiece("H")));
EXPECT_CALL(visitor_, OnPushPromiseFramePayload(QuicStringPiece("e")));
EXPECT_CALL(visitor_, OnPushPromiseFramePayload(QuicStringPiece("a")));
@@ -248,6 +256,15 @@
ProcessInputCharByChar(input);
EXPECT_EQ(QUIC_NO_ERROR, decoder_.error());
EXPECT_EQ("", decoder_.error_detail());
+
+ // Process push id incrementally and append headers with last byte of push id.
+ EXPECT_CALL(visitor_, OnPushPromiseFrameStart(257, 2, 8));
+ EXPECT_CALL(visitor_, OnPushPromiseFramePayload(QuicStringPiece("Headers")));
+ EXPECT_CALL(visitor_, OnPushPromiseFrameEnd());
+ ProcessInputCharByChar(input.substr(0, 9));
+ EXPECT_EQ(8u, ProcessInput(input.substr(9)));
+ EXPECT_EQ(QUIC_NO_ERROR, decoder_.error());
+ EXPECT_EQ("", decoder_.error_detail());
}
TEST_F(HttpDecoderTest, MaxPushId) {
@@ -845,7 +862,7 @@
{"\x05" // type (PUSH_PROMISE)
"\x01" // length
"\x40", // first byte of two-byte varint push id
- "Unable to read push_id"},
+ "PUSH_PROMISE frame malformed."},
{"\x0D" // type (MAX_PUSH_ID)
"\x01" // length
"\x40", // first byte of two-byte varint push id
