brbzull0 commented on code in PR #13627:
URL: https://github.com/apache/trafficserver/pull/13627#discussion_r3976723928
##########
src/proxy/http3/test/test_Http3Frame.cc:
##########
@@ -166,6 +166,43 @@ TEST_CASE("Load SETTINGS Frame", "[http3]")
}
}
+// A SETTINGS identifier is a full QUIC variable-length integer (up to 62
bits).
+// Parsing must compare the decoded identifier against the known-ID set at full
+// width; narrowing the decoded value into a smaller integer type can cause an
Review Comment:
Done in b1342b11fe. The identifier is now `0x1f * 33824 + 0x21 == 0x100001`,
encoded as `0x80 0x10 0x00 0x01`; the frame length and the assertions are
unchanged. Re-confirmed the case still fails with the `uint16_t` restored.
https://github.com/apache/trafficserver/pull/13627/commits/b1342b11fed651b8fe981b31dc427d45983114f1
##########
src/proxy/http3/test/test_Http3Frame.cc:
##########
@@ -166,6 +166,43 @@ TEST_CASE("Load SETTINGS Frame", "[http3]")
}
}
+// A SETTINGS identifier is a full QUIC variable-length integer (up to 62
bits).
+// Parsing must compare the decoded identifier against the known-ID set at full
+// width; narrowing the decoded value into a smaller integer type can cause an
+// identifier outside the known set to be misinterpreted as a known one.
+// Here, encoding 0x10001 as a 4-byte QUIC varint (0x80 0x01 0x00 0x01) shares
+// its low 16 bits with HEADER_TABLE_SIZE (0x01). The parser must treat the
+// identifier as unknown and skip the setting rather than store it under
+// HEADER_TABLE_SIZE.
+TEST_CASE("Load SETTINGS Frame ignores wide unknown identifier",
"[http3][http3-settings-id-width]")
+{
+ uint8_t buf[] = {
+ 0x04, // Type
+ 0x05, // Length
+ 0x80, 0x01, 0x00, 0x01, // Identifier: QUIC varint encoding of 0x10001
+ 0x2a, // Value (1-byte QUIC varint = 42)
+ };
+ 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();
Review Comment:
Done in b1342b11fe and d1b88d56f0. "Load DATA Frame" and "Load SETTINGS
Frame" now release the frames before `free_MIOBuffer()`, carrying the same
explanatory comment as the new case.
https://github.com/apache/trafficserver/pull/13627/commits/b1342b11fed651b8fe981b31dc427d45983114f1
https://github.com/apache/trafficserver/pull/13627/commits/d1b88d56f0d48707de7c652a17bda682773e08fc
##########
src/proxy/http3/test/test_Http3Frame.cc:
##########
@@ -166,6 +166,43 @@ TEST_CASE("Load SETTINGS Frame", "[http3]")
}
Review Comment:
Done in b1342b11fe — the case is tagged `[http3]` now.
https://github.com/apache/trafficserver/pull/13627/commits/b1342b11fed651b8fe981b31dc427d45983114f1
--
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]
