Change Settings frame's setting id from 2 byte integer to variable length integer. gfe-relnote: n/a --under version 99, which is not turned on. PiperOrigin-RevId: 244771758 Change-Id: I0e1b329ff0696255f5ca0c10087bf39090b81e76
diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc index eff9efc..e0c7383 100644 --- a/quic/core/http/http_decoder.cc +++ b/quic/core/http/http_decoder.cc
@@ -308,10 +308,6 @@ break; } case 0x4: { // SETTINGS - // TODO(rch): Handle overly large SETTINGS frames. Either: - // 1. Impose a limit on SETTINGS frame size, and close the connection if - // exceeded - // 2. Implement a streaming parsing mode. SettingsFrame frame; QuicDataReader reader(buffer_.data(), current_frame_length_); if (!ParseSettingsFrame(&reader, &frame)) { @@ -480,8 +476,8 @@ bool HttpDecoder::ParseSettingsFrame(QuicDataReader* reader, SettingsFrame* frame) { while (!reader->IsDoneReading()) { - uint16_t id; - if (!reader->ReadUInt16(&id)) { + uint64_t id; + if (!reader->ReadVarInt62(&id)) { RaiseError(QUIC_INTERNAL_ERROR, "Unable to read settings frame identifier"); return false;
diff --git a/quic/core/http/http_decoder_test.cc b/quic/core/http/http_decoder_test.cc index 614e4f9..a4903b5 100644 --- a/quic/core/http/http_decoder_test.cc +++ b/quic/core/http/http_decoder_test.cc
@@ -304,26 +304,29 @@ // type (SETTINGS) 0x04, // length - 0x06, + 0x07, // identifier (SETTINGS_NUM_PLACEHOLDERS) - 0x00, 0x03, // content 0x02, // identifier (SETTINGS_MAX_HEADER_LIST_SIZE) - 0x00, 0x06, // content 0x05, - }; + // identifier (256 in variable length integer) + 0x40 + 0x01, + 0x00, + // content + 0x04}; // clang-format on SettingsFrame frame; frame.values[3] = 2; frame.values[6] = 5; + frame.values[256] = 4; // Process the full frame. - EXPECT_CALL(visitor_, OnSettingsFrameStart(Http3FrameLengths(2, 6))); + EXPECT_CALL(visitor_, OnSettingsFrameStart(Http3FrameLengths(2, 7))); EXPECT_CALL(visitor_, OnSettingsFrame(frame)); EXPECT_EQ(QUIC_ARRAYSIZE(input), decoder_.ProcessInput(input, QUIC_ARRAYSIZE(input))); @@ -331,7 +334,7 @@ EXPECT_EQ("", decoder_.error_detail()); // Process the frame incremently. - EXPECT_CALL(visitor_, OnSettingsFrameStart(Http3FrameLengths(2, 6))); + EXPECT_CALL(visitor_, OnSettingsFrameStart(Http3FrameLengths(2, 7))); EXPECT_CALL(visitor_, OnSettingsFrame(frame)); for (char c : input) { EXPECT_EQ(1u, decoder_.ProcessInput(&c, 1));
diff --git a/quic/core/http/http_encoder.cc b/quic/core/http/http_encoder.cc index 9de1571..01f9c10 100644 --- a/quic/core/http/http_encoder.cc +++ b/quic/core/http/http_encoder.cc
@@ -48,8 +48,6 @@ static const size_t kPriorityWeightLength = 1; // Length of a priority frame's first byte. static const size_t kPriorityFirstByteLength = 1; -// Length of a key in the map of a settings frame. -static const size_t kSettingsMapKeyLength = 2; } // namespace @@ -71,6 +69,8 @@ if (WriteFrameHeader(payload_length, HttpFrameType::DATA, &writer)) { return header_length; } + QUIC_DLOG(ERROR) + << "Http encoder failed when attempting to serialize data frame header."; return 0; } @@ -89,6 +89,9 @@ if (WriteFrameHeader(payload_length, HttpFrameType::HEADERS, &writer)) { return header_length; } + QUIC_DLOG(ERROR) + << "Http encoder failed when attempting to serialize headers " + "frame header."; return 0; } @@ -107,6 +110,8 @@ QuicDataWriter writer(total_length, output->get()); if (!WriteFrameHeader(payload_length, HttpFrameType::PRIORITY, &writer)) { + QUIC_DLOG(ERROR) << "Http encoder failed when attempting to serialize " + "priority frame header."; return 0; } @@ -124,6 +129,8 @@ writer.WriteUInt8(priority.weight)) { return total_length; } + QUIC_DLOG(ERROR) << "Http encoder failed when attempting to serialize " + "priority frame payload."; return 0; } @@ -142,16 +149,18 @@ writer.WriteVarInt62(cancel_push.push_id)) { return total_length; } + QUIC_DLOG(ERROR) + << "Http encoder failed when attempting to serialize cancel push frame."; return 0; } QuicByteCount HttpEncoder::SerializeSettingsFrame( const SettingsFrame& settings, std::unique_ptr<char[]>* output) { - // Calculate the key sizes. - QuicByteCount payload_length = settings.values.size() * kSettingsMapKeyLength; - // Calculate the value sizes. + QuicByteCount payload_length = 0; + // Calculate the payload length. for (auto it = settings.values.begin(); it != settings.values.end(); ++it) { + payload_length += QuicDataWriter::GetVarInt62Len(it->first); payload_length += QuicDataWriter::GetVarInt62Len(it->second); } @@ -162,11 +171,15 @@ QuicDataWriter writer(total_length, output->get()); if (!WriteFrameHeader(payload_length, HttpFrameType::SETTINGS, &writer)) { + QUIC_DLOG(ERROR) << "Http encoder failed when attempting to serialize " + "settings frame header."; return 0; } for (auto it = settings.values.begin(); it != settings.values.end(); ++it) { - if (!writer.WriteUInt16(it->first) || !writer.WriteVarInt62(it->second)) { + if (!writer.WriteVarInt62(it->first) || !writer.WriteVarInt62(it->second)) { + QUIC_DLOG(ERROR) << "Http encoder failed when attempting to serialize " + "settings frame payload."; return 0; } } @@ -194,6 +207,8 @@ writer.WriteVarInt62(push_promise.push_id)) { return total_length; } + QUIC_DLOG(ERROR) + << "Http encoder failed when attempting to serialize push promise frame."; return 0; } @@ -212,6 +227,8 @@ writer.WriteVarInt62(goaway.stream_id)) { return total_length; } + QUIC_DLOG(ERROR) + << "Http encoder failed when attempting to serialize goaway frame."; return 0; } @@ -230,6 +247,8 @@ writer.WriteVarInt62(max_push_id.push_id)) { return total_length; } + QUIC_DLOG(ERROR) + << "Http encoder failed when attempting to serialize max push id frame."; return 0; } @@ -249,6 +268,8 @@ writer.WriteVarInt62(duplicate_push.push_id)) { return total_length; } + QUIC_DLOG(ERROR) << "Http encoder failed when attempting to serialize " + "duplicate push frame."; return 0; }
diff --git a/quic/core/http/http_encoder_test.cc b/quic/core/http/http_encoder_test.cc index 9088fb5..1ff5d89 100644 --- a/quic/core/http/http_encoder_test.cc +++ b/quic/core/http/http_encoder_test.cc
@@ -92,22 +92,23 @@ SettingsFrame settings; settings.values[3] = 2; settings.values[6] = 5; - char output[] = { - // type (SETTINGS) - 0x04, - // length - 0x06, - // identifier (SETTINGS_NUM_PLACEHOLDERS) - 0x00, - 0x03, - // content - 0x02, - // identifier (SETTINGS_MAX_HEADER_LIST_SIZE) - 0x00, - 0x06, - // content - 0x05, - }; + settings.values[256] = 4; + char output[] = {// type (SETTINGS) + 0x04, + // length + 0x07, + // identifier (SETTINGS_NUM_PLACEHOLDERS) + 0x03, + // content + 0x02, + // identifier (SETTINGS_MAX_HEADER_LIST_SIZE) + 0x06, + // content + 0x05, + // identifier (256 in variable length integer) + 0x40 + 0x01, 0x00, + // content + 0x04}; std::unique_ptr<char[]> buffer; uint64_t length = encoder_.SerializeSettingsFrame(settings, &buffer); EXPECT_EQ(QUIC_ARRAYSIZE(output), length);
diff --git a/quic/core/http/http_frames.h b/quic/core/http/http_frames.h index ea11557..f1aa7ff 100644 --- a/quic/core/http/http_frames.h +++ b/quic/core/http/http_frames.h
@@ -90,8 +90,7 @@ // affect how endpoints communicate, such as preferences and constraints // on peer behavior -using SettingsId = uint16_t; -using SettingsMap = std::map<SettingsId, uint64_t>; +using SettingsMap = std::map<uint64_t, uint64_t>; struct SettingsFrame { SettingsMap values;