Change HTTP/3 frame type from 1 byte integer to variable length integer, and
allow partial delivery.
gfe-relnote: Used in version 99 only.
PiperOrigin-RevId: 244287295
Change-Id: Ic9a58f2822815006295f47f23f312b1f2c040f4e
diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc
index ec0062a..653652f 100644
--- a/quic/core/http/http_decoder.cc
+++ b/quic/core/http/http_decoder.cc
@@ -24,8 +24,6 @@
return (flags >> offset) & GetMaskFromNumBits(num_bits);
}
-// Length of the type field of HTTP/3 frames.
-static const QuicByteCount kFrameTypeLength = 1;
// Length of the weight field of a priority frame.
static const size_t kPriorityWeightLength = 1;
// Length of a priority frame's first byte.
@@ -41,6 +39,8 @@
remaining_length_field_length_(0),
current_frame_length_(0),
remaining_frame_length_(0),
+ current_type_field_length_(0),
+ remaining_type_field_length_(0),
error_(QUIC_NO_ERROR),
error_detail_("") {}
@@ -97,9 +97,39 @@
void HttpDecoder::ReadFrameType(QuicDataReader* reader) {
DCHECK_NE(0u, reader->BytesRemaining());
- if (!reader->ReadUInt8(¤t_frame_type_)) {
- RaiseError(QUIC_INTERNAL_ERROR, "Unable to read frame type");
- return;
+ if (current_type_field_length_ == 0) {
+ // A new frame is coming.
+ current_type_field_length_ = reader->PeekVarInt62Length();
+ if (current_type_field_length_ == 0) {
+ RaiseError(QUIC_INTERNAL_ERROR, "Unable to read frame type length");
+ visitor_->OnError(this);
+ return;
+ }
+ if (current_type_field_length_ <= reader->BytesRemaining()) {
+ // The reader has all type data needed, so no need to buffer.
+ if (!reader->ReadVarInt62(¤t_frame_type_)) {
+ RaiseError(QUIC_INTERNAL_ERROR, "Unable to read frame type");
+ return;
+ }
+ } else {
+ // Buffer a new type field.
+ remaining_type_field_length_ = current_type_field_length_;
+ BufferFrameType(reader);
+ return;
+ }
+ } else {
+ // Buffer the existing type field.
+ BufferFrameType(reader);
+ // The frame is still not buffered completely.
+ if (remaining_type_field_length_ != 0) {
+ return;
+ }
+ QuicDataReader type_reader(type_buffer_.data(), current_type_field_length_);
+ if (!type_reader.ReadVarInt62(¤t_frame_type_)) {
+ RaiseError(QUIC_INTERNAL_ERROR, "Unable to read buffered frame type");
+ visitor_->OnError(this);
+ return;
+ }
}
if (current_frame_length_ > MaxFrameLength(current_frame_type_)) {
@@ -112,13 +142,16 @@
// frame payload.
if (current_frame_type_ == 0x0) {
visitor_->OnDataFrameStart(Http3FrameLengths(
- current_length_field_size_ + kFrameTypeLength, current_frame_length_));
+ current_length_field_size_ + current_type_field_length_,
+ current_frame_length_));
} else if (current_frame_type_ == 0x1) {
visitor_->OnHeadersFrameStart(Http3FrameLengths(
- current_length_field_size_ + kFrameTypeLength, current_frame_length_));
+ current_length_field_size_ + current_type_field_length_,
+ current_frame_length_));
} else if (current_frame_type_ == 0x4) {
visitor_->OnSettingsFrameStart(Http3FrameLengths(
- current_length_field_size_ + kFrameTypeLength, current_frame_length_));
+ current_length_field_size_ + current_type_field_length_,
+ current_frame_length_));
}
state_ = (remaining_frame_length_ == 0) ? STATE_FINISH_PARSING
@@ -331,6 +364,7 @@
}
current_length_field_size_ = 0;
+ current_type_field_length_ = 0;
state_ = STATE_READING_FRAME_LENGTH;
}
@@ -346,6 +380,7 @@
if (remaining_frame_length_ == 0) {
state_ = STATE_READING_FRAME_LENGTH;
current_length_field_size_ = 0;
+ current_type_field_length_ = 0;
}
}
@@ -391,6 +426,22 @@
remaining_length_field_length_ -= bytes_to_read;
}
+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());
+ if (!reader->ReadBytes(type_buffer_.data() + current_type_field_length_ -
+ remaining_type_field_length_,
+ bytes_to_read)) {
+ RaiseError(QUIC_INTERNAL_ERROR, "Unable to buffer frame type bytes.");
+ visitor_->OnError(this);
+ return;
+ }
+ remaining_type_field_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 55de8ad..8ed7973 100644
--- a/quic/core/http/http_decoder.h
+++ b/quic/core/http/http_decoder.h
@@ -117,6 +117,7 @@
QuicErrorCode error() const { return error_; }
const std::string& error_detail() const { return error_detail_; }
+ uint64_t current_frame_type() const { return current_frame_type_; }
private:
// Represents the current state of the parsing state machine.
@@ -152,9 +153,12 @@
void BufferFramePayload(QuicDataReader* reader);
// Buffers any remaining frame length field from |reader| into
- // |length_buffer_|
+ // |length_buffer_|.
void BufferFrameLength(QuicDataReader* reader);
+ // Buffers any remaining frame type field from |reader| into |type_buffer_|.
+ void BufferFrameType(QuicDataReader* reader);
+
// Sets |error_| and |error_detail_| accordingly.
void RaiseError(QuicErrorCode error, std::string error_detail);
@@ -172,7 +176,7 @@
// Current state of the parsing.
HttpDecoderState state_;
// Type of the frame currently being parsed.
- uint8_t current_frame_type_;
+ uint64_t current_frame_type_;
// Size of the frame's length field.
QuicByteCount current_length_field_size_;
// Remaining length that's needed for the frame's length field.
@@ -181,6 +185,10 @@
QuicByteCount current_frame_length_;
// Remaining payload bytes to be parsed.
QuicByteCount remaining_frame_length_;
+ // Length of the frame's type field.
+ QuicByteCount current_type_field_length_;
+ // Remaining length that's needed for the frame's type field.
+ QuicByteCount remaining_type_field_length_;
// Last error.
QuicErrorCode error_;
// The issue which caused |error_|
@@ -189,6 +197,8 @@
std::string buffer_;
// Remaining unparsed length field data.
std::string length_buffer_;
+ // Remaining unparsed type field data.
+ std::array<char, sizeof(uint64_t)> type_buffer_;
};
} // namespace quic
diff --git a/quic/core/http/http_decoder_test.cc b/quic/core/http/http_decoder_test.cc
index 7f0dcd4..b0a3f6b 100644
--- a/quic/core/http/http_decoder_test.cc
+++ b/quic/core/http/http_decoder_test.cc
@@ -7,6 +7,7 @@
#include "net/third_party/quiche/src/quic/core/http/http_encoder.h"
#include "net/third_party/quiche/src/quic/core/quic_data_writer.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_arraysize.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_ptr_util.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_test.h"
using testing::InSequence;
@@ -54,51 +55,99 @@
}
TEST_F(HttpDecoderTest, ReservedFramesNoPayload) {
+ std::unique_ptr<char[]> input;
for (int n = 0; n < 8; ++n) {
const uint8_t type = 0xB + 0x1F * n;
- char input[] = {// length
- 0x00,
- // type
- type};
+ QuicByteCount total_length = QuicDataWriter::GetVarInt62Len(0x00) +
+ QuicDataWriter::GetVarInt62Len(type);
+ input = QuicMakeUnique<char[]>(total_length);
+ QuicDataWriter writer(total_length, input.get());
+ writer.WriteVarInt62(0x00);
+ writer.WriteVarInt62(type);
- EXPECT_EQ(2u, decoder_.ProcessInput(input, QUIC_ARRAYSIZE(input))) << n;
+ EXPECT_EQ(total_length, decoder_.ProcessInput(input.get(), total_length))
+ << n;
EXPECT_EQ(QUIC_NO_ERROR, decoder_.error());
ASSERT_EQ("", decoder_.error_detail());
+ EXPECT_EQ(type, decoder_.current_frame_type());
}
+ // Test on a arbitrary reserved frame with 2-byte type field by hard coding
+ // variable length integer.
+ char in[] = {// length
+ 0x00,
+ // type 0xB + 0x1F*3
+ 0x40, 0x68};
+ EXPECT_EQ(3u, decoder_.ProcessInput(in, QUIC_ARRAYSIZE(in)));
+ EXPECT_EQ(QUIC_NO_ERROR, decoder_.error());
+ ASSERT_EQ("", decoder_.error_detail());
+ EXPECT_EQ(0xB + 0x1F * 3, decoder_.current_frame_type());
}
TEST_F(HttpDecoderTest, ReservedFramesSmallPayload) {
+ std::unique_ptr<char[]> input;
+ const uint8_t payload_size = 50;
+ std::string data(payload_size, 'a');
for (int n = 0; n < 8; ++n) {
const uint8_t type = 0xB + 0x1F * n;
- const uint8_t payload_size = 50;
- char input[payload_size + 2] = {// length
- payload_size,
- // type
- type};
-
- EXPECT_EQ(QUIC_ARRAYSIZE(input),
- decoder_.ProcessInput(input, QUIC_ARRAYSIZE(input)))
+ QuicByteCount total_length = QuicDataWriter::GetVarInt62Len(payload_size) +
+ QuicDataWriter::GetVarInt62Len(type) +
+ payload_size;
+ input = QuicMakeUnique<char[]>(total_length);
+ QuicDataWriter writer(total_length, input.get());
+ writer.WriteVarInt62(payload_size);
+ writer.WriteVarInt62(type);
+ writer.WriteStringPiece(data);
+ EXPECT_EQ(total_length, decoder_.ProcessInput(input.get(), total_length))
<< n;
EXPECT_EQ(QUIC_NO_ERROR, decoder_.error());
ASSERT_EQ("", decoder_.error_detail());
+ EXPECT_EQ(type, decoder_.current_frame_type());
}
+
+ // Test on a arbitrary reserved frame with 2-byte type field by hard coding
+ // variable length integer.
+ char in[payload_size + 3] = {// length
+ payload_size,
+ // type 0xB + 0x1F*3
+ 0x40, 0x68};
+ EXPECT_EQ(QUIC_ARRAYSIZE(in), decoder_.ProcessInput(in, QUIC_ARRAYSIZE(in)));
+ EXPECT_EQ(QUIC_NO_ERROR, decoder_.error());
+ ASSERT_EQ("", decoder_.error_detail());
+ EXPECT_EQ(0xB + 0x1F * 3, decoder_.current_frame_type());
}
TEST_F(HttpDecoderTest, ReservedFramesLargePayload) {
+ std::unique_ptr<char[]> input;
+ const QuicByteCount payload_size = 256;
+ std::string data(payload_size, 'a');
for (int n = 0; n < 8; ++n) {
const uint8_t type = 0xB + 0x1F * n;
- const QuicByteCount payload_size = 256;
- char input[payload_size + 3] = {// length
- 0x40 + 0x01, 0x00,
- // type
- type};
+ QuicByteCount total_length = QuicDataWriter::GetVarInt62Len(payload_size) +
+ QuicDataWriter::GetVarInt62Len(type) +
+ payload_size;
+ input = QuicMakeUnique<char[]>(total_length);
+ QuicDataWriter writer(total_length, input.get());
+ writer.WriteVarInt62(payload_size);
+ writer.WriteVarInt62(type);
+ writer.WriteStringPiece(data);
- EXPECT_EQ(QUIC_ARRAYSIZE(input),
- decoder_.ProcessInput(input, QUIC_ARRAYSIZE(input)))
+ EXPECT_EQ(total_length, decoder_.ProcessInput(input.get(), total_length))
<< n;
EXPECT_EQ(QUIC_NO_ERROR, decoder_.error());
ASSERT_EQ("", decoder_.error_detail());
+ EXPECT_EQ(type, decoder_.current_frame_type());
}
+
+ // Test on a arbitrary reserved frame with 2-byte type field by hard coding
+ // variable length integer.
+ char in[payload_size + 4] = {// length
+ 0x40 + 0x01, 0x00,
+ // type 0xB + 0x1F*3
+ 0x40, 0x68};
+ EXPECT_EQ(QUIC_ARRAYSIZE(in), decoder_.ProcessInput(in, QUIC_ARRAYSIZE(in)));
+ EXPECT_EQ(QUIC_NO_ERROR, decoder_.error());
+ ASSERT_EQ("", decoder_.error_detail());
+ EXPECT_EQ(0xB + 0x1F * 3, decoder_.current_frame_type());
}
TEST_F(HttpDecoderTest, CancelPush) {
@@ -352,6 +401,27 @@
EXPECT_EQ("", decoder_.error_detail());
}
+TEST_F(HttpDecoderTest, PartialDeliveryOfLargeFrameType) {
+ // Use a reserved type that's more than 1 byte in VarInt62.
+ const uint8_t type = 0xB + 0x1F * 3;
+ std::unique_ptr<char[]> input;
+ QuicByteCount total_length = QuicDataWriter::GetVarInt62Len(0x00) +
+ QuicDataWriter::GetVarInt62Len(type);
+ input.reset(new char[total_length]);
+ QuicDataWriter writer(total_length, input.get());
+ writer.WriteVarInt62(0x00);
+ writer.WriteVarInt62(type);
+
+ auto raw_input = input.get();
+ for (uint64_t i = 0; i < total_length; ++i) {
+ char c = raw_input[i];
+ EXPECT_EQ(1u, decoder_.ProcessInput(&c, 1));
+ }
+ EXPECT_EQ(QUIC_NO_ERROR, decoder_.error());
+ EXPECT_EQ("", decoder_.error_detail());
+ EXPECT_EQ(type, decoder_.current_frame_type());
+}
+
TEST_F(HttpDecoderTest, GoAway) {
char input[] = {// length
0x1,
diff --git a/quic/core/http/http_encoder.cc b/quic/core/http/http_encoder.cc
index beeef9b..df04be2 100644
--- a/quic/core/http/http_encoder.cc
+++ b/quic/core/http/http_encoder.cc
@@ -44,8 +44,6 @@
}
}
-// Length of the type field of a frame.
-static const size_t kFrameTypeLength = 1;
// Length of the weight field of a priority frame.
static const size_t kPriorityWeightLength = 1;
// Length of a priority frame's first byte.
@@ -63,8 +61,9 @@
QuicByteCount payload_length,
std::unique_ptr<char[]>* output) {
DCHECK_NE(0u, payload_length);
- QuicByteCount header_length =
- QuicDataWriter::GetVarInt62Len(payload_length) + kFrameTypeLength;
+ QuicByteCount header_length = QuicDataWriter::GetVarInt62Len(payload_length) +
+ QuicDataWriter::GetVarInt62Len(
+ static_cast<uint64_t>(HttpFrameType::DATA));
output->reset(new char[header_length]);
QuicDataWriter writer(header_length, output->get());
@@ -80,7 +79,9 @@
std::unique_ptr<char[]>* output) {
DCHECK_NE(0u, payload_length);
QuicByteCount header_length =
- QuicDataWriter::GetVarInt62Len(payload_length) + kFrameTypeLength;
+ QuicDataWriter::GetVarInt62Len(payload_length) +
+ QuicDataWriter::GetVarInt62Len(
+ static_cast<uint64_t>(HttpFrameType::HEADERS));
output->reset(new char[header_length]);
QuicDataWriter writer(header_length, output->get());
@@ -99,7 +100,8 @@
QuicDataWriter::GetVarInt62Len(priority.prioritized_element_id) +
QuicDataWriter::GetVarInt62Len(priority.element_dependency_id) +
kPriorityWeightLength;
- QuicByteCount total_length = GetTotalLength(payload_length);
+ QuicByteCount total_length =
+ GetTotalLength(payload_length, HttpFrameType::PRIORITY);
output->reset(new char[total_length]);
QuicDataWriter writer(total_length, output->get());
@@ -130,7 +132,8 @@
std::unique_ptr<char[]>* output) {
QuicByteCount payload_length =
QuicDataWriter::GetVarInt62Len(cancel_push.push_id);
- QuicByteCount total_length = GetTotalLength(payload_length);
+ QuicByteCount total_length =
+ GetTotalLength(payload_length, HttpFrameType::CANCEL_PUSH);
output->reset(new char[total_length]);
QuicDataWriter writer(total_length, output->get());
@@ -152,7 +155,8 @@
payload_length += QuicDataWriter::GetVarInt62Len(it->second);
}
- QuicByteCount total_length = GetTotalLength(payload_length);
+ QuicByteCount total_length =
+ GetTotalLength(payload_length, HttpFrameType::SETTINGS);
output->reset(new char[total_length]);
QuicDataWriter writer(total_length, output->get());
@@ -178,7 +182,9 @@
push_promise.headers.length();
// GetTotalLength() is not used because headers will not be serialized.
QuicByteCount total_length =
- QuicDataWriter::GetVarInt62Len(payload_length) + kFrameTypeLength +
+ QuicDataWriter::GetVarInt62Len(payload_length) +
+ QuicDataWriter::GetVarInt62Len(
+ static_cast<uint64_t>(HttpFrameType::PUSH_PROMISE)) +
QuicDataWriter::GetVarInt62Len(push_promise.push_id);
output->reset(new char[total_length]);
@@ -196,7 +202,8 @@
std::unique_ptr<char[]>* output) {
QuicByteCount payload_length =
QuicDataWriter::GetVarInt62Len(goaway.stream_id);
- QuicByteCount total_length = GetTotalLength(payload_length);
+ QuicByteCount total_length =
+ GetTotalLength(payload_length, HttpFrameType::GOAWAY);
output->reset(new char[total_length]);
QuicDataWriter writer(total_length, output->get());
@@ -213,7 +220,8 @@
std::unique_ptr<char[]>* output) {
QuicByteCount payload_length =
QuicDataWriter::GetVarInt62Len(max_push_id.push_id);
- QuicByteCount total_length = GetTotalLength(payload_length);
+ QuicByteCount total_length =
+ GetTotalLength(payload_length, HttpFrameType::MAX_PUSH_ID);
output->reset(new char[total_length]);
QuicDataWriter writer(total_length, output->get());
@@ -230,7 +238,8 @@
std::unique_ptr<char[]>* output) {
QuicByteCount payload_length =
QuicDataWriter::GetVarInt62Len(duplicate_push.push_id);
- QuicByteCount total_length = GetTotalLength(payload_length);
+ QuicByteCount total_length =
+ GetTotalLength(payload_length, HttpFrameType::DUPLICATE_PUSH);
output->reset(new char[total_length]);
QuicDataWriter writer(total_length, output->get());
@@ -247,11 +256,13 @@
HttpFrameType type,
QuicDataWriter* writer) {
return writer->WriteVarInt62(length) &&
- writer->WriteUInt8(static_cast<uint8_t>(type));
+ writer->WriteVarInt62(static_cast<uint64_t>(type));
}
-QuicByteCount HttpEncoder::GetTotalLength(QuicByteCount payload_length) {
- return QuicDataWriter::GetVarInt62Len(payload_length) + kFrameTypeLength +
+QuicByteCount HttpEncoder::GetTotalLength(QuicByteCount payload_length,
+ HttpFrameType type) {
+ return QuicDataWriter::GetVarInt62Len(payload_length) +
+ QuicDataWriter::GetVarInt62Len(static_cast<uint64_t>(type)) +
payload_length;
}
diff --git a/quic/core/http/http_encoder.h b/quic/core/http/http_encoder.h
index f04e6e4..6691e31 100644
--- a/quic/core/http/http_encoder.h
+++ b/quic/core/http/http_encoder.h
@@ -77,7 +77,8 @@
HttpFrameType type,
QuicDataWriter* writer);
- QuicByteCount GetTotalLength(QuicByteCount payload_length);
+ QuicByteCount GetTotalLength(QuicByteCount payload_length,
+ HttpFrameType type);
};
} // namespace quic
