Change HTTP/3 frames from Lengh-Type-Value to Type-Length-Value. Updating to IETF draft 19. gfe-relnote: n/a --version 99 only, not yet enabled. PiperOrigin-RevId: 244687780 Change-Id: Id8f42caad0c1dd2978e1a0a45aa2f462dcd0bcf8
diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc index 653652f..eff9efc 100644 --- a/quic/core/http/http_decoder.cc +++ b/quic/core/http/http_decoder.cc
@@ -33,7 +33,7 @@ HttpDecoder::HttpDecoder() : visitor_(nullptr), - state_(STATE_READING_FRAME_LENGTH), + state_(STATE_READING_FRAME_TYPE), current_frame_type_(0), current_length_field_size_(0), remaining_length_field_length_(0), @@ -51,12 +51,12 @@ while (error_ == QUIC_NO_ERROR && (reader.BytesRemaining() != 0 || state_ == STATE_FINISH_PARSING)) { switch (state_) { - case STATE_READING_FRAME_LENGTH: - ReadFrameLength(&reader); - break; case STATE_READING_FRAME_TYPE: ReadFrameType(&reader); break; + case STATE_READING_FRAME_LENGTH: + ReadFrameLength(&reader); + break; case STATE_READING_FRAME_PAYLOAD: ReadFramePayload(&reader); break; @@ -77,24 +77,6 @@ return len - reader.BytesRemaining(); } -void HttpDecoder::ReadFrameLength(QuicDataReader* reader) { - DCHECK_NE(0u, reader->BytesRemaining()); - BufferFrameLength(reader); - if (remaining_length_field_length_ != 0) { - return; - } - QuicDataReader length_reader(length_buffer_.data(), - current_length_field_size_); - if (!length_reader.ReadVarInt62(¤t_frame_length_)) { - RaiseError(QUIC_INTERNAL_ERROR, "Unable to read frame length"); - visitor_->OnError(this); - return; - } - - state_ = STATE_READING_FRAME_TYPE; - remaining_frame_length_ = current_frame_length_; -} - void HttpDecoder::ReadFrameType(QuicDataReader* reader) { DCHECK_NE(0u, reader->BytesRemaining()); if (current_type_field_length_ == 0) { @@ -132,6 +114,23 @@ } } + state_ = STATE_READING_FRAME_LENGTH; +} + +void HttpDecoder::ReadFrameLength(QuicDataReader* reader) { + DCHECK_NE(0u, reader->BytesRemaining()); + BufferFrameLength(reader); + if (remaining_length_field_length_ != 0) { + return; + } + QuicDataReader length_reader(length_buffer_.data(), + current_length_field_size_); + if (!length_reader.ReadVarInt62(¤t_frame_length_)) { + RaiseError(QUIC_INTERNAL_ERROR, "Unable to read frame length"); + visitor_->OnError(this); + return; + } + if (current_frame_length_ > MaxFrameLength(current_frame_type_)) { RaiseError(QUIC_INTERNAL_ERROR, "Frame is too large"); visitor_->OnError(this); @@ -154,6 +153,7 @@ current_frame_length_)); } + remaining_frame_length_ = current_frame_length_; state_ = (remaining_frame_length_ == 0) ? STATE_FINISH_PARSING : STATE_READING_FRAME_PAYLOAD; } @@ -266,6 +266,7 @@ QUIC_FALLTHROUGH_INTENDED; default: DiscardFramePayload(reader); + return; } if (remaining_frame_length_ == 0) { @@ -365,7 +366,7 @@ current_length_field_size_ = 0; current_type_field_length_ = 0; - state_ = STATE_READING_FRAME_LENGTH; + state_ = STATE_READING_FRAME_TYPE; } void HttpDecoder::DiscardFramePayload(QuicDataReader* reader) { @@ -378,7 +379,7 @@ } remaining_frame_length_ -= payload.length(); if (remaining_frame_length_ == 0) { - state_ = STATE_READING_FRAME_LENGTH; + state_ = STATE_READING_FRAME_TYPE; current_length_field_size_ = 0; current_type_field_length_ = 0; }
diff --git a/quic/core/http/http_decoder.h b/quic/core/http/http_decoder.h index 8ed7973..43a6d2c 100644 --- a/quic/core/http/http_decoder.h +++ b/quic/core/http/http_decoder.h
@@ -129,15 +129,15 @@ STATE_ERROR }; - // Reads the length of a frame from |reader|. Sets error_ and error_detail_ - // if there are any errors. - void ReadFrameLength(QuicDataReader* reader); - // Reads the type of a frame from |reader|. Sets error_ and error_detail_ // if there are any errors. Also calls OnDataFrameStart() or // OnHeadersFrameStart() for appropriate frame types. void ReadFrameType(QuicDataReader* reader); + // Reads the length of a frame from |reader|. Sets error_ and error_detail_ + // if there are any errors. + void ReadFrameLength(QuicDataReader* reader); + // Reads the payload of the current frame from |reader| and processes it, // possibly buffering the data or invoking the visitor. void ReadFramePayload(QuicDataReader* reader);
diff --git a/quic/core/http/http_decoder_test.cc b/quic/core/http/http_decoder_test.cc index d37cdd4..614e4f9 100644 --- a/quic/core/http/http_decoder_test.cc +++ b/quic/core/http/http_decoder_test.cc
@@ -62,8 +62,8 @@ QuicDataWriter::GetVarInt62Len(type); input = QuicMakeUnique<char[]>(total_length); QuicDataWriter writer(total_length, input.get()); - writer.WriteVarInt62(0x00); writer.WriteVarInt62(type); + writer.WriteVarInt62(0x00); EXPECT_EQ(total_length, decoder_.ProcessInput(input.get(), total_length)) << n; @@ -73,10 +73,10 @@ } // 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}; + char in[] = {// type 0xB + 0x1F*3 + 0x40, 0x68, + // length + 0x00}; EXPECT_EQ(3u, decoder_.ProcessInput(in, QUIC_ARRAYSIZE(in))); EXPECT_EQ(QUIC_NO_ERROR, decoder_.error()); ASSERT_EQ("", decoder_.error_detail()); @@ -94,8 +94,8 @@ payload_size; input = QuicMakeUnique<char[]>(total_length); QuicDataWriter writer(total_length, input.get()); - writer.WriteVarInt62(payload_size); writer.WriteVarInt62(type); + writer.WriteVarInt62(payload_size); writer.WriteStringPiece(data); EXPECT_EQ(total_length, decoder_.ProcessInput(input.get(), total_length)) << n; @@ -106,10 +106,10 @@ // 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}; + char in[payload_size + 3] = {// type 0xB + 0x1F*3 + 0x40, 0x68, + // length + payload_size}; EXPECT_EQ(QUIC_ARRAYSIZE(in), decoder_.ProcessInput(in, QUIC_ARRAYSIZE(in))); EXPECT_EQ(QUIC_NO_ERROR, decoder_.error()); ASSERT_EQ("", decoder_.error_detail()); @@ -127,8 +127,8 @@ payload_size; input = QuicMakeUnique<char[]>(total_length); QuicDataWriter writer(total_length, input.get()); - writer.WriteVarInt62(payload_size); writer.WriteVarInt62(type); + writer.WriteVarInt62(payload_size); writer.WriteStringPiece(data); EXPECT_EQ(total_length, decoder_.ProcessInput(input.get(), total_length)) @@ -140,10 +140,10 @@ // 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}; + char in[payload_size + 4] = {// type 0xB + 0x1F*3 + 0x40, 0x68, + // length + 0x40 + 0x01, 0x00}; EXPECT_EQ(QUIC_ARRAYSIZE(in), decoder_.ProcessInput(in, QUIC_ARRAYSIZE(in))); EXPECT_EQ(QUIC_NO_ERROR, decoder_.error()); ASSERT_EQ("", decoder_.error_detail()); @@ -151,10 +151,10 @@ } TEST_F(HttpDecoderTest, CancelPush) { - char input[] = {// length - 0x1, - // type (CANCEL_PUSH) + char input[] = {// type (CANCEL_PUSH) 0x03, + // length + 0x1, // Push Id 0x01}; @@ -175,10 +175,10 @@ } TEST_F(HttpDecoderTest, PushPromiseFrame) { - char input[] = {// length - 0x8, - // type (PUSH_PROMISE) + char input[] = {// type (PUSH_PROMISE) 0x05, + // length + 0x8, // Push Id 0x01, // Header Block @@ -212,10 +212,10 @@ } TEST_F(HttpDecoderTest, MaxPushId) { - char input[] = {// length - 0x1, - // type (MAX_PUSH_ID) + char input[] = {// type (MAX_PUSH_ID) 0x0D, + // length + 0x1, // Push Id 0x01}; @@ -236,10 +236,10 @@ } TEST_F(HttpDecoderTest, DuplicatePush) { - char input[] = {// length - 0x1, - // type (DUPLICATE_PUSH) + char input[] = {// type (DUPLICATE_PUSH) 0x0E, + // length + 0x1, // Push Id 0x01}; // Process the full frame. @@ -259,10 +259,10 @@ } TEST_F(HttpDecoderTest, PriorityFrame) { - char input[] = {// length - 0x4, - // type (PRIORITY) + char input[] = {// type (PRIORITY) 0x2, + // length + 0x4, // request stream, request stream, exclusive 0x01, // prioritized_element_id @@ -301,10 +301,10 @@ TEST_F(HttpDecoderTest, SettingsFrame) { // clang-format off char input[] = { - // length - 0x06, // type (SETTINGS) 0x04, + // length + 0x06, // identifier (SETTINGS_NUM_PLACEHOLDERS) 0x00, 0x03, @@ -341,10 +341,10 @@ } TEST_F(HttpDecoderTest, DataFrame) { - char input[] = {// length - 0x05, - // type (DATA) + char input[] = {// type (DATA) 0x00, + // length + 0x05, // data 'D', 'a', 't', 'a', '!'}; @@ -409,8 +409,8 @@ QuicDataWriter::GetVarInt62Len(type); input.reset(new char[total_length]); QuicDataWriter writer(total_length, input.get()); - writer.WriteVarInt62(0x00); writer.WriteVarInt62(type); + writer.WriteVarInt62(0x00); auto raw_input = input.get(); for (uint64_t i = 0; i < total_length; ++i) { @@ -423,10 +423,10 @@ } TEST_F(HttpDecoderTest, GoAway) { - char input[] = {// length - 0x1, - // type (GOAWAY) + char input[] = {// type (GOAWAY) 0x07, + // length + 0x1, // StreamId 0x01}; @@ -447,10 +447,10 @@ } TEST_F(HttpDecoderTest, HeadersFrame) { - char input[] = {// length - 0x07, - // type (HEADERS) + char input[] = {// type (HEADERS) 0x01, + // length + 0x07, // headers 'H', 'e', 'a', 'd', 'e', 'r', 's'}; @@ -482,8 +482,8 @@ } TEST_F(HttpDecoderTest, EmptyDataFrame) { - char input[] = {0x00, // length - 0x00}; // type (DATA) + char input[] = {0x00, // type (DATA) + 0x00}; // length // Process the full frame. InSequence s; @@ -505,8 +505,8 @@ } TEST_F(HttpDecoderTest, EmptyHeadersFrame) { - char input[] = {0x00, // length - 0x01}; // type (HEADERS) + char input[] = {0x01, // type (HEADERS) + 0x00}; // length // Process the full frame. InSequence s; @@ -528,8 +528,8 @@ } TEST_F(HttpDecoderTest, PushPromiseFrameNoHeaders) { - char input[] = {0x01, // length - 0x05, // type (PUSH_PROMISE) + char input[] = {0x05, // type (PUSH_PROMISE) + 0x01, // length 0x01}; // Push Id // Process the full frame. @@ -552,8 +552,8 @@ } TEST_F(HttpDecoderTest, MalformedFrameWithOverlyLargePayload) { - char input[] = {0x10, // length - 0x03, // type (CANCEL_PUSH) + char input[] = {0x03, // type (CANCEL_PUSH) + 0x10, // length 0x15}; // malformed payload // Process the full frame. EXPECT_CALL(visitor_, OnError(&decoder_)); @@ -565,10 +565,10 @@ TEST_F(HttpDecoderTest, MalformedSettingsFrame) { char input[30]; QuicDataWriter writer(30, input); - // Write length. - writer.WriteVarInt62(2048 * 1024); // Write type SETTINGS. writer.WriteUInt8(0x04); + // Write length. + writer.WriteVarInt62(2048 * 1024); writer.WriteStringPiece("Malformed payload"); EXPECT_CALL(visitor_, OnError(&decoder_));
diff --git a/quic/core/http/http_encoder.cc b/quic/core/http/http_encoder.cc index df04be2..9de1571 100644 --- a/quic/core/http/http_encoder.cc +++ b/quic/core/http/http_encoder.cc
@@ -255,8 +255,8 @@ bool HttpEncoder::WriteFrameHeader(QuicByteCount length, HttpFrameType type, QuicDataWriter* writer) { - return writer->WriteVarInt62(length) && - writer->WriteVarInt62(static_cast<uint64_t>(type)); + return writer->WriteVarInt62(static_cast<uint64_t>(type)) && + writer->WriteVarInt62(length); } QuicByteCount HttpEncoder::GetTotalLength(QuicByteCount payload_length,
diff --git a/quic/core/http/http_encoder_test.cc b/quic/core/http/http_encoder_test.cc index 154d768..9088fb5 100644 --- a/quic/core/http/http_encoder_test.cc +++ b/quic/core/http/http_encoder_test.cc
@@ -22,10 +22,10 @@ std::unique_ptr<char[]> buffer; uint64_t length = encoder_.SerializeDataFrameHeader(/* payload_length = */ 5, &buffer); - char output[] = {// length - 0x05, - // type (DATA) - 0x00}; + char output[] = {// type (DATA) + 0x00, + // length + 0x05}; EXPECT_EQ(QUIC_ARRAYSIZE(output), length); CompareCharArraysWithHexError("DATA", buffer.get(), length, output, QUIC_ARRAYSIZE(output)); @@ -35,10 +35,10 @@ std::unique_ptr<char[]> buffer; uint64_t length = encoder_.SerializeHeadersFrameHeader(/* payload_length = */ 7, &buffer); - char output[] = {// length - 0x07, - // type (HEADERS) - 0x01}; + char output[] = {// type (HEADERS) + 0x01, + // length + 0x07}; EXPECT_EQ(QUIC_ARRAYSIZE(output), length); CompareCharArraysWithHexError("HEADERS", buffer.get(), length, output, QUIC_ARRAYSIZE(output)); @@ -52,10 +52,10 @@ priority.prioritized_element_id = 0x03; priority.element_dependency_id = 0x04; priority.weight = 0xFF; - char output[] = {// length - 0x4, - // type (PRIORITY) + char output[] = {// type (PRIORITY) 0x2, + // length + 0x4, // request stream, request stream, exclusive 0x01, // prioritized_element_id @@ -75,10 +75,10 @@ TEST_F(HttpEncoderTest, SerializeCancelPushFrame) { CancelPushFrame cancel_push; cancel_push.push_id = 0x01; - char output[] = {// length - 0x1, - // type (CANCEL_PUSH) + char output[] = {// type (CANCEL_PUSH) 0x03, + // length + 0x1, // Push Id 0x01}; std::unique_ptr<char[]> buffer; @@ -93,10 +93,10 @@ settings.values[3] = 2; settings.values[6] = 5; char output[] = { - // length - 0x06, // type (SETTINGS) 0x04, + // length + 0x06, // identifier (SETTINGS_NUM_PLACEHOLDERS) 0x00, 0x03, @@ -119,10 +119,10 @@ PushPromiseFrame push_promise; push_promise.push_id = 0x01; push_promise.headers = "Headers"; - char output[] = {// length - 0x8, - // type (PUSH_PROMISE) + char output[] = {// type (PUSH_PROMISE) 0x05, + // length + 0x8, // Push Id 0x01}; std::unique_ptr<char[]> buffer; @@ -136,10 +136,10 @@ TEST_F(HttpEncoderTest, SerializeGoAwayFrame) { GoAwayFrame goaway; goaway.stream_id = 0x1; - char output[] = {// length - 0x1, - // type (GOAWAY) + char output[] = {// type (GOAWAY) 0x07, + // length + 0x1, // StreamId 0x01}; std::unique_ptr<char[]> buffer; @@ -152,10 +152,10 @@ TEST_F(HttpEncoderTest, SerializeMaxPushIdFrame) { MaxPushIdFrame max_push_id; max_push_id.push_id = 0x1; - char output[] = {// length - 0x1, - // type (MAX_PUSH_ID) + char output[] = {// type (MAX_PUSH_ID) 0x0D, + // length + 0x1, // Push Id 0x01}; std::unique_ptr<char[]> buffer; @@ -168,10 +168,10 @@ TEST_F(HttpEncoderTest, SerializeDuplicatePushFrame) { DuplicatePushFrame duplicate_push; duplicate_push.push_id = 0x1; - char output[] = {// length - 0x1, - // type (DUPLICATE_PUSH) + char output[] = {// type (DUPLICATE_PUSH) 0x0E, + // length + 0x1, // Push Id 0x01}; std::unique_ptr<char[]> buffer;