From 82cc93924c431de5cd8365a7ac4045b106596bd7 Mon Sep 17 00:00:00 2001 From: jake champion Date: Thu, 3 Sep 2026 15:28:47 +0100 Subject: [PATCH 1/5] Fix cache object version compatibility on read `unmarshal_helper` picks `HTTPInfo::unmarshal_v24_1` for objects older than `CACHE_DB_VERSION`, but the real boundary is `24.2`, where `HTTPInfo`'s marshalled layout changed. They coincide today, so nothing misbehaves; a minor version bump parts them and every existing object would then be read with the wrong unmarshaller. Pin the comparison to a constant naming that boundary, with a static_assert so the two cannot cross. `scanObject` skipped the dispatch entirely and called `HTTPInfo::unmarshal` directly, so scanning a pre-24.2 object already used the wrong unmarshaller. It now shares `unmarshal_helper`. `load_http_info` recomputes the well known string indices persisted in marshalled headers for older objects, and skipped anything from the RAM cache as already fixed up. That does not hold when the RAM cache is compressed: those entries are stored still marshalled and decompressed into a fresh buffer on every hit, so the indices stay stale. Move the fixup into `unmarshal_helper`, gated on the alt still being `MARSHALED`, which holds exactly when this reader owns the buffer it repairs. The old placement had no such guard and could mutate a block other readers were aliasing. --- include/iocore/cache/CacheDefs.h | 14 ++++++--- include/tscore/Version.h | 12 ++++---- src/iocore/cache/Cache.cc | 2 -- src/iocore/cache/CacheHttp.cc | 28 +----------------- src/iocore/cache/CacheRead.cc | 12 +------- src/iocore/cache/CacheVC.cc | 50 +++++++++++++++++--------------- src/iocore/cache/CacheVC.h | 11 +++---- src/iocore/cache/P_CacheHttp.h | 1 - 8 files changed, 51 insertions(+), 79 deletions(-) diff --git a/include/iocore/cache/CacheDefs.h b/include/iocore/cache/CacheDefs.h index c5b3e304bf4..1a098faab44 100644 --- a/include/iocore/cache/CacheDefs.h +++ b/include/iocore/cache/CacheDefs.h @@ -39,10 +39,16 @@ enum class CacheInitState : int { static const uint8_t CACHE_DB_MAJOR_VERSION = 24; static const uint8_t CACHE_DB_MINOR_VERSION = 2; -// This is used in various comparisons because otherwise if the minor version is 0, -// the compile fails because the condition is always true or false. Running it through -// VersionNumber prevents that. -extern const ts::VersionNumber CACHE_DB_VERSION; +// Comparisons go through VersionNumber so a zero minor version does not make +// conditions tautological (always true or false) at compile time. +inline constexpr ts::VersionNumber CACHE_DB_VERSION{CACHE_DB_MAJOR_VERSION, CACHE_DB_MINOR_VERSION}; + +// The version at which HTTPInfo's marshalled layout last changed; objects older than +// this need HTTPInfo::unmarshal_v24_1. Fixed rather than tracking CACHE_DB_VERSION, +// which moves for unrelated reasons. +inline constexpr ts::VersionNumber CACHE_DB_VERSION_HTTPINFO_V24_2{24, 2}; +static_assert(CACHE_DB_VERSION >= CACHE_DB_VERSION_HTTPINFO_V24_2, + "CACHE_DB_VERSION must not be older than the HTTPInfo layout boundary"); static const uint8_t CACHE_DIR_MAJOR_VERSION = 18; static const uint8_t CACHE_DIR_MINOR_VERSION = 0; diff --git a/include/tscore/Version.h b/include/tscore/Version.h index 699e4b007b9..66af9f64c08 100644 --- a/include/tscore/Version.h +++ b/include/tscore/Version.h @@ -48,37 +48,37 @@ struct VersionNumber { inline constexpr VersionNumber::VersionNumber(unsigned short major, unsigned short minor) : _major(major), _minor(minor) {} -inline bool +inline constexpr bool operator<(VersionNumber const &lhs, VersionNumber const &rhs) { return lhs._major < rhs._major || (lhs._major == rhs._major && lhs._minor < rhs._minor); } -inline bool +inline constexpr bool operator==(VersionNumber const &lhs, VersionNumber const &rhs) { return lhs._major == rhs._major && lhs._minor == rhs._minor; } -inline bool +inline constexpr bool operator!=(VersionNumber const &lhs, VersionNumber const &rhs) { return !(lhs == rhs); } -inline bool +inline constexpr bool operator>(VersionNumber const &lhs, VersionNumber const &rhs) { return rhs < lhs; } -inline bool +inline constexpr bool operator<=(VersionNumber const &lhs, VersionNumber const &rhs) { return !(lhs > rhs); } -inline bool +inline constexpr bool operator>=(VersionNumber const &lhs, VersionNumber const &rhs) { return !(rhs > lhs); diff --git a/src/iocore/cache/Cache.cc b/src/iocore/cache/Cache.cc index d5dcf3df76b..eff54fbfeb4 100644 --- a/src/iocore/cache/Cache.cc +++ b/src/iocore/cache/Cache.cc @@ -52,8 +52,6 @@ extern void register_cache_stats(CacheStatsBlock *rsb, const std::string &prefix); -constexpr ts::VersionNumber CACHE_DB_VERSION(CACHE_DB_MAJOR_VERSION, CACHE_DB_MINOR_VERSION); - // Configuration int64_t cache_config_ram_cache_size = AUTO_SIZE_RAM_CACHE; diff --git a/src/iocore/cache/CacheHttp.cc b/src/iocore/cache/CacheHttp.cc index 4c48db5adb0..08bb6e2fa09 100644 --- a/src/iocore/cache/CacheHttp.cc +++ b/src/iocore/cache/CacheHttp.cc @@ -178,32 +178,6 @@ CacheHTTPInfoVector::marshal(char *buf, int length) return buf - start; } -int -CacheHTTPInfoVector::unmarshal(const char *buf, int length, RefCountObj *block_ptr) -{ - ink_assert(!(((intptr_t)buf) & 3)); // buf must be aligned - - const char *start = buf; - CacheHTTPInfo info; - xcount = 0; - - while (length - (buf - start) > static_cast(sizeof(HTTPCacheAlt))) { - int tmp = HTTPInfo::unmarshal(const_cast(buf), length - (buf - start), block_ptr); - if (tmp < 0) { - return -1; - } - info.m_alt = reinterpret_cast(const_cast(buf)); - buf += tmp; - - data(xcount).alternate = info; - xcount++; - } - - return (const_cast(buf) - const_cast(start)); -} - -/*------------------------------------------------------------------------- - -------------------------------------------------------------------------*/ uint32_t CacheHTTPInfoVector::get_handles(const char *buf, int length, RefCountObj *block_ptr) { @@ -218,7 +192,7 @@ CacheHTTPInfoVector::get_handles(const char *buf, int length, RefCountObj *block while (length - (buf - start) > static_cast(sizeof(HTTPCacheAlt))) { int tmp = info.get_handle(const_cast(buf), length - (buf - start)); if (tmp < 0) { - ink_assert(!"CacheHTTPInfoVector::unmarshal get_handle() failed"); + ink_assert(!"CacheHTTPInfoVector::get_handles get_handle() failed"); return static_cast(-1); } buf += tmp; diff --git a/src/iocore/cache/CacheRead.cc b/src/iocore/cache/CacheRead.cc index 5c60a68d700..9e219569a32 100644 --- a/src/iocore/cache/CacheRead.cc +++ b/src/iocore/cache/CacheRead.cc @@ -73,17 +73,7 @@ static constexpr bool test_force_corrupt_doc = false; uint32_t CacheVC::load_http_info(CacheHTTPInfoVector *info, Doc *doc, RefCountObj *block_ptr) { - uint32_t zret = info->get_handles(doc->hdr(), doc->hlen, block_ptr); - if (!this->f.doc_from_ram_cache && // ram cache is always already fixed up. - // If this is an old object, the object version will be old or 0, in either case this is - // correct. Forget the 4.2 compatibility, always update older versioned objects. - ts::VersionNumber(doc->v_major, doc->v_minor) < CACHE_DB_VERSION) { - for (int i = info->xcount - 1; i >= 0; --i) { - info->data(i).alternate.m_alt->m_response_hdr.m_mime->recompute_accelerators_and_presence_bits(); - info->data(i).alternate.m_alt->m_request_hdr.m_mime->recompute_accelerators_and_presence_bits(); - } - } - return zret; + return info->get_handles(doc->hdr(), doc->hlen, block_ptr); } int diff --git a/src/iocore/cache/CacheVC.cc b/src/iocore/cache/CacheVC.cc index b7443703b14..b31b9457128 100644 --- a/src/iocore/cache/CacheVC.cc +++ b/src/iocore/cache/CacheVC.cc @@ -318,31 +318,45 @@ CacheVC::dead(int /* event ATS_UNUSED */, Event * /*e ATS_UNUSED */) return EVENT_DONE; } -static void -unmarshal_helper(Doc *doc, Ptr &buf, int &okay) +static bool +unmarshal_helper(Doc *doc, Ptr &buf) { using UnmarshalFunc = int(char *buf, int len, RefCountObj *block_ref); UnmarshalFunc *unmarshal_func = &HTTPInfo::unmarshal; ts::VersionNumber version(doc->v_major, doc->v_minor); - // introduced by https://github.com/apache/trafficserver/pull/4874, this is used to distinguish the doc version - // before and after #4847 - if (version < CACHE_DB_VERSION) { + if (version < CACHE_DB_VERSION_HTTPINFO_V24_2) { unmarshal_func = &HTTPInfo::unmarshal_v24_1; } + // Objects written by an older version can carry stale well known string indices and + // presence bits. Repair them only on the MARSHALED to ALIVE transition, since an already + // ALIVE block may be shared with other readers. All alts of a doc transition together, so + // the first one answers for the whole header block. + bool const needs_wks_fixup = version < CACHE_DB_VERSION && doc->hlen > 0 && + reinterpret_cast(doc->hdr())->m_magic == CacheAltMagic::MARSHALED; + char *tmp = doc->hdr(); int len = doc->hlen; while (len > 0) { int r = unmarshal_func(tmp, len, buf.get()); + if (r < 0) { - ink_assert(!"CacheVC::handleReadDone unmarshal failed"); - okay = 0; - break; + ink_assert(!"unmarshal_helper: HTTPInfo unmarshal failed"); + return false; + } + if (needs_wks_fixup) { + auto *alt = reinterpret_cast(tmp); + for (HTTPHdr *hdr : {&alt->m_response_hdr, &alt->m_request_hdr}) { + if (hdr->valid()) { + hdr->m_mime->recompute_accelerators_and_presence_bits(); + } + } } len -= r; tmp += r; } + return true; } // [amc] I think this is where all disk reads from cache funnel through here. @@ -421,7 +435,7 @@ CacheVC::handleReadDone(int event, Event * /* e ATS_UNUSED */) // If http doc we need to unmarshal the headers before putting in the ram cache // unless it could be compressed if (!http_copy_hdr && doc->doc_type == CACHE_FRAG_TYPE_HTTP && doc->hlen && okay) { - unmarshal_helper(doc, buf, okay); + okay = unmarshal_helper(doc, buf); } // Put the request in the ram cache only if its a open_read or lookup if (vio.op == VIO::READ && okay) { @@ -452,7 +466,7 @@ CacheVC::handleReadDone(int event, Event * /* e ATS_UNUSED */) } // end VIO::READ check // If it could be compressed, unmarshal after if (http_copy_hdr && doc->doc_type == CACHE_FRAG_TYPE_HTTP && doc->hlen && okay) { - unmarshal_helper(doc, buf, okay); + okay = unmarshal_helper(doc, buf); } } // end io.ok() check } @@ -787,25 +801,15 @@ CacheVC::scanObject(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED */) // Bounds-check in unsigned domain: doc must lie within the // buffer, with room for the Doc header, and doc->hlen must // fit in the remaining bytes before doc->hdr() and - // HTTPInfo::unmarshal walk it. + // unmarshal_helper walk it. if (io.aiocb.aio_nbytes < doc_off || (io.aiocb.aio_nbytes - doc_off) < sizeof(Doc) || (io.aiocb.aio_nbytes - doc_off - sizeof(Doc)) < doc->hlen) { might_need_overlap_read = true; goto Lskip; } } - { - char *tmp = doc->hdr(); - int len = doc->hlen; - while (len > 0) { - int r = HTTPInfo::unmarshal(tmp, len, buf.get()); - if (r < 0) { - ink_assert(!"CacheVC::scanObject unmarshal failed"); - goto Lskip; - } - len -= r; - tmp += r; - } + if (!unmarshal_helper(doc, buf)) { + goto Lskip; } if (this->load_http_info(&vector, doc) != doc->hlen) { goto Lskip; diff --git a/src/iocore/cache/CacheVC.h b/src/iocore/cache/CacheVC.h index 601c3f9ea04..2243242b6ce 100644 --- a/src/iocore/cache/CacheVC.h +++ b/src/iocore/cache/CacheVC.h @@ -201,13 +201,14 @@ struct CacheVC : public CacheVConnection { or @c nullptr if there is no fragment table. */ virtual HTTPInfo::FragOffset *get_frag_table(); - /** Load alt pointers and do fixups if needed. + /** Load alt pointers from an already unmarshalled header block. + The block may be shared with other readers, so it must not be modified here. @return Length of header data used for alternates. */ - virtual uint32_t load_http_info(CacheHTTPInfoVector *info, struct Doc *doc, RefCountObj *block_ptr = nullptr); - bool is_pread_capable() override; - bool set_pin_in_cache(time_t time_pin) override; - time_t get_pin_in_cache() override; + uint32_t load_http_info(CacheHTTPInfoVector *info, struct Doc *doc, RefCountObj *block_ptr = nullptr); + bool is_pread_capable() override; + bool set_pin_in_cache(time_t time_pin) override; + time_t get_pin_in_cache() override; // number of bytes to memset to 0 in the CacheVC when we free // it. All member variables starting from vio are memset to 0. diff --git a/src/iocore/cache/P_CacheHttp.h b/src/iocore/cache/P_CacheHttp.h index 77f84713202..e61d39aef89 100644 --- a/src/iocore/cache/P_CacheHttp.h +++ b/src/iocore/cache/P_CacheHttp.h @@ -63,7 +63,6 @@ struct CacheHTTPInfoVector { int marshal_length(); int marshal(char *buf, int length); uint32_t get_handles(const char *buf, int length, RefCountObj *block_ptr = nullptr); - int unmarshal(const char *buf, int length, RefCountObj *block_ptr); CacheArray data; int xcount = 0; From c0397c86d149fbeafe76b5a5b29b46c89fad7eee Mon Sep 17 00:00:00 2001 From: jake champion Date: Fri, 4 Sep 2026 12:00:17 +0100 Subject: [PATCH 2/5] Add version compatibility tests for cache header unmarshalling --- src/iocore/cache/CMakeLists.txt | 1 + src/iocore/cache/CacheVC.cc | 14 +- src/iocore/cache/CacheVC.h | 5 + .../cache/unit_tests/test_Unmarshal_Compat.cc | 249 ++++++++++++++++++ 4 files changed, 262 insertions(+), 7 deletions(-) create mode 100644 src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc diff --git a/src/iocore/cache/CMakeLists.txt b/src/iocore/cache/CMakeLists.txt index c9ba5bd2fde..390808b8f17 100644 --- a/src/iocore/cache/CMakeLists.txt +++ b/src/iocore/cache/CMakeLists.txt @@ -94,6 +94,7 @@ if(BUILD_TESTING) add_cache_test(CacheStripe unit_tests/test_Stripe.cc) add_cache_test(CacheAggregateWriteBuffer unit_tests/test_AggregateWriteBuffer.cc) add_cache_test(RamCacheCompressEntries unit_tests/test_RamCacheCompressEntries.cc) + add_cache_test(CacheUnmarshalCompat unit_tests/test_Unmarshal_Compat.cc) # Only the shutdown test attaches a live segment; the rest need no shm syscall. add_cache_test(CacheShm unit_tests/test_CacheShm.cc) if(TS_USE_CACHE_SHM) diff --git a/src/iocore/cache/CacheVC.cc b/src/iocore/cache/CacheVC.cc index b31b9457128..ceb3741a34c 100644 --- a/src/iocore/cache/CacheVC.cc +++ b/src/iocore/cache/CacheVC.cc @@ -318,8 +318,8 @@ CacheVC::dead(int /* event ATS_UNUSED */, Event * /*e ATS_UNUSED */) return EVENT_DONE; } -static bool -unmarshal_helper(Doc *doc, Ptr &buf) +bool +CacheVC::unmarshal_http_info(Doc *doc, Ptr &buf) { using UnmarshalFunc = int(char *buf, int len, RefCountObj *block_ref); UnmarshalFunc *unmarshal_func = &HTTPInfo::unmarshal; @@ -342,7 +342,7 @@ unmarshal_helper(Doc *doc, Ptr &buf) int r = unmarshal_func(tmp, len, buf.get()); if (r < 0) { - ink_assert(!"unmarshal_helper: HTTPInfo unmarshal failed"); + ink_assert(!"CacheVC::unmarshal_http_info: HTTPInfo unmarshal failed"); return false; } if (needs_wks_fixup) { @@ -435,7 +435,7 @@ CacheVC::handleReadDone(int event, Event * /* e ATS_UNUSED */) // If http doc we need to unmarshal the headers before putting in the ram cache // unless it could be compressed if (!http_copy_hdr && doc->doc_type == CACHE_FRAG_TYPE_HTTP && doc->hlen && okay) { - okay = unmarshal_helper(doc, buf); + okay = CacheVC::unmarshal_http_info(doc, buf); } // Put the request in the ram cache only if its a open_read or lookup if (vio.op == VIO::READ && okay) { @@ -466,7 +466,7 @@ CacheVC::handleReadDone(int event, Event * /* e ATS_UNUSED */) } // end VIO::READ check // If it could be compressed, unmarshal after if (http_copy_hdr && doc->doc_type == CACHE_FRAG_TYPE_HTTP && doc->hlen && okay) { - okay = unmarshal_helper(doc, buf); + okay = CacheVC::unmarshal_http_info(doc, buf); } } // end io.ok() check } @@ -801,14 +801,14 @@ CacheVC::scanObject(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED */) // Bounds-check in unsigned domain: doc must lie within the // buffer, with room for the Doc header, and doc->hlen must // fit in the remaining bytes before doc->hdr() and - // unmarshal_helper walk it. + // unmarshal_http_info walk it. if (io.aiocb.aio_nbytes < doc_off || (io.aiocb.aio_nbytes - doc_off) < sizeof(Doc) || (io.aiocb.aio_nbytes - doc_off - sizeof(Doc)) < doc->hlen) { might_need_overlap_read = true; goto Lskip; } } - if (!unmarshal_helper(doc, buf)) { + if (!CacheVC::unmarshal_http_info(doc, buf)) { goto Lskip; } if (this->load_http_info(&vector, doc) != doc->hlen) { diff --git a/src/iocore/cache/CacheVC.h b/src/iocore/cache/CacheVC.h index 2243242b6ce..6ede80c7bd6 100644 --- a/src/iocore/cache/CacheVC.h +++ b/src/iocore/cache/CacheVC.h @@ -201,6 +201,11 @@ struct CacheVC : public CacheVConnection { or @c nullptr if there is no fragment table. */ virtual HTTPInfo::FragOffset *get_frag_table(); + /** Unmarshal every alt in @a doc's header block, selecting the decoder for the + version @a doc was written with and repairing anything that version left stale. + @return @c true if the whole block was unmarshalled. + */ + static bool unmarshal_http_info(struct Doc *doc, Ptr &buf); /** Load alt pointers from an already unmarshalled header block. The block may be shared with other readers, so it must not be modified here. @return Length of header data used for alternates. diff --git a/src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc b/src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc new file mode 100644 index 00000000000..aeb49bd95c3 --- /dev/null +++ b/src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc @@ -0,0 +1,249 @@ +/** @file + + Version compatibility tests for CacheVC::unmarshal_http_info. + + @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 "main.h" + +#include "../P_CacheDoc.h" +#include "iocore/cache/CacheDefs.h" +#include "proxy/hdrs/HdrHeap.h" +#include "tscore/ink_memory.h" + +#include +#include + +int cache_vols = 1; +bool reuse_existing_cache = false; + +namespace +{ +int constexpr ALT_MARSHAL_SIZE = HdrHeapMarshalBlocks{swoc::round_up(sizeof(HTTPCacheAlt))}; +int constexpr N_INTEGRAL = HTTPCacheAlt::N_INTEGRAL_FRAG_OFFSETS; +int constexpr FRAG_COUNT = N_INTEGRAL + 2; + +using FragOffset = HTTPInfo::FragOffset; + +/** A Doc followed by a header block, versioned as an on-disk object would be. */ +class DocBuffer +{ +public: + DocBuffer(uint8_t major, uint8_t minor, int hlen) : _storage((sizeof(Doc) + hlen + 7) / sizeof(uint64_t) + 1, 0) + { + Doc *d = this->doc(); + + d->magic = DOC_MAGIC; + d->doc_type = CACHE_FRAG_TYPE_HTTP; + d->v_major = major; + d->v_minor = minor; + d->hlen = hlen; + d->len = sizeof(Doc) + hlen; + } + + Doc * + doc() + { + return reinterpret_cast(_storage.data()); + } + + HTTPCacheAlt * + alt() + { + return reinterpret_cast(this->doc()->hdr()); + } + +private: + std::vector _storage; +}; + +/** Initialize the alt header the way HTTPInfo::marshal leaves it, with no header heaps. */ +void +init_marshalled_alt(HTTPCacheAlt *alt, int frag_count, intptr_t frag_table_offset) +{ + alt->m_magic = CacheAltMagic::MARSHALED; + alt->m_writeable = 0; + alt->m_unmarshal_len = -1; + alt->m_frag_offset_count = frag_count; + + *reinterpret_cast(&alt->m_frag_offsets) = frag_table_offset; +} + +/** The 24.2 layout: the whole fragment offset table follows the alt. */ +DocBuffer +make_v24_2_doc(uint8_t major, uint8_t minor) +{ + DocBuffer buffer{major, minor, static_cast(ALT_MARSHAL_SIZE + FRAG_COUNT * sizeof(FragOffset))}; + + init_marshalled_alt(buffer.alt(), FRAG_COUNT, ALT_MARSHAL_SIZE); + + auto *table = reinterpret_cast(buffer.doc()->hdr() + ALT_MARSHAL_SIZE); + + for (int i = 0; i < FRAG_COUNT; ++i) { + table[i] = i; + } + return buffer; +} + +/** The 24.1 layout: the first N_INTEGRAL offsets are inline, only the rest follow. */ +DocBuffer +make_v24_1_doc(uint8_t major, uint8_t minor) +{ + int constexpr extra = FRAG_COUNT - N_INTEGRAL; + DocBuffer buffer{major, minor, static_cast(ALT_MARSHAL_SIZE + extra * sizeof(FragOffset))}; + + init_marshalled_alt(buffer.alt(), FRAG_COUNT, ALT_MARSHAL_SIZE); + + for (int i = 0; i < N_INTEGRAL; ++i) { + buffer.alt()->m_integral_frag_offsets[i] = i; + } + + auto *table = reinterpret_cast(buffer.doc()->hdr() + ALT_MARSHAL_SIZE); + + for (int i = 0; i < extra; ++i) { + table[i] = N_INTEGRAL + i; + } + return buffer; +} + +void +check_offsets_are_sequential(HTTPCacheAlt *alt) +{ + REQUIRE(alt->m_frag_offsets != nullptr); + for (int i = 0; i < FRAG_COUNT; ++i) { + CHECK(alt->m_frag_offsets[i] == static_cast(i)); + } +} + +/** unmarshal_v24_1 copies the table onto the heap; the caller owns it. */ +void +free_frag_offsets(HTTPCacheAlt *alt) +{ + if (alt->m_frag_offsets != nullptr && alt->m_frag_offsets != alt->m_integral_frag_offsets) { + ats_free(alt->m_frag_offsets); + alt->m_frag_offsets = nullptr; + } +} + +} // end anonymous namespace + +TEST_CASE("unmarshal_http_info selects the decoder matching the object version", "[cache][unmarshal][compat]") +{ + Ptr buf; + + SECTION("an object at the layout boundary is read with the current decoder") + { + DocBuffer doc{make_v24_2_doc(CACHE_DB_VERSION_HTTPINFO_V24_2._major, CACHE_DB_VERSION_HTTPINFO_V24_2._minor)}; + + REQUIRE(CacheVC::unmarshal_http_info(doc.doc(), buf)); + check_offsets_are_sequential(doc.alt()); + // Only the current decoder leaves the table in the buffer; the 24.1 one copies it out, + // so this is what says which of the two ran. + CHECK(reinterpret_cast(doc.alt()->m_frag_offsets) == doc.doc()->hdr() + ALT_MARSHAL_SIZE); + } + + SECTION("an object below the layout boundary is read with the 24.1 decoder") + { + DocBuffer doc{make_v24_1_doc(CACHE_DB_VERSION_HTTPINFO_V24_2._major, CACHE_DB_VERSION_HTTPINFO_V24_2._minor - 1)}; + + // The current decoder would reject this layout outright: it expects the whole table + // to follow the alt, and only the offsets past the integral ones are there. + REQUIRE(CacheVC::unmarshal_http_info(doc.doc(), buf)); + check_offsets_are_sequential(doc.alt()); + CHECK(reinterpret_cast(doc.alt()->m_frag_offsets) != doc.doc()->hdr() + ALT_MARSHAL_SIZE); + free_frag_offsets(doc.alt()); + } +} + +TEST_CASE("unmarshal_http_info repairs stale accelerators only when it owns the block", "[cache][unmarshal][compat]") +{ + HTTPInfo info; + + info.create(); + build_hdrs(info, "http://www.example.com/test.html"); + + int const hlen = info.marshal_length(); + std::vector marshalled(hlen / sizeof(uint64_t) + 1, 0); + + REQUIRE(info.marshal(reinterpret_cast(marshalled.data()), hlen) == hlen); + + auto load = [&](DocBuffer &doc) { memcpy(doc.doc()->hdr(), marshalled.data(), hlen); }; + + // Unmarshal an untouched copy to learn where the response MIME header lands and what + // its presence bits should be. Every copy below is byte identical, so the offset holds. + ptrdiff_t mime_offset = 0; + uint64_t correct_bits = 0; + { + DocBuffer doc{CACHE_DB_MAJOR_VERSION, CACHE_DB_MINOR_VERSION, hlen}; + Ptr buf; + + load(doc); + REQUIRE(CacheVC::unmarshal_http_info(doc.doc(), buf)); + REQUIRE(doc.alt()->m_response_hdr.valid()); + + mime_offset = reinterpret_cast(doc.alt()->m_response_hdr.m_mime) - doc.doc()->hdr(); + correct_bits = doc.alt()->m_response_hdr.m_mime->m_presence_bits; + REQUIRE(correct_bits != 0); + } + + auto corrupt_presence_bits = [&](DocBuffer &doc) { + reinterpret_cast(doc.doc()->hdr() + mime_offset)->m_presence_bits = 0; + }; + + SECTION("an older object is repaired") + { + DocBuffer doc{CACHE_DB_MAJOR_VERSION, CACHE_DB_MINOR_VERSION - 1, hlen}; + Ptr buf; + + load(doc); + corrupt_presence_bits(doc); + REQUIRE(CacheVC::unmarshal_http_info(doc.doc(), buf)); + CHECK(doc.alt()->m_response_hdr.m_mime->m_presence_bits == correct_bits); + } + + SECTION("a current object is left alone") + { + DocBuffer doc{CACHE_DB_MAJOR_VERSION, CACHE_DB_MINOR_VERSION, hlen}; + Ptr buf; + + load(doc); + corrupt_presence_bits(doc); + REQUIRE(CacheVC::unmarshal_http_info(doc.doc(), buf)); + CHECK(doc.alt()->m_response_hdr.m_mime->m_presence_bits == 0); + } + + SECTION("an already unmarshalled block is left alone even for an older object") + { + // The block may be shared with other readers at this point, so the repair must not + // run a second time. This is what the MARSHALED check buys. + DocBuffer doc{CACHE_DB_MAJOR_VERSION, CACHE_DB_MINOR_VERSION - 1, hlen}; + Ptr buf; + + load(doc); + REQUIRE(CacheVC::unmarshal_http_info(doc.doc(), buf)); + REQUIRE(doc.alt()->m_magic == CacheAltMagic::ALIVE); + + doc.alt()->m_response_hdr.m_mime->m_presence_bits = 0; + REQUIRE(CacheVC::unmarshal_http_info(doc.doc(), buf)); + CHECK(doc.alt()->m_response_hdr.m_mime->m_presence_bits == 0); + } + + info.destroy(); +} From 52355b809abefc714f0225739c93e98c5ef8ebb8 Mon Sep 17 00:00:00 2001 From: jake champion Date: Fri, 4 Sep 2026 12:20:38 +0100 Subject: [PATCH 3/5] Guard the header block walk against a truncated tail --- src/iocore/cache/CacheVC.cc | 8 +++++++- src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc | 9 +++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/iocore/cache/CacheVC.cc b/src/iocore/cache/CacheVC.cc index ceb3741a34c..752b8708cd7 100644 --- a/src/iocore/cache/CacheVC.cc +++ b/src/iocore/cache/CacheVC.cc @@ -333,12 +333,18 @@ CacheVC::unmarshal_http_info(Doc *doc, Ptr &buf) // presence bits. Repair them only on the MARSHALED to ALIVE transition, since an already // ALIVE block may be shared with other readers. All alts of a doc transition together, so // the first one answers for the whole header block. - bool const needs_wks_fixup = version < CACHE_DB_VERSION && doc->hlen > 0 && + bool const needs_wks_fixup = version < CACHE_DB_VERSION && doc->hlen >= sizeof(HTTPCacheAlt) && reinterpret_cast(doc->hdr())->m_magic == CacheAltMagic::MARSHALED; char *tmp = doc->hdr(); int len = doc->hlen; while (len > 0) { + // The decoders read the alt header before they check any length, so a tail too short + // to hold one has to be rejected here rather than passed down. + if (static_cast(len) < sizeof(HTTPCacheAlt)) { + ink_assert(!"CacheVC::unmarshal_http_info: truncated header block"); + return false; + } int r = unmarshal_func(tmp, len, buf.get()); if (r < 0) { diff --git a/src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc b/src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc index aeb49bd95c3..c66f4ab9ed0 100644 --- a/src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc +++ b/src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc @@ -40,6 +40,11 @@ int constexpr ALT_MARSHAL_SIZE = HdrHeapMarshalBlocks{swoc::round_up(sizeof(HTTP int constexpr N_INTEGRAL = HTTPCacheAlt::N_INTEGRAL_FRAG_OFFSETS; int constexpr FRAG_COUNT = N_INTEGRAL + 2; +// The static_assert in CacheDefs.h holds CACHE_DB_VERSION at or above {24, 2}, so this stays +// older than the current version wherever that moves. Subtracting one from +// CACHE_DB_MINOR_VERSION instead would wrap the minor when a major bump resets it to zero. +ts::VersionNumber constexpr OLDER_THAN_CURRENT{24, 1}; + using FragOffset = HTTPInfo::FragOffset; /** A Doc followed by a header block, versioned as an on-disk object would be. */ @@ -209,7 +214,7 @@ TEST_CASE("unmarshal_http_info repairs stale accelerators only when it owns the SECTION("an older object is repaired") { - DocBuffer doc{CACHE_DB_MAJOR_VERSION, CACHE_DB_MINOR_VERSION - 1, hlen}; + DocBuffer doc{OLDER_THAN_CURRENT._major, OLDER_THAN_CURRENT._minor, hlen}; Ptr buf; load(doc); @@ -233,7 +238,7 @@ TEST_CASE("unmarshal_http_info repairs stale accelerators only when it owns the { // The block may be shared with other readers at this point, so the repair must not // run a second time. This is what the MARSHALED check buys. - DocBuffer doc{CACHE_DB_MAJOR_VERSION, CACHE_DB_MINOR_VERSION - 1, hlen}; + DocBuffer doc{OLDER_THAN_CURRENT._major, OLDER_THAN_CURRENT._minor, hlen}; Ptr buf; load(doc); From 1ed98baf3282c2ee3bd507b0916902768b50d260 Mon Sep 17 00:00:00 2001 From: jake champion Date: Fri, 4 Sep 2026 14:41:32 +0100 Subject: [PATCH 4/5] Reject malformed header lengths instead of reporting success --- src/iocore/cache/CacheVC.cc | 10 ++++++- .../cache/unit_tests/test_Unmarshal_Compat.cc | 26 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/iocore/cache/CacheVC.cc b/src/iocore/cache/CacheVC.cc index 752b8708cd7..04ee60a1e52 100644 --- a/src/iocore/cache/CacheVC.cc +++ b/src/iocore/cache/CacheVC.cc @@ -66,6 +66,7 @@ #include #include #include +#include namespace { @@ -325,6 +326,13 @@ CacheVC::unmarshal_http_info(Doc *doc, Ptr &buf) UnmarshalFunc *unmarshal_func = &HTTPInfo::unmarshal; ts::VersionNumber version(doc->v_major, doc->v_minor); + // hlen is unsigned and the walk below is not. Narrowing a header length this large would + // make the walk negative, skipping it and reporting success on a block nothing decoded. + if (doc->hlen > static_cast(std::numeric_limits::max())) { + Warning("CacheVC::unmarshal_http_info: header length %u exceeds the maximum - corrupt cache entry", doc->hlen); + return false; + } + if (version < CACHE_DB_VERSION_HTTPINFO_V24_2) { unmarshal_func = &HTTPInfo::unmarshal_v24_1; } @@ -342,7 +350,7 @@ CacheVC::unmarshal_http_info(Doc *doc, Ptr &buf) // The decoders read the alt header before they check any length, so a tail too short // to hold one has to be rejected here rather than passed down. if (static_cast(len) < sizeof(HTTPCacheAlt)) { - ink_assert(!"CacheVC::unmarshal_http_info: truncated header block"); + Warning("CacheVC::unmarshal_http_info: header block ends mid alternate - corrupt cache entry"); return false; } int r = unmarshal_func(tmp, len, buf.get()); diff --git a/src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc b/src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc index c66f4ab9ed0..10bb18d91b5 100644 --- a/src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc +++ b/src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc @@ -29,6 +29,7 @@ #include "tscore/ink_memory.h" #include +#include #include int cache_vols = 1; @@ -177,6 +178,31 @@ TEST_CASE("unmarshal_http_info selects the decoder matching the object version", } } +TEST_CASE("unmarshal_http_info reports failure on a malformed header length", "[cache][unmarshal][compat]") +{ + Ptr buf; + + // A header block nothing decoded must never be reported as unmarshalled: the caller + // would go on to read it as though the alts were live. + SECTION("a header length that cannot be walked as a signed length") + { + DocBuffer doc{make_v24_2_doc(CACHE_DB_VERSION_HTTPINFO_V24_2._major, CACHE_DB_VERSION_HTTPINFO_V24_2._minor)}; + + doc.doc()->hlen = static_cast(std::numeric_limits::max()) + 1; + CHECK_FALSE(CacheVC::unmarshal_http_info(doc.doc(), buf)); + CHECK(doc.alt()->m_magic == CacheAltMagic::MARSHALED); + } + + SECTION("a header block ending part way through an alternate") + { + DocBuffer doc{make_v24_2_doc(CACHE_DB_VERSION_HTTPINFO_V24_2._major, CACHE_DB_VERSION_HTTPINFO_V24_2._minor)}; + + doc.doc()->hlen = sizeof(HTTPCacheAlt) - 1; + CHECK_FALSE(CacheVC::unmarshal_http_info(doc.doc(), buf)); + CHECK(doc.alt()->m_magic == CacheAltMagic::MARSHALED); + } +} + TEST_CASE("unmarshal_http_info repairs stale accelerators only when it owns the block", "[cache][unmarshal][compat]") { HTTPInfo info; From 73fb752ebf43b8ef702318780464eb7dc05d9572 Mon Sep 17 00:00:00 2001 From: jake champion Date: Thu, 17 Sep 2026 14:09:41 +0100 Subject: [PATCH 5/5] Repair all persisted well known string indices on cache read --- include/proxy/hdrs/HTTP.h | 13 +++ include/proxy/hdrs/URL.h | 9 +++ src/iocore/cache/CacheVC.cc | 16 ++-- .../cache/unit_tests/test_Unmarshal_Compat.cc | 80 +++++++++++++++---- src/proxy/hdrs/HTTP.cc | 20 +++++ src/proxy/hdrs/URL.cc | 10 +++ 6 files changed, 128 insertions(+), 20 deletions(-) diff --git a/include/proxy/hdrs/HTTP.h b/include/proxy/hdrs/HTTP.h index c23f8dc353c..1ec6a632dbe 100644 --- a/include/proxy/hdrs/HTTP.h +++ b/include/proxy/hdrs/HTTP.h @@ -284,6 +284,19 @@ struct HTTPHdrImpl : public HdrHeapObjImpl { void move_strings(HdrStrHeap *new_heap); size_t strings_length(); + /** Rebuild everything in this header that indexes the well known string table. + * + * That is the request method index, the request URL's scheme index, and the field indexes, + * presence bits and slot accelerators of the MIME header. All of them are caches over strings + * the header carries itself, so they can always be rebuilt, and they have to be after the + * header is read back from a cached object written by a build whose table differed from this + * one's: a stale index makes the getters report a string the header does not hold. + * + * Call this only once the header is fully unmarshalled. It walks the MIME field blocks, which + * are separate heap objects and are not usable until their own pointers have been swizzled. + */ + void recompute_wks_indices(); + // Sanity Check Functions void check_strings(HeapCheck *heaps, int num_heaps); }; diff --git a/include/proxy/hdrs/URL.h b/include/proxy/hdrs/URL.h index e8aeee0978d..02d33b8268b 100644 --- a/include/proxy/hdrs/URL.h +++ b/include/proxy/hdrs/URL.h @@ -113,6 +113,15 @@ class URLImpl : public HdrHeapObjImpl void rehome_strings(HdrHeap *new_heap); size_t strings_length(); + /** Re-derive m_scheme_wks_idx from the scheme string. + * + * get_scheme() answers from the index whenever it is set and only falls back to m_ptr_scheme + * when it is not, so an index that no longer denotes the scheme stored beside it makes the URL + * report a scheme it does not hold. The string is authoritative, so the index can always be + * rebuilt from it. + */ + void recompute_wks_idx(); + // Sanity Check Functions void check_strings(HeapCheck *heaps, int num_heaps); diff --git a/src/iocore/cache/CacheVC.cc b/src/iocore/cache/CacheVC.cc index 04ee60a1e52..179b29f84f7 100644 --- a/src/iocore/cache/CacheVC.cc +++ b/src/iocore/cache/CacheVC.cc @@ -337,10 +337,16 @@ CacheVC::unmarshal_http_info(Doc *doc, Ptr &buf) unmarshal_func = &HTTPInfo::unmarshal_v24_1; } - // Objects written by an older version can carry stale well known string indices and - // presence bits. Repair them only on the MARSHALED to ALIVE transition, since an already - // ALIVE block may be shared with other readers. All alts of a doc transition together, so - // the first one answers for the whole header block. + // Objects written by an older version can carry stale well known string indices, and the + // presence bits and accelerators derived from them. Repair them only on the MARSHALED to + // ALIVE transition, since an already ALIVE block may be shared with other readers. All alts + // of a doc transition together, so the first one answers for the whole header block. + // + // This gate covers a version difference and nothing else. Nothing ties a revision of the well + // known string table in proxy/hdrs/HdrToken.cc to a cache version, so two builds at the same + // CACHE_DB_VERSION can still disagree about what a stored index means, and no version + // comparison can see that. Changing the table is what would call for rebuilding here + // unconditionally, which is why the table has been frozen rather than versioned. bool const needs_wks_fixup = version < CACHE_DB_VERSION && doc->hlen >= sizeof(HTTPCacheAlt) && reinterpret_cast(doc->hdr())->m_magic == CacheAltMagic::MARSHALED; @@ -363,7 +369,7 @@ CacheVC::unmarshal_http_info(Doc *doc, Ptr &buf) auto *alt = reinterpret_cast(tmp); for (HTTPHdr *hdr : {&alt->m_response_hdr, &alt->m_request_hdr}) { if (hdr->valid()) { - hdr->m_mime->recompute_accelerators_and_presence_bits(); + hdr->m_http->recompute_wks_indices(); } } } diff --git a/src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc b/src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc index 10bb18d91b5..eabb40c9154 100644 --- a/src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc +++ b/src/iocore/cache/unit_tests/test_Unmarshal_Compat.cc @@ -203,7 +203,7 @@ TEST_CASE("unmarshal_http_info reports failure on a malformed header length", "[ } } -TEST_CASE("unmarshal_http_info repairs stale accelerators only when it owns the block", "[cache][unmarshal][compat]") +TEST_CASE("unmarshal_http_info repairs stale well known string indices only when it owns the block", "[cache][unmarshal][compat]") { HTTPInfo info; @@ -217,10 +217,14 @@ TEST_CASE("unmarshal_http_info repairs stale accelerators only when it owns the auto load = [&](DocBuffer &doc) { memcpy(doc.doc()->hdr(), marshalled.data(), hlen); }; - // Unmarshal an untouched copy to learn where the response MIME header lands and what - // its presence bits should be. Every copy below is byte identical, so the offset holds. - ptrdiff_t mime_offset = 0; - uint64_t correct_bits = 0; + // Unmarshal an untouched copy to learn where the headers land and what their indices should + // be. Every copy below is byte identical and unmarshalling moves nothing, so the offsets hold. + ptrdiff_t mime_offset = 0; + ptrdiff_t request_offset = 0; + ptrdiff_t url_offset = 0; + uint64_t correct_bits = 0; + int16_t correct_method_idx = 0; + int16_t correct_scheme_idx = 0; { DocBuffer doc{CACHE_DB_MAJOR_VERSION, CACHE_DB_MINOR_VERSION, hlen}; Ptr buf; @@ -228,14 +232,47 @@ TEST_CASE("unmarshal_http_info repairs stale accelerators only when it owns the load(doc); REQUIRE(CacheVC::unmarshal_http_info(doc.doc(), buf)); REQUIRE(doc.alt()->m_response_hdr.valid()); + REQUIRE(doc.alt()->m_request_hdr.valid()); + + HTTPHdrImpl *request = doc.alt()->m_request_hdr.m_http; + + REQUIRE(request->u.req.m_url_impl != nullptr); + + mime_offset = reinterpret_cast(doc.alt()->m_response_hdr.m_mime) - doc.doc()->hdr(); + request_offset = reinterpret_cast(request) - doc.doc()->hdr(); + url_offset = reinterpret_cast(request->u.req.m_url_impl) - doc.doc()->hdr(); + + correct_bits = doc.alt()->m_response_hdr.m_mime->m_presence_bits; + correct_method_idx = request->u.req.m_method_wks_idx; + correct_scheme_idx = request->u.req.m_url_impl->m_scheme_wks_idx; - mime_offset = reinterpret_cast(doc.alt()->m_response_hdr.m_mime) - doc.doc()->hdr(); - correct_bits = doc.alt()->m_response_hdr.m_mime->m_presence_bits; REQUIRE(correct_bits != 0); + REQUIRE(correct_method_idx >= 0); + REQUIRE(correct_scheme_idx >= 0); } - auto corrupt_presence_bits = [&](DocBuffer &doc) { - reinterpret_cast(doc.doc()->hdr() + mime_offset)->m_presence_bits = 0; + // Everything below reaches the headers through these offsets rather than through the alt, + // since a block that was not repaired is still marshalled and its pointers not yet swizzled. + auto presence_bits = [&](DocBuffer &doc) -> uint64_t & { + return reinterpret_cast(doc.doc()->hdr() + mime_offset)->m_presence_bits; + }; + auto method_idx = [&](DocBuffer &doc) -> int16_t & { + return reinterpret_cast(doc.doc()->hdr() + request_offset)->u.req.m_method_wks_idx; + }; + auto scheme_idx = [&](DocBuffer &doc) -> int16_t & { + return reinterpret_cast(doc.doc()->hdr() + url_offset)->m_scheme_wks_idx; + }; + + // Stand in for an object written against a different well known string table: every stored + // index names an adjacent string, and the bits derived from them are gone. Shifting rather + // than clearing is what makes a repair that only rebuilds a subset of them visible. + int16_t const stale_method_idx = correct_method_idx + 1; + int16_t const stale_scheme_idx = correct_scheme_idx + 1; + + auto make_indices_stale = [&](DocBuffer &doc) { + presence_bits(doc) = 0; + method_idx(doc) = stale_method_idx; + scheme_idx(doc) = stale_scheme_idx; }; SECTION("an older object is repaired") @@ -244,9 +281,16 @@ TEST_CASE("unmarshal_http_info repairs stale accelerators only when it owns the Ptr buf; load(doc); - corrupt_presence_bits(doc); + make_indices_stale(doc); REQUIRE(CacheVC::unmarshal_http_info(doc.doc(), buf)); - CHECK(doc.alt()->m_response_hdr.m_mime->m_presence_bits == correct_bits); + + CHECK(presence_bits(doc) == correct_bits); + // The getters answer from these indices in preference to the strings stored beside them, so + // leaving either stale makes the cached request report a method or scheme it does not hold. + CHECK(method_idx(doc) == correct_method_idx); + CHECK(scheme_idx(doc) == correct_scheme_idx); + CHECK(doc.alt()->m_request_hdr.method_get() == "GET"); + CHECK(doc.alt()->m_request_hdr.scheme_get() == "http"); } SECTION("a current object is left alone") @@ -255,9 +299,12 @@ TEST_CASE("unmarshal_http_info repairs stale accelerators only when it owns the Ptr buf; load(doc); - corrupt_presence_bits(doc); + make_indices_stale(doc); REQUIRE(CacheVC::unmarshal_http_info(doc.doc(), buf)); - CHECK(doc.alt()->m_response_hdr.m_mime->m_presence_bits == 0); + + CHECK(presence_bits(doc) == 0); + CHECK(method_idx(doc) == stale_method_idx); + CHECK(scheme_idx(doc) == stale_scheme_idx); } SECTION("an already unmarshalled block is left alone even for an older object") @@ -271,9 +318,12 @@ TEST_CASE("unmarshal_http_info repairs stale accelerators only when it owns the REQUIRE(CacheVC::unmarshal_http_info(doc.doc(), buf)); REQUIRE(doc.alt()->m_magic == CacheAltMagic::ALIVE); - doc.alt()->m_response_hdr.m_mime->m_presence_bits = 0; + make_indices_stale(doc); REQUIRE(CacheVC::unmarshal_http_info(doc.doc(), buf)); - CHECK(doc.alt()->m_response_hdr.m_mime->m_presence_bits == 0); + + CHECK(presence_bits(doc) == 0); + CHECK(method_idx(doc) == stale_method_idx); + CHECK(scheme_idx(doc) == stale_scheme_idx); } info.destroy(); diff --git a/src/proxy/hdrs/HTTP.cc b/src/proxy/hdrs/HTTP.cc index 1f86b5fadfe..761116e9f0a 100644 --- a/src/proxy/hdrs/HTTP.cc +++ b/src/proxy/hdrs/HTTP.cc @@ -1965,6 +1965,26 @@ HTTPHdrImpl::unmarshal(intptr_t offset) HDR_UNMARSHAL_PTR(m_fields_impl, MIMEHdrImpl, offset); } +void +HTTPHdrImpl::recompute_wks_indices() +{ + if (m_polarity == HTTPType::REQUEST) { + // http_hdr_method_get() answers from the index when it is set, so a stale index would report a + // different method than the one stored here. Tokenize case sensitively, exactly as the parser + // does, so a method that only matches a well known string case insensitively stays untokenized. + u.req.m_method_wks_idx = u.req.m_ptr_method != nullptr ? + static_cast(hdrtoken_method_tokenize(u.req.m_ptr_method, u.req.m_len_method)) : + int16_t{-1}; + if (u.req.m_url_impl != nullptr) { + u.req.m_url_impl->recompute_wks_idx(); + } + } + + if (m_fields_impl != nullptr) { + m_fields_impl->recompute_accelerators_and_presence_bits(); + } +} + void HTTPHdrImpl::move_strings(HdrStrHeap *new_heap) { diff --git a/src/proxy/hdrs/URL.cc b/src/proxy/hdrs/URL.cc index b0d0e5329b3..61304e31527 100644 --- a/src/proxy/hdrs/URL.cc +++ b/src/proxy/hdrs/URL.cc @@ -319,6 +319,16 @@ URLImpl::unmarshal(intptr_t offset) // HDR_UNMARSHAL_STR(m_ptr_printed_string, offset); } +void +URLImpl::recompute_wks_idx() +{ + // Tokenize the same way url_parse_scheme() does, so the index lands where parsing the same + // scheme would have put it. hdrtoken_tokenize() is case insensitive, which is what makes a + // scheme stored with other casing resolve to the well known string all the same. + m_scheme_wks_idx = + m_ptr_scheme != nullptr ? static_cast(hdrtoken_tokenize(m_ptr_scheme, m_len_scheme, nullptr)) : int16_t{-1}; +} + void URLImpl::rehome_strings(HdrHeap *new_heap) {