Copilot commented on code in PR #13627:
URL: https://github.com/apache/trafficserver/pull/13627#discussion_r3960222558
##########
src/proxy/http3/test/test_Http3Frame.cc:
##########
@@ -162,10 +166,51 @@ TEST_CASE("Load SETTINGS Frame", "[http3]")
CHECK(settings_frame->get(Http3SettingsId::MAX_FIELD_SECTION_SIZE) ==
0x0400);
CHECK(settings_frame->get(Http3SettingsId::NUM_PLACEHOLDERS) == 0x0f);
+ settings_frame.reset();
+ frame.reset();
free_MIOBuffer(input);
}
}
+// A SETTINGS identifier is a full QUIC variable-length integer, and an
+// identifier the implementation does not understand must be ignored
+// (RFC 9114, Section 7.2.4). Comparing a narrowed copy of the decoded
+// identifier against the known-ID set lets an unknown identifier alias a
+// known one.
+//
+// Identifiers of the form 0x1f * N + 0x21 are reserved to exercise that
+// requirement, and endpoints are expected to send one (RFC 9114, Section
+// 7.2.4.1). 0x1f * 33824 + 0x21 == 0x100001, which shares its low 16 bits
+// with HEADER_TABLE_SIZE (0x01).
+TEST_CASE("Load SETTINGS Frame ignores reserved identifier", "[http3]")
+{
+ uint8_t buf[] = {
+ 0x04, // Type
+ 0x05, // Length
+ 0x80, 0x10, 0x00, 0x01, // Identifier: QUIC varint encoding of 0x100001
+ 0x2a, // Value
+ };
+ MIOBuffer *input = new_MIOBuffer(BUFFER_SIZE_INDEX_128);
+ input->write(buf, sizeof(buf));
+ IOBufferReader *input_reader = input->alloc_reader();
+
+ std::shared_ptr<Http3Frame> frame = Http3FrameFactory::create(*input_reader);
+ frame->update();
+ REQUIRE(frame->type() == Http3FrameType::SETTINGS);
+
+ std::shared_ptr<Http3SettingsFrame> settings_frame =
std::dynamic_pointer_cast<Http3SettingsFrame>(frame);
+ REQUIRE(settings_frame);
+ REQUIRE(settings_frame->is_valid());
+
+ CHECK_FALSE(settings_frame->contains(Http3SettingsId::HEADER_TABLE_SIZE));
+
+ // ~Http3Frame deallocates the reader through its MIOBuffer, so release the
+ // frames before freeing that buffer.
+ settings_frame.reset();
+ frame.reset();
Review Comment:
Manual `reset()` calls and repeated explanatory comments are now duplicated
across multiple tests. Consider restructuring these tests to rely on
scope-based lifetimes instead (e.g., put `frame`/`settings_frame` in an inner
scope and call `free_MIOBuffer(input);` after that scope). This reduces
repetition and makes the destruction order harder to get wrong in future edits.
##########
src/proxy/http3/test/test_Http3Frame.cc:
##########
@@ -162,10 +166,51 @@ TEST_CASE("Load SETTINGS Frame", "[http3]")
CHECK(settings_frame->get(Http3SettingsId::MAX_FIELD_SECTION_SIZE) ==
0x0400);
CHECK(settings_frame->get(Http3SettingsId::NUM_PLACEHOLDERS) == 0x0f);
+ settings_frame.reset();
+ frame.reset();
free_MIOBuffer(input);
}
}
+// A SETTINGS identifier is a full QUIC variable-length integer, and an
+// identifier the implementation does not understand must be ignored
+// (RFC 9114, Section 7.2.4). Comparing a narrowed copy of the decoded
+// identifier against the known-ID set lets an unknown identifier alias a
+// known one.
+//
+// Identifiers of the form 0x1f * N + 0x21 are reserved to exercise that
+// requirement, and endpoints are expected to send one (RFC 9114, Section
+// 7.2.4.1). 0x1f * 33824 + 0x21 == 0x100001, which shares its low 16 bits
+// with HEADER_TABLE_SIZE (0x01).
+TEST_CASE("Load SETTINGS Frame ignores reserved identifier", "[http3]")
+{
+ uint8_t buf[] = {
+ 0x04, // Type
+ 0x05, // Length
+ 0x80, 0x10, 0x00, 0x01, // Identifier: QUIC varint encoding of 0x100001
+ 0x2a, // Value
Review Comment:
The PR description says the test is tagged `[http3-settings-id-width]` and
uses the 4-byte varint encoding of `0x10001`, but the added test is tagged
`[http3]` and encodes `0x100001` (as also stated in the inline comment). Please
align the PR description with the actual test (or adjust the test/tag/value) to
avoid confusion when running or referencing this regression.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
