brbzull0 commented on code in PR #13636:
URL: https://github.com/apache/trafficserver/pull/13636#discussion_r3981508570
##########
src/proxy/hdrs/XPACK.cc:
##########
@@ -131,12 +131,14 @@ xpack_decode_string(Arena &arena, char **str, uint64_t
&str_length, const uint8_
if (isHuffman) {
// Allocate temporary area twice the size of before decoded data
uint32_t const str_len = encoded_string_len * 2;
- *str = arena.str_alloc(str_len);
+ char *decoded = arena.str_alloc(str_len);
- len = huffman_decode(*str, str_len, p, encoded_string_len);
+ len = huffman_decode(decoded, str_len, p, encoded_string_len);
if (len < 0) {
Review Comment:
Not reachable: `max_string_len` comes from
`proxy.config.http.header_field_max_size`,
which is `[0-65535]` in `RecordsConfig.cc:582` and additionally clamped to
`UINT16_MAX` in `HttpConfig.cc:1289`, so `encoded_string_len * 2` is at most
131070. The `uint32_t` on that line predates this PR, which only renamed the
pointer in that hunk. Leaving it.
##########
src/proxy/http3/test/test_QPACK.cc:
##########
@@ -472,3 +472,65 @@ TEST_CASE("Decoding", "[qpack-decode]")
}
}
}
+
+// Decodes one Literal Header Field Without Name Reference. That is the field
+// representation whose name and value are both allocated out of QPACK's
+// per-connection arena and released again once the header is attached, so it
is
+// the path that cares about the order those releases happen in.
+TEST_CASE("Decoding a literal header field without name reference",
"[qpack-literal-decode]")
+{
+ QUICApplicationDriver driver;
+ QPACK *qpack = new QPACK(driver.get_connection(),
UINT32_MAX, 0, 0, MAX_FIELD_SIZE);
+ TestQPACKEventHandler *event_handler = new TestQPACKEventHandler();
+
Review Comment:
Same shape as `test_encode` and `test_decode` in this file (lines 297-299,
355-359), and for the same reason: `decode()` schedules events on
`all_ethreads[0]` that target `event_handler`, so a stack object would be
torn
down with events for it still queued. The file's convention is to let these
test
objects live rather than race the event thread. Happy to clean up the whole
file
in a follow-up; not doing only the new case here.
--
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]
