From 3075d06bfd959df5e9189e5ab5c402928cbb0785 Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Thu, 11 Jun 2026 18:46:59 +0000 Subject: [PATCH 1/6] QPACK: speed up static-table lookup Replace the unconditional 99-entry linear scan in the static-table name lookup with a binary search over an auxiliary name-sorted index. The static table cannot be reordered (its indices are wire values), so the index is built once and sorted by name. Behavior is unchanged: the lookup returns the sole exact match or the highest-indexed name match, exactly as the linear scan did. Co-Authored-By: Claude Fable 5 --- src/proxy/http3/QPACK.cc | 54 +++++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/src/proxy/http3/QPACK.cc b/src/proxy/http3/QPACK.cc index dfdd2d278b3..05ab306ead6 100644 --- a/src/proxy/http3/QPACK.cc +++ b/src/proxy/http3/QPACK.cc @@ -27,6 +27,10 @@ #include "tscore/ink_defs.h" #include "tscore/ink_memory.h" +#include +#include +#include + #define QPACKDebug(fmt, ...) Dbg(dbg_ctl_qpack, "[%s] " fmt, this->_qc->cids().data(), ##__VA_ARGS__) #define QPACKDTDebug(fmt, ...) Dbg(dbg_ctl_qpack, "" fmt, ##__VA_ARGS__) @@ -1226,26 +1230,42 @@ QPACK::StaticTable::lookup(uint16_t index, const char **name, size_t *name_len, const XpackLookupResult QPACK::StaticTable::lookup(const char *name, size_t name_len, const char *value, size_t value_len) { + // The static table (RFC 9204 Appendix A) is not ordered by name and its + // indices are part of the wire encoding, so it cannot be reordered. Keep an + // auxiliary index sorted by name (ties broken by ascending table index) so a + // lookup scans only the entries sharing the requested name instead of all of + // them. Returns the same index the former linear scan did: the sole exact + // match, or the highest-indexed name match otherwise. + static const auto name_order = [] { + std::array order; + for (uint16_t i = 0; i < order.size(); ++i) { + order[i] = i; + } + std::sort(order.begin(), order.end(), [](uint16_t l, uint16_t r) { + const std::string_view ln{STATIC_HEADER_FIELDS[l].name, STATIC_HEADER_FIELDS[l].name_len}; + const std::string_view rn{STATIC_HEADER_FIELDS[r].name, STATIC_HEADER_FIELDS[r].name_len}; + return ln != rn ? ln < rn : l < r; + }); + return order; + }(); + + const std::string_view target{name, name_len}; + auto it = std::lower_bound(name_order.begin(), name_order.end(), target, [](uint16_t idx, std::string_view t) { + return std::string_view{STATIC_HEADER_FIELDS[idx].name, STATIC_HEADER_FIELDS[idx].name_len} < t; + }); + XpackLookupResult::MatchType match_type = XpackLookupResult::MatchType::NONE; - uint16_t i = 0; uint16_t candidate_index = 0; - int n = countof(STATIC_HEADER_FIELDS); - - for (; i < n; ++i) { - const Header &h = STATIC_HEADER_FIELDS[i]; - if (h.name_len == name_len) { - if (memcmp(name, h.name, name_len) == 0) { - candidate_index = i; - if (value_len == h.value_len && memcmp(value, h.value, value_len) == 0) { - // Exact match - match_type = XpackLookupResult::MatchType::EXACT; - break; - } else { - // Name match -- Keep it for no exact matches - match_type = XpackLookupResult::MatchType::NAME; - } - } + for (; it != name_order.end(); ++it) { + const Header &h = STATIC_HEADER_FIELDS[*it]; + if (h.name_len != name_len || memcmp(h.name, name, name_len) != 0) { + break; // Past the run of entries sharing this name. + } + candidate_index = *it; + if (value_len == h.value_len && memcmp(value, h.value, value_len) == 0) { + return {candidate_index, XpackLookupResult::MatchType::EXACT}; } + match_type = XpackLookupResult::MatchType::NAME; } return {candidate_index, match_type}; } From 19069d22450f955bf850b576e3caa0eea16159b3 Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Thu, 11 Jun 2026 18:48:04 +0000 Subject: [PATCH 2/6] QPACK: fix header-prefix decode bounds and validation The decoder's Required Insert Count / Delta Base prefix decoding had three defects on malformed input: - _decode_header read an uninitialized `tmp` and used `&&` instead of `||`, so a failed integer decode was usually not caught and `pos` was then advanced by a negative return value. - The Delta Base check compared `< 0xFFFF` (inverted), failing to reject oversized values. - `remain_len` was never updated as `pos` advanced, so per-instruction decoders received an end pointer past the real buffer, allowing an over-read on a crafted length. Decode against a fixed `end` pointer, initialize the prefix integers, reject on `|| value > 0xFFFF`, and recompute the remaining length each instruction. Co-Authored-By: Claude Fable 5 --- src/proxy/http3/QPACK.cc | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/proxy/http3/QPACK.cc b/src/proxy/http3/QPACK.cc index 05ab306ead6..6c7686ebfe4 100644 --- a/src/proxy/http3/QPACK.cc +++ b/src/proxy/http3/QPACK.cc @@ -287,7 +287,7 @@ QPACK::decode(uint64_t stream_id, const uint8_t *header_block, size_t header_blo uint64_t tmp = 0; int64_t ret = xpack_decode_integer(tmp, header_block, header_block + header_block_len, 8); - if (ret < 0 && tmp > 0xFFFF) { + if (ret < 0 || tmp > 0xFFFF) { return -1; } uint16_t largest_reference = tmp; @@ -917,21 +917,21 @@ QPACK::_decode_literal_header_field_with_postbase_name_ref(int16_t base_index, c int QPACK::_decode_header(const uint8_t *header_block, size_t header_block_len, HTTPHdr &hdr) { - const uint8_t *pos = header_block; - size_t remain_len = header_block_len; - int64_t ret; + const uint8_t *pos = header_block; + const uint8_t *const end = header_block + header_block_len; + int64_t ret; // Decode Header Data Prefix - uint64_t tmp; - if ((ret = xpack_decode_integer(tmp, pos, pos + remain_len, 8)) < 0 && tmp > 0xFFFF) { + uint64_t tmp = 0; + if ((ret = xpack_decode_integer(tmp, pos, end, 8)) < 0 || tmp > 0xFFFF) { return -1; } pos += ret; uint16_t largest_reference = tmp; - uint64_t delta_base_index; + uint64_t delta_base_index = 0; uint16_t base_index; - if ((ret = xpack_decode_integer(delta_base_index, pos, pos + remain_len, 7)) < 0 && delta_base_index < 0xFFFF) { + if ((ret = xpack_decode_integer(delta_base_index, pos, end, 7)) < 0 || delta_base_index > 0xFFFF) { return -2; } @@ -948,7 +948,8 @@ QPACK::_decode_header(const uint8_t *header_block, size_t header_block_len, HTTP uint32_t decoded_header_list_size = 0; // Decode Instructions - while (pos < header_block + header_block_len) { + while (pos < end) { + size_t remain_len = end - pos; uint32_t header_len = 0; if (pos[0] & 0x80) { // Index Header Field From 57e673b3269016fb2bb3337e4cb46b81949571be Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Thu, 11 Jun 2026 18:48:55 +0000 Subject: [PATCH 3/6] HTTP/3: refuse unimplemented QPACK dynamic-table config The QPACK dynamic table is not implemented (entries are never stored), so advertising a non-zero header_table_size or qpack_blocked_streams would make peers insert entries we drop and then reference, breaking decoding. Fail at config load if either is set non-zero instead of silently misbehaving on the wire. Co-Authored-By: Claude Fable 5 --- src/proxy/http3/Http3Config.cc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/proxy/http3/Http3Config.cc b/src/proxy/http3/Http3Config.cc index e66af7204cb..9f4e459deee 100644 --- a/src/proxy/http3/Http3Config.cc +++ b/src/proxy/http3/Http3Config.cc @@ -23,6 +23,8 @@ #include "proxy/http3/Http3Config.h" +#include "tscore/Diags.h" + int ts::Http3Config::_config_id = 0; // @@ -36,6 +38,16 @@ ts::Http3ConfigParams::initialize() RecEstablishStaticConfigUInt32(this->_qpack_blocked_streams, "proxy.config.http3.qpack_blocked_streams"); RecEstablishStaticConfigUInt32(this->_num_placeholders, "proxy.config.http3.num_placeholders"); RecEstablishStaticConfigUInt32(this->_max_settings, "proxy.config.http3.max_settings"); + + // The QPACK dynamic table is not implemented: XpackDynamicTable never stores + // entries (its capacity is fixed at zero and never raised). Advertising a + // non-zero table size or blocked-stream count would tell a peer it may + // insert entries our decoder silently drops and then reference them, which + // breaks decoding. Refuse to start until the dynamic table is implemented. + if (this->_header_table_size != 0 || this->_qpack_blocked_streams != 0) { + Fatal("HTTP/3 QPACK dynamic table is not implemented; proxy.config.http3.header_table_size and " + "proxy.config.http3.qpack_blocked_streams must be 0"); + } } uint32_t From b7ea8bc3f74fb49de139206b53894c55557102fa Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Thu, 11 Jun 2026 18:49:58 +0000 Subject: [PATCH 4/6] QPACK: restore RFC 9204 static table (drop zstd entries) The QPACK static table is normative: its 99 indices (RFC 9204 Appendix A) are wire values shared with every peer and cannot be extended or reordered. #12201 added a 100th entry ("content-encoding: zstd") in the middle and appended "zstd" to entry 31's value, shifting RFC indices 42-98 to 43-99 and altering index 31. A peer using the standard table then mis-resolves every static reference at or above 42 (content-type variants, the second :status block, user-agent, etc.), corrupting headers in both directions; it also makes "content-encoding: zstd" itself decode as "br" on a standard client. Restore the table to RFC 9204 exactly. zstd content-coding is unaffected: it is still encoded as a literal field, which is how every compliant QPACK implementation conveys values absent from the static table. Co-Authored-By: Claude Fable 5 --- src/proxy/http3/QPACK.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/proxy/http3/QPACK.cc b/src/proxy/http3/QPACK.cc index 6c7686ebfe4..3152c70e7da 100644 --- a/src/proxy/http3/QPACK.cc +++ b/src/proxy/http3/QPACK.cc @@ -73,7 +73,7 @@ const QPACK::Header QPACK::StaticTable::STATIC_HEADER_FIELDS[] = { {":status", "503" }, {"accept", "*/*" }, {"accept", "application/dns-message" }, - {"accept-encoding", "gzip, deflate, br, zstd" }, + {"accept-encoding", "gzip, deflate, br" }, {"accept-ranges", "bytes" }, {"access-control-allow-headers", "cache-control" }, {"access-control-allow-headers", "content-type" }, @@ -84,7 +84,6 @@ const QPACK::Header QPACK::StaticTable::STATIC_HEADER_FIELDS[] = { {"cache-control", "no-cache" }, {"cache-control", "no-store" }, {"cache-control", "public, max-age=31536000" }, - {"content-encoding", "zstd" }, {"content-encoding", "br" }, {"content-encoding", "gzip" }, {"content-type", "application/dns-message" }, From 8c6aa9ff55a15f081363f15f5d9155985f47bc13 Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Thu, 11 Jun 2026 19:08:48 +0000 Subject: [PATCH 5/6] QPACK: reject dynamic references when no dynamic table exists A header block with a non-zero Required Insert Count references the dynamic table. Entries are only stored once a non-zero table capacity is negotiated, and ATS advertises zero, so such a reference can never be satisfied -- the decoder would queue the stream as blocked forever. Fail the decode instead when the table capacity is zero. Adds a test. Co-Authored-By: Claude Fable 5 --- src/proxy/http3/QPACK.cc | 9 +++++++++ src/proxy/http3/test/test_QPACK.cc | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/proxy/http3/QPACK.cc b/src/proxy/http3/QPACK.cc index 3152c70e7da..d25a8690ace 100644 --- a/src/proxy/http3/QPACK.cc +++ b/src/proxy/http3/QPACK.cc @@ -291,6 +291,15 @@ QPACK::decode(uint64_t stream_id, const uint8_t *header_block, size_t header_blo } uint16_t largest_reference = tmp; + // A non-zero Required Insert Count references the dynamic table. We only + // store dynamic entries once a non-zero table capacity is negotiated (and we + // currently always advertise zero), so when the table capacity is zero such + // a reference can never be satisfied. Treat it as a decoding failure instead + // of queuing the stream as blocked forever. + if (largest_reference != 0 && this->_dynamic_table.maximum_size() == 0) { + return -1; + } + if (largest_reference != 0 && (this->_dynamic_table.is_empty() || this->_dynamic_table.largest_index() < largest_reference)) { // Blocked if (this->_add_to_blocked_list( diff --git a/src/proxy/http3/test/test_QPACK.cc b/src/proxy/http3/test/test_QPACK.cc index 438571d1149..f04f7dd4be7 100644 --- a/src/proxy/http3/test/test_QPACK.cc +++ b/src/proxy/http3/test/test_QPACK.cc @@ -469,3 +469,26 @@ TEST_CASE("Decoding", "[qpack-decode]") } } } + +// ATS advertises a zero-capacity QPACK dynamic table, so a header block whose +// Required Insert Count is non-zero references entries that can never exist. +// decode() must fail rather than queue the stream as blocked forever. +TEST_CASE("QPACK decode rejects a dynamic reference with no dynamic table", "[qpack]") +{ + QUICApplicationDriver driver; + QPACK *qpack = new QPACK(driver.get_connection(), UINT32_MAX, 0, 0); + TestQPACKEventHandler *event_handler = new TestQPACKEventHandler(); + + HTTPHdr hdr; + hdr.create(HTTPType::REQUEST); + + uint8_t block[8]; + int len = 0; + len += xpack_encode_integer(block + len, block + sizeof(block), 1, 8); // Required Insert Count = 1 + block[len++] = 0x00; // S = 0, Delta Base = 0 + + int ret = qpack->decode(1, block, len, hdr, event_handler, eventProcessor.all_ethreads[0]); + CHECK(ret < 0); + + hdr.destroy(); +} From ae70b60e1fb2a41884657392d6edee21792be21b Mon Sep 17 00:00:00 2001 From: "Phong X. Nguyen" Date: Thu, 11 Jun 2026 19:09:51 +0000 Subject: [PATCH 6/6] QPACK: add RFC 9204 static-table conformance test Decode header blocks that reference static-table indices spanning the range a prior change shifted (31, 42, 52, 71, 98) and assert each yields the exact RFC 9204 Appendix A name and value. This pins the normative table and fails if an entry is ever inserted or reordered. Co-Authored-By: Claude Fable 5 --- src/proxy/http3/test/test_QPACK.cc | 57 ++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/proxy/http3/test/test_QPACK.cc b/src/proxy/http3/test/test_QPACK.cc index f04f7dd4be7..f499ca0cfe6 100644 --- a/src/proxy/http3/test/test_QPACK.cc +++ b/src/proxy/http3/test/test_QPACK.cc @@ -470,6 +470,63 @@ TEST_CASE("Decoding", "[qpack-decode]") } } +// Encode an "Indexed Header Field" (RFC 9204 4.5.2) that references a +// static-table entry, returning the number of bytes written. +static int +append_static_indexed_field(uint8_t *buf, const uint8_t *buf_end, uint16_t index) +{ + int64_t n = xpack_encode_integer(buf, buf_end, index, 6); + buf[0] |= 0xC0; // '1' Indexed Header Field + 'T'=1 static table + return static_cast(n); +} + +// The QPACK static table (RFC 9204 Appendix A) is normative: its 99 indices are +// wire values shared with every peer and must not be reordered. These cases +// span the range a previous change shifted by inserting a non-standard entry; +// decoding each index must yield the exact RFC name and value. +TEST_CASE("QPACK static table conforms to RFC 9204", "[qpack]") +{ + struct { + uint16_t index; + const char *name; + const char *value; + } const cases[] = { + {31, "accept-encoding", "gzip, deflate, br" }, + {42, "content-encoding", "br" }, + {52, "content-type", "text/html; charset=utf-8"}, + {71, ":status", "500" }, + {98, "x-frame-options", "sameorigin" }, + }; + + QUICApplicationDriver driver; + QPACK *qpack = new QPACK(driver.get_connection(), UINT32_MAX, 0, 0); + TestQPACKEventHandler *event_handler = new TestQPACKEventHandler(); + + HTTPHdr hdr; + hdr.create(HTTPType::RESPONSE); + + uint8_t block[256]; + int len = 0; + block[len++] = 0x00; // Required Insert Count = 0 + block[len++] = 0x00; // S = 0, Delta Base = 0 + for (auto const &c : cases) { + len += append_static_indexed_field(block + len, block + sizeof(block), c.index); + } + + // decode() populates the header set synchronously; only the completion event + // is scheduled, so the fields can be inspected as soon as it returns. + int ret = qpack->decode(1, block, len, hdr, event_handler, eventProcessor.all_ethreads[0]); + REQUIRE(ret == 0); + + for (auto const &c : cases) { + MIMEField *field = hdr.field_find(std::string_view{c.name}); + REQUIRE(field != nullptr); + CHECK(field->value_get() == std::string_view{c.value}); + } + + hdr.destroy(); +} + // ATS advertises a zero-capacity QPACK dynamic table, so a header block whose // Required Insert Count is non-zero references entries that can never exist. // decode() must fail rather than queue the stream as blocked forever.