From 357a4ca5af9ca449d52a806569d0eb5eca1ae82a Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Mon, 8 Jun 2026 16:07:35 -0600 Subject: [PATCH 01/33] add bypass header config option to maxmind_acl plugin (#13160) * add bypass header config option to maxmind_acl plugin * fix for header value check, update docs with warning about usage of this new feature * require both header and value. clean up review suggestions * more explicit documentation about value match, limit debug log * maxmind: tighten up checks on yaml node (cherry picked from commit f891fe9c440b1e7676f699f6df82dcffed936e51) --- doc/admin-guide/plugins/maxmind_acl.en.rst | 47 +++++++++- .../experimental/maxmind_acl/maxmind_acl.cc | 4 + plugins/experimental/maxmind_acl/mmdb.cc | 92 +++++++++++++++++++ plugins/experimental/maxmind_acl/mmdb.h | 7 ++ 4 files changed, 149 insertions(+), 1 deletion(-) diff --git a/doc/admin-guide/plugins/maxmind_acl.en.rst b/doc/admin-guide/plugins/maxmind_acl.en.rst index d0a4aacb97a..32fc35d199e 100644 --- a/doc/admin-guide/plugins/maxmind_acl.en.rst +++ b/doc/admin-guide/plugins/maxmind_acl.en.rst @@ -113,4 +113,49 @@ The plugin also supports optional fields from GeoGuard databases which includes: ``vpn_datacenter`` ``relay_proxy`` ``proxy_over_vpn`` -``smart_dns_proxy`` \ No newline at end of file +``smart_dns_proxy`` + +Bypass +====== + +An optional ``bypass`` field allows a request to skip all geo checks entirely and pass through +unmodified. Both a header name and an expected value must be configured; when the named header +is present in the request **and** its value matches exactly, the plugin returns immediately +without performing any country, IP, regex, or anonymous evaluation. + +``header`` + Required sub-key. The name of the HTTP request header to look for, e.g. ``@GeoBypass``. + +``value`` + Required sub-key. The header field value must match this string exactly for the bypass to + trigger. Both ``header`` and ``value`` must be present and non-empty; omitting either + disables the bypass entirely and a warning is emitted to the ATS error log. + +The comparison uses the complete, raw field value of the first occurrence of the named header. +Duplicate headers with the same name (repeated lines) are ignored — only the first is evaluated. +Within that first field, the entire value must match exactly, so a comma-separated multi-value +(e.g. ``@GeoBypass: 1, extra``) in a single header line will not match a simple configured value. + +An example configuration :: + + maxmind: + database: GeoIP2-City.mmdb + bypass: + header: "@GeoBypass" + value: "1" + allow: + country: + - US + +This is useful for internal or trusted upstream services that should not be subject to geo +restrictions. If ``bypass`` is absent from the configuration, or if either ``header`` or +``value`` is missing, bypass is disabled and all requests are evaluated normally. + +.. warning:: + + Because the bypass skips **all** ACL checks, the configured header must be + unforgeable by external clients. Use an internal ``@``-prefixed header (e.g. + ``@GeoBypass``) that is set by ATS itself or a trusted upstream, or + ensure the edge strips/overwrites the header before it reaches this plugin. + Configuring a normal client-supplied header allows end users to opt out of + geo restrictions by simply sending the header in their request. \ No newline at end of file diff --git a/plugins/experimental/maxmind_acl/maxmind_acl.cc b/plugins/experimental/maxmind_acl/maxmind_acl.cc index a6c6a26948f..b05aa50dc4c 100644 --- a/plugins/experimental/maxmind_acl/maxmind_acl.cc +++ b/plugins/experimental/maxmind_acl/maxmind_acl.cc @@ -68,6 +68,10 @@ TSRemapDoRemap(void *ih, TSHttpTxn rh, TSRemapRequestInfo *rri) Dbg(dbg_ctl, "No ACLs configured"); } else { Acl *a = static_cast(ih); + if (a->check_bypass(rh)) { + Dbg(dbg_ctl, "bypassing geo check due to bypass header"); + return TSREMAP_NO_REMAP; + } if (!a->eval(rri, rh)) { Dbg(dbg_ctl, "denying request"); TSHttpTxnStatusSet(rh, TS_HTTP_STATUS_FORBIDDEN, PLUGIN_NAME); diff --git a/plugins/experimental/maxmind_acl/mmdb.cc b/plugins/experimental/maxmind_acl/mmdb.cc index c62dcc1a5b0..bd170555c64 100644 --- a/plugins/experimental/maxmind_acl/mmdb.cc +++ b/plugins/experimental/maxmind_acl/mmdb.cc @@ -117,6 +117,9 @@ Acl::init(char const *filename) _proxy_over_vpn = false; _smart_dns_proxy = false; + _bypass_header.clear(); + _bypass_header_value.clear(); + if (loadallow(maxmind["allow"])) { Dbg(dbg_ctl, "Loaded Allow ruleset"); status = true; @@ -135,6 +138,8 @@ Acl::init(char const *filename) _anonymous_blocking = loadanonymous(maxmind["anonymous"]); + loadbypass(maxmind["bypass"]); + if (!status) { Dbg(dbg_ctl, "Failed to load any rulesets, none specified"); status = false; @@ -425,6 +430,58 @@ Acl::parseregex(const YAML::Node ®ex, bool allow) } } +void +Acl::loadbypass(const YAML::Node &bypassNode) +{ + if (!bypassNode) { + Dbg(dbg_ctl, "No bypass set"); + return; + } + if (bypassNode.IsNull()) { + TSWarning("[%s] bypass node is NULL — bypass disabled", PLUGIN_NAME); + return; + } + + try { + if (bypassNode["header"]) { + const YAML::Node &headerNode = bypassNode["header"]; + if (headerNode.IsNull() || !headerNode.IsScalar()) { + TSWarning("[%s] bypass 'header' is null or non-scalar — bypass disabled", PLUGIN_NAME); + return; + } + + if (!bypassNode["value"]) { + TSWarning("[%s] bypass 'header' set without 'value' — bypass disabled; both are required", PLUGIN_NAME); + return; + } + const YAML::Node &valueNode = bypassNode["value"]; + if (valueNode.IsNull() || !valueNode.IsScalar()) { + TSWarning("[%s] bypass 'value' is null or non-scalar — bypass disabled", PLUGIN_NAME); + return; + } + + _bypass_header_value = valueNode.as(); + if (_bypass_header_value.empty()) { + TSWarning("[%s] bypass 'value' is empty — bypass disabled; a non-empty value is required", PLUGIN_NAME); + return; + } + _bypass_header = headerNode.as(); + if (_bypass_header.empty()) { + TSWarning("[%s] bypass 'header' is empty — bypass disabled; a non-empty header is required", PLUGIN_NAME); + return; + } + Dbg(dbg_ctl, "bypass header set to: %s", _bypass_header.c_str()); + Dbg(dbg_ctl, "bypass value set to: %s", _bypass_header_value.c_str()); + } else { + TSWarning("[%s] bypass is set but missing 'header' key — bypass disabled", PLUGIN_NAME); + return; + } + } catch (const YAML::Exception &e) { + TSError("[%s] YAML::Exception %s when parsing bypass config", PLUGIN_NAME, e.what()); + return; + } +} + void Acl::loadhtml(const YAML::Node &htmlNode) { @@ -499,6 +556,41 @@ Acl::loaddb(const YAML::Node &dbNode) return true; } +bool +Acl::check_bypass(TSHttpTxn txnp) const +{ + if (_bypass_header.empty()) { + return false; + } + + TSMBuffer mbuf; + TSMLoc hdr_loc; + if (TS_SUCCESS != TSHttpTxnClientReqGet(txnp, &mbuf, &hdr_loc)) { + Dbg(dbg_ctl, "check_bypass: failed to get client request headers"); + return false; + } + + TSMLoc field_loc = TSMimeHdrFieldFind(mbuf, hdr_loc, _bypass_header.c_str(), static_cast(_bypass_header.size())); + if (TS_NULL_MLOC == field_loc) { + TSHandleMLocRelease(mbuf, TS_NULL_MLOC, hdr_loc); + return false; + } + + bool bypassed = false; + int val_len = 0; + const char *val = TSMimeHdrFieldValueStringGet(mbuf, hdr_loc, field_loc, -1, &val_len); + if (val != nullptr && 0 < val_len && std::string_view(val, val_len) == _bypass_header_value) { + Dbg(dbg_ctl, "check_bypass: bypass triggered"); + bypassed = true; + } else { + Dbg(dbg_ctl, "check_bypass: bypass header present but value did not match"); + } + + TSHandleMLocRelease(mbuf, hdr_loc, field_loc); + TSHandleMLocRelease(mbuf, TS_NULL_MLOC, hdr_loc); + return bypassed; +} + bool Acl::eval(TSRemapRequestInfo * /* rri ATS_UNUSED */, TSHttpTxn txnp) { diff --git a/plugins/experimental/maxmind_acl/mmdb.h b/plugins/experimental/maxmind_acl/mmdb.h index 9b1d622a204..bdcb5ad550d 100644 --- a/plugins/experimental/maxmind_acl/mmdb.h +++ b/plugins/experimental/maxmind_acl/mmdb.h @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -69,6 +70,7 @@ class Acl } bool eval(TSRemapRequestInfo *rri, TSHttpTxn txnp); + bool check_bypass(TSHttpTxn txnp) const; bool init(char const *filename); void @@ -111,6 +113,10 @@ class Acl bool _anonymous_blocking = false; + // Bypass header fields + std::string _bypass_header; + std::string _bypass_header_value; + // Do we want to allow by default or not? Useful // for deny only rules bool default_allow = false; @@ -121,6 +127,7 @@ class Acl bool loaddeny(const YAML::Node &denyNode); void loadhtml(const YAML::Node &htmlNode); bool loadanonymous(const YAML::Node &anonNode); + void loadbypass(const YAML::Node &bypassNode); bool eval_country(MMDB_entry_data_s *entry_data, const std::string &url); bool eval_anonymous(MMDB_entry_s *entry_data); void parseregex(const YAML::Node ®ex, bool allow); From f5deb14bc9820621fc8f5e0ac8ccdb09774bc767 Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Mon, 8 Jun 2026 05:49:42 -0500 Subject: [PATCH 02/33] Clamp HTTP3 frame type buf size to reader bytes (#13242) * Clamp HTTP3 frame type buf size to reader bytes The length of the source buffer for HTTP3 type parsing was always taken to be the maximum length of the type field. This seemed to work without UB when I tested it through `Http3FrameDispatcher`, but Kit Chan pointed out that it is risky (#11720). This patch refactors the type parsing to guarantee that the number of bytes passed to the parser will not be greater than the number of initialized bytes in the buffer. * Fix incorrect identifier name Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Fix incorrect identifier * Fix incorrect identifier name Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> (cherry picked from commit 318942389264f51e728624c7f02237eab3a09cec) --- src/proxy/http3/Http3Frame.cc | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/proxy/http3/Http3Frame.cc b/src/proxy/http3/Http3Frame.cc index 0060dad16d9..64b0a0a4651 100644 --- a/src/proxy/http3/Http3Frame.cc +++ b/src/proxy/http3/Http3Frame.cc @@ -27,6 +27,8 @@ #include "proxy/http3/Http3Frame.h" #include "proxy/http3/Http3Config.h" +#include + ClassAllocator http3FrameAllocator("http3FrameAllocator"); ClassAllocator http3DataFrameAllocator("http3DataFrameAllocator"); ClassAllocator http3HeadersFrameAllocator("http3HeadersFrameAllocator"); @@ -505,9 +507,10 @@ Http3FrameFactory::create(IOBufferReader &reader) ts::Http3Config::scoped_config params; Http3Frame *frame = nullptr; - uint8_t type_buf[FRAME_TYPE_MAX_BYTES]{}; - reader.memcpy(type_buf, sizeof(type_buf)); - Http3FrameType type = Http3Frame::type(type_buf, sizeof(type_buf)); + uint8_t type_buf[FRAME_TYPE_MAX_BYTES]{}; + std::size_t const type_avail{std::min(reader.read_avail(), sizeof(type_buf))}; + reader.memcpy(type_buf, type_avail); + Http3FrameType type = Http3Frame::type(type_buf, type_avail); switch (type) { case Http3FrameType::HEADERS: @@ -534,9 +537,10 @@ Http3FrameFactory::create(IOBufferReader &reader) std::shared_ptr Http3FrameFactory::fast_create(IOBufferReader &reader) { - uint8_t type_buf[FRAME_TYPE_MAX_BYTES]{}; - reader.memcpy(type_buf, sizeof(type_buf)); - Http3FrameType type = Http3Frame::type(type_buf, sizeof(type_buf)); + uint8_t type_buf[FRAME_TYPE_MAX_BYTES]{}; + std::size_t const type_avail{std::min(reader.read_avail(), sizeof(type_buf))}; + reader.memcpy(type_buf, type_avail); + Http3FrameType type = Http3Frame::type(type_buf, type_avail); if (type == Http3FrameType::UNKNOWN) { if (!this->_unknown_frame) { this->_unknown_frame = Http3FrameFactory::create(reader); From fafb8bdcf43684b5c6b56ae2aa15bb6d88542c74 Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Tue, 9 Jun 2026 09:51:52 -0600 Subject: [PATCH 03/33] slice: fix stpcpy off-by-one for header value extraction (#13181) * slice: fix stpcpy off-by-one for header value extraction The off by one wastes a single character, it doesn't overrun. * HttpHeader: check for pass in zero length value Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> (cherry picked from commit b71ea748f2bc3524cf7ecf0080181c06aff92b78) --- plugins/slice/HttpHeader.cc | 28 ++++++++++++++++------------ plugins/slice/HttpHeader.h | 4 +++- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/plugins/slice/HttpHeader.cc b/plugins/slice/HttpHeader.cc index bfaa2253bc1..d08e76db7a9 100644 --- a/plugins/slice/HttpHeader.cc +++ b/plugins/slice/HttpHeader.cc @@ -168,33 +168,37 @@ HttpHeader::removeKey(char const *const keystr, int const keylen) bool HttpHeader::valueForKey(char const *const keystr, int const keylen, char *const valstr, int *const vallen, int const index) const { - if (!isValid()) { + if (nullptr == valstr || nullptr == vallen) { + return false; + } + if (!isValid() || index < -1) { *vallen = 0; return false; } bool status = false; + int const valcap = *vallen; + *vallen = 0; + if (valcap <= 0) { + return false; + } + valstr[0] = '\0'; TSMLoc const locfield = TSMimeHdrFieldFind(m_buffer, m_lochdr, keystr, keylen); if (nullptr != locfield) { int getlen = 0; char const *const getstr = TSMimeHdrFieldValueStringGet(m_buffer, m_lochdr, locfield, index, &getlen); - - int const valcap = *vallen; - if (nullptr != getstr && 0 < getlen && getlen < (valcap - 1)) { + if (nullptr != getstr && 0 < getlen && getlen < valcap) { char *const endp = stpncpy(valstr, getstr, getlen); - - *vallen = endp - valstr; - status = (*vallen < valcap); - - if (status) { - *endp = '\0'; + int const len = endp - valstr; + if (len < valcap) { + *endp = '\0'; + *vallen = len; + status = true; } } TSHandleMLocRelease(m_buffer, m_lochdr, locfield); - } else { - *vallen = 0; } return status; diff --git a/plugins/slice/HttpHeader.h b/plugins/slice/HttpHeader.h index 0c99df1921d..c52738e5d18 100644 --- a/plugins/slice/HttpHeader.h +++ b/plugins/slice/HttpHeader.h @@ -114,7 +114,9 @@ struct HttpHeader { // returns false if header invalid or something went wrong with removal. bool removeKey(char const *const key, int const keylen); - // retrieves header value as a char* + // retrieves header value as a null terminated char* in valstr. + // caller must ensure the valstr buffer has sufficient capacity. + // null termination only guaranteed on success. bool valueForKey(char const *const keystr, int const keylen, char *const valstr, // <-- return string value int *const vallen, // <-- pass in capacity, returns len of string From 08102c00870f535dabddc88881a85058f36da603 Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Tue, 9 Jun 2026 06:56:28 -0500 Subject: [PATCH 04/33] Move some directory helpers to `Directory` (#13245) * Move `dir_init_segment` to `Directory` * Move `dir_init_segment` to `Directory` * Move `dir_bucket_loop_fix` to `Directory` * Move `unlink_from_freelist` to `Directory` * Move `dir_delete_entry` to `Directory` Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> (cherry picked from commit 0847dc6f853a045b2dc7335fec052e748a5645a3) --- src/iocore/cache/CacheDir.cc | 97 +++++++++-------------------------- src/iocore/cache/CacheVC.cc | 5 +- src/iocore/cache/P_CacheDir.h | 85 +++++++++++++++++++++++++----- 3 files changed, 97 insertions(+), 90 deletions(-) diff --git a/src/iocore/cache/CacheDir.cc b/src/iocore/cache/CacheDir.cc index ee14d521ff3..7997c501086 100644 --- a/src/iocore/cache/CacheDir.cc +++ b/src/iocore/cache/CacheDir.cc @@ -210,16 +210,16 @@ dir_bucket_loop_check(Dir *start_dir, Dir *seg) // adds all the directory entries // in a segment to the segment freelist void -dir_init_segment(int s, Directory *directory) +Directory::init_segment(int s) { - directory->header->freelist[s] = 0; - Dir *seg = directory->get_segment(s); + this->header->freelist[s] = 0; + Dir *seg = this->get_segment(s); int l, b; - memset(static_cast(seg), 0, SIZEOF_DIR * DIR_DEPTH * directory->buckets); + memset(static_cast(seg), 0, SIZEOF_DIR * DIR_DEPTH * this->buckets); for (l = 1; l < DIR_DEPTH; l++) { - for (b = 0; b < directory->buckets; b++) { + for (b = 0; b < this->buckets; b++) { Dir *bucket = dir_bucket(b, seg); - directory->free_entry(dir_bucket_row(bucket, l), s); + this->free_entry(dir_bucket_row(bucket, l), s); } } } @@ -227,11 +227,11 @@ dir_init_segment(int s, Directory *directory) // break the infinite loop in directory entries // Note : abuse of the token bit in dir entries int -dir_bucket_loop_fix(Dir *start_dir, int s, Directory *directory) +Directory::bucket_loop_fix(Dir *start_dir, int s) { - if (!dir_bucket_loop_check(start_dir, directory->get_segment(s))) { + if (!dir_bucket_loop_check(start_dir, this->get_segment(s))) { Warning("Dir loop exists, clearing segment %d", s); - dir_init_segment(s, directory); + this->init_segment(s); return 1; } return 0; @@ -243,7 +243,7 @@ Directory::freelist_length(int s) int free = 0; Dir *seg = this->get_segment(s); Dir *e = dir_from_offset(this->header->freelist[s], seg); - if (dir_bucket_loop_fix(e, s, this)) { + if (this->bucket_loop_fix(e, s)) { return (DIR_DEPTH - 1) * this->buckets; } while (e) { @@ -260,7 +260,7 @@ Directory::bucket_length(Dir *b, int s) int i = 0; Dir *seg = this->get_segment(s); #ifdef LOOP_CHECK_MODE - if (dir_bucket_loop_fix(b, s, this)) + if (this->bucket_loop_fix(b, s)) return 1; #endif while (e) { @@ -296,55 +296,6 @@ Directory::check() return 1; } -inline void -unlink_from_freelist(Dir *e, int s, Directory *directory) -{ - Dir *seg = directory->get_segment(s); - Dir *p = dir_from_offset(dir_prev(e), seg); - if (p) { - dir_set_next(p, dir_next(e)); - } else { - directory->header->freelist[s] = dir_next(e); - } - Dir *n = dir_from_offset(dir_next(e), seg); - if (n) { - dir_set_prev(n, dir_prev(e)); - } -} - -inline Dir * -dir_delete_entry(Dir *e, Dir *p, int s, Directory *directory) -{ - Dir *seg = directory->get_segment(s); - int no = dir_next(e); - directory->header->dirty = 1; - if (p) { - unsigned int fo = directory->header->freelist[s]; - unsigned int eo = dir_to_offset(e, seg); - dir_clear(e); - dir_set_next(p, no); - dir_set_next(e, fo); - if (fo) { - dir_set_prev(dir_from_offset(fo, seg), eo); - } - directory->header->freelist[s] = eo; - } else { - Dir *n = next_dir(e, seg); - if (n) { - // "Shuffle" here means that we're copying the second entry's data to the head entry's location, and removing the second entry - // - because the head entry can't be moved. - ATS_PROBE3(cache_dir_shuffle, s, dir_to_offset(e, seg), dir_to_offset(n, seg)); - dir_assign(e, n); - dir_delete_entry(n, e, s, directory); - return e; - } else { - dir_clear(e); - return nullptr; - } - } - return dir_from_offset(no, seg); -} - inline void dir_clean_bucket(Dir *b, int s, StripeSM *stripe) { @@ -357,7 +308,7 @@ dir_clean_bucket(Dir *b, int s, StripeSM *stripe) #ifdef LOOP_CHECK_MODE loop_count++; if (loop_count > DIR_LOOP_THRESHOLD) { - if (dir_bucket_loop_fix(b, s, vol->directory)) + if (vol->directory.bucket_loop_fix(b, s)) return; } #endif @@ -372,7 +323,7 @@ dir_clean_bucket(Dir *b, int s, StripeSM *stripe) } // Match cache_dir_remove arguments ATS_PROBE7(cache_dir_remove_clean_bucket, stripe->fd, s, dir_to_offset(e, seg), dir_offset(e), dir_approx_size(e), 0, 0); - e = dir_delete_entry(e, p, s, &stripe->directory); + e = stripe->directory.delete_entry(e, p, s); continue; } p = e; @@ -462,7 +413,7 @@ freelist_pop(int s, StripeSM *stripe) stripe->directory.header->freelist[s] = dir_next(e); // if the freelist if bad, punt. if (dir_offset(e)) { - dir_init_segment(s, &stripe->directory); + stripe->directory.init_segment(s); return nullptr; } Dir *h = dir_from_offset(stripe->directory.header->freelist[s], seg); @@ -495,7 +446,7 @@ Directory::probe(const CacheKey *key, StripeSM *stripe, Dir *result, Dir **last_ Dir *e = nullptr, *p = nullptr, *collision = *last_collision; CHECK_DIR(d); #ifdef LOOP_CHECK_MODE - if (dir_bucket_loop_fix(dir_bucket(b, seg), s, this)) + if (this->bucket_loop_fix(dir_bucket(b, seg), s)) return 0; #endif Lagain: @@ -506,7 +457,7 @@ Directory::probe(const CacheKey *key, StripeSM *stripe, Dir *result, Dir **last_ ink_assert(dir_offset(e)); // Bug: 51680. Need to check collision before checking // dir_valid(). In case of a collision, if !dir_valid(), we - // don't want to call dir_delete_entry. + // don't want to call Directory::delete_entry. if (collision) { if (collision == e) { collision = nullptr; @@ -533,7 +484,7 @@ Directory::probe(const CacheKey *key, StripeSM *stripe, Dir *result, Dir **last_ ts::Metrics::Gauge::decrement(stripe->cache_vol->vol_rsb.direntries_used); ATS_PROBE7(cache_dir_remove_invalid, stripe->fd, s, dir_to_offset(e, seg), dir_offset(e), dir_approx_size(e), key->slice64(0), key->slice64(1)); - e = dir_delete_entry(e, p, s, this); + e = this->delete_entry(e, p, s); continue; } } else { @@ -585,7 +536,7 @@ Directory::insert(const CacheKey *key, StripeSM *stripe, Dir *to_part) for (l = 1; l < DIR_DEPTH; l++) { e = dir_bucket_row(b, l); if (dir_is_empty(e)) { - unlink_from_freelist(e, s, this); + this->unlink_from_freelist(e, s); goto Llink; } } @@ -653,7 +604,7 @@ Directory::overwrite(const CacheKey *key, StripeSM *stripe, Dir *dir, Dir *overw #ifdef LOOP_CHECK_MODE loop_count++; if (loop_count > DIR_LOOP_THRESHOLD && loop_possible) { - if (dir_bucket_loop_fix(b, s, this)) { + if (this->bucket_loop_fix(b, s)) { loop_possible = false; goto Lagain; } @@ -679,7 +630,7 @@ Directory::overwrite(const CacheKey *key, StripeSM *stripe, Dir *dir, Dir *overw for (l = 1; l < DIR_DEPTH; l++) { e = dir_bucket_row(b, l); if (dir_is_empty(e)) { - unlink_from_freelist(e, s, this); + this->unlink_from_freelist(e, s); goto Llink; } } @@ -733,7 +684,7 @@ Directory::remove(const CacheKey *key, StripeSM *stripe, Dir *del) #ifdef LOOP_CHECK_MODE loop_count++; if (loop_count > DIR_LOOP_THRESHOLD) { - if (dir_bucket_loop_fix(dir_bucket(b, seg), s, this)) + if (this->bucket_loop_fix(dir_bucket(b, seg), s)) return 0; } #endif @@ -743,7 +694,7 @@ Directory::remove(const CacheKey *key, StripeSM *stripe, Dir *del) ts::Metrics::Gauge::decrement(stripe->cache_vol->vol_rsb.direntries_used); ATS_PROBE7(cache_dir_remove, stripe->fd, s, dir_to_offset(e, seg), offset, dir_approx_size(e), key->slice64(0), key->slice64(1)); - dir_delete_entry(e, p, s, this); + this->delete_entry(e, p, s); CHECK_DIR(d); return 1; } @@ -939,7 +890,7 @@ Directory::entries_used() sfull = 0; for (int b = 0; b < this->buckets; b++) { Dir *e = dir_bucket(b, seg); - if (dir_bucket_loop_fix(e, s, this)) { + if (this->bucket_loop_fix(e, s)) { sfull = 0; break; } @@ -1068,7 +1019,7 @@ CacheSync::mainEvent(int event, Event * /* e ATS_UNUSED */) /* Don't sync the directory to disk if its not dirty. Syncing the clean directory to disk is also the cause of INKqa07151. Increasing the serial number causes the cache to recover more data than necessary. - The dirty bit it set in dir_insert, overwrite and dir_delete_entry + The dirty bit is set in dir_insert, overwrite and Directory::delete_entry */ if (!stripe->directory.header->dirty) { Dbg(dbg_ctl_cache_dir_sync, "Dir %s not dirty", stripe->hash_text.get()); diff --git a/src/iocore/cache/CacheVC.cc b/src/iocore/cache/CacheVC.cc index 8dc154a8872..83bc9ea8d6a 100644 --- a/src/iocore/cache/CacheVC.cc +++ b/src/iocore/cache/CacheVC.cc @@ -111,9 +111,6 @@ next_in_map(Stripe *stripe, char *vol_map, off_t offset) return new_off + start_offset; } -// Function in CacheDir.cc that we need for make_vol_map(). -int dir_bucket_loop_fix(Dir *start_dir, int s, Directory *directory); - // TODO: If we used a bit vector, we could make a smaller map structure. // TODO: If we saved a high water mark we could have a smaller buf, and avoid searching it // when we are asked about the highest interesting offset. @@ -137,7 +134,7 @@ make_vol_map(Stripe *stripe) Dir *seg = stripe->directory.get_segment(s); for (int b = 0; b < stripe->directory.buckets; b++) { Dir *e = dir_bucket(b, seg); - if (dir_bucket_loop_fix(e, s, &stripe->directory)) { + if (stripe->directory.bucket_loop_fix(e, s)) { break; } while (e) { diff --git a/src/iocore/cache/P_CacheDir.h b/src/iocore/cache/P_CacheDir.h index fbd2f0b25bc..0a9150f4832 100644 --- a/src/iocore/cache/P_CacheDir.h +++ b/src/iocore/cache/P_CacheDir.h @@ -30,6 +30,8 @@ #include "tscore/Version.h" #include "tscore/hugepages.h" +#include + #include #include @@ -286,7 +288,9 @@ struct StripeHeaderFooter { uint16_t freelist[1]; }; -struct Directory { +class Directory +{ +public: char *raw_dir{nullptr}; Dir *dir{}; StripeHeaderFooter *header{}; @@ -316,19 +320,13 @@ struct Directory { int bucket_length(Dir *b, int s); int freelist_length(int s); void clean_segment(int s, StripeSM *stripe); -}; - -inline int -Directory::entries() const -{ - return this->buckets * DIR_DEPTH * this->segments; -} + void init_segment(int s); + int bucket_loop_fix(Dir *start_dir, int s); + Dir *delete_entry(Dir *e, Dir *p, int s); -inline Dir * -Directory::get_segment(int s) const -{ - return reinterpret_cast((reinterpret_cast(this->dir)) + (s * this->buckets) * DIR_DEPTH * SIZEOF_DIR); -} +private: + void unlink_from_freelist(Dir *e, int s); +}; // Global Functions @@ -394,3 +392,64 @@ dir_bucket_row(Dir *b, int64_t i) { return dir_in_seg(b, i); } + +inline int +Directory::entries() const +{ + return this->buckets * DIR_DEPTH * this->segments; +} + +inline Dir * +Directory::get_segment(int s) const +{ + return reinterpret_cast((reinterpret_cast(this->dir)) + (s * this->buckets) * DIR_DEPTH * SIZEOF_DIR); +} + +inline void +Directory::unlink_from_freelist(Dir *e, int s) +{ + Dir *seg = this->get_segment(s); + Dir *p = dir_from_offset(dir_prev(e), seg); + if (p) { + dir_set_next(p, dir_next(e)); + } else { + this->header->freelist[s] = dir_next(e); + } + Dir *n = dir_from_offset(dir_next(e), seg); + if (n) { + dir_set_prev(n, dir_prev(e)); + } +} + +inline Dir * +Directory::delete_entry(Dir *e, Dir *p, int s) +{ + Dir *seg = this->get_segment(s); + int no = dir_next(e); + this->header->dirty = 1; + if (p) { + unsigned int fo = this->header->freelist[s]; + unsigned int eo = dir_to_offset(e, seg); + dir_clear(e); + dir_set_next(p, no); + dir_set_next(e, fo); + if (fo) { + dir_set_prev(dir_from_offset(fo, seg), eo); + } + this->header->freelist[s] = eo; + } else { + Dir *n = next_dir(e, seg); + if (n) { + // "Shuffle" here means that we're copying the second entry's data to the head entry's location, and removing the second entry + // - because the head entry can't be moved. + ATS_PROBE3(cache_dir_shuffle, s, dir_to_offset(e, seg), dir_to_offset(n, seg)); + dir_assign(e, n); + this->delete_entry(n, e, s); + return e; + } else { + dir_clear(e); + return nullptr; + } + } + return dir_from_offset(no, seg); +} From 8fea5042fcfb7bbe73e678273420d220a31a58e5 Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Tue, 9 Jun 2026 20:43:20 -0500 Subject: [PATCH 05/33] Fix compilation under `LOOP_CHECK_MODE` (#13250) In earlier work, `vol` was renamed to `stripe` throughout the cache. A few occurences of `vol` were missed because they are conditionally compiled. This patch renames `vol` to `stripe` within code switched by the `LOOP_CHECK_MODE` definition. (cherry picked from commit 77253da933af73970c16727bd08c99f126e2f88f) --- src/iocore/cache/CacheDir.cc | 2 +- src/iocore/cache/unit_tests/test_CacheDir.cc | 60 ++++++++++---------- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/iocore/cache/CacheDir.cc b/src/iocore/cache/CacheDir.cc index 7997c501086..99e9fba47b4 100644 --- a/src/iocore/cache/CacheDir.cc +++ b/src/iocore/cache/CacheDir.cc @@ -308,7 +308,7 @@ dir_clean_bucket(Dir *b, int s, StripeSM *stripe) #ifdef LOOP_CHECK_MODE loop_count++; if (loop_count > DIR_LOOP_THRESHOLD) { - if (vol->directory.bucket_loop_fix(b, s)) + if (stripe->directory.bucket_loop_fix(b, s)) return; } #endif diff --git a/src/iocore/cache/unit_tests/test_CacheDir.cc b/src/iocore/cache/unit_tests/test_CacheDir.cc index 5fabd608bbe..d8f3ba38195 100644 --- a/src/iocore/cache/unit_tests/test_CacheDir.cc +++ b/src/iocore/cache/unit_tests/test_CacheDir.cc @@ -169,58 +169,58 @@ class CacheDirTest : public CacheInit #ifdef LOOP_CHECK_MODE // probe in bucket with loop rand_CacheKey(&key); - s1 = key.slice32(0) % vol->segments; - b1 = key.slice32(1) % vol->buckets; - dir_corrupt_bucket(dir_bucket(b1, vol->directory.get_segment(s1)), s1, vol); - stripe->directory.insert(&key, vol, &dir); + s1 = key.slice32(0) % stripe->directory.segments; + b1 = key.slice32(1) % stripe->directory.buckets; + dir_corrupt_bucket(dir_bucket(b1, stripe->directory.get_segment(s1)), s1, stripe); + stripe->directory.insert(&key, stripe, &dir); Dir *last_collision = 0; - vol->directory.probe(&key, vol, &dir, &last_collision); + stripe->directory.probe(&key, stripe, &dir, &last_collision); rand_CacheKey(&key); - s1 = key.slice32(0) % vol->segments; - b1 = key.slice32(1) % vol->buckets; - dir_corrupt_bucket(dir_bucket(b1, vol->directory.get_segment(s1)), s1, vol); + s1 = key.slice32(0) % stripe->directory.segments; + b1 = key.slice32(1) % stripe->directory.buckets; + dir_corrupt_bucket(dir_bucket(b1, stripe->directory.get_segment(s1)), s1, stripe); last_collision = 0; - vol->directory.probe(&key, vol, &dir, &last_collision); + stripe->directory.probe(&key, stripe, &dir, &last_collision); // overwrite in bucket with loop rand_CacheKey(&key); - s1 = key.slice32(0) % vol->segments; - b1 = key.slice32(1) % vol->buckets; + s1 = key.slice32(0) % stripe->directory.segments; + b1 = key.slice32(1) % stripe->directory.buckets; CacheKey key1; key1.b[1] = 127; dir1 = dir; dir_set_offset(&dir1, 23); - stripe->directory.insert(&key1, vol, &dir1); - stripe->directory.insert(&key, vol, &dir); + stripe->directory.insert(&key1, stripe, &dir1); + stripe->directory.insert(&key, stripe, &dir); key1.b[1] = 80; - stripe->directory.insert(&key1, vol, &dir1); - dir_corrupt_bucket(dir_bucket(b1, vol->directory.get_segment(s1)), s1, vol); - vol->directory.overwrite(&key, vol, &dir, &dir, 1); + stripe->directory.insert(&key1, stripe, &dir1); + dir_corrupt_bucket(dir_bucket(b1, stripe->directory.get_segment(s1)), s1, stripe); + stripe->directory.overwrite(&key, stripe, &dir, &dir, 1); rand_CacheKey(&key); - s1 = key.slice32(0) % vol->segments; - b1 = key.slice32(1) % vol->buckets; + s1 = key.slice32(0) % stripe->directory.segments; + b1 = key.slice32(1) % stripe->directory.buckets; key.b[1] = 23; - stripe->directory.insert(&key, vol, &dir1); - dir_corrupt_bucket(dir_bucket(b1, vol->directory.get_segment(s1)), s1, vol); - vol->directory.overwrite(&key, vol, &dir, &dir, 0); + stripe->directory.insert(&key, stripe, &dir1); + dir_corrupt_bucket(dir_bucket(b1, stripe->directory.get_segment(s1)), s1, stripe); + stripe->directory.overwrite(&key, stripe, &dir, &dir, 0); rand_CacheKey(&key); - s1 = key.slice32(0) % vol->segments; - Dir *seg1 = vol->directory.get_segment(s1); + s1 = key.slice32(0) % stripe->directory.segments; + Dir *seg1 = stripe->directory.get_segment(s1); // freelist_length in freelist with loop - dir_corrupt_bucket(dir_from_offset(vol->header->freelist[s], seg1), s1, vol); - vol->directory.freelist_length(s1); + dir_corrupt_bucket(dir_from_offset(stripe->directory.header->freelist[s], seg1), s1, stripe); + stripe->directory.freelist_length(s1); rand_CacheKey(&key); - s1 = key.slice32(0) % vol->segments; - b1 = key.slice32(1) % vol->buckets; + s1 = key.slice32(0) % stripe->directory.segments; + b1 = key.slice32(1) % stripe->directory.buckets; // bucket_length in bucket with loop - dir_corrupt_bucket(dir_bucket(b1, vol->directory.get_segment(s1)), s1, vol); - vol->directory.bucket_length(dir_bucket(b1, vol->directory.get_segment(s1)), s1, vol); - CHECK(vol->directory.check()); + dir_corrupt_bucket(dir_bucket(b1, stripe->directory.get_segment(s1)), s1, stripe); + stripe->directory.bucket_length(dir_bucket(b1, stripe->directory.get_segment(s1)), s1); + CHECK(stripe->directory.check()); #else // test corruption detection rand_CacheKey(&key); From 7b5db7927814baf7fe408a170b63b23c538629da Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Thu, 11 Jun 2026 14:32:15 -0600 Subject: [PATCH 06/33] authproxy: Release client request handle in StateAuthorized (#13258) TSHttpTxnClientReqGet was not paired with TSHandleMLocRelease, unlike every other call site in this file. The handle is a top-level HTTP_HEADER mloc so the release is effectively a no-op today, but matching the documented API contract avoids surprises if the SDK implementation ever changes. (cherry picked from commit e79182f7e22423fca5e60b9903c3a9a104b5ac13) --- plugins/authproxy/authproxy.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/authproxy/authproxy.cc b/plugins/authproxy/authproxy.cc index 252d7168597..3d697241a48 100644 --- a/plugins/authproxy/authproxy.cc +++ b/plugins/authproxy/authproxy.cc @@ -686,6 +686,8 @@ StateAuthorized(AuthRequestContext *auth, void *) TSHandleMLocRelease(auth->rheader.buffer, auth->rheader.header, field_loc); field_loc = next_field_loc; } + + TSHandleMLocRelease(request_bufp, TS_NULL_MLOC, request_hdr); } // Proceed with the modified request From 62085459d93ca3443c6056ca3ea4c8d05667e6e1 Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Mon, 15 Jun 2026 18:10:56 -0500 Subject: [PATCH 07/33] Remove unused EThread members (#13268) This removes the following members: - EThread::diskHandler - EThread::aio_ops (cherry picked from commit a2e02e144912302811338d67abcc1ead224795a7) --- include/iocore/eventsystem/EThread.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/include/iocore/eventsystem/EThread.h b/include/iocore/eventsystem/EThread.h index 99569b9fffc..ff3b0e0933f 100644 --- a/include/iocore/eventsystem/EThread.h +++ b/include/iocore/eventsystem/EThread.h @@ -47,7 +47,6 @@ using hwloc_obj_t = hwloc_obj *; // instead. #define MUTEX_RETRY_DELAY HRTIME_MSECONDS(20) -class DiskHandler; struct EventIO; class ServerSessionPool; @@ -328,12 +327,6 @@ class EThread : public Thread /** Block of memory to allocate thread specific data e.g. stat system arrays. */ char thread_private[PER_THREAD_DATA]; - /** Private Data for the Disk Processor. */ - DiskHandler *diskHandler = nullptr; - - /** Private Data for AIO. */ - Que(Continuation, link) aio_ops; - ProtectedQueue EventQueueExternal; PriorityEventQueue EventQueue; From bdbea812e2c6f9be16c0b31816762c4af227dd96 Mon Sep 17 00:00:00 2001 From: Robert Clendenin Date: Tue, 16 Jun 2026 10:15:35 -0500 Subject: [PATCH 08/33] Fix tsapi build with ENABLE_PROBES=ON (#13276) Cache headers transitively pull in via P_CacheDir.h. When ENABLE_PROBES=ON, ENABLE_SYSTEMTAP_PROBES is defined and ats_probe.h `#include `. tsapi's include path didn't have lib/systemtap, so the build failed. Mirror what tscore already does (src/tscore/CMakeLists.txt:110), gated on ENABLE_PROBES so the dependency only attaches when probes are actually enabled. (cherry picked from commit 52a124bcd543f6889ba26af582a9d9927b7f7e6e) --- src/api/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/api/CMakeLists.txt b/src/api/CMakeLists.txt index 642f868d48d..ca3ea344862 100644 --- a/src/api/CMakeLists.txt +++ b/src/api/CMakeLists.txt @@ -28,6 +28,9 @@ set(TSAPI_PUBLIC_HEADERS ${PROJECT_SOURCE_DIR}/include/ts/ts.h ${PROJECT_SOURCE_ # OpenSSL needs to be listed in before other libraries that can be found in the system default lib directory (See #11511) target_link_libraries(tsapi PRIVATE libswoc::libswoc yaml-cpp::yaml-cpp OpenSSL::SSL) +if(ENABLE_PROBES) + target_link_libraries(tsapi PRIVATE systemtap::systemtap) +endif() set_target_properties(tsapi PROPERTIES PUBLIC_HEADER "${TSAPI_PUBLIC_HEADERS}") # Items common between api and other ts libraries From c897a6fd905d5f18b438c2bca96ca3197c94a69f Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Tue, 16 Jun 2026 12:30:22 -0500 Subject: [PATCH 09/33] Remove TsBuffer.h (#13281) Fixes #1570. This removes the unused `ConstBuffer` class. The header was not included anywhere. The only mention of `ConstBuffer` in the docs is in the history of `TextView`. Therefore, that mention has been preserved. (cherry picked from commit 1a10d127a7dee3eb73abaaec3322ab871c9c54af) --- include/tscore/TsBuffer.h | 508 -------------------------------------- 1 file changed, 508 deletions(-) delete mode 100644 include/tscore/TsBuffer.h diff --git a/include/tscore/TsBuffer.h b/include/tscore/TsBuffer.h deleted file mode 100644 index 857437eb511..00000000000 --- a/include/tscore/TsBuffer.h +++ /dev/null @@ -1,508 +0,0 @@ -/** @file - Definitions for a buffer type, to carry a reference to a chunk of memory. - - @section license License - - Licensed to the Apache Software Foundation (ASF) under one - or more contributor license agreements. See the NOTICE file - distributed with this work for additional information - regarding copyright ownership. The ASF licenses this file - to you under the Apache License, Version 2.0 (the - "License"); you may not use this file except in compliance - with the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - */ - -#pragma once - -#if defined _MSC_VER -#include -#else -#include -#endif - -// For memcmp() -#include -#include - -/// Apache Traffic Server commons. -namespace ts -{ -struct ConstBuffer; -/** A chunk of writable memory. - A convenience class because we pass this kind of pair frequently. - - @note The default construct leaves the object - uninitialized. This is for performance reasons. To construct an - empty @c Buffer use @c Buffer(0). - */ -struct Buffer { - using self = Buffer; ///< Self reference type. - using pseudo_bool = bool (self::*)() const; - - char *_ptr = nullptr; ///< Pointer to base of memory chunk. - size_t _size = 0; ///< Size of memory chunk. - - /// Default constructor (empty buffer). - Buffer(); - - /** Construct from pointer and size. - @note Due to ambiguity issues do not call this with - two arguments if the first argument is 0. - */ - Buffer(char *ptr, ///< Pointer to buffer. - size_t n ///< Size of buffer. - ); - /** Construct from two pointers. - @note This presumes a half open range, (start, end] - */ - Buffer(char *start, ///< First valid character. - char *end ///< First invalid character. - ); - - /** Equality. - @return @c true if @a that refers to the same memory as @a this, - @c false otherwise. - */ - bool operator==(self const &that) const; - /** Inequality. - @return @c true if @a that does not refer to the same memory as @a this, - @c false otherwise. - */ - bool operator!=(self const &that) const; - /** Equality for a constant buffer. - @return @c true if @a that refers to the same memory as @a this. - @c false otherwise. - */ - bool operator==(ConstBuffer const &that) const; - /** Inequality. - @return @c true if @a that does not refer to the same memory as @a this, - @c false otherwise. - */ - bool operator!=(ConstBuffer const &that) const; - - /// @return The first character in the buffer. - char operator*() const; - /** Discard the first character in the buffer. - @return @a this object. - */ - self &operator++(); - - /// Check for empty buffer. - /// @return @c true if the buffer has a zero pointer @b or size. - bool operator!() const; - /// Check for non-empty buffer. - /// @return @c true if the buffer has a non-zero pointer @b and size. - operator pseudo_bool() const; - - /// @name Accessors. - //@{ - /// Get the data in the buffer. - char *data() const; - /// Get the size of the buffer. - size_t size() const; - //@} - - /// Set the chunk. - /// Any previous values are discarded. - /// @return @c this object. - self &set(char *ptr, ///< Buffer address. - size_t n = 0 ///< Buffer size. - ); - /// Reset to empty. - self &reset(); -}; - -/** A chunk of read only memory. - A convenience class because we pass this kind of pair frequently. - */ -struct ConstBuffer { - using self = ConstBuffer; ///< Self reference type. - using pseudo_bool = bool (self::*)() const; - - char const *_ptr = nullptr; ///< Pointer to base of memory chunk. - size_t _size = 0; ///< Size of memory chunk. - - /// Default constructor (empty buffer). - ConstBuffer(); - - /** Construct from pointer and size. - */ - ConstBuffer(char const *ptr, ///< Pointer to buffer. - size_t n ///< Size of buffer. - ); - /** Construct from two pointers. - @note This presumes a half open range (start, end] - @note Due to ambiguity issues do not invoke this with - @a start == 0. - */ - ConstBuffer(char const *start, ///< First valid character. - char const *end ///< First invalid character. - ); - /// Construct from writable buffer. - ConstBuffer(Buffer const &buffer ///< Buffer to copy. - ); - - /** Equality. - @return @c true if @a that refers to the same memory as @a this, - @c false otherwise. - */ - bool operator==(self const &that) const; - /** Equality. - @return @c true if @a that refers to the same memory as @a this, - @c false otherwise. - */ - bool operator==(Buffer const &that) const; - /** Inequality. - @return @c true if @a that does not refer to the same memory as @a this, - @c false otherwise. - */ - bool operator!=(self const &that) const; - /** Inequality. - @return @c true if @a that does not refer to the same memory as @a this, - @c false otherwise. - */ - bool operator!=(Buffer const &that) const; - /// Assign from non-const Buffer. - self &operator=(Buffer const &that ///< Source buffer. - ); - - /// @return The first character in the buffer. - char operator*() const; - /** Discard the first character in the buffer. - @return @a this object. - */ - self &operator++(); - /** Discard the first @a n characters. - @return @a this object. - */ - self &operator+=(size_t n); - - /// Check for empty buffer. - /// @return @c true if the buffer has a zero pointer @b or size. - bool operator!() const; - /// Check for non-empty buffer. - /// @return @c true if the buffer has a non-zero pointer @b and size. - operator pseudo_bool() const; - - operator std::string_view() const { return {_ptr, _size}; } - - /// @name Accessors. - //@{ - /// Get the data in the buffer. - char const *data() const; - /// Get the size of the buffer. - size_t size() const; - /// Access a character (no bounds check). - char operator[](int n) const; - //@} - /// @return @c true if @a p points at a character in @a this. - bool contains(char const *p) const; - - /// Set the chunk. - /// Any previous values are discarded. - /// @return @c this object. - self &set(char const *ptr, ///< Buffer address. - size_t n = 0 ///< Buffer size. - ); - /** Set from 2 pointers. - @note This presumes a half open range (start, end] - */ - self &set(char const *start, ///< First valid character. - char const *end ///< First invalid character. - ); - /// Reset to empty. - self &reset(); - - /** Find a character. - @return A pointer to the first occurrence of @a c in @a this - or @c nullptr if @a c is not found. - */ - char const *find(char c) const; - - /** Split the buffer on the character at @a p. - - The buffer is split in to two parts and the character at @a p - is discarded. @a this retains all data @b after @a p. The - initial part of the buffer is returned. Neither buffer will - contain the character at @a p. - - This is convenient when tokenizing and @a p points at the token - separator. - - @note If @a *p is not in the buffer then @a this is not changed - and an empty buffer is returned. This means the caller can - simply pass the result of @c find and check for an empty - buffer returned to detect no more separators. - - @return A buffer containing data up to but not including @a p. - */ - self splitOn(char const *p); - - /** Split the buffer on the character @a c. - - The buffer is split in to two parts and the occurrence of @a c - is discarded. @a this retains all data @b after @a c. The - initial part of the buffer is returned. Neither buffer will - contain the first occurrence of @a c. - - This is convenient when tokenizing and @a c is the token - separator. - - @note If @a c is not found then @a this is not changed and an - empty buffer is returned. - - @return A buffer containing data up to but not including @a p. - */ - self splitOn(char c); - /** Get a trailing segment of the buffer. - - @return A buffer that contains all data after @a p. - */ - self after(char const *p) const; - /** Get a trailing segment of the buffer. - - @return A buffer that contains all data after the first - occurrence of @a c. - */ - self after(char c) const; - /** Remove trailing segment. - - Data at @a p and beyond is removed from the buffer. - If @a p is not in the buffer, no change is made. - - @return @a this. - */ - self &clip(char const *p); -}; - -// ---------------------------------------------------------- -// Inline implementations. - -inline Buffer::Buffer() {} -inline Buffer::Buffer(char *ptr, size_t n) : _ptr(ptr), _size(n) {} -inline Buffer & -Buffer::set(char *ptr, size_t n) -{ - _ptr = ptr; - _size = n; - return *this; -} -inline Buffer::Buffer(char *start, char *end) : _ptr(start), _size(end - start) {} -inline Buffer & -Buffer::reset() -{ - _ptr = nullptr; - _size = 0; - return *this; -} -inline bool -Buffer::operator!=(self const &that) const -{ - return !(*this == that); -} -inline bool -Buffer::operator!=(ConstBuffer const &that) const -{ - return !(*this == that); -} -inline bool -Buffer::operator==(self const &that) const -{ - return _size == that._size && _ptr == that._ptr; -} -inline bool -Buffer::operator==(ConstBuffer const &that) const -{ - return _size == that._size && _ptr == that._ptr; -} -inline bool -Buffer::operator!() const -{ - return !(_ptr && _size); -} -inline Buffer::operator pseudo_bool() const -{ - return _ptr && _size ? &self::operator! : nullptr; -} -inline char -Buffer::operator*() const -{ - return *_ptr; -} -inline Buffer & -Buffer::operator++() -{ - ++_ptr; - --_size; - return *this; -} -inline char * -Buffer::data() const -{ - return _ptr; -} -inline size_t -Buffer::size() const -{ - return _size; -} - -inline ConstBuffer::ConstBuffer() {} -inline ConstBuffer::ConstBuffer(char const *ptr, size_t n) : _ptr(ptr), _size(n) {} -inline ConstBuffer::ConstBuffer(char const *start, char const *end) : _ptr(start), _size(end - start) {} -inline ConstBuffer::ConstBuffer(Buffer const &that) : _ptr(that._ptr), _size(that._size) {} -inline ConstBuffer & -ConstBuffer::set(char const *ptr, size_t n) -{ - _ptr = ptr; - _size = n; - return *this; -} - -inline ConstBuffer & -ConstBuffer::set(char const *start, char const *end) -{ - _ptr = start; - _size = end - start; - return *this; -} - -inline ConstBuffer & -ConstBuffer::reset() -{ - _ptr = nullptr; - _size = 0; - return *this; -} -inline bool -ConstBuffer::operator!=(self const &that) const -{ - return !(*this == that); -} -inline bool -ConstBuffer::operator!=(Buffer const &that) const -{ - return !(*this == that); -} -inline bool -ConstBuffer::operator==(self const &that) const -{ - return _size == that._size && 0 == memcmp(_ptr, that._ptr, _size); -} -inline ConstBuffer & -ConstBuffer::operator=(Buffer const &that) -{ - _ptr = that._ptr; - _size = that._size; - return *this; -} -inline bool -ConstBuffer::operator==(Buffer const &that) const -{ - return _size == that._size && 0 == memcmp(_ptr, that._ptr, _size); -} -inline bool -ConstBuffer::operator!() const -{ - return !(_ptr && _size); -} -inline ConstBuffer::operator pseudo_bool() const -{ - return _ptr && _size ? &self::operator! : nullptr; -} -inline char -ConstBuffer::operator*() const -{ - return *_ptr; -} -inline ConstBuffer & -ConstBuffer::operator++() -{ - ++_ptr; - --_size; - return *this; -} -inline ConstBuffer & -ConstBuffer::operator+=(size_t n) -{ - _ptr += n; - _size -= n; - return *this; -} -inline char const * -ConstBuffer::data() const -{ - return _ptr; -} -inline char -ConstBuffer::operator[](int n) const -{ - return _ptr[n]; -} -inline size_t -ConstBuffer::size() const -{ - return _size; -} -inline bool -ConstBuffer::contains(char const *p) const -{ - return _ptr <= p && p < _ptr + _size; -} - -inline ConstBuffer -ConstBuffer::splitOn(char const *p) -{ - self zret; // default to empty return. - if (this->contains(p)) { - size_t n = p - _ptr; - zret.set(_ptr, n); - _ptr = p + 1; - _size -= n + 1; - } - return zret; -} - -inline char const * -ConstBuffer::find(char c) const -{ - return static_cast(memchr(_ptr, c, _size)); -} - -inline ConstBuffer -ConstBuffer::splitOn(char c) -{ - return this->splitOn(this->find(c)); -} - -inline ConstBuffer -ConstBuffer::after(char const *p) const -{ - return this->contains(p) ? self(p + 1, (_size - (p - _ptr)) - 1) : self(); -} -inline ConstBuffer -ConstBuffer::after(char c) const -{ - return this->after(this->find(c)); -} -inline ConstBuffer & -ConstBuffer::clip(char const *p) -{ - if (this->contains(p)) { - _size = p - _ptr; - } - return *this; -} - -} // namespace ts - -using TsBuffer = ts::Buffer; -using TsConstBuffer = ts::ConstBuffer; From 5e15f087abeda55ca3b9485333ca4dd26f39f716 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Thu, 18 Jun 2026 10:41:21 -0500 Subject: [PATCH 10/33] Fix txn_box unused find result (#13291) Clang 22 treats the ignored std::unordered_map::find result in the txn_box unit perf test as a warning, and the project builds with warnings promoted to errors. The test therefore fails to compile even though the lookup is intentionally only being timed. This explicitly discards the lookup result in the benchmark lambda and corrects the printed label to describe the lookup operation. (cherry picked from commit 309d832ddf1874bbe7b672b274b3a5c89328621c) --- plugins/experimental/txn_box/unit_tests/test_accl_utils.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/experimental/txn_box/unit_tests/test_accl_utils.cc b/plugins/experimental/txn_box/unit_tests/test_accl_utils.cc index 1359a1c0f1e..ccc998e870b 100644 --- a/plugins/experimental/txn_box/unit_tests/test_accl_utils.cc +++ b/plugins/experimental/txn_box/unit_tests/test_accl_utils.cc @@ -337,8 +337,9 @@ TEST_CASE("Very basic perf test") } { func_timer<> f; - auto const &took = f.run([&map]() { map.find("ASF.com"); }); - std::cout << "std::unordered_map - insert(\"ASF.com\") took " << took << to_string::unit>::value << std::endl; + // Only time the lookup dispatch; the returned iterator is intentionally unused. + auto const &took = f.run([&map]() { static_cast(map.find("ASF.com")); }); + std::cout << "std::unordered_map - find(\"ASF.com\") took " << took << to_string::unit>::value << std::endl; } } } From 0beee2c46f6449602fe0393a7bb1d4050b1e193f Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Mon, 22 Jun 2026 12:54:38 -0500 Subject: [PATCH 11/33] Add unit tests for `Continuation` logic (#13283) * Add unit tests for `Continuation` logic This change was generated by Claude Opus 4.7 from API contracts I have not yet opened PRs for. It does not touch existing files other than one CMakeLists.txt, so it is low-risk. I reviewed each test case and they are good. I have not reviewed code semantics carefully for correctness. The default delay for the `CountingContinuation` wait is five seconds; I think a wait with a timeout is an acceptable way to test this, and five seconds should be plenty. If an event scheduled to run immediately doesn't call back the associated continuation within five seconds, that's probably unacceptable performance and worth failing the test for. :D (cherry picked from commit a344dec29c5463f5b6d3f4dba5c61947b50fd94c) --- src/iocore/eventsystem/CMakeLists.txt | 4 + .../unit_tests/inkevent_test_fixtures.h | 159 +++++++++++ .../unit_tests/test_Continuation.cc | 260 ++++++++++++++++++ 3 files changed, 423 insertions(+) create mode 100644 src/iocore/eventsystem/unit_tests/inkevent_test_fixtures.h create mode 100644 src/iocore/eventsystem/unit_tests/test_Continuation.cc diff --git a/src/iocore/eventsystem/CMakeLists.txt b/src/iocore/eventsystem/CMakeLists.txt index e3130eb7452..4d4a8c50c52 100644 --- a/src/iocore/eventsystem/CMakeLists.txt +++ b/src/iocore/eventsystem/CMakeLists.txt @@ -64,6 +64,10 @@ if(BUILD_TESTING) add_catch2_test(NAME test_IOBuffer COMMAND test_IOBuffer) add_catch2_test(NAME test_MIOBufferWriter COMMAND test_MIOBufferWriter) + add_executable(test_Continuation unit_tests/test_Continuation.cc) + target_link_libraries(test_Continuation ts::inkevent configmanager Catch2::Catch2WithMain) + add_catch2_test(NAME test_Continuation COMMAND test_Continuation) + endif() clang_tidy_check(inkevent) diff --git a/src/iocore/eventsystem/unit_tests/inkevent_test_fixtures.h b/src/iocore/eventsystem/unit_tests/inkevent_test_fixtures.h new file mode 100644 index 00000000000..e5b7c431f08 --- /dev/null +++ b/src/iocore/eventsystem/unit_tests/inkevent_test_fixtures.h @@ -0,0 +1,159 @@ +/** @file + + Shared test fixtures for inkevent Catch2 unit tests. + + This header provides EventSystem.h. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#pragma once + +#include + +#include + +#include + +#include +#include +#include +#include + +#include +#include +#include + +namespace inkevent_test +{ + +inline constexpr int DEFAULT_TEST_THREADS = 2; +inline constexpr size_t DEFAULT_TEST_STACKSIZE = 1048576; +inline constexpr auto DEFAULT_TIMEOUT = std::chrono::seconds{5}; + +/** + Catch2 event listener that boots the inkevent eventProcessor once per + test executable. Mirrors the in-file listener used by test_EventSystem.cc + / test_IOBuffer.cc; lifted here so each new inkevent test file registers + the listener with a single CATCH_REGISTER_LISTENER call rather than + duplicating the boot logic. +*/ +struct EventProcessorListener : Catch::EventListenerBase { + using EventListenerBase::EventListenerBase; + + void + testRunStarting(Catch::TestRunInfo const & /* testRunInfo ATS_UNUSED */) override + { + Layout::create(); + init_diags("", nullptr); + RecProcessInit(); + + ink_event_system_init(EVENT_SYSTEM_MODULE_PUBLIC_VERSION); + eventProcessor.start(DEFAULT_TEST_THREADS, DEFAULT_TEST_STACKSIZE); + + EThread *main_thread = new EThread; + main_thread->set_specific(); + } +}; + +/** + Atomic boolean flag with a bounded acquire-load wait. Used by + multi-threaded tests to observe a one-shot signal from a Continuation + handler without sleeping in the assertion path. +*/ +class AtomicFlag +{ +public: + void + set() + { + flag.store(true, std::memory_order_release); + } + + bool + is_set() const + { + return flag.load(std::memory_order_acquire); + } + + bool + wait_until_set(std::chrono::milliseconds timeout = std::chrono::duration_cast(DEFAULT_TIMEOUT)) + { + auto deadline = std::chrono::steady_clock::now() + timeout; + while (!flag.load(std::memory_order_acquire)) { + if (std::chrono::steady_clock::now() >= deadline) { + return false; + } + std::this_thread::yield(); + } + return true; + } + +private: + std::atomic flag{false}; +}; + +/** + Continuation whose handler counts every dispatch into a public atomic + counter. The first consumer is the Continuation tests in + test_Continuation.cc; later inkevent tests reuse it whenever they need + to observe handler invocations as an externally-visible side effect. + + Usage: + CountingContinuation cont{new_ProxyMutex()}; + eventProcessor.schedule_imm(&cont, ET_CALL); + REQUIRE(cont.wait_until_at_least(1)); +*/ +class CountingContinuation : public Continuation +{ +public: + explicit CountingContinuation(ProxyMutex *amutex) : Continuation(amutex) { SET_HANDLER(&CountingContinuation::handle_event); } + + int + count() const + { + return counter.load(std::memory_order_acquire); + } + + bool + wait_until_at_least(int n, + std::chrono::milliseconds timeout = std::chrono::duration_cast(DEFAULT_TIMEOUT)) + { + auto deadline = std::chrono::steady_clock::now() + timeout; + while (counter.load(std::memory_order_acquire) < n) { + if (std::chrono::steady_clock::now() >= deadline) { + return false; + } + std::this_thread::yield(); + } + return true; + } + +private: + int + handle_event(int /* event ATS_UNUSED */, void * /* data ATS_UNUSED */) + { + counter.fetch_add(1, std::memory_order_release); + return 0; + } + + std::atomic counter{0}; +}; + +} // namespace inkevent_test diff --git a/src/iocore/eventsystem/unit_tests/test_Continuation.cc b/src/iocore/eventsystem/unit_tests/test_Continuation.cc new file mode 100644 index 00000000000..0cd2f3bba51 --- /dev/null +++ b/src/iocore/eventsystem/unit_tests/test_Continuation.cc @@ -0,0 +1,260 @@ +/** @file + + Catch2 unit tests for the Continuation boundary contract. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include + +#include "inkevent_test_fixtures.h" + +using inkevent_test::AtomicFlag; +using inkevent_test::CountingContinuation; +using inkevent_test::EventProcessorListener; + +CATCH_REGISTER_LISTENER(EventProcessorListener) + +namespace +{ + +class FlagContinuation : public Continuation +{ +public: + explicit FlagContinuation(ProxyMutex *m) : Continuation(m) { SET_HANDLER(&FlagContinuation::on_event); } + + AtomicFlag flag; + int last_event = -1; + void *last_data = nullptr; + +private: + int + on_event(int event, void *data) + { + last_event = event; + last_data = data; + flag.set(); + return 0; + } +}; + +class TwoHandlerContinuation : public Continuation +{ +public: + explicit TwoHandlerContinuation(ProxyMutex *m) : Continuation(m) { SET_HANDLER(&TwoHandlerContinuation::first); } + + std::atomic first_calls{0}; + std::atomic second_calls{0}; + + int + first(int /* event */, void * /* data */) + { + first_calls.fetch_add(1, std::memory_order_release); + SET_HANDLER(&TwoHandlerContinuation::second); + return 0; + } + + int + second(int /* event */, void * /* data */) + { + second_calls.fetch_add(1, std::memory_order_release); + return 0; + } +}; + +} // namespace + +TEST_CASE("Continuation constructed with a raw ProxyMutex pointer retains the mutex and leaves all other fields in their " + "documented initial state", + "[inkevent][continuation]") +{ + Ptr mutex{new_ProxyMutex()}; + FlagContinuation cont{mutex.get()}; + + REQUIRE(cont.getMutex() == mutex.get()); + REQUIRE(cont.getThreadAffinity() == nullptr); + REQUIRE(cont.handler != nullptr); +} + +TEST_CASE("Continuation constructed with a null ProxyMutex pointer reports a null mutex via getMutex", "[inkevent][continuation]") +{ + FlagContinuation cont{nullptr}; + + REQUIRE(cont.getMutex() == nullptr); + REQUIRE(cont.getThreadAffinity() == nullptr); +} + +TEST_CASE("Continuation::getMutex returns the same raw pointer for repeated calls without changing the held reference count", + "[inkevent][continuation]") +{ + Ptr mutex{new_ProxyMutex()}; + int const initial_refcount = mutex->refcount(); + + { + FlagContinuation cont{mutex.get()}; + ProxyMutex *first = cont.getMutex(); + ProxyMutex *second = cont.getMutex(); + + REQUIRE(first == second); + REQUIRE(first == mutex.get()); + REQUIRE(mutex->refcount() == initial_refcount + 1); + } + + REQUIRE(mutex->refcount() == initial_refcount); +} + +TEST_CASE("Continuation::setThreadAffinity returns true and stores the EThread when the argument is non-null", + "[inkevent][continuation]") +{ + Ptr mutex{new_ProxyMutex()}; + FlagContinuation cont{mutex.get()}; + EThread *me = this_ethread(); + REQUIRE(me != nullptr); + + bool const accepted = cont.setThreadAffinity(me); + + REQUIRE(accepted); + REQUIRE(cont.getThreadAffinity() == me); +} + +TEST_CASE("Continuation::setThreadAffinity returns false and leaves the affinity unchanged when called with a null EThread pointer", + "[inkevent][continuation]") +{ + Ptr mutex{new_ProxyMutex()}; + FlagContinuation cont{mutex.get()}; + EThread *me = this_ethread(); + REQUIRE(cont.setThreadAffinity(me)); + + bool const accepted = cont.setThreadAffinity(nullptr); + + REQUIRE_FALSE(accepted); + REQUIRE(cont.getThreadAffinity() == me); +} + +TEST_CASE("Continuation::clearThreadAffinity restores the no-preference state observable through getThreadAffinity", + "[inkevent][continuation]") +{ + Ptr mutex{new_ProxyMutex()}; + FlagContinuation cont{mutex.get()}; + cont.setThreadAffinity(this_ethread()); + REQUIRE(cont.getThreadAffinity() != nullptr); + + cont.clearThreadAffinity(); + + REQUIRE(cont.getThreadAffinity() == nullptr); +} + +TEST_CASE("Continuation::handleEvent forwards the event code and data pointer to the installed handler", "[inkevent][continuation]") +{ + Ptr mutex{new_ProxyMutex()}; + FlagContinuation cont{mutex.get()}; + + int payload = 42; + int event = 7; + { + SCOPED_MUTEX_LOCK(lock, mutex, this_ethread()); + cont.handleEvent(event, &payload); + } + + REQUIRE(cont.flag.is_set()); + REQUIRE(cont.last_event == event); + REQUIRE(cont.last_data == static_cast(&payload)); +} + +TEST_CASE("Continuation::handleEvent uses CONTINUATION_EVENT_NONE and a null data pointer when called with no arguments", + "[inkevent][continuation]") +{ + Ptr mutex{new_ProxyMutex()}; + FlagContinuation cont{mutex.get()}; + + { + SCOPED_MUTEX_LOCK(lock, mutex, this_ethread()); + cont.handleEvent(); + } + + REQUIRE(cont.last_event == CONTINUATION_EVENT_NONE); + REQUIRE(cont.last_data == nullptr); +} + +TEST_CASE("SET_HANDLER replaces the active handler so the next handleEvent dispatch invokes the new method", + "[inkevent][continuation]") +{ + Ptr mutex{new_ProxyMutex()}; + TwoHandlerContinuation cont{mutex.get()}; + + { + SCOPED_MUTEX_LOCK(lock, mutex, this_ethread()); + cont.handleEvent(); + cont.handleEvent(); + } + + REQUIRE(cont.first_calls.load() == 1); + REQUIRE(cont.second_calls.load() == 1); +} + +TEST_CASE("SET_CONTINUATION_HANDLER installs a handler on a peer Continuation pointed to by the caller", "[inkevent][continuation]") +{ + Ptr mutex{new_ProxyMutex()}; + TwoHandlerContinuation cont{mutex.get()}; + REQUIRE(cont.handler != nullptr); + + TwoHandlerContinuation *peer = &cont; + SET_CONTINUATION_HANDLER(peer, &TwoHandlerContinuation::second); + + { + SCOPED_MUTEX_LOCK(lock, mutex, this_ethread()); + cont.handleEvent(); + } + + REQUIRE(cont.second_calls.load() == 1); + REQUIRE(cont.first_calls.load() == 0); +} + +TEST_CASE("Scheduling a CountingContinuation onto eventProcessor invokes its handler on a dispatching EThread", + "[inkevent][continuation][multithread]") +{ + CountingContinuation cont{new_ProxyMutex()}; + Event *e = eventProcessor.schedule_imm(&cont, ET_CALL); + REQUIRE(e != nullptr); + + REQUIRE(cont.wait_until_at_least(1)); + REQUIRE(cont.count() >= 1); +} + +TEST_CASE("Continuation::handleEvent returns the value the handler returns", "[inkevent][continuation]") +{ + Ptr mutex{new_ProxyMutex()}; + + struct Echo : public Continuation { + int echo_value; + explicit Echo(ProxyMutex *m, int v) : Continuation(m), echo_value(v) { SET_HANDLER(&Echo::handler_method); } + int + handler_method(int /* event */, void * /* data */) + { + return echo_value; + } + }; + + Echo a{mutex.get(), CONTINUATION_DONE}; + Echo b{mutex.get(), CONTINUATION_CONT}; + + SCOPED_MUTEX_LOCK(lock, mutex, this_ethread()); + REQUIRE(a.handleEvent(0, nullptr) == CONTINUATION_DONE); + REQUIRE(b.handleEvent(0, nullptr) == CONTINUATION_CONT); +} From 4454dc5d6ea05e1025f4a216c6a48c8ce9580277 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Tue, 23 Jun 2026 12:57:11 -0500 Subject: [PATCH 12/33] Add TSMutex lock guard (#13188) Plugin code that protects small critical sections with TSMutex has to pair every early return with a matching unlock. That pattern is easy to get wrong and makes the intended lock lifetime harder to see. This adds a small TSMutexLockGuard helper to the plugin API and uses it in plugin code where the mutex naturally stays locked until a return path. (cherry picked from commit 094c755d7d2c503187e569a5df0d047453573125) --- include/ts/ts.h | 20 +++++++++++ plugins/background_fetch/background_fetch.cc | 3 +- plugins/certifier/certifier.cc | 35 ++++++++----------- .../cache_fill/background_fetch.h | 3 +- .../experimental/rate_limit/ip_reputation.cc | 16 +++------ .../stale_response/stale_response.cc | 7 ++-- .../experimental/system_stats/system_stats.cc | 4 +-- plugins/experimental/wasm/ats_context.cc | 7 ++-- plugins/experimental/wasm/wasm_main.cc | 7 ++-- plugins/lua/ts_lua.cc | 5 +-- plugins/lua/ts_lua_fetch.cc | 3 +- plugins/lua/ts_lua_util.cc | 9 +---- plugins/prefetch/fetch.cc | 4 +-- plugins/prefetch/fetch.h | 3 +- 14 files changed, 56 insertions(+), 70 deletions(-) diff --git a/include/ts/ts.h b/include/ts/ts.h index ad266054cb5..5631bee0f66 100644 --- a/include/ts/ts.h +++ b/include/ts/ts.h @@ -1168,6 +1168,26 @@ TSReturnCode TSMutexLockTry(TSMutex mutexp); void TSMutexUnlock(TSMutex mutexp); +/** Scoped lock guard for a @c TSMutex. + + Locks @a mutexp on construction and unlocks it when the guard leaves scope. + */ +class [[nodiscard]] TSMutexLockGuard +{ +public: + explicit TSMutexLockGuard(TSMutex mutexp) : m_mutex(mutexp) { TSMutexLock(m_mutex); } + + TSMutexLockGuard(const TSMutexLockGuard &) = delete; + TSMutexLockGuard &operator=(const TSMutexLockGuard &) = delete; + TSMutexLockGuard(TSMutexLockGuard &&) = delete; + TSMutexLockGuard &operator=(TSMutexLockGuard &&) = delete; + + ~TSMutexLockGuard() { TSMutexUnlock(m_mutex); } + +private: + TSMutex m_mutex = nullptr; +}; + /* -------------------------------------------------------------------------- cachekey */ /** diff --git a/plugins/background_fetch/background_fetch.cc b/plugins/background_fetch/background_fetch.cc index 90ce73345ed..09979e1a58c 100644 --- a/plugins/background_fetch/background_fetch.cc +++ b/plugins/background_fetch/background_fetch.cc @@ -117,14 +117,13 @@ class BgFetchState { bool ret; - TSMutexLock(_lock); + TSMutexLockGuard lock(_lock); if (_urls.end() == _urls.find(url)) { ret = false; } else { _urls.erase(url); ret = true; } - TSMutexUnlock(_lock); return ret; } diff --git a/plugins/certifier/certifier.cc b/plugins/certifier/certifier.cc index 6dd3ec82259..2274e1f141c 100644 --- a/plugins/certifier/certifier.cc +++ b/plugins/certifier/certifier.cc @@ -132,12 +132,12 @@ class SslLRUList SSL_CTX * lookup_and_create(const char *servername, void *edata, bool &wontdo) { - SslData *ssl_data = nullptr; - scoped_SslData scoped_ssl_data = nullptr; - SSL_CTX *ref_ctx = nullptr; - std::string commonName(servername); - TSMutexLock(list_mutex); - auto dataItr = cnDataMap.find(commonName); + SslData *ssl_data = nullptr; + scoped_SslData scoped_ssl_data = nullptr; + SSL_CTX *ref_ctx = nullptr; + std::string commonName(servername); + TSMutexLockGuard lock(list_mutex); + auto dataItr = cnDataMap.find(commonName); /// If such a context exists in dict if (dataItr != cnDataMap.end()) { /// Reuse context if already built, self queued if not @@ -165,7 +165,6 @@ class SslLRUList ssl_data->scheduled = true; } } - TSMutexUnlock(list_mutex); return ref_ctx; } @@ -265,27 +264,24 @@ class SslLRUList SslData * get_newest() { - TSMutexLock(list_mutex); - SslData *ret = head; - TSMutexUnlock(list_mutex); + TSMutexLockGuard lock(list_mutex); + SslData *ret = head; return ret; } SslData * get_oldest() { - TSMutexLock(list_mutex); - SslData *ret = tail; - TSMutexUnlock(list_mutex); + TSMutexLockGuard lock(list_mutex); + SslData *ret = tail; return ret; } int get_size() { - TSMutexLock(list_mutex); - int ret = size; - TSMutexUnlock(list_mutex); + TSMutexLockGuard lock(list_mutex); + int ret = size; return ret; } @@ -293,14 +289,13 @@ class SslLRUList int set_schedule(const std::string &commonName, bool flag) { - int ret = -1; - TSMutexLock(list_mutex); - auto iter = cnDataMap.find(commonName); + int ret = -1; + TSMutexLockGuard lock(list_mutex); + auto iter = cnDataMap.find(commonName); if (iter != cnDataMap.end()) { iter->second->scheduled = flag; ret = 0; } - TSMutexUnlock(list_mutex); return ret; } }; diff --git a/plugins/experimental/cache_fill/background_fetch.h b/plugins/experimental/cache_fill/background_fetch.h index 011d5e1d37d..a2744f7ab2f 100644 --- a/plugins/experimental/cache_fill/background_fetch.h +++ b/plugins/experimental/cache_fill/background_fetch.h @@ -99,14 +99,13 @@ class BgFetchState { bool ret; - TSMutexLock(_lock); + TSMutexLockGuard lock(_lock); if (_urls.end() == _urls.find(url)) { ret = false; } else { _urls.erase(url); ret = true; } - TSMutexUnlock(_lock); return ret; } diff --git a/plugins/experimental/rate_limit/ip_reputation.cc b/plugins/experimental/rate_limit/ip_reputation.cc index 520b12d2745..1853861c496 100644 --- a/plugins/experimental/rate_limit/ip_reputation.cc +++ b/plugins/experimental/rate_limit/ip_reputation.cc @@ -144,7 +144,7 @@ SieveLru::parseYaml(const YAML::Node &node) std::tuple SieveLru::increment(KeyClass key) { - TSMutexLock(_lock); + TSMutexLockGuard lock(_lock); TSAssert(_initialized); auto map_it = _map.find(key); @@ -166,7 +166,6 @@ SieveLru::increment(KeyClass key) lru->push_front({key, 1, entryBucket(), SystemClock::now()}); } _map[key] = lru->begin(); - TSMutexUnlock(_lock); return {entryBucket(), 1}; } else { @@ -213,7 +212,6 @@ SieveLru::increment(KeyClass key) lru->moveTop(lru.get(), map_item); } } - TSMutexUnlock(_lock); return {bucket, count}; } @@ -223,21 +221,17 @@ SieveLru::increment(KeyClass key) std::tuple SieveLru::lookup(KeyClass key) const { - TSMutexLock(_lock); + TSMutexLockGuard lock(_lock); TSAssert(_initialized); auto map_it = _map.find(key); if (_map.end() == map_it) { - TSMutexUnlock(_lock); - return {0, entryBucket()}; // Nothing found, return 0 hits and the entry bucket # } else { auto &[map_key, map_item] = *map_it; auto &[list_key, count, bucket, added] = *map_item; - TSMutexUnlock(_lock); - return {bucket, count}; } } @@ -247,7 +241,7 @@ SieveLru::lookup(KeyClass key) const int32_t SieveLru::move_bucket(KeyClass key, uint32_t to_bucket) { - TSMutexLock(_lock); + TSMutexLockGuard lock(_lock); TSAssert(_initialized); auto map_it = _map.find(key); @@ -289,7 +283,6 @@ SieveLru::move_bucket(KeyClass key, uint32_t to_bucket) added = SystemClock::now(); } } - TSMutexUnlock(_lock); return to_bucket; // Just as a convenience, return the destination bucket for this entry } @@ -336,7 +329,7 @@ SieveBucket::memorySize() const size_t SieveLru::memoryUsed() const { - TSMutexLock(_lock); + TSMutexLockGuard lock(_lock); TSAssert(_initialized); size_t total = sizeof(SieveLru); @@ -347,7 +340,6 @@ SieveLru::memoryUsed() const total += _map.size() * (sizeof(void *) + sizeof(SieveBucket::iterator)); total += _map.bucket_count() * (sizeof(size_t) + sizeof(void *)); - TSMutexUnlock(_lock); return total; } diff --git a/plugins/experimental/stale_response/stale_response.cc b/plugins/experimental/stale_response/stale_response.cc index 02a2bbff8c1..0f1b4f2f46f 100644 --- a/plugins/experimental/stale_response/stale_response.cc +++ b/plugins/experimental/stale_response/stale_response.cc @@ -202,11 +202,12 @@ free_state_info(StateInfo *state) int64_t aync_memory_total_add(ConfigInfo *plugin_config, int64_t change) { - int64_t total; - TSMutexLock(plugin_config->body_data_mutex); + int64_t total; + TSMutexLockGuard lock(plugin_config->body_data_mutex); + plugin_config->body_data_memory_usage += change; total = plugin_config->body_data_memory_usage; - TSMutexUnlock(plugin_config->body_data_mutex); + return total; } /*-----------------------------------------------------------------------------------------------*/ diff --git a/plugins/experimental/system_stats/system_stats.cc b/plugins/experimental/system_stats/system_stats.cc index 1e98cfd3fd7..1bb42e85e28 100644 --- a/plugins/experimental/system_stats/system_stats.cc +++ b/plugins/experimental/system_stats/system_stats.cc @@ -103,7 +103,7 @@ statAdd(const char *name, TSRecordDataType record_type, TSMutex create_mutex) { int stat_id = -1; - TSMutexLock(create_mutex); + TSMutexLockGuard lock(create_mutex); if (TS_ERROR == TSStatFindName(name, &stat_id)) { stat_id = TSStatCreate(name, record_type, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM); @@ -114,8 +114,6 @@ statAdd(const char *name, TSRecordDataType record_type, TSMutex create_mutex) } } - TSMutexUnlock(create_mutex); - return stat_id; } diff --git a/plugins/experimental/wasm/ats_context.cc b/plugins/experimental/wasm/ats_context.cc index ec96d5c9c2f..c7c2b636f36 100644 --- a/plugins/experimental/wasm/ats_context.cc +++ b/plugins/experimental/wasm/ats_context.cc @@ -496,9 +496,9 @@ Context::getConfiguration() WasmResult Context::setTimerPeriod(std::chrono::milliseconds period, uint32_t *timer_token_ptr) { - Wasm *wasm = this->wasm(); - Context *root_context = this->root_context(); - TSMutexLock(wasm->mutex()); + Wasm *wasm = this->wasm(); + Context *root_context = this->root_context(); + TSMutexLockGuard lock(wasm->mutex()); if (!wasm->existsTimerPeriod(root_context->id())) { Dbg(dbg_ctl, "[%s] no previous timer period set", __FUNCTION__); TSCont contp = root_context->scheduler_cont(); @@ -511,7 +511,6 @@ Context::setTimerPeriod(std::chrono::milliseconds period, uint32_t *timer_token_ wasm->setTimerPeriod(root_context->id(), period); *timer_token_ptr = 0; - TSMutexUnlock(wasm->mutex()); return WasmResult::Ok; } diff --git a/plugins/experimental/wasm/wasm_main.cc b/plugins/experimental/wasm/wasm_main.cc index 6e7c32a31ae..e2c16170faf 100644 --- a/plugins/experimental/wasm/wasm_main.cc +++ b/plugins/experimental/wasm/wasm_main.cc @@ -304,14 +304,13 @@ schedule_handler(TSCont contp, TSEvent /*event*/, void * /*data*/) auto *c = static_cast(TSContDataGet(contp)); - auto *old_wasm = static_cast(c->wasm()); - TSMutexLock(old_wasm->mutex()); + auto *old_wasm = static_cast(c->wasm()); + TSMutexLockGuard lock(old_wasm->mutex()); c->onTick(0); // use 0 as token if (wasm_config->configs.empty()) { TSError("[wasm][%s] Configuration objects are empty", __FUNCTION__); - TSMutexUnlock(old_wasm->mutex()); return 0; } @@ -365,8 +364,6 @@ schedule_handler(TSCont contp, TSEvent /*event*/, void * /*data*/) Dbg(ats_wasm::dbg_ctl, "[%s] config wasm has changed. thus not scheduling", __FUNCTION__); } - TSMutexUnlock(old_wasm->mutex()); - return 0; } diff --git a/plugins/lua/ts_lua.cc b/plugins/lua/ts_lua.cc index 02b07817118..eeaa4f03a5b 100644 --- a/plugins/lua/ts_lua.cc +++ b/plugins/lua/ts_lua.cc @@ -525,7 +525,7 @@ ts_lua_remap_plugin_init(void *ih, TSHttpTxn rh, TSRemapRequestInfo *rri) pthread_setspecific(lua_state_key, main_ctx); } - TSMutexLock(main_ctx->mutexp); + TSMutexLockGuard lock(main_ctx->mutexp); http_ctx = ts_lua_create_http_ctx(main_ctx, instance_conf); @@ -551,7 +551,6 @@ ts_lua_remap_plugin_init(void *ih, TSHttpTxn rh, TSRemapRequestInfo *rri) if (lua_type(L, -1) != LUA_TFUNCTION) { lua_pop(L, 1); ts_lua_destroy_http_ctx(http_ctx); - TSMutexUnlock(main_ctx->mutexp); return TSREMAP_NO_REMAP; } @@ -574,8 +573,6 @@ ts_lua_remap_plugin_init(void *ih, TSHttpTxn rh, TSRemapRequestInfo *rri) ts_lua_destroy_http_ctx(http_ctx); } - TSMutexUnlock(main_ctx->mutexp); - return TSRemapStatus(ret); } diff --git a/plugins/lua/ts_lua_fetch.cc b/plugins/lua/ts_lua_fetch.cc index b14b918eabc..45a80557c55 100644 --- a/plugins/lua/ts_lua_fetch.cc +++ b/plugins/lua/ts_lua_fetch.cc @@ -542,7 +542,7 @@ ts_lua_fetch_multi_handler(TSCont contp, TSEvent event ATS_UNUSED, void *edata) } // all finish - TSMutexLock(lmutex); + TSMutexLockGuard lock(lmutex); if (fmi->total == 1 && !fmi->multi) { ts_lua_fill_one_result(L, fi); @@ -559,7 +559,6 @@ ts_lua_fetch_multi_handler(TSCont contp, TSEvent event ATS_UNUSED, void *edata) TSContCall(ci->contp, TSEvent(TS_LUA_EVENT_COROUTINE_CONT), reinterpret_cast(1)); } - TSMutexUnlock(lmutex); return 0; } diff --git a/plugins/lua/ts_lua_util.cc b/plugins/lua/ts_lua_util.cc index bf1577f68f9..10baf236b79 100644 --- a/plugins/lua/ts_lua_util.cc +++ b/plugins/lua/ts_lua_util.cc @@ -289,7 +289,7 @@ ts_lua_add_module(ts_lua_instance_conf *conf, ts_lua_main_ctx *arr, int n, int a conf->_first = (i == 0) ? 1 : 0; conf->_last = (i == n - 1) ? 1 : 0; - TSMutexLock(arr[i].mutexp); + TSMutexLockGuard lock(arr[i].mutexp); L = arr[i].lua; @@ -308,7 +308,6 @@ ts_lua_add_module(ts_lua_instance_conf *conf, ts_lua_main_ctx *arr, int n, int a if (luaL_loadstring(L, conf->content)) { snprintf(errbuf, errbuf_size, "[%s] luaL_loadstring failed: %s", __FUNCTION__, lua_tostring(L, -1)); lua_pop(L, 1); - TSMutexUnlock(arr[i].mutexp); return -1; } @@ -316,7 +315,6 @@ ts_lua_add_module(ts_lua_instance_conf *conf, ts_lua_main_ctx *arr, int n, int a if (luaL_loadfile(L, conf->script)) { snprintf(errbuf, errbuf_size, "[%s] luaL_loadfile %s failed: %s", __FUNCTION__, conf->script, lua_tostring(L, -1)); lua_pop(L, 1); - TSMutexUnlock(arr[i].mutexp); return -1; } } @@ -324,7 +322,6 @@ ts_lua_add_module(ts_lua_instance_conf *conf, ts_lua_main_ctx *arr, int n, int a if (lua_pcall(L, 0, 0, 0)) { snprintf(errbuf, errbuf_size, "[%s] lua_pcall %s failed: %s", __FUNCTION__, conf->script, lua_tostring(L, -1)); lua_pop(L, 1); - TSMutexUnlock(arr[i].mutexp); return -1; } @@ -346,7 +343,6 @@ ts_lua_add_module(ts_lua_instance_conf *conf, ts_lua_main_ctx *arr, int n, int a if (lua_pcall(L, 1, 1, 0)) { snprintf(errbuf, errbuf_size, "[%s] lua_pcall %s failed: %s", __FUNCTION__, conf->script, lua_tostring(L, -1)); lua_pop(L, 1); - TSMutexUnlock(arr[i].mutexp); return -1; } @@ -354,7 +350,6 @@ ts_lua_add_module(ts_lua_instance_conf *conf, ts_lua_main_ctx *arr, int n, int a lua_pop(L, 1); if (ret) { - TSMutexUnlock(arr[i].mutexp); return -1; /* script parse error */ } @@ -375,8 +370,6 @@ ts_lua_add_module(ts_lua_instance_conf *conf, ts_lua_main_ctx *arr, int n, int a } else { Dbg(dbg_ctl, "ljgc = %d, NOT running LuaJIT Garbage Collector...", conf->ljgc); } - - TSMutexUnlock(arr[i].mutexp); } return 0; diff --git a/plugins/prefetch/fetch.cc b/plugins/prefetch/fetch.cc index f35b137aa3c..b0d5c7615c1 100644 --- a/plugins/prefetch/fetch.cc +++ b/plugins/prefetch/fetch.cc @@ -231,7 +231,7 @@ BgFetchState::init(const PrefetchConfig &config) TSMutexUnlock(_lock); /* Initialize fetching policy */ - TSMutexLock(_policyLock); + TSMutexLockGuard policy_lock(_policyLock); if (!config.getFetchPolicy().empty() && 0 != config.getFetchPolicy().compare("simple")) { status &= initializePolicy(_policy, config.getFetchPolicy().c_str()); @@ -242,8 +242,6 @@ BgFetchState::init(const PrefetchConfig &config) PrefetchDebug("Policy not specified or 'simple' policy chosen (skipping)"); } - TSMutexUnlock(_policyLock); - return status; } diff --git a/plugins/prefetch/fetch.h b/plugins/prefetch/fetch.h index 2a1b7082a19..66ae24d561a 100644 --- a/plugins/prefetch/fetch.h +++ b/plugins/prefetch/fetch.h @@ -140,7 +140,7 @@ class BgFetchStates BgFetchState *state; std::map::iterator it; - TSMutexLock(_prefetchStates->_lock); + TSMutexLockGuard lock(_prefetchStates->_lock); it = _prefetchStates->_states.find(space); if (it != _prefetchStates->_states.end()) { state = it->second; @@ -148,7 +148,6 @@ class BgFetchStates state = new BgFetchState(); _prefetchStates->_states[space] = state; } - TSMutexUnlock(_prefetchStates->_lock); return state; } From a4666a6c1c6acf60e8e8e23827dd04b04ad69b5b Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Tue, 23 Jun 2026 12:57:58 -0500 Subject: [PATCH 13/33] redo_cache_lookup: move to examples; fix `fallback` lifetime (#13209) The redo_cache_lookup plugin kept the fallback URL as a pointer into the plugin.config argv storage. That storage can be released after plugin initialization, leaving cache-lookup-complete callbacks to dereference stale memory. This copies the parsed fallback URL into plugin-owned storage and passes its owned bytes to TSHttpTxnRedoCacheLookup. Also, while investigating this, it looks like this plugin was made simply to demonstrate the use of TSHttpTxnRedoCacheLookup rather than being a production-useful plugin. The initial commit says as much and there is no customer-facing documentation for this plugin. As such, I'm moving this to the examples plugin. (cherry picked from commit 468e76405a932efa55332a9ff70745e0a04c3b35) --- cmake/ExperimentalPlugins.cmake | 1 - example/plugins/c-api/CMakeLists.txt | 8 ++ .../c-api/redo_cache_lookup/readme.txt | 13 ++ .../redo_cache_lookup/redo_cache_lookup.cc | 91 ++++++++++++++ .../redo_cache_lookup_config.h | 78 ++++++++++++ .../test_redo_cache_lookup_config.cc | 66 ++++++++++ plugins/experimental/CMakeLists.txt | 3 - .../redo_cache_lookup/CMakeLists.txt | 21 ---- .../experimental/redo_cache_lookup/README.md | 11 -- .../redo_cache_lookup/redo_cache_lookup.cc | 115 ------------------ 10 files changed, 256 insertions(+), 151 deletions(-) create mode 100644 example/plugins/c-api/redo_cache_lookup/readme.txt create mode 100644 example/plugins/c-api/redo_cache_lookup/redo_cache_lookup.cc create mode 100644 example/plugins/c-api/redo_cache_lookup/redo_cache_lookup_config.h create mode 100644 example/plugins/c-api/redo_cache_lookup/unit_tests/test_redo_cache_lookup_config.cc delete mode 100644 plugins/experimental/redo_cache_lookup/CMakeLists.txt delete mode 100644 plugins/experimental/redo_cache_lookup/README.md delete mode 100644 plugins/experimental/redo_cache_lookup/redo_cache_lookup.cc diff --git a/cmake/ExperimentalPlugins.cmake b/cmake/ExperimentalPlugins.cmake index 5e73ffa5ec0..4aff4543abf 100644 --- a/cmake/ExperimentalPlugins.cmake +++ b/cmake/ExperimentalPlugins.cmake @@ -83,7 +83,6 @@ auto_option( ) auto_option(RATE_LIMIT FEATURE_VAR BUILD_RATE_LIMIT DEFAULT ${_DEFAULT}) auto_option(REALIP FEATURE_VAR BUILD_REALIP DEFAULT ${_DEFAULT}) -auto_option(REDO_CACHE_LOOKUP FEATURE_VAR BUILD_REDO_CACHE_LOOKUP DEFAULT ${_DEFAULT}) auto_option(SSLHEADERS FEATURE_VAR BUILD_SSLHEADERS DEFAULT ${_DEFAULT}) auto_option(STALE_RESPONSE FEATURE_VAR BUILD_STALE_RESPONSE DEFAULT ${_DEFAULT}) auto_option( diff --git a/example/plugins/c-api/CMakeLists.txt b/example/plugins/c-api/CMakeLists.txt index 65594cd391e..ea40f39ceac 100644 --- a/example/plugins/c-api/CMakeLists.txt +++ b/example/plugins/c-api/CMakeLists.txt @@ -24,6 +24,7 @@ add_atsplugin(secure_link ./secure_link/secure_link.cc) target_link_libraries(secure_link PRIVATE OpenSSL::SSL) add_atsplugin(remap ./remap/remap.cc) add_atsplugin(redirect_1 ./redirect_1/redirect_1.cc) +add_atsplugin(redo_cache_lookup ./redo_cache_lookup/redo_cache_lookup.cc) add_atsplugin(query_remap ./query_remap/query_remap.cc) add_atsplugin(thread_pool ./thread_pool/psi.cc ./thread_pool/thread.cc) add_atsplugin(bnull_transform ./bnull_transform/bnull_transform.cc) @@ -66,3 +67,10 @@ add_atsplugin(protocol_stack ./protocol_stack/protocol_stack.cc) add_atsplugin(client_context_dump ./client_context_dump/client_context_dump.cc) target_link_libraries(client_context_dump PRIVATE OpenSSL::SSL libswoc::libswoc) add_atsplugin(custom_logfield ./custom_logfield/custom_logfield.cc) + +if(BUILD_TESTING) + add_executable(test_redo_cache_lookup ./redo_cache_lookup/unit_tests/test_redo_cache_lookup_config.cc) + target_include_directories(test_redo_cache_lookup PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/redo_cache_lookup) + target_link_libraries(test_redo_cache_lookup PRIVATE Catch2::Catch2WithMain) + add_catch2_test(NAME test_redo_cache_lookup COMMAND test_redo_cache_lookup) +endif() diff --git a/example/plugins/c-api/redo_cache_lookup/readme.txt b/example/plugins/c-api/redo_cache_lookup/readme.txt new file mode 100644 index 00000000000..9f3008400f3 --- /dev/null +++ b/example/plugins/c-api/redo_cache_lookup/readme.txt @@ -0,0 +1,13 @@ +# Redo Cache Lookup Example Plugin + +This plugin shows how to use the `TSHttpTxnRedoCacheLookup` C API. It +checks cache lookup results and asks ATS to retry the lookup with a fallback +URL when the original lookup misses or is skipped. + +## Configuration + +Add this plugin to `plugin.config` with the `--fallback` option: + +``` +redo_cache_lookup.so --fallback http://example.com/fallback_url +``` diff --git a/example/plugins/c-api/redo_cache_lookup/redo_cache_lookup.cc b/example/plugins/c-api/redo_cache_lookup/redo_cache_lookup.cc new file mode 100644 index 00000000000..c370fae0e22 --- /dev/null +++ b/example/plugins/c-api/redo_cache_lookup/redo_cache_lookup.cc @@ -0,0 +1,91 @@ +/** @file + + An example plugin to redo cache lookups with a fallback URL. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include +#include + +#include "ts/ts.h" +#include "redo_cache_lookup_config.h" + +#define PLUGIN_NAME "redo_cache_lookup" + +namespace +{ +DbgCtl dbg_ctl{PLUGIN_NAME}; + +struct RedoCacheLookupConfig { + RedoCacheLookupConfig(std::string_view fallback) : fallback(fallback) {} + + std::string fallback; +}; + +int +handle_cache_lookup_complete(TSCont contp, TSEvent event, void *edata) +{ + if (event != TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE) { + return 0; + } + + TSHttpTxn txnp = static_cast(edata); + auto *config = static_cast(TSContDataGet(contp)); + int status = TS_CACHE_LOOKUP_MISS; + + if (TSHttpTxnCacheLookupStatusGet(txnp, &status) != TS_SUCCESS || status == TS_CACHE_LOOKUP_MISS || + status == TS_CACHE_LOOKUP_SKIPPED) { + Dbg(dbg_ctl, "rewinding to check for fallback url: %s", config->fallback.c_str()); + TSHttpTxnRedoCacheLookup(txnp, config->fallback.c_str(), static_cast(config->fallback.size())); + } + + TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); + return 0; +} +} // namespace + +void +TSPluginInit(int argc, const char *argv[]) +{ + TSPluginRegistrationInfo info; + + Dbg(dbg_ctl, "Init"); + info.plugin_name = PLUGIN_NAME; + info.vendor_name = "Apache Software Foundation"; + info.support_email = "dev@trafficserver.apache.org"; + + if (TSPluginRegister(&info) != TS_SUCCESS) { + TSError("[%s] Plugin registration failed", PLUGIN_NAME); + return; + } + + auto fallback = redo_cache_lookup::parse_fallback_url(argc, argv); + + if (!fallback) { + Dbg(dbg_ctl, "Missing fallback option."); + TSError("[%s] Missing fallback option", PLUGIN_NAME); + return; + } + Dbg(dbg_ctl, "Initialized with fallback: %s", fallback->c_str()); + + TSCont contp = TSContCreate(handle_cache_lookup_complete, nullptr); + TSContDataSet(contp, new RedoCacheLookupConfig(*fallback)); + TSHttpHookAdd(TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK, contp); +} diff --git a/example/plugins/c-api/redo_cache_lookup/redo_cache_lookup_config.h b/example/plugins/c-api/redo_cache_lookup/redo_cache_lookup_config.h new file mode 100644 index 00000000000..8158e4aec0f --- /dev/null +++ b/example/plugins/c-api/redo_cache_lookup/redo_cache_lookup_config.h @@ -0,0 +1,78 @@ +/** @file + + Configuration helpers for the redo_cache_lookup plugin. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#pragma once + +#include +#include +#include + +namespace redo_cache_lookup +{ +/** Parse the configured fallback URL from plugin arguments. + * + * @param[in] argc The number of plugin argument entries in @a argv. + * @param[in] argv The plugin arguments supplied from @c plugin.config. + * @return The configured fallback URL, or @c std::nullopt if no fallback URL + * is configured. + */ +inline std::optional +parse_fallback_url(int argc, const char *argv[]) +{ + std::optional fallback; + + static const struct option longopts[] = { + {"fallback", required_argument, nullptr, 'f'}, + {nullptr, 0, nullptr, 0 }, + }; + +#if (!defined(kfreebsd) && defined(freebsd)) || defined(darwin) + optreset = 1; +#endif +#if defined(__GLIBC__) + optind = 0; +#else + optind = 1; +#endif + opterr = 0; + optarg = nullptr; + + int opt = 0; + + while (opt >= 0) { + opt = getopt_long(argc, const_cast(argv), "f:", longopts, nullptr); + switch (opt) { + case 'f': + fallback = optarg; + break; + case -1: + case '?': + break; + default: + return std::nullopt; + } + } + + return fallback; +} +} // namespace redo_cache_lookup diff --git a/example/plugins/c-api/redo_cache_lookup/unit_tests/test_redo_cache_lookup_config.cc b/example/plugins/c-api/redo_cache_lookup/unit_tests/test_redo_cache_lookup_config.cc new file mode 100644 index 00000000000..905af6dfe61 --- /dev/null +++ b/example/plugins/c-api/redo_cache_lookup/unit_tests/test_redo_cache_lookup_config.cc @@ -0,0 +1,66 @@ +/** @file + + Tests for redo_cache_lookup plugin configuration parsing. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include + +#include + +#include "redo_cache_lookup_config.h" + +TEST_CASE("redo_cache_lookup fallback option is copied", "[redo_cache_lookup]") +{ + char plugin_name[] = "redo_cache_lookup.so"; + char fallback_opt[] = "--fallback"; + char fallback_url[] = "http://example.test/fallback"; + const char *argv[] = {plugin_name, fallback_opt, fallback_url}; + + auto parsed = redo_cache_lookup::parse_fallback_url(3, argv); + + REQUIRE(parsed.has_value()); + + std::memset(fallback_url, 'x', sizeof(fallback_url) - 1); + + REQUIRE(*parsed == "http://example.test/fallback"); +} + +TEST_CASE("redo_cache_lookup fallback option accepts short form", "[redo_cache_lookup]") +{ + char plugin_name[] = "redo_cache_lookup.so"; + char fallback_opt[] = "-f"; + char fallback_url[] = "http://example.test/short"; + const char *argv[] = {plugin_name, fallback_opt, fallback_url}; + + auto parsed = redo_cache_lookup::parse_fallback_url(3, argv); + + REQUIRE(parsed == "http://example.test/short"); +} + +TEST_CASE("redo_cache_lookup fallback option is required", "[redo_cache_lookup]") +{ + char plugin_name[] = "redo_cache_lookup.so"; + const char *argv[] = {plugin_name}; + + auto parsed = redo_cache_lookup::parse_fallback_url(1, argv); + + REQUIRE_FALSE(parsed.has_value()); +} diff --git a/plugins/experimental/CMakeLists.txt b/plugins/experimental/CMakeLists.txt index fe7897c5854..bd8cf64e0f3 100644 --- a/plugins/experimental/CMakeLists.txt +++ b/plugins/experimental/CMakeLists.txt @@ -98,9 +98,6 @@ endif() if(BUILD_REALIP) add_subdirectory(realip) endif() -if(BUILD_REDO_CACHE_LOOKUP) - add_subdirectory(redo_cache_lookup) -endif() if(BUILD_SSLHEADERS) add_subdirectory(sslheaders) endif() diff --git a/plugins/experimental/redo_cache_lookup/CMakeLists.txt b/plugins/experimental/redo_cache_lookup/CMakeLists.txt deleted file mode 100644 index 1b6c41d47b0..00000000000 --- a/plugins/experimental/redo_cache_lookup/CMakeLists.txt +++ /dev/null @@ -1,21 +0,0 @@ -####################### -# -# Licensed to the Apache Software Foundation (ASF) under one or more contributor license -# agreements. See the NOTICE file distributed with this work for additional information regarding -# copyright ownership. The ASF licenses this file to you under the Apache License, Version 2.0 -# (the "License"); you may not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software distributed under the License -# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express -# or implied. See the License for the specific language governing permissions and limitations under -# the License. -# -####################### - -add_atsplugin(redo_cache_lookup redo_cache_lookup.cc) - -target_link_libraries(redo_cache_lookup PRIVATE ts::tscppapi) -verify_global_plugin(redo_cache_lookup) diff --git a/plugins/experimental/redo_cache_lookup/README.md b/plugins/experimental/redo_cache_lookup/README.md deleted file mode 100644 index 7ad5ddae36e..00000000000 --- a/plugins/experimental/redo_cache_lookup/README.md +++ /dev/null @@ -1,11 +0,0 @@ -# Redo Cache Lookup Plugin - -This plugin shows how to use the experimental `TSHttpTxnRedoCacheLookup` api. It works by checking the cache for a fallback url if the cache lookup failed for any given url. - -## Configuration - -Add this plugin to `plugin.config` with the `--fallback` option: - -``` -redo_cache_lookup.so --fallback http://example.com/fallback_url -``` \ No newline at end of file diff --git a/plugins/experimental/redo_cache_lookup/redo_cache_lookup.cc b/plugins/experimental/redo_cache_lookup/redo_cache_lookup.cc deleted file mode 100644 index 6e917c700a0..00000000000 --- a/plugins/experimental/redo_cache_lookup/redo_cache_lookup.cc +++ /dev/null @@ -1,115 +0,0 @@ -/** @file - - A plugin to redo cache lookups with a fallback if cache lookups fail for specific urls. - - @section license License - - Licensed to the Apache Software Foundation (ASF) under one - or more contributor license agreements. See the NOTICE file - distributed with this work for additional information - regarding copyright ownership. The ASF licenses this file - to you under the Apache License, Version 2.0 (the - "License"); you may not use this file except in compliance - with the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - */ - -#include -#include -#include -#include -#include -#include - -#include -#include "tscpp/api/GlobalPlugin.h" -#include "tscpp/api/utils.h" - -#define PLUGIN_NAME "redo_cache_lookup" - -using namespace atscppapi; - -namespace -{ -GlobalPlugin *plugin; - -DbgCtl dbg_ctl{PLUGIN_NAME}; -} // namespace - -class RedoCacheLookupPlugin : public GlobalPlugin -{ -public: - RedoCacheLookupPlugin(const char *fallback) : fallback(fallback) - { - Dbg(dbg_ctl, "registering transaction hooks"); - RedoCacheLookupPlugin::registerHook(HOOK_CACHE_LOOKUP_COMPLETE); - } - - void - handleReadCacheLookupComplete(Transaction &transaction) override - { - Transaction::CacheStatus status = transaction.getCacheStatus(); - - if (status == Transaction::CacheStatus::CACHE_LOOKUP_NONE || status == Transaction::CacheStatus::CACHE_LOOKUP_SKIPED || - status == Transaction::CacheStatus::CACHE_LOOKUP_MISS) { - Dbg(dbg_ctl, "rewinding to check for fallback url: %s", fallback); - TSHttpTxn txnp = static_cast(transaction.getAtsHandle()); - TSHttpTxnRedoCacheLookup(txnp, fallback, strlen(fallback)); - } - - transaction.resume(); - } - -private: - const char *fallback; -}; - -void -TSPluginInit(int argc, const char *argv[]) -{ - Dbg(dbg_ctl, "Init"); - if (!RegisterGlobalPlugin("RedoCacheLookupPlugin", PLUGIN_NAME, "dev@trafficserver.apache.org")) { - return; - } - - const char *fallback = nullptr; - - // Read options from plugin.config - static const struct option longopts[] = { - {"fallback", required_argument, nullptr, 'f'} - }; - - int opt = 0; - - while (opt >= 0) { - opt = getopt_long(argc, const_cast(argv), "f:", longopts, nullptr); - switch (opt) { - case 'f': - fallback = optarg; - break; - case -1: - case '?': - break; - default: - Dbg(dbg_ctl, "Unexpected option: %i", opt); - TSError("[%s] Unexpected options error.", PLUGIN_NAME); - return; - } - } - - if (nullptr == fallback) { - Dbg(dbg_ctl, "Missing fallback option."); - TSError("[%s] Missing fallback option", PLUGIN_NAME); - return; - } - Dbg(dbg_ctl, "Initialized with fallback: %s", fallback); - - plugin = new RedoCacheLookupPlugin(fallback); -} From d91895f95d5697bdd74da1c808c5e6e71ca71d20 Mon Sep 17 00:00:00 2001 From: Phong Nguyen Date: Tue, 23 Jun 2026 15:46:51 -0700 Subject: [PATCH 14/33] Update FastLZ to b1342da (#13230) * Update FastLZ to b1342da * Add copyright to the NOTICE (cherry picked from commit 9ebcdd2c6d1f8bdf35f12fc40f0921d14a2c92b2) --- NOTICE | 1 + lib/fastlz/README.md | 64 ++++--- lib/fastlz/fastlz.cc | 442 +++++++++++++++---------------------------- lib/fastlz/fastlz.h | 14 +- 4 files changed, 209 insertions(+), 312 deletions(-) diff --git a/NOTICE b/NOTICE index ad0c5f11b54..dfc08aa4ba2 100644 --- a/NOTICE +++ b/NOTICE @@ -95,6 +95,7 @@ https://github.com/jbeder/yaml-cpp ~~ fastlz: an ANSI C/C90 implementation of Lempel-Ziv 77 algorithm (LZ77) of lossless data compression. +Copyright (C) 2005-2020 Ariya Hidayat (MIT License) https://github.com/ariya/FastLZ ~~ diff --git a/lib/fastlz/README.md b/lib/fastlz/README.md index 6ec851ac909..9d7bf696b5b 100644 --- a/lib/fastlz/README.md +++ b/lib/fastlz/README.md @@ -34,31 +34,49 @@ For [Vcpkg](https://github.com/microsoft/vcpkg) users, FastLZ is [already availa A simple file compressor called `6pack` is included as an example on how to use FastLZ. The corresponding decompressor is `6unpack`. -FastLZ supports any standard-conforming ANSI C/C90 compiler, including the popular ones such as GCC, Clang, Intel C++ Compiler, Visual Studio and even Tiny CC. FastLZ works well on a number of architectures (32-bit and 64-bit, big endian and little endian), from Intel/AMD, ARM, and MIPS. +FastLZ supports any standard-conforming ANSI C/C90 compiler, including the popular ones such as [GCC](https://gcc.gnu.org/), [Clang](https://clang.llvm.org/), [Visual Studio](https://visualstudio.microsoft.com/vs/features/cplusplus/), and even [Tiny CC](https://bellard.org/tcc/). FastLZ works well on a number of architectures (32-bit and 64-bit, big endian and little endian), from Intel/AMD, PowerPC, System z, ARM, MIPS, and RISC-V. The continuous integration system runs an extensive set of compression-decompression round trips on the following systems: For more details, check the corresponding [GitHub Actions build logs](https://github.com/ariya/FastLZ/actions). -| | | | | -|--------------|---------------------------------------------------------------------------------------------|---------------------------------------------------------------------------------------------------|---------------------------------------------------------------------------------------------| -| **amd64** | **Linux** | **Windows** | **macOS** | -| GCC | ![amd64_linux_gcc](https://github.com/ariya/FastLZ/workflows/amd64_linux_gcc/badge.svg) | ![amd64_windows_gcc](https://github.com/ariya/FastLZ/workflows/amd64_windows_gcc/badge.svg) | ![amd64_macos_gcc](https://github.com/ariya/FastLZ/workflows/amd64_macos_gcc/badge.svg) | -| Clang | ![amd64_linux_clang](https://github.com/ariya/FastLZ/workflows/amd64_linux_clang/badge.svg) | ![amd64_windows_clang](https://github.com/ariya/FastLZ/workflows/amd64_windows_clang/badge.svg) | ![amd64_macos_clang](https://github.com/ariya/FastLZ/workflows/amd64_macos_clang/badge.svg) | -| Intel CC | ![amd64_linux_icc](https://github.com/ariya/FastLZ/workflows/amd64_linux_icc/badge.svg) | | | -| TinyCC | ![amd64_linux_tcc](https://github.com/ariya/FastLZ/workflows/amd64_linux_tcc/badge.svg) | | | | -| VS 2017 | | ![amd64_windows_vs2017](https://github.com/ariya/FastLZ/workflows/amd64_windows_vs2017/badge.svg) | | -| VS 2019 | | ![amd64_windows_vs2019](https://github.com/ariya/FastLZ/workflows/amd64_windows_vs2019/badge.svg) | | -| **i686** | **Linux** | **Windows** | **macOS** | -| GCC | ![i686_linux_gcc](https://github.com/ariya/FastLZ/workflows/i686_linux_gcc/badge.svg) | | | -| Clang | ![i686_linux_clang](https://github.com/ariya/FastLZ/workflows/i686_linux_clang/badge.svg) | | | -| VS 2017 | | ![i686_windows_vs2017](https://github.com/ariya/FastLZ/workflows/i686_windows_vs2017/badge.svg) | | -| VS 2019 | | ![i686_windows_vs2019](https://github.com/ariya/FastLZ/workflows/i686_windows_vs2019/badge.svg) | | -| **arm64** | **Linux** | **Windows** | **macOS** | -| GCC | ![arm64_linux_gcc](https://github.com/ariya/FastLZ/workflows/arm64_linux_gcc/badge.svg) | | | -| **mips64** | **Linux** | **Windows** | **macOS** | -| GCC | ![mips64_linux_gcc](https://github.com/ariya/FastLZ/workflows/mips64_linux_gcc/badge.svg) | | | +| | | | | +|----------------------|--------------------------------------------------------------------------------------------------------:|--------------------------------------------------------------------------------------------------:|--------------------------------------------------------------------------------------------:| +| **amd64** | **Linux** | **Windows** | **macOS** | +| GCC | ![amd64_linux_gcc](https://github.com/ariya/FastLZ/workflows/amd64_linux_gcc/badge.svg) | ![amd64_windows_gcc](https://github.com/ariya/FastLZ/workflows/amd64_windows_gcc/badge.svg) | ![amd64_macos_gcc](https://github.com/ariya/FastLZ/workflows/amd64_macos_gcc/badge.svg) | +| Clang | ![amd64_linux_clang](https://github.com/ariya/FastLZ/workflows/amd64_linux_clang/badge.svg) | ![amd64_windows_clang](https://github.com/ariya/FastLZ/workflows/amd64_windows_clang/badge.svg) | ![amd64_macos_clang](https://github.com/ariya/FastLZ/workflows/amd64_macos_clang/badge.svg) | +| TinyCC | ![amd64_linux_tcc](https://github.com/ariya/FastLZ/workflows/amd64_linux_tcc/badge.svg) | ![amd64_windows_tcc](https://github.com/ariya/FastLZ/workflows/amd64_windows_tcc/badge.svg) | | +| VS 2019 | | ![amd64_windows_vs2019](https://github.com/ariya/FastLZ/workflows/amd64_windows_vs2019/badge.svg) | | +| **i686** | **Linux** | **Windows** | **macOS** | +| GCC | ![i686_linux_gcc](https://github.com/ariya/FastLZ/workflows/i686_linux_gcc/badge.svg) | | | +| Clang | ![i686_linux_clang](https://github.com/ariya/FastLZ/workflows/i686_linux_clang/badge.svg) | | | +| TinyCC | | ![i686_windows_tcc](https://github.com/ariya/FastLZ/workflows/i686_windows_tcc/badge.svg) | | +| VS 2019 | | ![i686_windows_vs2019](https://github.com/ariya/FastLZ/workflows/i686_windows_vs2019/badge.svg) | | +| **i586** | **Linux** | **DOS** | | +| GCC | | ![i586_dos_gcc_cross](https://github.com/ariya/FastLZ/workflows/i586_dos_gcc_cross/badge.svg) | | +| | **Linux** | | | +| **powerpc** | | | | +| GCC | ![powerpc_linux_gcc](https://github.com/ariya/FastLZ/workflows/powerpc_linux_gcc/badge.svg) | | | +| **ppc64(le)** | | | | +| GCC | ![ppc64_linux_gcc](https://github.com/ariya/FastLZ/workflows/ppc64_linux_gcc/badge.svg) | | | +| GCC | ![ppc64le_linux_gcc](https://github.com/ariya/FastLZ/workflows/ppc64le_linux_gcc/badge.svg) | | | +| **s390x** | | | | +| GCC | ![s390x_linux_gcc](https://github.com/ariya/FastLZ/workflows/s390x_linux_gcc/badge.svg) | | | +| **armhf** | | | | +| GCC | ![armhf_linux_gcc](https://github.com/ariya/FastLZ/workflows/armhf_linux_gcc/badge.svg) | | | +| **arm64** | | | | +| GCC | ![arm64_linux_gcc](https://github.com/ariya/FastLZ/workflows/arm64_linux_gcc/badge.svg) | | | +| **mips(el)** | | | | +| GCC | ![mipsel_linux_gcc](https://github.com/ariya/FastLZ/workflows/mipsel_linux_gcc/badge.svg) | | | +| GCC | ![mips_linux_gcc](https://github.com/ariya/FastLZ/workflows/mips_linux_gcc/badge.svg) | | | +| **mips64(el)** | | | | +| GCC | ![mips64el_linux_gcc](https://github.com/ariya/FastLZ/workflows/mips64el_linux_gcc/badge.svg) | | | +| GCC | ![mips64_linux_gcc](https://github.com/ariya/FastLZ/workflows/mips64_linux_gcc/badge.svg) | | | +| **riscv** | | | | +| GCC | ![riscv_linux_gcc](https://github.com/ariya/FastLZ/workflows/riscv_linux_gcc/badge.svg) | | | +| **riscv64** | | | | +| GCC | ![riscv64_linux_gcc](https://github.com/ariya/FastLZ/workflows/riscv64_linux_gcc/badge.svg) | | | @@ -66,7 +84,7 @@ For more details, check the corresponding [GitHub Actions build logs](https://gi Let us assume that FastLZ compresses an array of bytes, called the _uncompressed block_, into another array of bytes, called the _compressed block_. To understand what will be stored in the compressed block, it is illustrative to demonstrate how FastLZ will _decompress_ the block to retrieve the original uncompressed block. -The first 5-bit of the block, i.e. the 5 most-significant bits of the first byte, is the **block tag**. Currently the block tag determines the compression level used to produce the compressed block. +The first 3-bit of the block, i.e. the 3 most-significant bits of the first byte, is the **block tag**. Currently the block tag determines the compression level used to produce the compressed block. |Block tag|Compression level| |---------|-----------------| @@ -77,16 +95,16 @@ The content of the block will vary depending on the compression level. ### Block Format for Level 1 -FastLZ Level 1 impements LZ77 compression algorithm with 8 KB sliding window and up to 264 bytes of match length. +FastLZ Level 1 implements LZ77 compression algorithm with 8 KB sliding window and up to 264 bytes of match length. The compressed block consists of one or more **instructions**. Each instruction starts with a 1-byte opcode, 2-byte opcode, or 3-byte opcode. | Instruction type | Opcode[0] | Opcode[1] | Opcode[2] |-----------|------------------|--------------------|--| -| Literal run | `000`, L₅-L₀ | -|- | +| Literal run | `000`, L₄-L₀ | -|- | | Short match | M₂-M₀, R₁₂-R₈ | R₇-R₀ | - | -| Long match | `111`, R₁₂-R₈ | M₇-R₀ | R₇-R₀ | +| Long match | `111`, R₁₂-R₈ | M₇-M₀ | R₇-R₀ | Note that the _very first_ instruction in a compressed block is always a literal run. diff --git a/lib/fastlz/fastlz.cc b/lib/fastlz/fastlz.cc index 2e9bfb3d72d..f99bb10b0f3 100644 --- a/lib/fastlz/fastlz.cc +++ b/lib/fastlz/fastlz.cc @@ -25,14 +25,8 @@ #include -/* - * Always check for bound when decompressing. - * Generally it is best to leave it defined. - */ -#define FASTLZ_SAFE -#if defined(FASTLZ_USE_SAFE_DECOMPRESSOR) && (FASTLZ_USE_SAFE_DECOMPRESSOR == 0) -#undef FASTLZ_SAFE -#endif +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wimplicit-fallthrough" /* * Give hints to the compiler for branch prediction optimization. @@ -48,33 +42,26 @@ /* * Specialize custom 64-bit implementation for speed improvements. */ -#if defined(__x86_64__) || defined(_M_X64) +#if defined(__x86_64__) || defined(_M_X64) || defined(__aarch64__) #define FLZ_ARCH64 #endif -#if defined(FASTLZ_SAFE) -#define FASTLZ_BOUND_CHECK(cond) \ - if (FASTLZ_UNLIKELY(!(cond))) \ - return 0; -#else -#define FASTLZ_BOUND_CHECK(cond) \ - do { \ - } while (0) +/* + * Workaround for DJGPP to find uint8_t, uint16_t, etc. + */ +#if defined(__MSDOS__) && defined(__GNUC__) +#include #endif #if defined(FASTLZ_USE_MEMMOVE) && (FASTLZ_USE_MEMMOVE == 0) -static void -fastlz_memmove(uint8_t *dest, const uint8_t *src, uint32_t count) -{ +static void fastlz_memmove(uint8_t* dest, const uint8_t* src, uint32_t count) { do { *dest++ = *src++; } while (--count); } -static void -fastlz_memcpy(uint8_t *dest, const uint8_t *src, uint32_t count) -{ +static void fastlz_memcpy(uint8_t* dest, const uint8_t* src, uint32_t count) { return fastlz_memmove(dest, src, count); } @@ -82,146 +69,64 @@ fastlz_memcpy(uint8_t *dest, const uint8_t *src, uint32_t count) #include -static void -fastlz_memmove(uint8_t *dest, const uint8_t *src, uint32_t count) -{ +static void fastlz_memmove(uint8_t* dest, const uint8_t* src, uint32_t count) { if ((count > 4) && (dest >= src + count)) { memmove(dest, src, count); } else { switch (count) { - default: - do { + default: + do { + *dest++ = *src++; + } while (--count); + break; + case 3: + *dest++ = *src++; + case 2: *dest++ = *src++; - } while (--count); - break; - case 3: - *dest++ = *src++; - [[fallthrough]]; - case 2: - *dest++ = *src++; - [[fallthrough]]; - case 1: - *dest++ = *src++; - [[fallthrough]]; - case 0: - break; + case 1: + *dest++ = *src++; + case 0: + break; } } } -static void -fastlz_memcpy(uint8_t *dest, const uint8_t *src, uint32_t count) -{ - memcpy(dest, src, count); -} +static void fastlz_memcpy(uint8_t* dest, const uint8_t* src, uint32_t count) { memcpy(dest, src, count); } #endif #if defined(FLZ_ARCH64) -static uint32_t -flz_readu32(const void *ptr) -{ - return *(const uint32_t *)ptr; -} - -static uint64_t -flz_readu64(const void *ptr) -{ - return *(const uint64_t *)ptr; -} +static uint32_t flz_readu32(const void* ptr) { return *(const uint32_t*)ptr; } -static uint32_t -flz_cmp(const uint8_t *p, const uint8_t *q, const uint8_t *r) -{ - const uint8_t *start = p; +static uint32_t flz_cmp(const uint8_t* p, const uint8_t* q, const uint8_t* r) { + const uint8_t* start = p; - if (flz_readu64(p) == flz_readu64(q)) { - p += 8; - q += 8; - } if (flz_readu32(p) == flz_readu32(q)) { p += 4; q += 4; } while (q < r) - if (*p++ != *q++) - break; + if (*p++ != *q++) break; return p - start; } -static void -flz_copy64(uint8_t *dest, const uint8_t *src, uint32_t count) -{ - const uint64_t *p = (const uint64_t *)src; - uint64_t *q = (uint64_t *)dest; - if (count < 16) { - if (count >= 8) { - *q++ = *p++; - } - *q++ = *p++; - } else { - *q++ = *p++; - *q++ = *p++; - *q++ = *p++; - *q++ = *p++; - } -} - -static void -flz_copy256(void *dest, const void *src) -{ - const uint64_t *p = (const uint64_t *)src; - uint64_t *q = (uint64_t *)dest; - *q++ = *p++; - *q++ = *p++; - *q++ = *p++; - *q++ = *p++; -} - #endif /* FLZ_ARCH64 */ #if !defined(FLZ_ARCH64) -static uint32_t -flz_readu32(const void *ptr) -{ - const uint8_t *p = (const uint8_t *)ptr; +static uint32_t flz_readu32(const void* ptr) { + const uint8_t* p = (const uint8_t*)ptr; return (p[3] << 24) | (p[2] << 16) | (p[1] << 8) | p[0]; } -static uint32_t -flz_cmp(const uint8_t *p, const uint8_t *q, const uint8_t *r) -{ - const uint8_t *start = p; +static uint32_t flz_cmp(const uint8_t* p, const uint8_t* q, const uint8_t* r) { + const uint8_t* start = p; while (q < r) - if (*p++ != *q++) - break; + if (*p++ != *q++) break; return p - start; } -static void -flz_copy64(uint8_t *dest, const uint8_t *src, uint32_t count) -{ - const uint8_t *p = (const uint8_t *)src; - uint8_t *q = (uint8_t *)dest; - unsigned int c; - for (c = 0; c < count * 8; ++c) { - *q++ = *p++; - } -} - -static void -flz_copy256(void *dest, const void *src) -{ - const uint8_t *p = (const uint8_t *)src; - uint8_t *q = (uint8_t *)dest; - int c; - for (c = 0; c < 32; ++c) { - *q++ = *p++; - } -} - #endif /* !FLZ_ARCH64 */ #define MAX_COPY 32 @@ -230,60 +135,54 @@ flz_copy256(void *dest, const void *src) #define MAX_L2_DISTANCE 8191 #define MAX_FARDISTANCE (65535 + MAX_L2_DISTANCE - 1) -#define HASH_LOG 14 +#define HASH_LOG 13 #define HASH_SIZE (1 << HASH_LOG) #define HASH_MASK (HASH_SIZE - 1) -static uint16_t -flz_hash(uint32_t v) -{ +static uint16_t flz_hash(uint32_t v) { uint32_t h = (v * 2654435769LL) >> (32 - HASH_LOG); return h & HASH_MASK; } -static uint8_t * -flz_literals(uint32_t runs, const uint8_t *src, uint8_t *dest) -{ - while (runs >= MAX_COPY) { - *dest++ = MAX_COPY - 1; - flz_copy256(dest, src); - src += MAX_COPY; - dest += MAX_COPY; - runs -= MAX_COPY; - } - if (runs > 0) { - *dest++ = runs - 1; - flz_copy64(dest, src, runs); - dest += runs; - } - return dest; -} - -/* special case of memcpy: at most 32 bytes */ -static void -flz_smallcopy(uint8_t *dest, const uint8_t *src, uint32_t count) -{ +/* special case of memcpy: at most MAX_COPY bytes */ +static void flz_smallcopy(uint8_t* dest, const uint8_t* src, uint32_t count) { #if defined(FLZ_ARCH64) - if (count >= 8) { - const uint64_t *p = (const uint64_t *)src; - uint64_t *q = (uint64_t *)dest; - while (count > 8) { + if (count >= 4) { + const uint32_t* p = (const uint32_t*)src; + uint32_t* q = (uint32_t*)dest; + while (count > 4) { *q++ = *p++; - count -= 8; - dest += 8; - src += 8; + count -= 4; + dest += 4; + src += 4; } } #endif fastlz_memcpy(dest, src, count); } -static uint8_t * -flz_finalize(uint32_t runs, const uint8_t *src, uint8_t *dest) -{ +/* special case of memcpy: exactly MAX_COPY bytes */ +static void flz_maxcopy(void* dest, const void* src) { +#if defined(FLZ_ARCH64) + const uint32_t* p = (const uint32_t*)src; + uint32_t* q = (uint32_t*)dest; + *q++ = *p++; + *q++ = *p++; + *q++ = *p++; + *q++ = *p++; + *q++ = *p++; + *q++ = *p++; + *q++ = *p++; + *q++ = *p++; +#else + fastlz_memcpy(dest, src, MAX_COPY); +#endif +} + +static uint8_t* flz_literals(uint32_t runs, const uint8_t* src, uint8_t* dest) { while (runs >= MAX_COPY) { *dest++ = MAX_COPY - 1; - flz_smallcopy(dest, src, MAX_COPY); + flz_maxcopy(dest, src); src += MAX_COPY; dest += MAX_COPY; runs -= MAX_COPY; @@ -296,9 +195,7 @@ flz_finalize(uint32_t runs, const uint8_t *src, uint8_t *dest) return dest; } -static uint8_t * -flz1_match(uint32_t len, uint32_t distance, uint8_t *op) -{ +static uint8_t* flz1_match(uint32_t len, uint32_t distance, uint8_t* op) { --distance; if (FASTLZ_UNLIKELY(len > MAX_LEN - 2)) while (len > MAX_LEN - 2) { @@ -318,46 +215,44 @@ flz1_match(uint32_t len, uint32_t distance, uint8_t *op) return op; } -int -fastlz1_compress(const void *input, int length, void *output) -{ - const uint8_t *ip = (const uint8_t *)input; - const uint8_t *ip_start = ip; - const uint8_t *ip_bound = ip + length - 4; /* because readU32 */ - const uint8_t *ip_limit = ip + length - 12 - 1; - uint8_t *op = (uint8_t *)output; +#define FASTLZ_BOUND_CHECK(cond) \ + if (FASTLZ_UNLIKELY(!(cond))) return 0; + +static int fastlz1_compress(const void* input, int length, void* output) { + const uint8_t* ip = (const uint8_t*)input; + const uint8_t* ip_start = ip; + const uint8_t* ip_bound = ip + length - 4; /* because readU32 */ + const uint8_t* ip_limit = ip + length - 12 - 1; + uint8_t* op = (uint8_t*)output; uint32_t htab[HASH_SIZE]; uint32_t seq, hash; /* initializes hash table */ - for (hash = 0; hash < HASH_SIZE; ++hash) - htab[hash] = 0; + for (hash = 0; hash < HASH_SIZE; ++hash) htab[hash] = 0; /* we start with literal copy */ - const uint8_t *anchor = ip; + const uint8_t* anchor = ip; ip += 2; /* main loop */ while (FASTLZ_LIKELY(ip < ip_limit)) { - const uint8_t *ref; + const uint8_t* ref; uint32_t distance, cmp; /* find potential match */ do { - seq = flz_readu32(ip) & 0xffffff; - hash = flz_hash(seq); - ref = ip_start + htab[hash]; + seq = flz_readu32(ip) & 0xffffff; + hash = flz_hash(seq); + ref = ip_start + htab[hash]; htab[hash] = ip - ip_start; - distance = ip - ref; - cmp = FASTLZ_LIKELY(distance < MAX_L1_DISTANCE) ? flz_readu32(ref) & 0xffffff : 0x1000000; - if (FASTLZ_UNLIKELY(ip >= ip_limit)) - break; + distance = ip - ref; + cmp = FASTLZ_LIKELY(distance < MAX_L1_DISTANCE) ? flz_readu32(ref) & 0xffffff : 0x1000000; + if (FASTLZ_UNLIKELY(ip >= ip_limit)) break; ++ip; } while (seq != cmp); - if (FASTLZ_UNLIKELY(ip >= ip_limit)) - break; + if (FASTLZ_UNLIKELY(ip >= ip_limit)) break; --ip; if (FASTLZ_LIKELY(ip > anchor)) { @@ -365,41 +260,39 @@ fastlz1_compress(const void *input, int length, void *output) } uint32_t len = flz_cmp(ref + 3, ip + 3, ip_bound); - op = flz1_match(len, distance, op); + op = flz1_match(len, distance, op); /* update the hash at match boundary */ ip += len; - seq = flz_readu32(ip); - hash = flz_hash(seq & 0xffffff); + seq = flz_readu32(ip); + hash = flz_hash(seq & 0xffffff); htab[hash] = ip++ - ip_start; seq >>= 8; - hash = flz_hash(seq); + hash = flz_hash(seq); htab[hash] = ip++ - ip_start; anchor = ip; } - uint32_t copy = (uint8_t *)input + length - anchor; - op = flz_finalize(copy, anchor, op); + uint32_t copy = (uint8_t*)input + length - anchor; + op = flz_literals(copy, anchor, op); - return op - (uint8_t *)output; + return op - (uint8_t*)output; } -int -fastlz1_decompress(const void *input, int length, void *output, int maxout) -{ - const uint8_t *ip = (const uint8_t *)input; - const uint8_t *ip_limit = ip + length; - const uint8_t *ip_bound = ip_limit - 2; - uint8_t *op = (uint8_t *)output; - uint8_t *op_limit = op + maxout; - uint32_t ctrl = (*ip++) & 31; +static int fastlz1_decompress(const void* input, int length, void* output, int maxout) { + const uint8_t* ip = (const uint8_t*)input; + const uint8_t* ip_limit = ip + length; + const uint8_t* ip_bound = ip_limit - 2; + uint8_t* op = (uint8_t*)output; + uint8_t* op_limit = op + maxout; + uint32_t ctrl = (*ip++) & 31; while (1) { if (ctrl >= 32) { - uint32_t len = (ctrl >> 5) - 1; - uint32_t ofs = (ctrl & 31) << 8; - const uint8_t *ref = op - ofs - 1; + uint32_t len = (ctrl >> 5) - 1; + uint32_t ofs = (ctrl & 31) << 8; + const uint8_t* ref = op - ofs - 1; if (len == 7 - 1) { FASTLZ_BOUND_CHECK(ip <= ip_bound); len += *ip++; @@ -407,7 +300,7 @@ fastlz1_decompress(const void *input, int length, void *output, int maxout) ref -= *ip++; len += 3; FASTLZ_BOUND_CHECK(op + len <= op_limit); - FASTLZ_BOUND_CHECK(ref >= (uint8_t *)output); + FASTLZ_BOUND_CHECK(ref >= (uint8_t*)output); fastlz_memmove(op, ref, len); op += len; } else { @@ -419,17 +312,14 @@ fastlz1_decompress(const void *input, int length, void *output, int maxout) op += ctrl; } - if (FASTLZ_UNLIKELY(ip > ip_bound)) - break; + if (FASTLZ_UNLIKELY(ip > ip_bound)) break; ctrl = *ip++; } - return op - (uint8_t *)output; + return op - (uint8_t*)output; } -static uint8_t * -flz2_match(uint32_t len, uint32_t distance, uint8_t *op) -{ +static uint8_t* flz2_match(uint32_t len, uint32_t distance, uint8_t* op) { --distance; if (distance < MAX_L2_DISTANCE) { if (len < 7) { @@ -437,8 +327,7 @@ flz2_match(uint32_t len, uint32_t distance, uint8_t *op) *op++ = (distance & 255); } else { *op++ = (7 << 5) + (distance >> 8); - for (len -= 7; len >= 255; len -= 255) - *op++ = 255; + for (len -= 7; len >= 255; len -= 255) *op++ = 255; *op++ = len; *op++ = (distance & 255); } @@ -453,8 +342,7 @@ flz2_match(uint32_t len, uint32_t distance, uint8_t *op) } else { distance -= MAX_L2_DISTANCE; *op++ = (7 << 5) + 31; - for (len -= 7; len >= 255; len -= 255) - *op++ = 255; + for (len -= 7; len >= 255; len -= 255) *op++ = 255; *op++ = len; *op++ = 255; *op++ = distance >> 8; @@ -464,46 +352,41 @@ flz2_match(uint32_t len, uint32_t distance, uint8_t *op) return op; } -int -fastlz2_compress(const void *input, int length, void *output) -{ - const uint8_t *ip = (const uint8_t *)input; - const uint8_t *ip_start = ip; - const uint8_t *ip_bound = ip + length - 4; /* because readU32 */ - const uint8_t *ip_limit = ip + length - 12 - 1; - uint8_t *op = (uint8_t *)output; +static int fastlz2_compress(const void* input, int length, void* output) { + const uint8_t* ip = (const uint8_t*)input; + const uint8_t* ip_start = ip; + const uint8_t* ip_bound = ip + length - 4; /* because readU32 */ + const uint8_t* ip_limit = ip + length - 12 - 1; + uint8_t* op = (uint8_t*)output; uint32_t htab[HASH_SIZE]; uint32_t seq, hash; /* initializes hash table */ - for (hash = 0; hash < HASH_SIZE; ++hash) - htab[hash] = 0; + for (hash = 0; hash < HASH_SIZE; ++hash) htab[hash] = 0; /* we start with literal copy */ - const uint8_t *anchor = ip; + const uint8_t* anchor = ip; ip += 2; /* main loop */ while (FASTLZ_LIKELY(ip < ip_limit)) { - const uint8_t *ref; + const uint8_t* ref; uint32_t distance, cmp; /* find potential match */ do { - seq = flz_readu32(ip) & 0xffffff; - hash = flz_hash(seq); - ref = ip_start + htab[hash]; + seq = flz_readu32(ip) & 0xffffff; + hash = flz_hash(seq); + ref = ip_start + htab[hash]; htab[hash] = ip - ip_start; - distance = ip - ref; - cmp = FASTLZ_LIKELY(distance < MAX_FARDISTANCE) ? flz_readu32(ref) & 0xffffff : 0x1000000; - if (FASTLZ_UNLIKELY(ip >= ip_limit)) - break; + distance = ip - ref; + cmp = FASTLZ_LIKELY(distance < MAX_FARDISTANCE) ? flz_readu32(ref) & 0xffffff : 0x1000000; + if (FASTLZ_UNLIKELY(ip >= ip_limit)) break; ++ip; } while (seq != cmp); - if (FASTLZ_UNLIKELY(ip >= ip_limit)) - break; + if (FASTLZ_UNLIKELY(ip >= ip_limit)) break; --ip; @@ -520,48 +403,45 @@ fastlz2_compress(const void *input, int length, void *output) } uint32_t len = flz_cmp(ref + 3, ip + 3, ip_bound); - op = flz2_match(len, distance, op); + op = flz2_match(len, distance, op); /* update the hash at match boundary */ ip += len; - seq = flz_readu32(ip); - hash = flz_hash(seq & 0xffffff); + seq = flz_readu32(ip); + hash = flz_hash(seq & 0xffffff); htab[hash] = ip++ - ip_start; seq >>= 8; - hash = flz_hash(seq); + hash = flz_hash(seq); htab[hash] = ip++ - ip_start; anchor = ip; } - uint32_t copy = (uint8_t *)input + length - anchor; - op = flz_finalize(copy, anchor, op); + uint32_t copy = (uint8_t*)input + length - anchor; + op = flz_literals(copy, anchor, op); /* marker for fastlz2 */ - *(uint8_t *)output |= (1 << 5); + *(uint8_t*)output |= (1 << 5); - return op - (uint8_t *)output; + return op - (uint8_t*)output; } -int -fastlz2_decompress(const void *input, int length, void *output, int maxout) -{ - const uint8_t *ip = (const uint8_t *)input; - const uint8_t *ip_limit = ip + length; - const uint8_t *ip_bound = ip_limit - 2; - uint8_t *op = (uint8_t *)output; - uint8_t *op_limit = op + maxout; - uint32_t ctrl = (*ip++) & 31; +static int fastlz2_decompress(const void* input, int length, void* output, int maxout) { + const uint8_t* ip = (const uint8_t*)input; + const uint8_t* ip_limit = ip + length; + const uint8_t* ip_bound = ip_limit - 2; + uint8_t* op = (uint8_t*)output; + uint8_t* op_limit = op + maxout; + uint32_t ctrl = (*ip++) & 31; while (1) { if (ctrl >= 32) { - uint32_t len = (ctrl >> 5) - 1; - uint32_t ofs = (ctrl & 31) << 8; - const uint8_t *ref = op - ofs - 1; + uint32_t len = (ctrl >> 5) - 1; + uint32_t ofs = (ctrl & 31) << 8; + const uint8_t* ref = op - ofs - 1; uint8_t code; - if (len == 7 - 1) - do { + if (len == 7 - 1) do { FASTLZ_BOUND_CHECK(ip <= ip_bound); code = *ip++; len += code; @@ -580,7 +460,7 @@ fastlz2_decompress(const void *input, int length, void *output, int maxout) } FASTLZ_BOUND_CHECK(op + len <= op_limit); - FASTLZ_BOUND_CHECK(ref >= (uint8_t *)output); + FASTLZ_BOUND_CHECK(ref >= (uint8_t*)output); fastlz_memmove(op, ref, len); op += len; } else { @@ -592,47 +472,37 @@ fastlz2_decompress(const void *input, int length, void *output, int maxout) op += ctrl; } - if (FASTLZ_UNLIKELY(ip >= ip_limit)) - break; + if (FASTLZ_UNLIKELY(ip >= ip_limit)) break; ctrl = *ip++; } - return op - (uint8_t *)output; + return op - (uint8_t*)output; } -int -fastlz_compress(const void *input, int length, void *output) -{ +int fastlz_compress(const void* input, int length, void* output) { /* for short block, choose fastlz1 */ - if (length < 65536) - return fastlz1_compress(input, length, output); + if (length < 65536) return fastlz1_compress(input, length, output); /* else... */ return fastlz2_compress(input, length, output); } -int -fastlz_decompress(const void *input, int length, void *output, int maxout) -{ +int fastlz_decompress(const void* input, int length, void* output, int maxout) { /* magic identifier for compression level */ - int level = ((*(const uint8_t *)input) >> 5) + 1; + int level = ((*(const uint8_t*)input) >> 5) + 1; - if (level == 1) - return fastlz1_decompress(input, length, output, maxout); - if (level == 2) - return fastlz2_decompress(input, length, output, maxout); + if (level == 1) return fastlz1_decompress(input, length, output, maxout); + if (level == 2) return fastlz2_decompress(input, length, output, maxout); /* unknown level, trigger error */ return 0; } -int -fastlz_compress_level(int level, const void *input, int length, void *output) -{ - if (level == 1) - return fastlz1_compress(input, length, output); - if (level == 2) - return fastlz2_compress(input, length, output); +int fastlz_compress_level(int level, const void* input, int length, void* output) { + if (level == 1) return fastlz1_compress(input, length, output); + if (level == 2) return fastlz2_compress(input, length, output); return 0; } + +#pragma GCC diagnostic pop diff --git a/lib/fastlz/fastlz.h b/lib/fastlz/fastlz.h index fd41e961e04..9172d74de10 100644 --- a/lib/fastlz/fastlz.h +++ b/lib/fastlz/fastlz.h @@ -32,6 +32,10 @@ #define FASTLZ_VERSION_STRING "0.5.0" +#if defined(__cplusplus) +extern "C" { +#endif + /** Compress a block of data in the input buffer and returns the size of compressed block. The size of input buffer is specified by length. The @@ -54,7 +58,7 @@ decompressed using the function fastlz_decompress below. */ -int fastlz_compress_level(int level, const void *input, int length, void *output); +int fastlz_compress_level(int level, const void* input, int length, void* output); /** Decompress a block of compressed data and returns the size of the @@ -72,7 +76,7 @@ int fastlz_compress_level(int level, const void *input, int length, void *output producing the compressed block). */ -int fastlz_decompress(const void *input, int length, void *output, int maxout); +int fastlz_decompress(const void* input, int length, void* output, int maxout); /** DEPRECATED. @@ -84,6 +88,10 @@ int fastlz_decompress(const void *input, int length, void *output, int maxout); version. */ -int fastlz_compress(const void *input, int length, void *output); +int fastlz_compress(const void* input, int length, void* output); + +#if defined(__cplusplus) +} +#endif #endif /* FASTLZ_H */ From 2bdef3a851ae1b7d2f06d3722c84fa9176cc2544 Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Tue, 23 Jun 2026 13:30:47 -0500 Subject: [PATCH 15/33] Use fixture listener in test_EventSystem (#13308) This removes the Catch2 listener from test_EventSystem.cc, using the identical listener from inkevent_test_fixtures.h instead. There is a similar listener in the IOBuffer test, but it is not an exact duplicate (it calls `LibRecordsConfigInit`). (cherry picked from commit bd785f4bd79194836985d01c7637cd6bd45a3c5d) --- .../unit_tests/test_EventSystem.cc | 23 +++---------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/src/iocore/eventsystem/unit_tests/test_EventSystem.cc b/src/iocore/eventsystem/unit_tests/test_EventSystem.cc index b9c24764c7d..22a9d21cb9e 100644 --- a/src/iocore/eventsystem/unit_tests/test_EventSystem.cc +++ b/src/iocore/eventsystem/unit_tests/test_EventSystem.cc @@ -21,6 +21,8 @@ limitations under the License. */ +#include "inkevent_test_fixtures.h" + #include #include #include @@ -28,10 +30,9 @@ #include "iocore/eventsystem/EventSystem.h" #include "tscore/ink_atomic.h" -#include "tscore/Layout.h" #include "tscore/TSSystemState.h" -#include "iocore/utils/diags.i" +using inkevent_test::EventProcessorListener; #define TEST_TIME_SECOND 60 #define TEST_THREADS 2 @@ -83,24 +84,6 @@ TEST_CASE("EventSystem", "[iocore]") } } -struct EventProcessorListener : Catch::EventListenerBase { - using EventListenerBase::EventListenerBase; - - void - testRunStarting(Catch::TestRunInfo const & /* testRunInfo ATS_UNUSED */) override - { - Layout::create(); - init_diags("", nullptr); - RecProcessInit(); - - ink_event_system_init(EVENT_SYSTEM_MODULE_PUBLIC_VERSION); - eventProcessor.start(TEST_THREADS, 1048576); // Hardcoded stacksize at 1MB - - EThread *main_thread = new EThread; - main_thread->set_specific(); - } -}; - CATCH_REGISTER_LISTENER(EventProcessorListener); TEST_CASE("EventSystemUnixSocket", "[iocore][sock]") From 56d477ccce93b48f2e414b8d0bb6332fe4668b38 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Thu, 25 Jun 2026 09:44:40 -0500 Subject: [PATCH 16/33] Downgrade inbound H2 stream error log (#13316) Malformed client HTTP/2 streams can produce noisy error-level diagnostics even though the malformed parse details are now available in transaction logs via apache/trafficserver#13059. This downgrades the inbound rcv_frame stream-error diagnostic to Http2StreamDebug while leaving outbound stream creation errors at Error level. This also updates the malformed request AuTest to look for the debug diagnostic in traffic.out and relies on Http2StreamDebug to include the session and stream identifiers. (cherry picked from commit 87cf5ec24cedc2dd95a33fdb743f3421d10824a1) --- src/proxy/http2/Http2ConnectionState.cc | 4 ++-- tests/gold_tests/connect/h2_malformed_request_logging.test.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/proxy/http2/Http2ConnectionState.cc b/src/proxy/http2/Http2ConnectionState.cc index 36d16d70d74..ca7b0eeb707 100644 --- a/src/proxy/http2/Http2ConnectionState.cc +++ b/src/proxy/http2/Http2ConnectionState.cc @@ -1521,8 +1521,8 @@ Http2ConnectionState::rcv_frame(const Http2Frame *frame) // The Http2ClientSession will shutdown because connection_state.is_state_closed() will be true } else if (error.cls == Http2ErrorClass::HTTP2_ERROR_CLASS_STREAM) { if (error.msg) { - Error("HTTP/2 stream error code=0x%02x client_ip=%s session_id=%" PRId64 " stream_id=%u %s", static_cast(error.code), - client_ip, session->get_connection_id(), stream_id, error.msg); + Http2StreamDebug(session, stream_id, "HTTP/2 stream error code=0x%02x client_ip=%s %s", static_cast(error.code), + client_ip, error.msg); } this->send_rst_stream_frame(stream_id, error.code); diff --git a/tests/gold_tests/connect/h2_malformed_request_logging.test.py b/tests/gold_tests/connect/h2_malformed_request_logging.test.py index 79749677891..95ff7952210 100644 --- a/tests/gold_tests/connect/h2_malformed_request_logging.test.py +++ b/tests/gold_tests/connect/h2_malformed_request_logging.test.py @@ -115,7 +115,7 @@ def _setup_ts(self): format: malformed_h2_request mode: ascii """.split('\n')) - self._ts.Disk.diags_log.Content = Testers.ContainsExpression( + self._ts.Disk.traffic_out.Content += Testers.ContainsExpression( 'recv headers malformed request', 'ATS should reject malformed requests at the HTTP/2 layer.', ) From 48a18a065c2b39130ca5587d6fbb9754a69a1d68 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Wed, 24 Jun 2026 20:43:15 -0500 Subject: [PATCH 17/33] Fix stale_response FORCE_SIE enum (#13326) Give the FORCE_SIE test mode its own enum value so the stale-if-error rows are not aliases of FORCE_SWR. Add a load-time guard to catch future duplicate OptionType values before the autest runs. (cherry picked from commit 9fe25cef35e85fa0c9b2698bf03bf03f352430ee) --- .../pluginTest/stale_response/stale_response.test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/gold_tests/pluginTest/stale_response/stale_response.test.py b/tests/gold_tests/pluginTest/stale_response/stale_response.test.py index 272814ac6cb..b073c45760b 100644 --- a/tests/gold_tests/pluginTest/stale_response/stale_response.test.py +++ b/tests/gold_tests/pluginTest/stale_response/stale_response.test.py @@ -33,7 +33,10 @@ class OptionType(Enum): NONE = 0 DEFAULT_DIRECTIVES = 1 FORCE_SWR = 2 - FORCE_SIE = 2 + FORCE_SIE = 3 + + +assert len({option.value for option in OptionType.__members__.values()}) == len(OptionType.__members__) class TestStaleResponse: From 108f5ad4f137db31382406e04617af7d80f28596 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Wed, 24 Jun 2026 20:43:27 -0500 Subject: [PATCH 18/33] Clarify HttpSM cache action cleanup (#13327) Describe what cancel_pending_action() does in the Coverity suppression rationale. The helper cancels pending cache work and clears tracked pointers rather than only setting flags. (cherry picked from commit 00adbb80764c2a91209d1090ba9b7562ee7ad375) --- src/proxy/http/HttpSM.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index e0ec5937d0b..9a623ffa9e4 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -293,7 +293,7 @@ HttpSM::~HttpSM() Error("Exception in ~HttpSM during m_remap->release"); } - // coverity[exn_spec_violation] - cancel_pending_action() only sets boolean flags + // coverity[exn_spec_violation] - cancel_pending_action() cancels pending cache work and clears tracked pointers cache_sm.cancel_pending_action(); mutex.clear(); From 097879f652fad5f4069ff365b983601f538ffd17 Mon Sep 17 00:00:00 2001 From: mmustafasenoglu Date: Mon, 6 Jul 2026 19:13:19 +0300 Subject: [PATCH 19/33] docs: add call condition note for TSUrlHostGet (#13313) * docs: add call condition note for TSUrlHostGet Add a note to the TSUrlHostGet documentation indicating that it should only be called after TS_HTTP_POST_REMAP_HOOK. For earlier hooks like TS_HTTP_READ_REQUEST_HDR_HOOK, TSHttpHdrHostGet should be used instead. Fixes #5742 * docs: fix unknown interpreted text role data Replace :data: with double backticks for hook names, consistent with the rest of the documentation. Fixes #5742 * docs: trigger CI for TSUrlHostGet call condition note * docs: document TSHttpHdrUrlGet hook availability, fix redirect_1 to use TSHttpHdrHostGet - TSHttpHdrUrlGet: add note that URL components may not be available at early hooks, recommend TSHttpHdrHostGet for reliable host retrieval - TSUrlHostGet: add call condition note (TS_HTTP_POST_REMAP_HOOK onwards) and cross-reference to TSHttpHdrHostGet - TSHttpHdrHostGet: add cross-references to TSHttpHdrUrlGet and TSUrlHostGet - redirect_1: replace TSHttpHdrUrlGet+TSUrlHostGet with TSHttpHdrHostGet which works correctly at TS_HTTP_READ_REQUEST_HDR_HOOK * fix(docs): replace :c:macro: with double backticks for TS_HTTP_READ_REQUEST_HDR_HOOK * fix(docs): replace :c:macro: with double backticks for TS_HTTP_READ_REQUEST_HDR_HOOK * fix(docs): replace :c:macro: with double backticks for TS_HTTP_READ_REQUEST_HDR_HOOK * fix: restore TSHttpHdrHostGet docs content and add cross-references - Restored accidentally emptied TSHttpHdrHostGet.en.rst - Added call condition note explaining TSHttpHdrHostGet vs TSUrlHostGet - Added See Also cross-references to related APIs Fixes docs build warning reported by JosiahWI * fix: add trailing newline to redirect_1.cc * fix: restore upstream license header formatting * fix: restore upstream license header formatting * fix: restore upstream license header formatting * fix: restore upstream license header in redirect_1.cc * fix: restore license header and fix broken string literal * fix: restore blank line after title underline * fix: add blank line between title and Synopsis * fix: apply code changes on top of upstream cleanly * fix: restore redirect_1.cc with correct API changes * fix: restore redirect_1.cc with correct TSHttpHdrHostGet usage * fix: apply TSHttpHdrHostGet changes cleanly on upstream * fix: restore blank line between title and Synopsis --------- Co-authored-by: Mustafa Senoglu (cherry picked from commit 16c434c28251804ac8022950cbbe913a6a866b4a) --- .../api/functions/TSHttpHdrHostGet.en.rst | 16 ++++++++++++++++ .../api/functions/TSHttpHdrUrlGet.en.rst | 14 +++++++++++++- .../api/functions/TSUrlHostGet.en.rst | 11 +++++++++++ example/plugins/c-api/redirect_1/redirect_1.cc | 13 ++----------- 4 files changed, 42 insertions(+), 12 deletions(-) diff --git a/doc/developer-guide/api/functions/TSHttpHdrHostGet.en.rst b/doc/developer-guide/api/functions/TSHttpHdrHostGet.en.rst index 381add15a64..57f1e1b9828 100644 --- a/doc/developer-guide/api/functions/TSHttpHdrHostGet.en.rst +++ b/doc/developer-guide/api/functions/TSHttpHdrHostGet.en.rst @@ -43,3 +43,19 @@ header field. This is much faster than calling :func:`TSHttpTxnEffectiveUrlStringGet` and extracting the host from the result. + +.. note:: + + :func:`TSHttpHdrHostGet` checks both the URL and the ``Host`` header field, + making it reliable at any hook stage. In contrast, :func:`TSUrlHostGet` + operates only on the URL object obtained from :func:`TSHttpHdrUrlGet`. In + early hooks such as ``TS_HTTP_READ_REQUEST_HDR_HOOK``, the URL object may + not yet be fully parsed, and :func:`TSUrlHostGet` may return ``NULL`` even + when a ``Host`` header is present. + +See Also +======== + +:func:`TSUrlHostGet`, +:func:`TSHttpHdrUrlGet`, +:func:`TSHttpTxnEffectiveUrlStringGet` diff --git a/doc/developer-guide/api/functions/TSHttpHdrUrlGet.en.rst b/doc/developer-guide/api/functions/TSHttpHdrUrlGet.en.rst index f3ecc7737cc..ddeca6ed080 100644 --- a/doc/developer-guide/api/functions/TSHttpHdrUrlGet.en.rst +++ b/doc/developer-guide/api/functions/TSHttpHdrUrlGet.en.rst @@ -42,6 +42,16 @@ The value placed in :arg:`locp` is stable only for a single callback, as other c change the URL object itself (see :func:`TSHttpHdrUrlSet`), not just the data in it. That value is also valid only if this function return ``TS_SUCCESS``. +.. note:: + + Not all URL components may be available at every hook stage. In early hooks + such as ``TS_HTTP_READ_REQUEST_HDR_HOOK``, the URL object may not yet be + fully parsed. In particular, the host component retrieved via + :func:`TSUrlHostGet` may be ``NULL`` even when a ``Host`` header is present. + For reliable host retrieval across all hook stages, use + :func:`TSHttpHdrHostGet` instead, which checks both the URL and the ``Host`` + header field. + See Also ======== @@ -49,4 +59,6 @@ See Also :manpage:`TSHttpTxnClientReqGet(3ts)`, :manpage:`TSHttpTxnServerReqGet(3ts)`, :manpage:`TSHttpTxnServerRespGet(3ts)`, -:manpage:`TSHttpTxnClientRespGet(3ts)` +:manpage:`TSHttpTxnClientRespGet(3ts)`, +:manpage:`TSHttpHdrHostGet(3ts)`, +:manpage:`TSUrlHostGet(3ts)` diff --git a/doc/developer-guide/api/functions/TSUrlHostGet.en.rst b/doc/developer-guide/api/functions/TSUrlHostGet.en.rst index 1497e625c95..6b7b587fbe3 100644 --- a/doc/developer-guide/api/functions/TSUrlHostGet.en.rst +++ b/doc/developer-guide/api/functions/TSUrlHostGet.en.rst @@ -71,6 +71,16 @@ scheme. :arg:`offset` within the marshal buffer :arg:`bufp`. If there is no explicit port number in the URL, zero is returned. +.. note:: + + :func:`TSUrlHostGet` operates on a URL object obtained from + :func:`TSHttpHdrUrlGet`. In early hooks such as + ``TS_HTTP_READ_REQUEST_HDR_HOOK``, the URL object may not yet be fully + parsed, and :func:`TSUrlHostGet` may return ``NULL`` even when a ``Host`` + header is present. For reliable host retrieval at any hook stage, use + :func:`TSHttpHdrHostGet` instead, which checks both the URL and the + ``Host`` header field. + Return Values ============= @@ -90,6 +100,7 @@ See Also :manpage:`TSAPI(3ts)`, :manpage:`TSUrlCreate(3ts)`, :manpage:`TSHttpHdrUrlGet(3ts)`, +:manpage:`TSHttpHdrHostGet(3ts)`, :manpage:`TSUrlHostSet(3ts)`, :manpage:`TSUrlStringGet(3ts)`, :manpage:`TSUrlPercentEncode(3ts)` diff --git a/example/plugins/c-api/redirect_1/redirect_1.cc b/example/plugins/c-api/redirect_1/redirect_1.cc index 701446a5661..6a9a9a71fa6 100644 --- a/example/plugins/c-api/redirect_1/redirect_1.cc +++ b/example/plugins/c-api/redirect_1/redirect_1.cc @@ -97,7 +97,7 @@ static void handle_client_lookup(TSHttpTxn txnp, TSCont contp) { TSMBuffer bufp; - TSMLoc hdr_loc, url_loc; + TSMLoc hdr_loc; int host_length; in_addr_t clientip = 0; @@ -130,16 +130,9 @@ handle_client_lookup(TSHttpTxn txnp, TSCont contp) goto done; } - if (TSHttpHdrUrlGet(bufp, hdr_loc, &url_loc) != TS_SUCCESS) { - TSError("[%s] Couldn't retrieve request url", PLUGIN_NAME); - TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); - goto done; - } - - host = TSUrlHostGet(bufp, url_loc, &host_length); + host = TSHttpHdrHostGet(bufp, hdr_loc, &host_length); if (!host) { TSError("[%s] Couldn't retrieve request hostname", PLUGIN_NAME); - TSHandleMLocRelease(bufp, hdr_loc, url_loc); TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); goto done; } @@ -148,7 +141,6 @@ handle_client_lookup(TSHttpTxn txnp, TSCont contp) * Check to see if the client is already headed to the redirect site. */ if (strncmp(host, url_redirect, host_length) == 0) { - TSHandleMLocRelease(bufp, hdr_loc, url_loc); TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); goto done; } @@ -159,7 +151,6 @@ handle_client_lookup(TSHttpTxn txnp, TSCont contp) update_redirected_method_stats(bufp, hdr_loc); - TSHandleMLocRelease(bufp, hdr_loc, url_loc); TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc); /* From e2633c5764cec64edab505363d4133e208e6ed68 Mon Sep 17 00:00:00 2001 From: Javid Khan Date: Tue, 7 Jul 2026 05:45:12 +0530 Subject: [PATCH 20/33] Track remaining length while decoding qpack header block (#13361) (cherry picked from commit 082e87545eeeb6141b492d833d298dab2f9e03d5) --- src/proxy/http3/QPACK.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/proxy/http3/QPACK.cc b/src/proxy/http3/QPACK.cc index dfdd2d278b3..1469d69de12 100644 --- a/src/proxy/http3/QPACK.cc +++ b/src/proxy/http3/QPACK.cc @@ -923,6 +923,7 @@ QPACK::_decode_header(const uint8_t *header_block, size_t header_block_len, HTTP return -1; } pos += ret; + remain_len -= ret; uint16_t largest_reference = tmp; uint64_t delta_base_index; @@ -939,7 +940,8 @@ QPACK::_decode_header(const uint8_t *header_block, size_t header_block_len, HTTP } else { base_index = largest_reference + delta_base_index; } - pos += ret; + pos += ret; + remain_len -= ret; uint32_t decoded_header_list_size = 0; @@ -969,7 +971,8 @@ QPACK::_decode_header(const uint8_t *header_block, size_t header_block_len, HTTP break; } - pos += ret; + pos += ret; + remain_len -= ret; } return ret; From 78e4961103453b2d8d0a415e652116a826e636a2 Mon Sep 17 00:00:00 2001 From: Masakazu Kitajo Date: Thu, 9 Jul 2026 12:09:29 -0600 Subject: [PATCH 21/33] jax_fingerprint: Reduce allocations and gate methods at build time (#13338) * jax_fingerprint: Reduce allocations and gate methods at build time Trim per-connection memory work in the hybrid (global + remap) setup by collapsing the per-connection table of fingerprint contexts to an inline structure and by passing fingerprints into the context without an intermediate copy. Lookup behavior is unchanged. Add ENABLE_JAX_METHODS as the configure-time switch for which fingerprint methods are compiled in. CMake derives the per-method preprocessor defines, the dispatcher table in plugin.cc, and the slot count of the inline context table from the same list. An empty list or an unknown method directory fails at configure time. Include a developer README covering the per-method file layout, the build-time switches, and the naming rules that the CMake glob relies on. * Address copilot comments (cherry picked from commit a9e26aa45387087b40063db05212713456925bd3) --- .../jax_fingerprint/CMakeLists.txt | 56 ++++--- plugins/experimental/jax_fingerprint/README | 144 ++++++++++++++++++ .../experimental/jax_fingerprint/context.cc | 2 +- .../experimental/jax_fingerprint/context.h | 3 +- .../jax_fingerprint/context_map.h | 106 ++++++------- .../experimental/jax_fingerprint/plugin.cc | 41 +++-- 6 files changed, 264 insertions(+), 88 deletions(-) create mode 100644 plugins/experimental/jax_fingerprint/README diff --git a/plugins/experimental/jax_fingerprint/CMakeLists.txt b/plugins/experimental/jax_fingerprint/CMakeLists.txt index 8ace4738047..d87f29c2bf7 100644 --- a/plugins/experimental/jax_fingerprint/CMakeLists.txt +++ b/plugins/experimental/jax_fingerprint/CMakeLists.txt @@ -15,6 +15,37 @@ # ####################### +set(ENABLE_JAX_METHODS + "ja3;ja4;ja4h" + CACHE STRING "Semicolon-separated list of fingerprint methods to compile into jax_fingerprint" +) + +list(LENGTH ENABLE_JAX_METHODS _jax_method_count) +if(_jax_method_count EQUAL 0) + message(FATAL_ERROR "ENABLE_JAX_METHODS must list at least one method (e.g. ja3;ja4;ja4h)") +endif() + +set(_jax_plugin_method_sources "") +set(_jax_test_method_sources "") +set(_jax_method_defs "") +foreach(_m IN LISTS ENABLE_JAX_METHODS) + if(NOT IS_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/${_m}") + message(FATAL_ERROR "ENABLE_JAX_METHODS references unknown method '${_m}' (no ${_m}/ directory)") + endif() + file(GLOB _srcs CONFIGURE_DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/${_m}/*.cc") + set(_plugin_srcs ${_srcs}) + set(_test_srcs ${_srcs}) + list(FILTER _plugin_srcs EXCLUDE REGEX "/test\\.cc$") + # method.cc and tls_client_hello_summary.cc bind algorithm logic to ATS APIs (TSClientHello + # etc.) that the test binary does not link; everything else is pure algorithm/data and is + # safe to include in test_jax. + list(FILTER _test_srcs EXCLUDE REGEX "/(method|tls_client_hello_summary)\\.cc$") + list(APPEND _jax_plugin_method_sources ${_plugin_srcs}) + list(APPEND _jax_test_method_sources ${_test_srcs}) + string(TOUPPER ${_m} _m_upper) + list(APPEND _jax_method_defs "ENABLE_JAX_METHOD_${_m_upper}") +endforeach() + add_atsplugin( jax_fingerprint plugin.cc @@ -22,37 +53,20 @@ add_atsplugin( userarg.cc header.cc log.cc - ja3/method.cc - ja3/utils.cc - ja4/method.cc - ja4/ja4.cc - ja4/datasource.cc - ja4/tls_client_hello_summary.cc - ja4h/method.cc - ja4h/ja4h.cc - ja4h/datasource.cc common/utils.cc + ${_jax_plugin_method_sources} ) target_link_libraries(jax_fingerprint PRIVATE OpenSSL::Crypto OpenSSL::SSL) target_include_directories(jax_fingerprint BEFORE PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) +target_compile_definitions(jax_fingerprint PRIVATE ${_jax_method_defs} JAX_FINGERPRINT_MAX_METHODS=${_jax_method_count}) verify_global_plugin(jax_fingerprint) verify_remap_plugin(jax_fingerprint) if(BUILD_TESTING) - add_executable( - test_jax - ja3/test.cc - ja3/utils.cc - ja4/test.cc - ja4/ja4.cc - ja4/datasource.cc - ja4h/test.cc - ja4h/ja4h.cc - ja4h/datasource.cc - common/utils.cc - ) + add_executable(test_jax common/utils.cc ${_jax_test_method_sources}) target_link_libraries(test_jax PRIVATE Catch2::Catch2WithMain OpenSSL::Crypto OpenSSL::SSL) target_include_directories(test_jax BEFORE PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) + target_compile_definitions(test_jax PRIVATE ${_jax_method_defs}) add_catch2_test(NAME test_jax COMMAND test_jax) endif() diff --git a/plugins/experimental/jax_fingerprint/README b/plugins/experimental/jax_fingerprint/README new file mode 100644 index 00000000000..ca9502e0834 --- /dev/null +++ b/plugins/experimental/jax_fingerprint/README @@ -0,0 +1,144 @@ +ATS (Apache Traffic Server) JAx Fingerprint Plugin + +User-facing documentation lives in +`doc/admin-guide/plugins/jax_fingerprint.en.rst`. This README covers the +developer side: how the plugin is organised, and what it takes to add a new +fingerprinting method. + + +Plugin layout +------------- + +``` +jax_fingerprint/ + plugin.cc / plugin.h Plugin entry points, hook handlers, method dispatch. + context.cc / .h JAxContext: per-connection (or per-txn) fingerprint state. + context_map.h Inline fixed-size table that multiplexes JAxContexts + by method name on a single user-arg slot. + userarg.cc / userarg.h Shared user-arg slot reservation and accessors. + header.cc / .h Set/append/remove the fingerprint and via headers. + log.cc / .h Optional log file output for fingerprints. + method.h Abstract Method struct (name, type, callbacks). + Unaware of any concrete method. + config.h PluginConfig, parsing helpers. + common/ Shared utilities (hash stringification, etc.). + ja3/ ja4/ ja4h/ One subdirectory per fingerprinting method. +``` + +Each method subdirectory follows the conventions described below. + + +Adding a new method +------------------- + +Suppose you want to add a method called `mymethod`. The steps are: + + 1. Create a directory `mymethod/` under this plugin. + + 2. Drop in the source files (see *File naming* below for what each name + means to the build): + + mymethod/method.h Declares `extern struct Method method;` in + your namespace (e.g. `namespace mymethod`). + mymethod/method.cc Defines `Method method = { "MYMETHOD", ... }` + and the on_client_hello / on_request callback. + This is the file allowed to use TS APIs + (TSVConn, TSClientHello, ...). + mymethod/mymethod.cc/.h Pure algorithm. No TS API. + mymethod/datasource.cc/.h Abstract data source the algorithm consumes + (mirror ja4/datasource.h). + mymethod/test.cc Catch2 unit tests for the algorithm. + + 3. Register the method in plugin.cc. The dispatcher is a `constexpr` table + gated by `ENABLE_JAX_METHOD_*` macros. Add two entries: + + #ifdef ENABLE_JAX_METHOD_MYMETHOD + #include "mymethod/method.h" + #endif + + ... + + constexpr Method const *METHODS[] = { + ... + #ifdef ENABLE_JAX_METHOD_MYMETHOD + &mymethod::method, + #endif + }; + + 4. Enable the method at build time: + + cmake -B build -DENABLE_JAX_METHODS="ja3;ja4;ja4h;mymethod" + + CMake automatically picks up `mymethod/*.cc`, defines + `ENABLE_JAX_METHOD_MYMETHOD`, and bumps `JAX_FINGERPRINT_MAX_METHODS` to + match the list length. + + 5. Use the method: + + jax_fingerprint.so --method MYMETHOD --header x-my-fingerprint + + +File naming +----------- + +The top-level CMakeLists.txt globs `*.cc` from each enabled method directory +and applies these filters: + + test.cc Built only into the test_jax binary (Catch2 unit tests). + + method.cc Built only into jax_fingerprint.so. This is the one file + in each method directory that is allowed to call TS APIs + (TSVConn, TSClientHello, ...). Keep all TS glue here. + + every other Built into BOTH jax_fingerprint.so and test_jax. Keep these + .cc files free of TS API calls so the unit-test binary, which + does not link the TS plugin SDK, still builds. + +If you genuinely need to spread TS-API code across more than `method.cc` in +a method directory, you have to extend the exclusion regex in +CMakeLists.txt. There is one such historical exception today -- +`ja4/tls_client_hello_summary.cc` -- and the regex carves it out by name. +Prefer not to add new files in that category: split the algorithm so the +TS-facing piece stays in `method.cc` and the pure-data piece is its own +file. If a future case really does justify extending the regex, name the +file something that documents the constraint (for instance `*_ts.cc`) and +update the regex accordingly. + +The method directory name (lower case) maps to the macro +`ENABLE_JAX_METHOD_` (so `mymethod` -> `ENABLE_JAX_METHOD_MYMETHOD`). +`Method::name` should be the user-visible spelling (e.g. "MYMETHOD"), +matched against `--method` on the command line. + +`Method::name` must reference a string with static storage duration (a +string literal is the natural fit). ContextMap stores `std::string_view` +slot keys that alias `Method::name`; if `name` ever pointed at temporary +storage, the keys would dangle. + + +Build-time configuration +------------------------ + + ENABLE_JAX_METHODS Semicolon-separated list of methods to + compile in. Default: "ja3;ja4;ja4h". + Empty list or unknown directory name causes + a FATAL_ERROR at configure time. + + JAX_FINGERPRINT_MAX_METHODS Auto-derived from the length of + ENABLE_JAX_METHODS, passed to the plugin as + a compile definition. Bounds the inline + ContextMap slot array. The header has a + fallback `#define JAX_FINGERPRINT_MAX_METHODS 8` + so it is also valid standalone (e.g. for + IDE indexing). + + +Unit tests vs. AuTest +--------------------- + + * Catch2 unit tests (`test_jax`) live in each `*/test.cc` and cover pure + algorithm logic. Built when BUILD_TESTING is on; run via ctest or the + test_jax binary directly. + + * End-to-end AuTests live under `tests/gold_tests/pluginTest/jax_fingerprint/` + and exercise the plugin via Proxy Verifier replay yamls. They require a + full install (traffic_server, traffic_layout, the plugin .so). diff --git a/plugins/experimental/jax_fingerprint/context.cc b/plugins/experimental/jax_fingerprint/context.cc index f9f6b06a1c4..9eac2a093b9 100644 --- a/plugins/experimental/jax_fingerprint/context.cc +++ b/plugins/experimental/jax_fingerprint/context.cc @@ -59,7 +59,7 @@ JAxContext::get_fingerprint() const } void -JAxContext::set_fingerprint(const std::string &fingerprint) +JAxContext::set_fingerprint(std::string_view fingerprint) { this->_fingerprint = fingerprint; Dbg(dbg_ctl, "Fingerprint: %s", this->_fingerprint.c_str()); diff --git a/plugins/experimental/jax_fingerprint/context.h b/plugins/experimental/jax_fingerprint/context.h index 2d3bdaff6ed..ed727152dae 100644 --- a/plugins/experimental/jax_fingerprint/context.h +++ b/plugins/experimental/jax_fingerprint/context.h @@ -28,6 +28,7 @@ #include #include +#include class JAxContext { @@ -36,7 +37,7 @@ class JAxContext ~JAxContext(); const std::string &get_fingerprint() const; - void set_fingerprint(const std::string &fingerprint); + void set_fingerprint(std::string_view fingerprint); const char *get_addr() const; const char *get_method_name() const; diff --git a/plugins/experimental/jax_fingerprint/context_map.h b/plugins/experimental/jax_fingerprint/context_map.h index aa9b77a9bd8..1bdcf53bbe0 100644 --- a/plugins/experimental/jax_fingerprint/context_map.h +++ b/plugins/experimental/jax_fingerprint/context_map.h @@ -27,46 +27,62 @@ #pragma once -#include "config.h" #include "context.h" -#include +#include "ts/ts.h" + +#include +#include #include -#include -#include +#include + +#ifndef JAX_FINGERPRINT_MAX_METHODS +#define JAX_FINGERPRINT_MAX_METHODS 8 +#endif /** * @brief Container holding JAxContext instances for multiple methods. * * ATS has a limited number of user arg slots (~4 per type). When loading * many jax_fingerprint instances, we share a single slot and store all - * contexts in this map, keyed by method name. + * contexts in this inline fixed-size table, keyed by method name. The + * table size is set at build time via JAX_FINGERPRINT_MAX_METHODS. + * + * Lookup is a linear scan over std::string_view keys (Method::name points + * to a string literal with static storage duration, so storing the view + * is safe). */ class ContextMap { public: + static constexpr std::size_t MAX_METHODS = JAX_FINGERPRINT_MAX_METHODS; + static_assert(MAX_METHODS >= 1, "Must accommodate at least one fingerprinting method"); + ~ContextMap() { - for (auto &pair : m_contexts) { - delete pair.second; + for (std::size_t i = 0; i < _size; ++i) { + delete _slots[i].second; } } /** * @brief Store a context for a method. - * @param[in] method_name The method name (e.g., "JA3", "JA4"). + * @param[in] method_name The method name (e.g., "JA3", "JA4"). Must reference + * a string with lifetime >= the ContextMap (typically a string literal). * @param[in] ctx The context to store. Ownership is transferred. */ void set(std::string_view method_name, JAxContext *ctx) { - auto it = find_context(method_name); - if (it != m_contexts.end()) { - delete it->second; - it->second = ctx; - } else { - m_contexts.emplace(std::string{method_name}, ctx); + for (std::size_t i = 0; i < _size; ++i) { + if (_slots[i].first == method_name) { + delete _slots[i].second; + _slots[i].second = ctx; + return; + } } + TSReleaseAssert(_size < MAX_METHODS); + _slots[_size++] = {method_name, ctx}; } /** @@ -75,10 +91,14 @@ class ContextMap * @return The context, or nullptr if not found. */ JAxContext * - get(std::string_view method_name) + get(std::string_view method_name) const { - auto it = find_context(method_name); - return it != m_contexts.end() ? it->second : nullptr; + for (std::size_t i = 0; i < _size; ++i) { + if (_slots[i].first == method_name) { + return _slots[i].second; + } + } + return nullptr; } /** @@ -88,10 +108,16 @@ class ContextMap void remove(std::string_view method_name) { - auto it = find_context(method_name); - if (it != m_contexts.end()) { - delete it->second; - m_contexts.erase(it); + for (std::size_t i = 0; i < _size; ++i) { + if (_slots[i].first == method_name) { + delete _slots[i].second; + --_size; + if (i != _size) { + _slots[i] = _slots[_size]; + } + _slots[_size] = {}; + return; + } } } @@ -102,42 +128,10 @@ class ContextMap bool empty() const { - return m_contexts.empty(); + return _size == 0; } private: - using ContextStorage = std::unordered_map>; - - /** Find context by method name with C++20 generic lookup fallback. - * - * C++20 generic unordered lookup allows finding with std::string_view in a - * std::unordered_map without creating a temporary string. - * For standard libraries without this feature, we fall back to constructing - * a std::string for the lookup. - * - * @param[in] method_name The method name to look up. - * @return Iterator to the found element, or end() if not found. - */ - ContextStorage::iterator - find_context(std::string_view method_name) - { -#ifdef __cpp_lib_generic_unordered_lookup - return m_contexts.find(method_name); -#else - return m_contexts.find(std::string{method_name}); -#endif - } - - /** const_iterator @overload */ - ContextStorage::const_iterator - find_context(std::string_view method_name) const - { -#ifdef __cpp_lib_generic_unordered_lookup - return m_contexts.find(method_name); -#else - return m_contexts.find(std::string{method_name}); -#endif - } - - ContextStorage m_contexts; + std::array, MAX_METHODS> _slots{}; + std::size_t _size{0}; }; diff --git a/plugins/experimental/jax_fingerprint/plugin.cc b/plugins/experimental/jax_fingerprint/plugin.cc index aca0ba4effd..374d66faecb 100644 --- a/plugins/experimental/jax_fingerprint/plugin.cc +++ b/plugins/experimental/jax_fingerprint/plugin.cc @@ -27,9 +27,15 @@ #include "header.h" #include "log.h" +#ifdef ENABLE_JAX_METHOD_JA4 #include "ja4/method.h" +#endif +#ifdef ENABLE_JAX_METHOD_JA4H #include "ja4h/method.h" +#endif +#ifdef ENABLE_JAX_METHOD_JA3 #include "ja3/method.h" +#endif #include #include @@ -49,6 +55,21 @@ DbgCtl dbg_ctl{PLUGIN_NAME}; +namespace +{ +constexpr Method const *METHODS[] = { +#ifdef ENABLE_JAX_METHOD_JA4 + &ja4::method, +#endif +#ifdef ENABLE_JAX_METHOD_JA4H + &ja4h::method, +#endif +#ifdef ENABLE_JAX_METHOD_JA3 + &ja3::method, +#endif +}; +} // namespace + static bool read_config_option(int argc, char const *argv[], PluginConfig &config) { @@ -71,18 +92,20 @@ read_config_option(int argc, char const *argv[], PluginConfig &config) case '?': Dbg(dbg_ctl, "Unrecognized command argument."); break; - case 'M': - if (strcmp("JA4", optarg) == 0) { - config.method = ja4::method; - } else if (strcmp("JA4H", optarg) == 0) { - config.method = ja4h::method; - } else if (strcmp("JA3", optarg) == 0) { - config.method = ja3::method; - } else { + case 'M': { + bool found = false; + for (auto const *m : METHODS) { + if (m->name == optarg) { + config.method = *m; + found = true; + break; + } + } + if (!found) { Dbg(dbg_ctl, "Unexpected method: %s", optarg); return false; } - break; + } break; case 'm': if (strcmp("overwrite", optarg) == 0) { config.mode = Mode::OVERWRITE; From f5569b0c1b3fd1b8eecf5420b4bf4dd7ad6acd20 Mon Sep 17 00:00:00 2001 From: Phong Nguyen Date: Mon, 13 Jul 2026 15:29:08 -0700 Subject: [PATCH 22/33] Fix CLFUS RAM cache value metric broken by integer division (#13233) PR #11733 rewrote the CACHE_VALUE_HITS_SIZE cast so static_cast wraps the whole quotient, making (hits + 1) / (size + overhead) integer division. It truncates to 0 for normal object sizes, zeroing the value metric and collapsing CLFUS to FIFO: no promote-on-hit, no clock second chance, and no value-based ghost re-admission. Bind the cast to the numerator to restore floating-point division, and add the ram_cache_clfus_value regression test as a guard (it fails on the pre-fix macro and passes after). (cherry picked from commit d1d02c3f2638554fff3eed95de01fa778aecf10b) --- src/iocore/cache/RamCacheCLFUS.cc | 102 +++++++++++++++++++++--------- 1 file changed, 71 insertions(+), 31 deletions(-) diff --git a/src/iocore/cache/RamCacheCLFUS.cc b/src/iocore/cache/RamCacheCLFUS.cc index 639d2faa33c..feb09781eb8 100644 --- a/src/iocore/cache/RamCacheCLFUS.cc +++ b/src/iocore/cache/RamCacheCLFUS.cc @@ -31,24 +31,25 @@ #include "iocore/eventsystem/Tasks.h" #include "fastlz/fastlz.h" #include "tscore/CryptoHash.h" +#include "tscore/Regression.h" #include #ifdef HAVE_LZMA_H #include #endif -#define REQUIRED_COMPRESSION 0.9 // must get to this size or declared incompressible -#define REQUIRED_SHRINK 0.8 // must get to this size or keep original buffer (with padding) -#define HISTORY_HYSTERIA 10 // extra temporary history -#define ENTRY_OVERHEAD 256 // per-entry overhead to consider when computing cache value/size -#define LZMA_BASE_MEMLIMIT (64 * 1024 * 1024) // #define CHECK_ACOUNTING 1 // very expensive double checking of all sizes -#define REQUEUE_HITS(_h) ((_h) ? ((_h) - 1) : 0) -#define CACHE_VALUE_HITS_SIZE(_h, _s) (static_cast(((_h) + 1) / ((_s) + ENTRY_OVERHEAD))) -#define CACHE_VALUE(_x) CACHE_VALUE_HITS_SIZE((_x)->hits, (_x)->size) +constexpr double required_compression = 0.9; +constexpr double required_shrink = 0.8; +constexpr uint32_t history_hysteria = 10; +constexpr uint32_t entry_overhead = 256; // per-entry overhead to consider when computing cache value/size -#define AVERAGE_VALUE_OVER 100 -#define REQUEUE_LIMIT 100 +#ifdef HAVE_LZMA_H +constexpr uint32_t lzma_base_memlimit = 64 * 1024 * 1024; +#endif + +constexpr uint32_t average_value_over = 100; +constexpr uint32_t requeue_limit = 100; #ifdef DEBUG @@ -83,6 +84,24 @@ struct RamCacheCLFUSEntry { Ptr data; }; +constexpr uint64_t +requeue_hits(const uint64_t hits) +{ + return hits ? (hits - 1) : 0; +} + +constexpr double +cache_value_hits_size(const uint64_t hits, const uint32_t size) +{ + return static_cast(hits + 1) / (size + entry_overhead); +} + +constexpr double +cache_value(const RamCacheCLFUSEntry *const e) +{ + return cache_value_hits_size(e->hits, e->size); +} + class RamCacheCLFUS : public RamCache { public: @@ -230,7 +249,7 @@ check_accounting(RamCacheCLFUS *c) RamCacheCLFUSEntry *y = c->lru[0].head; while (y) { x++; - xsize += y->size + ENTRY_OVERHEAD; + xsize += y->size + entry_overhead; y = y->lru_link.next; } y = c->lru[1].head; @@ -259,7 +278,7 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey if (e->key == *key && e->auxkey == auxkey) { this->_move_compressed(e); if (!e->flag_bits.lru) { // in memory - if (CACHE_VALUE(e) > this->_average_value) { + if (cache_value(e) > this->_average_value) { this->_lru[e->flag_bits.lru].remove(e); this->_lru[e->flag_bits.lru].enqueue(e); } @@ -290,7 +309,7 @@ RamCacheCLFUS::get(CryptoHash *key, Ptr *ret_data, uint64_t auxkey #ifdef HAVE_LZMA_H case CACHE_COMPRESSION_LIBLZMA: { size_t l = static_cast(e->len), ipos = 0, opos = 0; - uint64_t memlimit = e->len * 2 + LZMA_BASE_MEMLIMIT; + uint64_t memlimit = e->len * 2 + lzma_base_memlimit; if (LZMA_OK != lzma_stream_buffer_decode(&memlimit, 0, nullptr, reinterpret_cast(e->data->data()), &ipos, e->compressed_len, reinterpret_cast(b), &opos, l)) { goto Lfailed; @@ -357,12 +376,12 @@ RamCacheCLFUS::_tick() } e->hits >>= 1; if (e->hits) { - e->hits = REQUEUE_HITS(e->hits); + e->hits = requeue_hits(e->hits); this->_lru[1].enqueue(e); } else { goto Lfree; } - if (this->_history <= this->_objects + HISTORY_HYSTERIA) { + if (this->_history <= this->_objects + history_hysteria) { return; } e = this->_lru[1].dequeue(); @@ -410,7 +429,7 @@ RamCacheCLFUS::_destroy(RamCacheCLFUSEntry *e) this->_lru[e->flag_bits.lru].remove(e); if (!e->flag_bits.lru) { this->_objects--; - this->_bytes -= e->size + ENTRY_OVERHEAD; + this->_bytes -= e->size + entry_overhead; ts::Metrics::Gauge::decrement(cache_rsb.ram_cache_bytes, e->size); ts::Metrics::Gauge::decrement(stripe->cache_vol->vol_rsb.ram_cache_bytes, e->size); e->data = nullptr; @@ -526,10 +545,10 @@ RamCacheCLFUS::compress_entries(EThread *thread, int do_at_most) goto Lcontinue; } } - if (l > REQUIRED_COMPRESSION * e->len) { + if (l > required_compression * e->len) { e->flag_bits.incompressible = true; } - if (l > REQUIRED_SHRINK * e->size) { + if (l > required_shrink * e->size) { goto Lfailed; } if (l < e->len) { @@ -581,10 +600,10 @@ RamCacheCLFUS::_requeue_victims(Que(RamCacheCLFUSEntry, lru_link) & victims) { RamCacheCLFUSEntry *victim = nullptr; while ((victim = victims.dequeue())) { - this->_bytes += victim->size + ENTRY_OVERHEAD; + this->_bytes += victim->size + entry_overhead; ts::Metrics::Gauge::increment(cache_rsb.ram_cache_bytes, victim->size); ts::Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.ram_cache_bytes, victim->size); - victim->hits = REQUEUE_HITS(victim->hits); + victim->hits = requeue_hits(victim->hits); this->_lru[0].enqueue(victim); } } @@ -637,7 +656,7 @@ RamCacheCLFUS::put(CryptoHash *key, IOBufferData *data, uint32_t len, bool copy, return 1; } else { this->_lru[1].remove(e); - if (CACHE_VALUE(e) < this->_average_value) { + if (cache_value(e) < this->_average_value) { this->_lru[1].enqueue(e); return 0; } @@ -645,7 +664,7 @@ RamCacheCLFUS::put(CryptoHash *key, IOBufferData *data, uint32_t len, bool copy, } Que(RamCacheCLFUSEntry, lru_link) victims; RamCacheCLFUSEntry *victim = nullptr; - int requeue_limit = REQUEUE_LIMIT; + int requeue_count = requeue_limit; if (!this->_lru[1].head) { // initial fill if (this->_bytes + size <= this->_max_bytes) { goto Linsert; @@ -674,12 +693,12 @@ RamCacheCLFUS::put(CryptoHash *key, IOBufferData *data, uint32_t len, bool copy, DDbg(dbg_ctl_ram_cache, "put %X %" PRId64 " NO VICTIM", key->slice32(3), auxkey); return 0; } - this->_average_value = (CACHE_VALUE(victim) + (this->_average_value * (AVERAGE_VALUE_OVER - 1))) / AVERAGE_VALUE_OVER; - if (CACHE_VALUE(victim) > this->_average_value && requeue_limit-- > 0) { + this->_average_value = (cache_value(victim) + (this->_average_value * (average_value_over - 1))) / average_value_over; + if (cache_value(victim) > this->_average_value && requeue_count-- > 0) { this->_lru[0].enqueue(victim); continue; } - this->_bytes -= victim->size + ENTRY_OVERHEAD; + this->_bytes -= victim->size + entry_overhead; ts::Metrics::Gauge::decrement(cache_rsb.ram_cache_bytes, victim->size); ts::Metrics::Gauge::decrement(stripe->cache_vol->vol_rsb.ram_cache_bytes, victim->size); victims.enqueue(victim); @@ -688,13 +707,13 @@ RamCacheCLFUS::put(CryptoHash *key, IOBufferData *data, uint32_t len, bool copy, } else { this->_ncompressed--; } - victim_value += CACHE_VALUE(victim); + victim_value += cache_value(victim); this->_tick(); if (!e) { goto Lhistory; } else { // e from history - DDbg(dbg_ctl_ram_cache_compare, "put %f %f", victim_value, CACHE_VALUE(e)); - if (this->_bytes + victim->size + size > this->_max_bytes && victim_value > CACHE_VALUE(e)) { + DDbg(dbg_ctl_ram_cache_compare, "put %f %f", victim_value, cache_value(e)); + if (this->_bytes + victim->size + size > this->_max_bytes && victim_value > cache_value(e)) { this->_requeue_victims(victims); this->_lru[1].enqueue(e); DDbg(dbg_ctl_ram_cache, "put %X %" PRId64 " size %d INC %" PRId64 " HISTORY", key->slice32(3), auxkey, e->size, e->hits); @@ -708,10 +727,10 @@ RamCacheCLFUS::put(CryptoHash *key, IOBufferData *data, uint32_t len, bool copy, Linsert: while ((victim = victims.dequeue())) { if (this->_bytes + size + victim->size <= this->_max_bytes) { - this->_bytes += victim->size + ENTRY_OVERHEAD; + this->_bytes += victim->size + entry_overhead; ts::Metrics::Gauge::increment(cache_rsb.ram_cache_bytes, victim->size); ts::Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.ram_cache_bytes, victim->size); - victim->hits = REQUEUE_HITS(victim->hits); + victim->hits = requeue_hits(victim->hits); this->_lru[0].enqueue(victim); } else { this->_victimize(victim); @@ -741,7 +760,7 @@ RamCacheCLFUS::put(CryptoHash *key, IOBufferData *data, uint32_t len, bool copy, e->data->_mem_type = DEFAULT_ALLOC; } e->flag_bits.copy = copy; - this->_bytes += size + ENTRY_OVERHEAD; + this->_bytes += size + entry_overhead; ts::Metrics::Gauge::increment(cache_rsb.ram_cache_bytes, size); ts::Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.ram_cache_bytes, size); e->size = size; @@ -792,3 +811,24 @@ new_RamCacheCLFUS() RamCacheCLFUS *r = new RamCacheCLFUS; return r; } + +// Guards against PR #11733-style regressions of the CLFUS value metric: the value density +// must be computed in floating point. Integer division truncates (hits + 1) / (size + overhead) +// to 0 for normal object sizes, zeroing the metric and silently collapsing CLFUS to FIFO (no +// promote-on-hit, no clock second chance, no value-based ghost re-admission). +REGRESSION_TEST(ram_cache_clfus_value)([[maybe_unused]] RegressionTest *t, [[maybe_unused]] int level, int *pstatus) +{ + *pstatus = REGRESSION_TEST_FAILED; + + constexpr float v_one = cache_value_hits_size(1u, 16384u); // a typical 16 KiB object, seen once + constexpr float v_hot = cache_value_hits_size(100u, 16384u); // same size, many more hits + constexpr float v_small = cache_value_hits_size(10u, 1024u); // smaller object, equal hits + constexpr float v_large = cache_value_hits_size(10u, 16384u); + + // A non-zero fraction: the integer-division regression makes this exactly 0.0f. + static_assert(v_one > 0.0f, "CLFUS value metric truncated to zero (integer division)"); + static_assert(v_hot > v_one, "CLFUS value metric does not increase with hits"); + static_assert(v_small > v_large, "CLFUS value metric does not decrease with size"); + + *pstatus = REGRESSION_TEST_PASSED; +} From da0e4d75a06959e0ed16c871309e096e3c2e4ffb Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Wed, 15 Jul 2026 07:21:14 -0500 Subject: [PATCH 23/33] Improve testing and documentation for client firewall marks (#13383) * Improve docs for `TSHttpTxnClientPacketMarkSet` * Add AuTest for `TSHttpTxnClientPacketMarkSet` * Make changes requested by Brian Neradt Allow PACKET_MARK in sock_option_flag_in Mention `CAP_NET_RAW` in docstring (cherry picked from commit 05c958a8db881288c9288e6a78dcff67818761d5) --- .../TSHttpTxnClientPacketMarkSet.en.rst | 17 +- include/ts/ts.h | 15 +- .../client_packet_mark.test.py | 103 +++++++++++ tests/tools/plugins/CMakeLists.txt | 1 + tests/tools/plugins/client_packet_mark.cc | 168 ++++++++++++++++++ 5 files changed, 299 insertions(+), 5 deletions(-) create mode 100644 tests/gold_tests/pluginTest/client_packet_mark/client_packet_mark.test.py create mode 100644 tests/tools/plugins/client_packet_mark.cc diff --git a/doc/developer-guide/api/functions/TSHttpTxnClientPacketMarkSet.en.rst b/doc/developer-guide/api/functions/TSHttpTxnClientPacketMarkSet.en.rst index 4cdbc5c9900..e0752c2dcfc 100644 --- a/doc/developer-guide/api/functions/TSHttpTxnClientPacketMarkSet.en.rst +++ b/doc/developer-guide/api/functions/TSHttpTxnClientPacketMarkSet.en.rst @@ -32,11 +32,24 @@ Synopsis Description =========== -Change packet firewall :arg:`mark` for the client side connection. +Change the packet firewall :arg:`mark` for the client side connection. The +entire firewall mark is replaced with :arg:`mark`, which is interpreted as a +32-bit unsigned bit pattern. + +Returns :const:`TS_SUCCESS` when the client connection was modified, and +:const:`TS_ERROR` when there is no client connection to modify. + +.. note:: + + The firewall mark is only honored on platforms whose OS supports it, + specifically Linux via ``SO_MARK``. On platforms without ``SO_MARK`` support + the call still returns :const:`TS_SUCCESS` when a client connection is + present, but setting the mark has no effect at the OS layer (it is a safe + no-op). .. note:: - Changes take effect immediately. + The change takes effect immediately on the live client connection. See Also ======== diff --git a/include/ts/ts.h b/include/ts/ts.h index 5631bee0f66..2a4eacfd4d4 100644 --- a/include/ts/ts.h +++ b/include/ts/ts.h @@ -1585,10 +1585,19 @@ TSReturnCode TSHttpSsnClientFdGet(TSHttpSsn ssnp, int *fdp); /* TS-1008 END */ /** Change packet firewall mark for the client side connection - * - @note The change takes effect immediately - @return TS_SUCCESS if the client connection was modified + Sets the entire client-side packet firewall mark to @a mark; the whole mark is replaced. @a mark + is interpreted as a 32-bit unsigned bit pattern. + + @note The firewall mark is only honored on platforms whose OS supports it, specifically Linux via + @c SO_MARK. On platforms without @c SO_MARK support the call still returns TS_SUCCESS when a + client connection is present, but setting the mark has no effect at the OS layer (it is a safe + no-op). + + @note The change takes effect immediately on the live client connection + + @return TS_SUCCESS if the client connection was modified, TS_ERROR if there is no client + connection to modify */ TSReturnCode TSHttpTxnClientPacketMarkSet(TSHttpTxn txnp, int mark); diff --git a/tests/gold_tests/pluginTest/client_packet_mark/client_packet_mark.test.py b/tests/gold_tests/pluginTest/client_packet_mark/client_packet_mark.test.py new file mode 100644 index 00000000000..796e6d5d44e --- /dev/null +++ b/tests/gold_tests/pluginTest/client_packet_mark/client_packet_mark.test.py @@ -0,0 +1,103 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import socket + +Test.Summary = ''' +Verify TSHttpTxnClientPacketMarkSet sets the client-side firewall mark to the +supplied value, using a test plugin that reads the applied mark back off the +client socket. +''' + + +def _can_set_so_mark() -> bool: + """Probe whether SO_MARK can actually be set on this host. + + Setting SO_MARK is Linux-only and requires CAP_NET_ADMIN or CAP_NET_RAW. + On any host that lacks the capability (or the platform), setsockopt raises, + and the applied value would be unobservable -- so the test is skipped + rather than failed. + """ + if not hasattr(socket, "SO_MARK"): + return False + try: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as probe: + probe.setsockopt(socket.SOL_SOCKET, socket.SO_MARK, 0x1) + return True + except (OSError, PermissionError): + return False + + +Test.SkipUnless( + Condition.IsPlatform("linux"), + Condition(_can_set_so_mark, "Setting SO_MARK requires Linux with CAP_NET_ADMIN or CAP_NET_RAW", True), +) + + +class ClientPacketMarkTest: + """Drive TSHttpTxnClientPacketMarkSet through a test plugin and assert on the + firewall mark read back off the client socket. + + The starting mark is seeded per process via + proxy.config.net.sock_packet_mark_in, applied at accept time. + """ + + # Value the plugin sets; the mark is expected to become exactly this. + SET_MARK = 0x0000000A + + def __init__(self): + self._server = self._make_server() + self._ts = self._make_ats("ts", seed_mark=0x0000FF00) + + def _make_server(self) -> 'Process': + server = Test.MakeOriginServer("server") + request_header = {"headers": "GET / HTTP/1.1\r\nHost: example.com\r\n\r\n", "timestamp": "1469733493.993", "body": ""} + response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""} + server.addResponse("sessionlog.json", request_header, response_header) + return server + + def _make_ats(self, name: str, seed_mark: int) -> 'Process': + ts = Test.MakeATSProcess(name, enable_cache=False) + ts.Disk.records_config.update( + { + 'proxy.config.net.sock_packet_mark_in': seed_mark, + 'proxy.config.net.sock_option_flag_in': 0x11, + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http|client_packet_mark', + 'proxy.config.url_remap.remap_required': 0, + # Keep ATS running as the invoking user inside sudo (no privilege drop). + 'proxy.config.admin.user_id': '#-1', + }) + ts.Disk.remap_config.AddLine(f"map / http://127.0.0.1:{self._server.Variables.Port}") + Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, 'client_packet_mark.so'), ts) + return ts + + def run(self): + # The mark is set to the supplied value, regardless of the seeded + # starting mark. + tr = Test.AddTestRun("TSHttpTxnClientPacketMarkSet sets the mark") + tr.Processes.Default.StartBefore(self._server) + tr.Processes.Default.StartBefore(self._ts) + tr.MakeCurlCommand( + f'--verbose --ipv4 --header "X-Set-Mark: 0x{self.SET_MARK:08x}" http://localhost:{self._ts.Variables.port}/', + ts=self._ts) + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.Streams.All += Testers.ContainsExpression( + f"X-Client-Packet-Mark: 0x{self.SET_MARK:08x}", f"Observed client packet mark should be 0x{self.SET_MARK:08x}") + + +ClientPacketMarkTest().run() diff --git a/tests/tools/plugins/CMakeLists.txt b/tests/tools/plugins/CMakeLists.txt index bbdc5866cd9..a658ba9aa24 100644 --- a/tests/tools/plugins/CMakeLists.txt +++ b/tests/tools/plugins/CMakeLists.txt @@ -15,6 +15,7 @@ # ####################### +add_autest_plugin(client_packet_mark client_packet_mark.cc) add_autest_plugin(conf_remap_stripped conf_remap_stripped.cc) add_autest_plugin(continuations_verify continuations_verify.cc) add_autest_plugin(cont_schedule cont_schedule.cc) diff --git a/tests/tools/plugins/client_packet_mark.cc b/tests/tools/plugins/client_packet_mark.cc new file mode 100644 index 00000000000..72acab439f9 --- /dev/null +++ b/tests/tools/plugins/client_packet_mark.cc @@ -0,0 +1,168 @@ +/** @file + + Test plugin for the TSHttpTxnClientPacketMarkSet API. + + On each request it reads a target mark from request headers, applies it to the + client-side connection via TSHttpTxnClientPacketMarkSet, then reads the mark + back off the client socket with getsockopt(SO_MARK) and echoes the observed + value into the X-Client-Packet-Mark response header for the AuTest to assert + on. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include + +extern "C" { +#include +} + +#include +#include +#include +#include +#include +#include + +namespace +{ +constexpr std::string_view PLUGIN_NAME = "client_packet_mark"; +constexpr std::string_view MARK_HEADER = "X-Set-Mark"; +constexpr std::string_view ECHO_HEADER = "X-Client-Packet-Mark"; + +DbgCtl dbg_ctl{PLUGIN_NAME.data()}; + +/** Read a header field and interpret its value as a 32-bit unsigned quantity. + + Values are parsed with strtoul (base 0), so "0x0000000A" and "10" are both + accepted. Returns std::nullopt if the header is absent. */ +std::optional +get_uint_header(TSMBuffer bufp, TSMLoc hdr_loc, std::string_view header) +{ + TSMLoc field_loc = TSMimeHdrFieldFind(bufp, hdr_loc, header.data(), static_cast(header.length())); + if (field_loc == TS_NULL_MLOC) { + return std::nullopt; + } + + int value_len = 0; + const char *value_str = TSMimeHdrFieldValueStringGet(bufp, hdr_loc, field_loc, -1, &value_len); + uint32_t result = 0; + if (value_str != nullptr && value_len > 0) { + std::string value(value_str, value_len); + result = static_cast(strtoul(value.c_str(), nullptr, 0)); + } + TSHandleMLocRelease(bufp, hdr_loc, field_loc); + return result; +} + +/** Create the echo header on the response with the value formatted as 0x%08x. */ +void +set_echo_header(TSMBuffer bufp, TSMLoc hdr_loc, uint32_t value) +{ + // 0x + 8 hex digits for a uint32_t + NUL = 11 bytes; 16 is comfortably enough. + char formatted[16]; + std::snprintf(formatted, sizeof(formatted), "0x%08x", value); + + TSMLoc field_loc = TS_NULL_MLOC; + if (TSMimeHdrFieldCreateNamed(bufp, hdr_loc, ECHO_HEADER.data(), static_cast(ECHO_HEADER.length()), &field_loc) == + TS_SUCCESS) { + // -1 length lets the API strlen the null-terminated buffer, so we do not + // rely on snprintf's return value (which is the would-be length, not the + // truncated length) as a byte count. + TSMimeHdrFieldValueStringSet(bufp, hdr_loc, field_loc, -1, formatted, -1); + TSMimeHdrFieldAppend(bufp, hdr_loc, field_loc); + TSHandleMLocRelease(bufp, hdr_loc, field_loc); + } +} + +int +handle_send_response(TSCont /* contp ATS_UNUSED */, TSEvent event, void *edata) +{ + TSHttpTxn txnp = static_cast(edata); + + if (event != TS_EVENT_HTTP_SEND_RESPONSE_HDR) { + TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); + return 0; + } + + TSMBuffer req_bufp = nullptr; + TSMLoc req_loc = TS_NULL_MLOC; + if (TSHttpTxnClientReqGet(txnp, &req_bufp, &req_loc) != TS_SUCCESS) { + TSError("[%s] Failed to get client request headers", PLUGIN_NAME.data()); + TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); + return 0; + } + + std::optional mark = get_uint_header(req_bufp, req_loc, MARK_HEADER); + TSHandleMLocRelease(req_bufp, TS_NULL_MLOC, req_loc); + + if (mark.has_value()) { + Dbg(dbg_ctl, "Setting client packet mark to 0x%08x", *mark); + TSHttpTxnClientPacketMarkSet(txnp, static_cast(*mark)); + } + + uint32_t observed = 0; +#if defined(SO_MARK) + int client_fd = -1; + if (TSHttpTxnClientFdGet(txnp, &client_fd) == TS_SUCCESS && client_fd >= 0) { + socklen_t optlen = sizeof(observed); + if (getsockopt(client_fd, SOL_SOCKET, SO_MARK, &observed, &optlen) != 0) { + TSError("[%s] getsockopt(SO_MARK) failed on fd %d", PLUGIN_NAME.data(), client_fd); + } + } else { + TSError("[%s] Failed to obtain client fd", PLUGIN_NAME.data()); + } +#else + // SO_MARK is Linux-only. On other platforms the accompanying AuTest is skipped + // via Test.SkipUnless, so this readback path is never exercised; keep it + // compilable so the plugin still builds everywhere. + TSError("[%s] SO_MARK is not supported on this platform", PLUGIN_NAME.data()); +#endif + + TSMBuffer resp_bufp = nullptr; + TSMLoc resp_loc = TS_NULL_MLOC; + if (TSHttpTxnClientRespGet(txnp, &resp_bufp, &resp_loc) == TS_SUCCESS) { + set_echo_header(resp_bufp, resp_loc, observed); + TSHandleMLocRelease(resp_bufp, TS_NULL_MLOC, resp_loc); + } else { + TSError("[%s] Failed to get client response headers", PLUGIN_NAME.data()); + } + + TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); + return 0; +} + +} // anonymous namespace + +void +TSPluginInit(int /* argc ATS_UNUSED */, const char ** /* argv ATS_UNUSED */) +{ + TSPluginRegistrationInfo info; + info.plugin_name = PLUGIN_NAME.data(); + info.vendor_name = "Apache Software Foundation"; + info.support_email = "dev@trafficserver.apache.org"; + + if (TSPluginRegister(&info) != TS_SUCCESS) { + TSError("[%s] Plugin registration failed", PLUGIN_NAME.data()); + return; + } + + TSCont contp = TSContCreate(handle_send_response, nullptr); + TSHttpHookAdd(TS_HTTP_SEND_RESPONSE_HDR_HOOK, contp); +} From 9523ffd4c3d5ebda8c7fa8e9c96f33a8b27b8812 Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Thu, 16 Jul 2026 09:53:26 -0500 Subject: [PATCH 24/33] Generalize client packet mark test (#13384) * Add util file for packet mark test plugin This introduces common utilities to make it easy to add a server-side packet mark test. * Generalize packet mark AuTest name * Ensure header value is null-terminated (cherry picked from commit 4efc34477e3523f27f906ceb8f82abe9f500e183) --- .../packet_mark.test.py} | 0 tests/tools/plugins/CMakeLists.txt | 2 +- tests/tools/plugins/client_packet_mark.cc | 118 ++--------- tests/tools/plugins/packet_mark_common.cc | 187 ++++++++++++++++++ tests/tools/plugins/packet_mark_common.h | 47 +++++ 5 files changed, 249 insertions(+), 105 deletions(-) rename tests/gold_tests/pluginTest/{client_packet_mark/client_packet_mark.test.py => packet_mark/packet_mark.test.py} (100%) create mode 100644 tests/tools/plugins/packet_mark_common.cc create mode 100644 tests/tools/plugins/packet_mark_common.h diff --git a/tests/gold_tests/pluginTest/client_packet_mark/client_packet_mark.test.py b/tests/gold_tests/pluginTest/packet_mark/packet_mark.test.py similarity index 100% rename from tests/gold_tests/pluginTest/client_packet_mark/client_packet_mark.test.py rename to tests/gold_tests/pluginTest/packet_mark/packet_mark.test.py diff --git a/tests/tools/plugins/CMakeLists.txt b/tests/tools/plugins/CMakeLists.txt index a658ba9aa24..701022dd27b 100644 --- a/tests/tools/plugins/CMakeLists.txt +++ b/tests/tools/plugins/CMakeLists.txt @@ -15,7 +15,7 @@ # ####################### -add_autest_plugin(client_packet_mark client_packet_mark.cc) +add_autest_plugin(client_packet_mark client_packet_mark.cc packet_mark_common.cc) add_autest_plugin(conf_remap_stripped conf_remap_stripped.cc) add_autest_plugin(continuations_verify continuations_verify.cc) add_autest_plugin(cont_schedule cont_schedule.cc) diff --git a/tests/tools/plugins/client_packet_mark.cc b/tests/tools/plugins/client_packet_mark.cc index 72acab439f9..6abd6aba88e 100644 --- a/tests/tools/plugins/client_packet_mark.cc +++ b/tests/tools/plugins/client_packet_mark.cc @@ -27,121 +27,31 @@ limitations under the License. */ -#include +#include "packet_mark_common.h" -extern "C" { -#include -} +#include -#include -#include -#include -#include -#include #include namespace { -constexpr std::string_view PLUGIN_NAME = "client_packet_mark"; -constexpr std::string_view MARK_HEADER = "X-Set-Mark"; -constexpr std::string_view ECHO_HEADER = "X-Client-Packet-Mark"; - -DbgCtl dbg_ctl{PLUGIN_NAME.data()}; - -/** Read a header field and interpret its value as a 32-bit unsigned quantity. - - Values are parsed with strtoul (base 0), so "0x0000000A" and "10" are both - accepted. Returns std::nullopt if the header is absent. */ -std::optional -get_uint_header(TSMBuffer bufp, TSMLoc hdr_loc, std::string_view header) -{ - TSMLoc field_loc = TSMimeHdrFieldFind(bufp, hdr_loc, header.data(), static_cast(header.length())); - if (field_loc == TS_NULL_MLOC) { - return std::nullopt; - } +constexpr char PLUGIN_NAME[] = "client_packet_mark"; +constexpr char MARK_HEADER[] = "X-Set-Mark"; +constexpr char ECHO_HEADER[] = "X-Client-Packet-Mark"; - int value_len = 0; - const char *value_str = TSMimeHdrFieldValueStringGet(bufp, hdr_loc, field_loc, -1, &value_len); - uint32_t result = 0; - if (value_str != nullptr && value_len > 0) { - std::string value(value_str, value_len); - result = static_cast(strtoul(value.c_str(), nullptr, 0)); - } - TSHandleMLocRelease(bufp, hdr_loc, field_loc); - return result; -} - -/** Create the echo header on the response with the value formatted as 0x%08x. */ -void -set_echo_header(TSMBuffer bufp, TSMLoc hdr_loc, uint32_t value) -{ - // 0x + 8 hex digits for a uint32_t + NUL = 11 bytes; 16 is comfortably enough. - char formatted[16]; - std::snprintf(formatted, sizeof(formatted), "0x%08x", value); - - TSMLoc field_loc = TS_NULL_MLOC; - if (TSMimeHdrFieldCreateNamed(bufp, hdr_loc, ECHO_HEADER.data(), static_cast(ECHO_HEADER.length()), &field_loc) == - TS_SUCCESS) { - // -1 length lets the API strlen the null-terminated buffer, so we do not - // rely on snprintf's return value (which is the would-be length, not the - // truncated length) as a byte count. - TSMimeHdrFieldValueStringSet(bufp, hdr_loc, field_loc, -1, formatted, -1); - TSMimeHdrFieldAppend(bufp, hdr_loc, field_loc); - TSHandleMLocRelease(bufp, hdr_loc, field_loc); - } -} +DbgCtl dbg_ctl{PLUGIN_NAME}; int handle_send_response(TSCont /* contp ATS_UNUSED */, TSEvent event, void *edata) { TSHttpTxn txnp = static_cast(edata); - if (event != TS_EVENT_HTTP_SEND_RESPONSE_HDR) { - TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); - return 0; - } - - TSMBuffer req_bufp = nullptr; - TSMLoc req_loc = TS_NULL_MLOC; - if (TSHttpTxnClientReqGet(txnp, &req_bufp, &req_loc) != TS_SUCCESS) { - TSError("[%s] Failed to get client request headers", PLUGIN_NAME.data()); - TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); - return 0; - } - - std::optional mark = get_uint_header(req_bufp, req_loc, MARK_HEADER); - TSHandleMLocRelease(req_bufp, TS_NULL_MLOC, req_loc); - - if (mark.has_value()) { - Dbg(dbg_ctl, "Setting client packet mark to 0x%08x", *mark); - TSHttpTxnClientPacketMarkSet(txnp, static_cast(*mark)); - } - - uint32_t observed = 0; -#if defined(SO_MARK) - int client_fd = -1; - if (TSHttpTxnClientFdGet(txnp, &client_fd) == TS_SUCCESS && client_fd >= 0) { - socklen_t optlen = sizeof(observed); - if (getsockopt(client_fd, SOL_SOCKET, SO_MARK, &observed, &optlen) != 0) { - TSError("[%s] getsockopt(SO_MARK) failed on fd %d", PLUGIN_NAME.data(), client_fd); - } - } else { - TSError("[%s] Failed to obtain client fd", PLUGIN_NAME.data()); - } -#else - // SO_MARK is Linux-only. On other platforms the accompanying AuTest is skipped - // via Test.SkipUnless, so this readback path is never exercised; keep it - // compilable so the plugin still builds everywhere. - TSError("[%s] SO_MARK is not supported on this platform", PLUGIN_NAME.data()); -#endif - - TSMBuffer resp_bufp = nullptr; - TSMLoc resp_loc = TS_NULL_MLOC; - if (TSHttpTxnClientRespGet(txnp, &resp_bufp, &resp_loc) == TS_SUCCESS) { - set_echo_header(resp_bufp, resp_loc, observed); - TSHandleMLocRelease(resp_bufp, TS_NULL_MLOC, resp_loc); - } else { - TSError("[%s] Failed to get client response headers", PLUGIN_NAME.data()); + if (event == TS_EVENT_HTTP_SEND_RESPONSE_HDR) { + // The client connection is live here; this applies the mark to it and reads + // it back off the client socket. + packet_mark::LogContext log{PLUGIN_NAME, dbg_ctl}; + packet_mark::apply_client_mark(log, txnp, MARK_HEADER); + packet_mark::echo_client_mark(log, txnp, ECHO_HEADER); } TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); @@ -154,12 +64,12 @@ void TSPluginInit(int /* argc ATS_UNUSED */, const char ** /* argv ATS_UNUSED */) { TSPluginRegistrationInfo info; - info.plugin_name = PLUGIN_NAME.data(); + info.plugin_name = PLUGIN_NAME; info.vendor_name = "Apache Software Foundation"; info.support_email = "dev@trafficserver.apache.org"; if (TSPluginRegister(&info) != TS_SUCCESS) { - TSError("[%s] Plugin registration failed", PLUGIN_NAME.data()); + TSError("[%s] Plugin registration failed", PLUGIN_NAME); return; } diff --git a/tests/tools/plugins/packet_mark_common.cc b/tests/tools/plugins/packet_mark_common.cc new file mode 100644 index 00000000000..ac47799c12f --- /dev/null +++ b/tests/tools/plugins/packet_mark_common.cc @@ -0,0 +1,187 @@ +/** @file + + Shared helpers for the packet-mark test plugins. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include "packet_mark_common.h" + +extern "C" { +#include +} + +#include +#include +#include +#include +#include +#include + +namespace packet_mark +{ +namespace +{ + // The tsapi setter and getters are bound as non-type template parameters below, + // which pins the correct client/server trio at compile time. These must be raw + // function-pointer types, not std::function: a non-type template parameter has + // to be a structural type, and std::function is a runtime type-erasure wrapper, + // so template Setter> is ill-formed. + using MarkSetter = TSReturnCode (*)(TSHttpTxn, int); + using FdGetter = TSReturnCode (*)(TSHttpTxn, int *); + using RespGetter = TSReturnCode (*)(TSHttpTxn, TSMBuffer *, TSMLoc *); + + std::optional + get_uint_header(TSMBuffer bufp, TSMLoc hdr_loc, std::string_view header) + { + // Values are parsed with strtoul (base 0), so "0x0000000A" and "10" are both + // accepted. Returns std::nullopt if the header is absent, empty, or not a valid + // number -- a malformed value is a test-harness error, not a silent 0. + TSMLoc field_loc{TSMimeHdrFieldFind(bufp, hdr_loc, header.data(), static_cast(header.length()))}; + if (field_loc == TS_NULL_MLOC) { + return std::nullopt; + } + + int value_len{0}; + char const *value_str{TSMimeHdrFieldValueStringGet(bufp, hdr_loc, field_loc, -1, &value_len)}; + + std::optional result{std::nullopt}; + if (value_str != nullptr && value_len > 0) { + std::string value{value_str, static_cast(value_len)}; + char *end{nullptr}; + errno = 0; + unsigned long const parsed{std::strtoul(value.c_str(), &end, 0)}; + // Reject empty, partially-numeric, or out-of-range values: a malformed + // header is a test-harness error, not a silent 0. + if (errno == 0 && end == value.c_str() + value.size() && parsed <= UINT32_MAX) { + result = static_cast(parsed); + } + } + TSHandleMLocRelease(bufp, hdr_loc, field_loc); + return result; + } + + void + set_echo_header(TSMBuffer bufp, TSMLoc hdr_loc, std::string_view header, uint32_t value) + { + // 0x + 8 hex digits for a uint32_t + NUL = 11 bytes; 16 is comfortably enough. + char formatted[16]; + std::snprintf(formatted, sizeof(formatted), "0x%08x", value); + + TSMLoc field_loc{TS_NULL_MLOC}; + if (TSMimeHdrFieldCreateNamed(bufp, hdr_loc, header.data(), static_cast(header.length()), &field_loc) == TS_SUCCESS) { + // -1 length lets the API strlen the null-terminated buffer, so we do not + // rely on snprintf's return value (which is the would-be length, not the + // truncated length) as a byte count. + TSMimeHdrFieldValueStringSet(bufp, hdr_loc, field_loc, -1, formatted, -1); + TSMimeHdrFieldAppend(bufp, hdr_loc, field_loc); + TSHandleMLocRelease(bufp, hdr_loc, field_loc); + } + } + + std::optional + get_so_mark([[maybe_unused]] int fd) + { +#if defined(SO_MARK) + if (fd < 0) { + return std::nullopt; + } + + uint32_t observed{0}; + socklen_t optlen{sizeof(observed)}; + if (getsockopt(fd, SOL_SOCKET, SO_MARK, &observed, &optlen) != 0) { + return std::nullopt; + } + return observed; +#else + // SO_MARK is Linux-only. On other platforms the accompanying AuTest is + // skipped via Test.SkipUnless, so this readback path is never exercised; + // keep it compilable so the plugins still build everywhere. + return std::nullopt; +#endif + } + + // Parameterized on the exact tsapi function and kept private to this file, + // driven only by the named entry points below. The public API is split by + // client/server rather than taking the function as an argument so each plugin + // links against exactly the tsapi trio it exercises. + template + void + apply_mark_from_header(const LogContext &log, TSHttpTxn txnp, std::string_view header) + { + TSMBuffer req_bufp{nullptr}; + TSMLoc req_loc{TS_NULL_MLOC}; + if (TSHttpTxnClientReqGet(txnp, &req_bufp, &req_loc) != TS_SUCCESS) { + TSError("[%.*s] Failed to get client request headers", static_cast(log.plugin_name.length()), log.plugin_name.data()); + return; + } + + std::optional const mark{get_uint_header(req_bufp, req_loc, header)}; + TSHandleMLocRelease(req_bufp, TS_NULL_MLOC, req_loc); + + if (mark.has_value()) { + Dbg(log.dbg_ctl, "Setting packet mark to 0x%08x (via %.*s)", *mark, static_cast(header.length()), header.data()); + if (Setter(txnp, static_cast(*mark)) != TS_SUCCESS) { + TSError("[%.*s] Failed to set packet mark 0x%08x", static_cast(log.plugin_name.length()), log.plugin_name.data(), + *mark); + } + } + } + + template + void + echo_observed_mark(const LogContext &log, TSHttpTxn txnp, std::string_view echo_header) + { + int fd{-1}; + if (FdGet(txnp, &fd) != TS_SUCCESS || fd < 0) { + TSError("[%.*s] Failed to obtain socket fd", static_cast(log.plugin_name.length()), log.plugin_name.data()); + return; + } + + std::optional const observed{get_so_mark(fd)}; + if (!observed.has_value()) { + TSError("[%.*s] Failed to read SO_MARK on fd %d", static_cast(log.plugin_name.length()), log.plugin_name.data(), fd); + return; + } + + TSMBuffer resp_bufp{nullptr}; + TSMLoc resp_loc{TS_NULL_MLOC}; + if (RespGet(txnp, &resp_bufp, &resp_loc) != TS_SUCCESS) { + TSError("[%.*s] Failed to get response headers", static_cast(log.plugin_name.length()), log.plugin_name.data()); + return; + } + + set_echo_header(resp_bufp, resp_loc, echo_header, *observed); + TSHandleMLocRelease(resp_bufp, TS_NULL_MLOC, resp_loc); + } +} // anonymous namespace + +void +apply_client_mark(const LogContext &log, TSHttpTxn txnp, std::string_view header) +{ + apply_mark_from_header(log, txnp, header); +} + +void +echo_client_mark(const LogContext &log, TSHttpTxn txnp, std::string_view echo_header) +{ + echo_observed_mark(log, txnp, echo_header); +} + +} // namespace packet_mark diff --git a/tests/tools/plugins/packet_mark_common.h b/tests/tools/plugins/packet_mark_common.h new file mode 100644 index 00000000000..3e43e53d0b8 --- /dev/null +++ b/tests/tools/plugins/packet_mark_common.h @@ -0,0 +1,47 @@ +/** @file + + Shared helpers for the packet-mark test plugins. + + The plugin reads a target mark out of a request header, applies it to a + connection via the tsapi under test, reads the applied mark back off the + relevant socket with getsockopt(SO_MARK), and echoes the observed value into a + response header for the accompanying AuTest to assert on. Everything except + the tsapi call and the fd getter is identical, so it lives here. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#pragma once + +#include + +#include + +namespace packet_mark +{ +struct LogContext { + std::string_view plugin_name; + const DbgCtl &dbg_ctl; +}; + +void apply_client_mark(const LogContext &log, TSHttpTxn txnp, std::string_view header); + +void echo_client_mark(const LogContext &log, TSHttpTxn txnp, std::string_view echo_header); + +} // namespace packet_mark From d34f7df7c298951e4bd82e19e35b9a1a9403dbd3 Mon Sep 17 00:00:00 2001 From: Miles Libbey Date: Sat, 18 Jul 2026 16:45:30 -0700 Subject: [PATCH 25/33] prefetch: admit --fetch-query requests only when the key is present (#13370) With --fetch-query configured, the front-end/first-pass gate set handleFetch=true whenever the query key was *configured*, not when the request carried it. So every request whose path matched no fetch-path-pattern was admitted anyway, ran the pattern replace, failed, and logged ERROR "failed to process the pattern" -- while scheduling no prefetch at all (BgFetch runs only on the success path). Admit only when the request's query actually contains the key, matched as a "=" parameter rather than a substring (which could hit another parameter's name or value). The same parameter test is now used by the hasValidQuery branch selector and the query-branch loop as well. (cherry picked from commit 03652e4928a0c18cc7256e1dfaed7e7001abf5db) --- plugins/prefetch/plugin.cc | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/plugins/prefetch/plugin.cc b/plugins/prefetch/plugin.cc index b935e7928db..a472bea0d50 100644 --- a/plugins/prefetch/plugin.cc +++ b/plugins/prefetch/plugin.cc @@ -348,6 +348,31 @@ getPristineUrlQuery(TSHttpTxn txnp) return pristineQuery; } +/** + * @brief Whether a single query parameter is the configured "=..." parameter. + */ +static bool +isQueryKeyParam(const String ¶m, const String &key) +{ + return param.size() > key.size() && param.compare(0, key.size(), key) == 0 && param[key.size()] == '='; +} + +/** + * @brief Whether the query string contains the configured "=..." parameter. + */ +static bool +hasQueryKeyParam(const String &query, const String &key) +{ + std::istringstream qs(query); + String param; + while (getline(qs, param, '&')) { + if (isQueryKeyParam(param, key)) { + return true; + } + } + return false; +} + static constexpr StringView CmcdHeader{"Cmcd-Request"}; static constexpr StringView CmcdNorFieldPrefix{"nor="}; static constexpr StringView CmcdNrrFieldPrefix{"nrr="}; @@ -590,9 +615,9 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) const String currentQuery = getPristineUrlQuery(txnp); bool hasValidQuery = false; - // If there is a --fetch-query defined in the config, and that string is found in the querystring, assume it is - // valid, and prefer the --fetch-query over the --fetch-path-pattern(s). - if (!config.getQueryKeyName().empty() && currentQuery.find(config.getQueryKeyName()) != String::npos) { + // If there is a --fetch-query defined in the config, and that parameter is present in the querystring, assume it + // is valid, and prefer the --fetch-query over the --fetch-path-pattern(s). + if (!config.getQueryKeyName().empty() && hasQueryKeyParam(currentQuery, config.getQueryKeyName())) { PrefetchDebug("Setting hasValidQuery to true"); hasValidQuery = true; } @@ -663,7 +688,7 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) String param; while (getline(cStringStream, param, '&')) { - if (param.find(config.getQueryKeyName()) != 0) { + if (!isQueryKeyParam(param, config.getQueryKeyName())) { continue; } if (config.getFetchCount() < done++) { @@ -851,8 +876,8 @@ TSRemapDoRemap(void *instance, TSHttpTxn txnp, TSRemapRequestInfo *rri) PrefetchDebug("failed to get path to (pre)match"); } - String queryKey = config.getQueryKeyName(); - if (!queryKey.empty()) { + const String &queryKey = config.getQueryKeyName(); + if (!handleFetch && !queryKey.empty() && hasQueryKeyParam(getPristineUrlQuery(txnp), queryKey)) { PrefetchDebug("handling for query-key: %s", queryKey.c_str()); handleFetch = true; } From 50ae689ee2af06158e5800f0810cd74e0f5967fe Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Mon, 20 Jul 2026 10:11:30 -0500 Subject: [PATCH 26/33] Improve testing and documentation for server firewall marks (#13385) * Improve docs for `TSHttpTxnServerPacketMarkSet` * Add AuTest for `TSHttpTxnServerPacketMarkSet` (cherry picked from commit b1465b4e0b464021972b15581579c5fdd1ac66fe) --- .../TSHttpTxnServerPacketMarkSet.en.rst | 22 ++- include/ts/ts.h | 16 ++- .../packet_mark/packet_mark.test.py | 130 ++++++++++++++---- tests/tools/plugins/CMakeLists.txt | 1 + tests/tools/plugins/packet_mark_common.cc | 20 ++- tests/tools/plugins/packet_mark_common.h | 12 +- tests/tools/plugins/server_packet_mark.cc | 107 ++++++++++++++ 7 files changed, 263 insertions(+), 45 deletions(-) create mode 100644 tests/tools/plugins/server_packet_mark.cc diff --git a/doc/developer-guide/api/functions/TSHttpTxnServerPacketMarkSet.en.rst b/doc/developer-guide/api/functions/TSHttpTxnServerPacketMarkSet.en.rst index c6641e5d4b2..8940f04e051 100644 --- a/doc/developer-guide/api/functions/TSHttpTxnServerPacketMarkSet.en.rst +++ b/doc/developer-guide/api/functions/TSHttpTxnServerPacketMarkSet.en.rst @@ -20,8 +20,6 @@ TSHttpTxnServerPacketMarkSet **************************** -Change packet firewall mark for the server side connection. - Synopsis ======== @@ -34,13 +32,25 @@ Synopsis Description =========== +Change the packet firewall :arg:`mark` for the server side (origin) connection. +The entire firewall mark is replaced with :arg:`mark`, which is interpreted as a +32-bit unsigned bit pattern. + +Always returns :const:`TS_SUCCESS`, including when no server connection has been +established yet. + .. note:: - The change takes effect immediately. If no OS connection has been - made, then this sets the mark that will be used. If an OS connection - is established + The firewall mark is only honored on platforms whose OS supports it, + specifically Linux via ``SO_MARK``. On platforms without ``SO_MARK`` support + the call still returns :const:`TS_SUCCESS`, but setting the mark has no effect + at the OS layer (it is a safe no-op). + +.. note:: -.. XXX Third sentence above needs to be completed. + If a live server connection exists, the mark is applied to it immediately; the + mark is also recorded on the transaction so that any subsequent server + connection for this transaction uses it. See Also ======== diff --git a/include/ts/ts.h b/include/ts/ts.h index 2a4eacfd4d4..cff3a73b6f8 100644 --- a/include/ts/ts.h +++ b/include/ts/ts.h @@ -1602,12 +1602,18 @@ TSReturnCode TSHttpSsnClientFdGet(TSHttpSsn ssnp, int *fdp); TSReturnCode TSHttpTxnClientPacketMarkSet(TSHttpTxn txnp, int mark); /** Change packet firewall mark for the server side connection - * - @note The change takes effect immediately, if no OS connection has been - made, then this sets the mark that will be used IF an OS connection - is established - @return TS_SUCCESS if the (future?) server connection was modified + Sets the entire server-side packet firewall mark to @a mark; the whole mark is replaced. @a mark + is interpreted as a 32-bit unsigned bit pattern. + + @note The firewall mark is only honored on platforms whose OS supports it, specifically Linux via + @c SO_MARK. On platforms without @c SO_MARK support the call still returns TS_SUCCESS, but setting + the mark has no effect at the OS layer (it is a safe no-op). + + @note If a live server connection exists, the mark is applied to it immediately; the mark is also + recorded on the transaction so that any subsequent server connection for this transaction uses it. + + @return TS_SUCCESS always, including when no server connection has been established yet. */ TSReturnCode TSHttpTxnServerPacketMarkSet(TSHttpTxn txnp, int mark); diff --git a/tests/gold_tests/pluginTest/packet_mark/packet_mark.test.py b/tests/gold_tests/pluginTest/packet_mark/packet_mark.test.py index 796e6d5d44e..1af23f2cbda 100644 --- a/tests/gold_tests/pluginTest/packet_mark/packet_mark.test.py +++ b/tests/gold_tests/pluginTest/packet_mark/packet_mark.test.py @@ -18,9 +18,11 @@ import socket Test.Summary = ''' -Verify TSHttpTxnClientPacketMarkSet sets the client-side firewall mark to the -supplied value, using a test plugin that reads the applied mark back off the -client socket. +Verify TSHttpTxnClientPacketMarkSet and TSHttpTxnServerPacketMarkSet set the +firewall mark on the client- and server-side connections respectively. Each is +driven by a test plugin that applies the mark and reads it back off the relevant +socket with getsockopt(SO_MARK), echoing the observed value into a response +header this test asserts on. ''' @@ -44,60 +46,136 @@ def _can_set_so_mark() -> bool: Test.SkipUnless( Condition.IsPlatform("linux"), - Condition(_can_set_so_mark, "Setting SO_MARK requires Linux with CAP_NET_ADMIN or CAP_NET_RAW", True), + # pass_value defaults to True: run only when the probe reports SO_MARK is settable. + Condition(_can_set_so_mark, "Setting SO_MARK requires Linux with CAP_NET_ADMIN or CAP_NET_RAW"), ) +# SOCK_OPT_PACKET_MARK (0x10) | SOCK_OPT_NO_DELAY (0x1). The mark is only pushed +# to the socket when the PACKET_MARK bit is set in the sock option flag. +SOCK_OPT_FLAG_PACKET_MARK = 0x11 -class ClientPacketMarkTest: - """Drive TSHttpTxnClientPacketMarkSet through a test plugin and assert on the - firewall mark read back off the client socket. - The starting mark is seeded per process via - proxy.config.net.sock_packet_mark_in, applied at accept time. +class PacketMarkTest: + """Drive a TSHttpTxn*PacketMarkSet API through a test plugin and assert on the + firewall mark read back off the relevant socket. + + This base holds the shared skeleton -- process setup, the common records, the + curl-and-assert case runner. Each subclass supplies its plugin and echo + header and extends _configure() (via super()) with the side-specific mark and + flag records. """ # Value the plugin sets; the mark is expected to become exactly this. SET_MARK = 0x0000000A + # Seeded starting mark, distinct from SET_MARK so a no-op would be visible. + SEED_MARK = 0x0000FF00 + + # Bumped per instance so each side gets uniquely-numbered processes. + _counter = 0 def __init__(self): + self._num = PacketMarkTest._counter + PacketMarkTest._counter += 1 self._server = self._make_server() - self._ts = self._make_ats("ts", seed_mark=0x0000FF00) + self._ts = self._make_ats() + self._configure(self._ts) + self._started = False - def _make_server(self) -> 'Process': - server = Test.MakeOriginServer("server") + def _make_server(self): + server = Test.MakeOriginServer(f"server{self._num}") request_header = {"headers": "GET / HTTP/1.1\r\nHost: example.com\r\n\r\n", "timestamp": "1469733493.993", "body": ""} response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""} - server.addResponse("sessionlog.json", request_header, response_header) + for _ in range(2): + server.addResponse("sessionlog.json", request_header, response_header) return server - def _make_ats(self, name: str, seed_mark: int) -> 'Process': - ts = Test.MakeATSProcess(name, enable_cache=False) + def _make_ats(self): + return Test.MakeATSProcess(f"ts{self._num}", enable_cache=False) + + def _configure(self, ts): + # Records and remap shared by both sides. Subclasses override to add the + # side-specific mark/flag records and load their plugin, calling super() + # for these. ts.Disk.records_config.update( { - 'proxy.config.net.sock_packet_mark_in': seed_mark, - 'proxy.config.net.sock_option_flag_in': 0x11, - 'proxy.config.diags.debug.enabled': 1, - 'proxy.config.diags.debug.tags': 'http|client_packet_mark', 'proxy.config.url_remap.remap_required': 0, # Keep ATS running as the invoking user inside sudo (no privilege drop). 'proxy.config.admin.user_id': '#-1', }) ts.Disk.remap_config.AddLine(f"map / http://127.0.0.1:{self._server.Variables.Port}") - Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, 'client_packet_mark.so'), ts) - return ts - def run(self): + def _add_case(self, echo_header: str, description: str, set_header: str): # The mark is set to the supplied value, regardless of the seeded - # starting mark. - tr = Test.AddTestRun("TSHttpTxnClientPacketMarkSet sets the mark") + # starting mark. The set is driven by whichever request header the plugin + # keys on; the observed mark is echoed into echo_header. The origin server + # and ATS are started before the first case, independent of the order in + # which cases are added. + tr = Test.AddTestRun(description) tr.Processes.Default.StartBefore(self._server) tr.Processes.Default.StartBefore(self._ts) + if not self._started: + tr.StillRunningAfter = self._server + tr.StillRunningAfter = self._ts + self._started = True tr.MakeCurlCommand( - f'--verbose --ipv4 --header "X-Set-Mark: 0x{self.SET_MARK:08x}" http://localhost:{self._ts.Variables.port}/', + f'--verbose --ipv4 --header "{set_header}: 0x{self.SET_MARK:08x}" http://localhost:{self._ts.Variables.port}/', ts=self._ts) tr.Processes.Default.ReturnCode = 0 tr.Processes.Default.Streams.All += Testers.ContainsExpression( - f"X-Client-Packet-Mark: 0x{self.SET_MARK:08x}", f"Observed client packet mark should be 0x{self.SET_MARK:08x}") + f"{echo_header}: 0x{self.SET_MARK:08x}", f"Observed packet mark should be 0x{self.SET_MARK:08x}") + + +class ClientPacketMarkTest(PacketMarkTest): + """Exercise TSHttpTxnClientPacketMarkSet. The client mark is seeded on the + inbound socket. + """ + + ECHO_HEADER = "X-Client-Packet-Mark" + + def _configure(self, ts): + super()._configure(ts) + ts.Disk.records_config.update( + { + 'proxy.config.net.sock_packet_mark_in': self.SEED_MARK, + 'proxy.config.net.sock_option_flag_in': SOCK_OPT_FLAG_PACKET_MARK, + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http|client_packet_mark', + }) + Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, f'client_packet_mark.so'), ts) + + def run(self): + self._add_case(self.ECHO_HEADER, "client_packet_mark sets the client-side mark on the live connection", "X-Set-Mark") + + +class ServerPacketMarkTest(PacketMarkTest): + """Exercise TSHttpTxnServerPacketMarkSet. The server mark is seeded on the + outbound socket. + + The server API additionally records the mark for a *future* origin + connection (TSHttpTxnConfigIntSet on TS_CONFIG_NET_SOCK_PACKET_MARK_OUT), + which the client API has no equivalent of. The server plugin exposes this by + honoring X-Set-Mark-Preconnect at READ_REQUEST_HDR, before any origin + connection exists -- so the mark can only reach the socket via that seed. + """ + + ECHO_HEADER = "X-Server-Packet-Mark" + + def _configure(self, ts): + super()._configure(ts) + ts.Disk.records_config.update( + { + 'proxy.config.net.sock_packet_mark_out': self.SEED_MARK, + 'proxy.config.net.sock_option_flag_out': SOCK_OPT_FLAG_PACKET_MARK, + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http|server_packet_mark', + }) + Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, f'server_packet_mark.so'), ts) + + def run(self): + self._add_case(self.ECHO_HEADER, "server_packet_mark sets the server-side mark on the live connection", "X-Set-Mark") + self._add_case( + self.ECHO_HEADER, "server_packet_mark seeds the mark for a future origin connection", "X-Set-Mark-Preconnect") ClientPacketMarkTest().run() +ServerPacketMarkTest().run() diff --git a/tests/tools/plugins/CMakeLists.txt b/tests/tools/plugins/CMakeLists.txt index 701022dd27b..a058eea6c63 100644 --- a/tests/tools/plugins/CMakeLists.txt +++ b/tests/tools/plugins/CMakeLists.txt @@ -25,6 +25,7 @@ add_autest_plugin(fatal_shutdown fatal_shutdown.cc) add_autest_plugin(hook_add_plugin hook_add_plugin.cc) add_autest_plugin(missing_mangled_definition missing_mangled_definition_c.c missing_mangled_definition_cpp.cc) add_autest_plugin(missing_ts_plugin_init missing_ts_plugin_init.cc) +add_autest_plugin(server_packet_mark server_packet_mark.cc packet_mark_common.cc) add_autest_plugin(ssl_client_verify_test ssl_client_verify_test.cc) add_autest_plugin(ssl_hook_test ssl_hook_test.cc) add_autest_plugin(ssl_secret_load_test ssl_secret_load_test.cc) diff --git a/tests/tools/plugins/packet_mark_common.cc b/tests/tools/plugins/packet_mark_common.cc index ac47799c12f..85b72d48d5b 100644 --- a/tests/tools/plugins/packet_mark_common.cc +++ b/tests/tools/plugins/packet_mark_common.cc @@ -1,6 +1,6 @@ /** @file - Shared helpers for the packet-mark test plugins. + Shared helpers for the client_packet_mark and server_packet_mark test plugins. @section license License @@ -118,9 +118,9 @@ namespace } // Parameterized on the exact tsapi function and kept private to this file, - // driven only by the named entry points below. The public API is split by - // client/server rather than taking the function as an argument so each plugin - // links against exactly the tsapi trio it exercises. + // driven only by the four named entry points below. See packet_mark_common.h + // for why the public API is split by client/server rather than taking the + // function as an argument. template void apply_mark_from_header(const LogContext &log, TSHttpTxn txnp, std::string_view header) @@ -178,10 +178,22 @@ apply_client_mark(const LogContext &log, TSHttpTxn txnp, std::string_view header apply_mark_from_header(log, txnp, header); } +void +apply_server_mark(const LogContext &log, TSHttpTxn txnp, std::string_view header) +{ + apply_mark_from_header(log, txnp, header); +} + void echo_client_mark(const LogContext &log, TSHttpTxn txnp, std::string_view echo_header) { echo_observed_mark(log, txnp, echo_header); } +void +echo_server_mark(const LogContext &log, TSHttpTxn txnp, std::string_view echo_header) +{ + echo_observed_mark(log, txnp, echo_header); +} + } // namespace packet_mark diff --git a/tests/tools/plugins/packet_mark_common.h b/tests/tools/plugins/packet_mark_common.h index 3e43e53d0b8..2acc840cfd7 100644 --- a/tests/tools/plugins/packet_mark_common.h +++ b/tests/tools/plugins/packet_mark_common.h @@ -1,10 +1,10 @@ /** @file - Shared helpers for the packet-mark test plugins. + Shared helpers for the client_packet_mark and server_packet_mark test plugins. - The plugin reads a target mark out of a request header, applies it to a - connection via the tsapi under test, reads the applied mark back off the - relevant socket with getsockopt(SO_MARK), and echoes the observed value into a + Both plugins read a target mark out of a request header, apply it to a + connection via the tsapi under test, read the applied mark back off the + relevant socket with getsockopt(SO_MARK), and echo the observed value into a response header for the accompanying AuTest to assert on. Everything except the tsapi call and the fd getter is identical, so it lives here. @@ -42,6 +42,10 @@ struct LogContext { void apply_client_mark(const LogContext &log, TSHttpTxn txnp, std::string_view header); +void apply_server_mark(const LogContext &log, TSHttpTxn txnp, std::string_view header); + void echo_client_mark(const LogContext &log, TSHttpTxn txnp, std::string_view echo_header); +void echo_server_mark(const LogContext &log, TSHttpTxn txnp, std::string_view echo_header); + } // namespace packet_mark diff --git a/tests/tools/plugins/server_packet_mark.cc b/tests/tools/plugins/server_packet_mark.cc new file mode 100644 index 00000000000..2d8c1cb56a5 --- /dev/null +++ b/tests/tools/plugins/server_packet_mark.cc @@ -0,0 +1,107 @@ +/** @file + + Test plugin for the TSHttpTxnServerPacketMarkSet API. + + The plugin exercises both halves of the TSHttpTxnServerPacketMarkSet contract, + selected by request header: + + - X-Set-Mark: applied at TS_HTTP_READ_RESPONSE_HDR_HOOK, when the origin + connection is already live. This tests the "apply to the live server + connection immediately" path. + + - X-Set-Mark-Preconnect: applied at TS_HTTP_READ_REQUEST_HDR_HOOK, before an + origin connection exists. This tests the server-only "record the mark so a + future origin connection is opened with it" path -- there is no live vc to + apply to at that point, so the mark reaches the socket only via the + transaction config seed. + + Regardless of which header drove the set, the readback happens at + TS_HTTP_READ_RESPONSE_HDR_HOOK: the origin fd is valid there and the server + response headers -- which propagate to the client response -- carry the + observed value (echoed into X-Server-Packet-Mark) back to curl. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include "packet_mark_common.h" + +#include + +#include + +namespace +{ +constexpr char PLUGIN_NAME[] = "server_packet_mark"; +constexpr char MARK_HEADER[] = "X-Set-Mark"; +constexpr char PRECONNECT_MARK_HEADER[] = "X-Set-Mark-Preconnect"; +constexpr char ECHO_HEADER[] = "X-Server-Packet-Mark"; + +DbgCtl dbg_ctl{PLUGIN_NAME}; + +int +handle_read_request(TSCont /* contp ATS_UNUSED */, TSEvent event, void *edata) +{ + TSHttpTxn txnp = static_cast(edata); + + if (event == TS_EVENT_HTTP_READ_REQUEST_HDR) { + // No origin connection exists yet; this exercises the "seed the mark for a + // future server connection" half of the contract. + packet_mark::LogContext log{PLUGIN_NAME, dbg_ctl}; + packet_mark::apply_server_mark(log, txnp, PRECONNECT_MARK_HEADER); + } + + TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); + return 0; +} + +int +handle_read_response(TSCont /* contp ATS_UNUSED */, TSEvent event, void *edata) +{ + TSHttpTxn txnp = static_cast(edata); + + if (event == TS_EVENT_HTTP_READ_RESPONSE_HDR) { + // The origin connection is live here; this applies the mark to it and reads + // it back off the server socket. + packet_mark::LogContext log{PLUGIN_NAME, dbg_ctl}; + packet_mark::apply_server_mark(log, txnp, MARK_HEADER); + packet_mark::echo_server_mark(log, txnp, ECHO_HEADER); + } + + TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); + return 0; +} + +} // anonymous namespace + +void +TSPluginInit(int /* argc ATS_UNUSED */, const char ** /* argv ATS_UNUSED */) +{ + TSPluginRegistrationInfo info; + info.plugin_name = PLUGIN_NAME; + info.vendor_name = "Apache Software Foundation"; + info.support_email = "dev@trafficserver.apache.org"; + + if (TSPluginRegister(&info) != TS_SUCCESS) { + TSError("[%s] Plugin registration failed", PLUGIN_NAME); + return; + } + + TSHttpHookAdd(TS_HTTP_READ_REQUEST_HDR_HOOK, TSContCreate(handle_read_request, nullptr)); + TSHttpHookAdd(TS_HTTP_READ_RESPONSE_HDR_HOOK, TSContCreate(handle_read_response, nullptr)); +} From b4b9a7aa7e991c9c097f9d498bf1516d306718ba Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Mon, 20 Jul 2026 15:04:45 -0500 Subject: [PATCH 27/33] Avoid Diags lock for syslog output (#13398) Syslog-only diagnostics take the same mutex used to serialize file output, adding avoidable contention when many threads report a shared failure. This limits the diagnostics mutex to FILE-backed destinations and removes the obsolete FreeBSD exception because syslog provides its own thread safety. Fixes: #7374 (cherry picked from commit 5fc8ad3585e267e966670743a0f20e885638ec32) --- src/tscore/Diags.cc | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/tscore/Diags.cc b/src/tscore/Diags.cc index 6fe5b79cf8b..a21ceb81bbd 100644 --- a/src/tscore/Diags.cc +++ b/src/tscore/Diags.cc @@ -238,8 +238,16 @@ Diags::print_va(const char *debug_tag, DiagsLevel diags_level, const SourceLocat // now, finally, output the message // ////////////////////////////////////// - lock(); - if (config.outputs[diags_level].to_diagslog) { + auto const &output = config.outputs[diags_level]; + + // FILE output must be serialized, but syslog provides its own thread safety. + bool const serialize_file_output = output.to_diagslog || output.to_stdout || output.to_stderr || regression_testing_on; + + if (serialize_file_output) { + lock(); + } + + if (output.to_diagslog) { if (diags_log && diags_log->m_fp) { va_list tmp; va_copy(tmp, ap); @@ -248,7 +256,7 @@ Diags::print_va(const char *debug_tag, DiagsLevel diags_level, const SourceLocat } } - if (config.outputs[diags_level].to_stdout) { + if (output.to_stdout) { if (stdout_log && stdout_log->m_fp) { va_list tmp; va_copy(tmp, ap); @@ -257,7 +265,7 @@ Diags::print_va(const char *debug_tag, DiagsLevel diags_level, const SourceLocat } } - if (config.outputs[diags_level].to_stderr || regression_testing_on) { + if (output.to_stderr || regression_testing_on) { if (stderr_log && stderr_log->m_fp) { va_list tmp; va_copy(tmp, ap); @@ -266,11 +274,11 @@ Diags::print_va(const char *debug_tag, DiagsLevel diags_level, const SourceLocat } } -#if !defined(freebsd) - unlock(); -#endif + if (serialize_file_output) { + unlock(); + } - if (config.outputs[diags_level].to_syslog) { + if (output.to_syslog) { int priority; char syslog_buffer[2048]; @@ -308,10 +316,6 @@ Diags::print_va(const char *debug_tag, DiagsLevel diags_level, const SourceLocat vsnprintf(syslog_buffer, sizeof(syslog_buffer), format_writer.data() + timestamp_offset, ap); syslog(priority, "%s", syslog_buffer); } - -#if defined(freebsd) - unlock(); -#endif } ////////////////////////////////////////////////////////////////////////////// From c09c61d2c1b158cb1f4f2c8cc554f3490426da30 Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Mon, 20 Jul 2026 15:12:59 -0700 Subject: [PATCH 28/33] doc: fix Via decoder ring URL (/tools/via moved to /via.html) (#13399) The Via decoder ring page moved from /tools/via to /via.html on the project website, leaving the documented URL returning 404. Update the references in the FAQ and records documentation to the new location. (cherry picked from commit 4b55a4d5689f66c414c798a82119128c8dd56592) --- doc/admin-guide/files/records.yaml.en.rst | 4 ++-- doc/appendices/faq.en.rst | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/admin-guide/files/records.yaml.en.rst b/doc/admin-guide/files/records.yaml.en.rst index b8cc4b791e6..378ae6a6760 100644 --- a/doc/admin-guide/files/records.yaml.en.rst +++ b/doc/admin-guide/files/records.yaml.en.rst @@ -982,7 +982,7 @@ allow-plain .. note:: - The ``Via`` transaction codes can be decoded with the `Via Decoder Ring `_. + The ``Via`` transaction codes can be decoded with the `Via Decoder Ring `_. .. ts:cv:: CONFIG proxy.config.http.request_via_str STRING ApacheTrafficServer/${PACKAGE_VERSION} :reloadable: @@ -1008,7 +1008,7 @@ allow-plain .. note:: - The ``Via`` transaction code can be decoded with the `Via Decoder Ring `_. + The ``Via`` transaction code can be decoded with the `Via Decoder Ring `_. .. ts:cv:: CONFIG proxy.config.http.response_via_str STRING ApacheTrafficServer/${PACKAGE_VERSION} :reloadable: diff --git a/doc/appendices/faq.en.rst b/doc/appendices/faq.en.rst index 2620fc9060a..eb4962a3263 100644 --- a/doc/appendices/faq.en.rst +++ b/doc/appendices/faq.en.rst @@ -106,7 +106,7 @@ Please refer to the :ref:`forward-proxy` documentation. How do I interpret the Via: header code? ---------------------------------------- -The ``Via`` header string can be decoded with the `Via Decoder Ring `_. +The ``Via`` header string can be decoded with the `Via Decoder Ring `_. The Via header is an optional HTTP header added by Traffic Server and other HTTP proxies. If a request goes through multiple proxies, each one appends its Via header content to the end of the existing Via header. Via header content is for general information and diagnostic use only and should not be used as a programmatic interface to Traffic Server. The header is cached by each intermediary with the object as received from its downstream node. Thus, the last node in the list to report a cache hit is the end of the transaction for that specific request. Nodes reported earlier were from a previous transaction. From 30e5bb7e46ca785033253ced79ab664c8eb9120b Mon Sep 17 00:00:00 2001 From: Sergey Blekher <287965768+blurman-ai@users.noreply.github.com> Date: Tue, 21 Jul 2026 01:15:07 +0300 Subject: [PATCH 29/33] Remove unused RemapProcessor.h include from RemapPlugins.h (#13403) RemapPlugins.h includes RemapProcessor.h, and RemapProcessor.h includes RemapPlugins.h back, forming an include cycle. RemapPlugins.h does not use RemapProcessor: the class derives from Continuation and its members are HttpTransact::State, URL and HTTPHdr, none of which come from RemapProcessor.h. Everything RemapPlugins.h needs already arrives through its other includes (EventSystem.h, HttpTransact.h, RemapPluginInfo.h), which are also the only things RemapProcessor.h contributed to the include closure. Removing the include breaks the cycle with no call-site changes. (cherry picked from commit 77b6efcf3378de3511d79de6efa2955a0b51d53b) --- include/proxy/http/remap/RemapPlugins.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/proxy/http/remap/RemapPlugins.h b/include/proxy/http/remap/RemapPlugins.h index 643231f1616..a0f01c728f1 100644 --- a/include/proxy/http/remap/RemapPlugins.h +++ b/include/proxy/http/remap/RemapPlugins.h @@ -26,7 +26,6 @@ #include "tscore/ink_platform.h" #include "iocore/eventsystem/EventSystem.h" -#include "proxy/http/remap/RemapProcessor.h" #include "proxy/http/remap/RemapPluginInfo.h" #include "proxy/http/HttpTransact.h" #include "proxy/ReverseProxy.h" From a306331e530a943e4a4c5a2d6cb9a3d42b44fa39 Mon Sep 17 00:00:00 2001 From: Javid Khan Date: Mon, 20 Jul 2026 22:02:18 +0530 Subject: [PATCH 30/33] copy only len_in bytes in the escapify no-escape path (#13404) (cherry picked from commit 110b5442a77b3f04bb9b694fb01abeea821c6438) --- src/tscore/Encoding.cc | 5 +++-- src/tscore/unit_tests/test_Encoding.cc | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/tscore/Encoding.cc b/src/tscore/Encoding.cc index 4bae0a9aa0b..e7257e12535 100644 --- a/src/tscore/Encoding.cc +++ b/src/tscore/Encoding.cc @@ -77,7 +77,7 @@ escapify_url_common(Arena *arena, char *url, size_t len_in, int *len_out, char * static char hex_digit[16] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'}; - if (!url || (dst && dst_size < len_in)) { + if (!url || (dst && dst_size <= len_in)) { *len_out = 0; return nullptr; } @@ -105,7 +105,8 @@ escapify_url_common(Arena *arena, char *url, size_t len_in, int *len_out, char * // *len_out = len_in; if (dst) { - ink_strlcpy(dst, url, dst_size); + memcpy(dst, url, len_in); + dst[len_in] = '\0'; } return url; } diff --git a/src/tscore/unit_tests/test_Encoding.cc b/src/tscore/unit_tests/test_Encoding.cc index b949964021d..dc670afcc64 100644 --- a/src/tscore/unit_tests/test_Encoding.cc +++ b/src/tscore/unit_tests/test_Encoding.cc @@ -22,6 +22,7 @@ */ #include +#include #include #include #include @@ -57,6 +58,30 @@ TEST_CASE("Encoding pure escapify url", "[pure_esc_url]") } } +TEST_CASE("Encoding escapify url without a terminator", "[esc_url_unterminated]") +{ + // The source is a counted string, not a C string, so nothing may be read past len_in. + // Sized exactly so that a read past the end is caught by a sanitizer. + constexpr std::string_view src{"abcdef"}; + + std::vector unterminated(src.begin(), src.end()); + + char output[128]; + int output_len; + + REQUIRE(Encoding::pure_escapify_url(nullptr, unterminated.data(), unterminated.size(), &output_len, output, sizeof(output)) != + nullptr); + CHECK(output_len == static_cast(src.size())); + CHECK(std::string_view(output, output_len) == src); + CHECK(output[output_len] == '\0'); + + REQUIRE(Encoding::escapify_url(nullptr, unterminated.data(), unterminated.size(), &output_len, output, sizeof(output)) != + nullptr); + CHECK(output_len == static_cast(src.size())); + CHECK(std::string_view(output, output_len) == src); + CHECK(output[output_len] == '\0'); +} + TEST_CASE("Encoding escapify url", "[esc_url]") { char input[][32] = { From 968eeec0f9d40a8e5a3b2be250db44ad5f0304ad Mon Sep 17 00:00:00 2001 From: Mo Chen Date: Mon, 20 Jul 2026 11:27:20 -0500 Subject: [PATCH 31/33] ssl: remove the dead SSL_HOOK_OP_TERMINATE op (#13407) No code has ever assigned hookOpRequested this value: the introducing commit (TS-3006, 2014) only wrote it to an example plugin's own struct, and the public TSSslVConnOp API that could have set it was removed in 2016 (TS-4658). A hook rejects a handshake via TSVConnReenableEx(TS_EVENT_ERROR) instead, so drop the enumerator and its unreachable arm in sslServerHandShakeEvent(). (cherry picked from commit c64d786be619a2e6e8f3f0d5e01d2ac68ba21a81) --- src/iocore/net/P_SSLNetVConnection.h | 5 ++--- src/iocore/net/SSLNetVConnection.cc | 3 --- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/iocore/net/P_SSLNetVConnection.h b/src/iocore/net/P_SSLNetVConnection.h index 3ff284ff8cc..ee1a04d9e26 100644 --- a/src/iocore/net/P_SSLNetVConnection.h +++ b/src/iocore/net/P_SSLNetVConnection.h @@ -81,9 +81,8 @@ constexpr int SSL_DEF_TLS_RECORD_MSEC_THRESHOLD = 1000; struct SSLCertLookup; enum class SslVConnOp { - SSL_HOOK_OP_DEFAULT, ///< Null / initialization value. Do normal processing. - SSL_HOOK_OP_TUNNEL, ///< Switch to blind tunnel - SSL_HOOK_OP_TERMINATE ///< Termination connection / transaction. + SSL_HOOK_OP_DEFAULT, ///< Null / initialization value. Do normal processing. + SSL_HOOK_OP_TUNNEL ///< Switch to blind tunnel }; enum class SSLHandshakeStatus { SSL_HANDSHAKE_ONGOING, SSL_HANDSHAKE_DONE, SSL_HANDSHAKE_ERROR }; diff --git a/src/iocore/net/SSLNetVConnection.cc b/src/iocore/net/SSLNetVConnection.cc index 0e08905b7e9..fcee0975947 100644 --- a/src/iocore/net/SSLNetVConnection.cc +++ b/src/iocore/net/SSLNetVConnection.cc @@ -1307,9 +1307,6 @@ SSLNetVConnection::sslServerHandShakeEvent(int &err) // we get out of this callback, and then will shuffle // over the buffered handshake packets to the O.S. return EVENT_DONE; - } else if (SslVConnOp::SSL_HOOK_OP_TERMINATE == hookOpRequested) { - sslHandshakeStatus = SSLHandshakeStatus::SSL_HANDSHAKE_DONE; - return EVENT_DONE; } Dbg(dbg_ctl_ssl, "Go on with the handshake state=%s", From cc19b74becbe077c56df760031686a7d8d5d4971 Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:43:07 -0500 Subject: [PATCH 32/33] Document `HTTPHdr` methods for #13420 review (#13422) * Document HTTP methods for #13420 review * Make changes requested by Brian Neradt Put brief sentence on opening line Use in/out/in,out parameter markers Clarify that `@` headers are also included in length (cherry picked from commit e24126528683cc2514ae9681c3893c2822b5d7cb) --- include/proxy/hdrs/HTTP.h | 77 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/include/proxy/hdrs/HTTP.h b/include/proxy/hdrs/HTTP.h index 571bd0fdc5e..eb73b68eb31 100644 --- a/include/proxy/hdrs/HTTP.h +++ b/include/proxy/hdrs/HTTP.h @@ -496,6 +496,23 @@ class HTTPHdr : public MIMEHdr int print(char *buf, int bufsize, int *bufindex, int *dumpoffset) const; + /** Returns the serialized byte length of the HTTP header. + * + * The count includes the request-line (for requests) or status-line (for + * responses), all header fields, and the terminating blank line. The message + * body is not included. + * + * @note Internal fields whose names begin with @c '@' are counted here even + * though @c print() omits them from its output, so this length may + * exceed the number of bytes @c print() actually writes. + * + * @return Serialized byte length of the header. + * + * @pre The header must be initialized. + * + * @par Thread Safety + * Not thread-safe. + */ int length_get() const; HTTPType type_get() const; @@ -616,10 +633,70 @@ class HTTPHdr : public MIMEHdr void mark_early_data(bool flag = true) const; bool is_early_data() const; + /** Parse an HTTP/1.x request header incrementally from a raw buffer. + * + * Parses input data into the header's request fields. Call repeatedly with the same @p parser + * until a result other than @c ParseResult::CONT is returned. When @c ParseResult::DONE is + * returned, the request method, URL, version, and header fields are set on this header. + * + * @param[in,out] parser Parser state. Must be the same object on each call for a given message. + * @param[in,out] start On entry, points to the first unparsed byte; on return, + * advanced past all consumed bytes. + * @param[in] end One past the last available byte of input. + * @param[in] eof @c true if no more data will follow @p end. + * @param[in] strict_uri_parsing URI compliance level: @c 0 performs no compliance check; @c 1 rejects + * the URI unless every character is a valid RFC 3986 URI character; @c 2 + * is more permissive, rejecting the URI only if it contains whitespace or + * non-printable characters. Other values behave like @c 0. + * @param[in] max_request_line_size Maximum byte length of the request line; exceeding it returns + * @c ParseResult::ERROR. + * @param[in] max_hdr_field_size Maximum byte length of a single header field; exceeding it + * returns @c ParseResult::ERROR. + * + * @return @c ParseResult::DONE if a complete valid request header has been parsed; + * @c ParseResult::CONT if more data is required; + * @c ParseResult::ERROR on a protocol error or exceeded limit. + * + * @pre The header must be initialized with @c HTTPType::REQUEST polarity. + * + * @par Thread Safety + * Not thread-safe. + */ ParseResult parse_req(HTTPParser *parser, const char **start, const char *end, bool eof, int strict_uri_parsing = 0, size_t max_request_line_size = UINT16_MAX, size_t max_hdr_field_size = 131070); ParseResult parse_resp(HTTPParser *parser, const char **start, const char *end, bool eof); + /** Parse an HTTP/1.x request header incrementally from an @c IOBufferReader. + * + * Reads and consumes data from @p r, parsing it into the header's request fields. Call + * repeatedly with the same @p parser until a result other than @c ParseResult::CONT is + * returned. When @c ParseResult::DONE is returned, the request method, URL, version, and + * header fields are set on this header. + * + * @param[in,out] parser Parser state. Must be the same object on each call for a given message. + * @param[in,out] r Source of input data; bytes consumed by the parser are removed from + * the reader. + * @param[out] bytes_used Must be non-null; set to the number of bytes consumed from @p r. + * @param[in] eof @c true if no more data will be provided after what is currently + * available on @p r. + * @param[in] strict_uri_parsing URI compliance level: @c 0 performs no compliance check; @c 1 rejects + * the URI unless every character is a valid RFC 3986 URI character; @c 2 + * is more permissive, rejecting the URI only if it contains whitespace or + * non-printable characters. Other values behave like @c 0. + * @param[in] max_request_line_size Maximum byte length of the request line; exceeding it returns + * @c ParseResult::ERROR. + * @param[in] max_hdr_field_size Maximum byte length of a single header field; exceeding it + * returns @c ParseResult::ERROR. + * + * @return @c ParseResult::DONE if a complete valid request header has been parsed; + * @c ParseResult::CONT if more data is required; + * @c ParseResult::ERROR on a protocol error or exceeded limit. + * + * @pre The header must be initialized with @c HTTPType::REQUEST polarity. + * + * @par Thread Safety + * Not thread-safe. + */ ParseResult parse_req(HTTPParser *parser, IOBufferReader *r, int *bytes_used, bool eof, int strict_uri_parsing = 0, size_t max_request_line_size = UINT16_MAX, size_t max_hdr_field_size = UINT16_MAX); ParseResult parse_resp(HTTPParser *parser, IOBufferReader *r, int *bytes_used, bool eof); From d1cc4e2137a426bb58b27a04668ab98087ae3bfb Mon Sep 17 00:00:00 2001 From: JosiahWI <41302989+JosiahWI@users.noreply.github.com> Date: Tue, 28 Jul 2026 06:06:04 -0500 Subject: [PATCH 33/33] Add unit tests for `Action` logic (#13332) * Add unit tests for `Action` logic Authored with Claude Sonnet and Claude Opus. (cherry picked from commit 04ee11ac10ff3987348bb0f4358f544a76cad725) --- src/iocore/eventsystem/CMakeLists.txt | 4 + .../eventsystem/unit_tests/test_Action.cc | 254 ++++++++++++++++++ 2 files changed, 258 insertions(+) create mode 100644 src/iocore/eventsystem/unit_tests/test_Action.cc diff --git a/src/iocore/eventsystem/CMakeLists.txt b/src/iocore/eventsystem/CMakeLists.txt index 4d4a8c50c52..a6f4d6fc436 100644 --- a/src/iocore/eventsystem/CMakeLists.txt +++ b/src/iocore/eventsystem/CMakeLists.txt @@ -68,6 +68,10 @@ if(BUILD_TESTING) target_link_libraries(test_Continuation ts::inkevent configmanager Catch2::Catch2WithMain) add_catch2_test(NAME test_Continuation COMMAND test_Continuation) + add_executable(test_Action unit_tests/test_Action.cc) + target_link_libraries(test_Action ts::inkevent configmanager Catch2::Catch2WithMain) + add_catch2_test(NAME test_Action COMMAND test_Action) + endif() clang_tidy_check(inkevent) diff --git a/src/iocore/eventsystem/unit_tests/test_Action.cc b/src/iocore/eventsystem/unit_tests/test_Action.cc new file mode 100644 index 00000000000..7f8890ad54c --- /dev/null +++ b/src/iocore/eventsystem/unit_tests/test_Action.cc @@ -0,0 +1,254 @@ +/** @file + + Catch2 unit tests for the Action boundary contract. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include "inkevent_test_fixtures.h" + +#include + +#include + +using inkevent_test::CountingContinuation; +using inkevent_test::EventProcessorListener; + +CATCH_REGISTER_LISTENER(EventProcessorListener) + +namespace +{ + +class RecordingAction : public Action +{ +public: + using Action::operator=; + + std::atomic derived_cancel_calls{0}; + + void + cancel(Continuation *c = nullptr) override + { + derived_cancel_calls.fetch_add(1, std::memory_order_release); + Action::cancel(c); + } +}; + +} // namespace + +TEST_CASE("A default-constructed Action has a null continuation, a null mutex reference, and a false cancelled flag", + "[inkevent][action]") +{ + Action a; + REQUIRE(a.continuation == nullptr); + REQUIRE(a.mutex.get() == nullptr); + REQUIRE(a.cancelled == false); +} + +TEST_CASE("Action::operator= with a non-null Continuation pins its observable post-conditions", "[inkevent][action]") +{ + Ptr mutex{new_ProxyMutex()}; + CountingContinuation cont{mutex.get()}; + int const before = mutex->refcount(); + Action a; + + Continuation *result = (a = &cont); + + SECTION("the operator returns its argument unchanged so callers can chain assignments") + { + REQUIRE(result == &cont); + } + + SECTION("the Action stores the assigned Continuation pointer and adopts that Continuation's ProxyMutex") + { + REQUIRE(a.continuation == &cont); + REQUIRE(a.mutex.get() == mutex.get()); + } + + SECTION("the Action retains an additional reference to the bound ProxyMutex") + { + REQUIRE(mutex->refcount() == before + 1); + } + + SECTION("the Action's cancelled flag is unchanged by binding a Continuation") + { + REQUIRE(a.cancelled == false); + } +} + +TEST_CASE("Action::operator= with nullptr clears the bound continuation and drops the retained ProxyMutex reference", + "[inkevent][action]") +{ + Ptr mutex{new_ProxyMutex()}; + CountingContinuation cont{mutex.get()}; + Action a; + a = &cont; + int const before_clear = mutex->refcount(); + + a = nullptr; + + REQUIRE(a.continuation == nullptr); + REQUIRE(a.mutex.get() == nullptr); + REQUIRE(mutex->refcount() == before_clear - 1); +} + +TEST_CASE( + "Reassigning Action::operator= to a different Continuation drops the previous ProxyMutex reference and adopts the new one", + "[inkevent][action]") +{ + Ptr mutex1{new_ProxyMutex()}; + Ptr mutex2{new_ProxyMutex()}; + CountingContinuation cont1{mutex1.get()}; + CountingContinuation cont2{mutex2.get()}; + + Action a; + a = &cont1; + int const ref1_one = mutex1->refcount(); + int const ref2_one = mutex2->refcount(); + + a = &cont2; + + REQUIRE(a.continuation == &cont2); + REQUIRE(a.mutex.get() == mutex2.get()); + REQUIRE(mutex1->refcount() == ref1_one - 1); + REQUIRE(mutex2->refcount() == ref2_one + 1); +} + +TEST_CASE( + "Action::operator= with a Continuation that has a null mutex stores the Continuation pointer and leaves Action's mutex null", + "[inkevent][action]") +{ + CountingContinuation cont{nullptr}; + Action a; + + a = &cont; + + REQUIRE(a.continuation == &cont); + REQUIRE(a.mutex.get() == nullptr); +} + +TEST_CASE("Action::cancel flips the cancelled flag to true whether the bound Continuation is passed explicitly or omitted", + "[inkevent][action]") +{ + bool const pass_continuation = GENERATE(true, false); + + Ptr mutex{new_ProxyMutex()}; + CountingContinuation cont{mutex.get()}; + Action a; + a = &cont; + REQUIRE(a.cancelled == false); + + { + SCOPED_MUTEX_LOCK(lock, mutex, this_ethread()); + a.cancel(pass_continuation ? &cont : nullptr); + } + + REQUIRE(a.cancelled == true); +} + +TEST_CASE("Action::cancel dispatches virtually so a derived Action's overridden cancel runs through an Action* base pointer", + "[inkevent][action]") +{ + Ptr mutex{new_ProxyMutex()}; + CountingContinuation cont{mutex.get()}; + RecordingAction derived; + derived = &cont; + Action *base = &derived; + + { + SCOPED_MUTEX_LOCK(lock, mutex, this_ethread()); + base->cancel(&cont); + } + + REQUIRE(derived.derived_cancel_calls.load() == 1); + REQUIRE(derived.cancelled == true); +} + +TEST_CASE("Action::cancel_action sets the cancelled flag without invoking a derived class's overridden cancel", + "[inkevent][action]") +{ + Ptr mutex{new_ProxyMutex()}; + CountingContinuation cont{mutex.get()}; + RecordingAction derived; + derived = &cont; + + { + SCOPED_MUTEX_LOCK(lock, mutex, this_ethread()); + derived.cancel_action(&cont); + } + + REQUIRE(derived.cancelled == true); + REQUIRE(derived.derived_cancel_calls.load() == 0); +} + +TEST_CASE("Destroying an Action that is bound to a Continuation drops its retained ProxyMutex reference", "[inkevent][action]") +{ + Ptr mutex{new_ProxyMutex()}; + CountingContinuation cont{mutex.get()}; + int const before = mutex->refcount(); + + { + Action a; + a = &cont; + REQUIRE(mutex->refcount() == before + 1); + } + + REQUIRE(mutex->refcount() == before); +} + +TEST_CASE("MAKE_ACTION_RESULT encodes a small integer with the low bit set so callers can distinguish sentinels from real Actions", + "[inkevent][action]") +{ + Action *s = MAKE_ACTION_RESULT(7); + REQUIRE((reinterpret_cast(s) & 1u) == 1u); + REQUIRE(reinterpret_cast(s) == ((7u << 1) + 1u)); +} + +TEST_CASE("ACTION_RESULT_DONE and ACTION_IO_ERROR are distinct sentinels with the low bit set", "[inkevent][action]") +{ + REQUIRE((reinterpret_cast(ACTION_RESULT_DONE) & 1u) == 1u); + REQUIRE((reinterpret_cast(ACTION_IO_ERROR) & 1u) == 1u); + REQUIRE(ACTION_RESULT_DONE != ACTION_IO_ERROR); +} + +TEST_CASE("A Continuation scheduled into the future and cancelled before its deadline receives no dispatched event", + "[inkevent][action][multithread]") +{ + Ptr target_mutex{new_ProxyMutex()}; + CountingContinuation target{target_mutex.get()}; + CountingContinuation barrier{new_ProxyMutex()}; + + { + SCOPED_MUTEX_LOCK(lock, target_mutex, this_ethread()); + Action *a = eventProcessor.schedule_in(&target, HRTIME_MSECONDS(20), ET_CALL); + REQUIRE(a != nullptr); + a->cancel(&target); + } + + // Schedule a barrier with a strictly later deadline than target's. The + // EThread cannot dispatch the barrier without first visiting target's slot + // (PriorityEventQueue dequeues by deadline), so awaiting the barrier proves + // the EThread actually ran past target's deadline — without which the + // count() == 0 assertion below would be vacuous on a stalled EThread. + Event *b = eventProcessor.schedule_in(&barrier, HRTIME_MSECONDS(150), ET_CALL); + REQUIRE(b != nullptr); + REQUIRE(barrier.wait_until_at_least(1)); + + REQUIRE(target.count() == 0); +}