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;
