This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 332fdbfce9d59b802e3886049fb08d6367de6d68 Author: Brian Neradt <[email protected]> AuthorDate: Wed May 7 16:18:16 2025 -0500 XPACK: Inspect _start_expanding_capacity before expanding (#12228) While expanding XPACK storage, crashes were happening with _old_data being nullptr. This will likely be fixed by checking the result of _start_expanding_capacity before trying to expand capacity. Fixes: #12225 (cherry picked from commit 50c7470e5ab05f050b283cecbd76b4db54fdc41e) --- include/proxy/hdrs/XPACK.h | 15 ++++++++++++++- src/proxy/hdrs/XPACK.cc | 7 +++++-- src/proxy/hdrs/unit_tests/test_XPACK.cc | 7 +++++++ 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/include/proxy/hdrs/XPACK.h b/include/proxy/hdrs/XPACK.h index da41b069f9..8a6cd1011c 100644 --- a/include/proxy/hdrs/XPACK.h +++ b/include/proxy/hdrs/XPACK.h @@ -165,7 +165,7 @@ public: /** Begin the storage expansion phase to the @a new_max_size. */ ExpandCapacityContext(XpackDynamicTableStorage &storage, uint32_t new_max_size) : _storage{storage} { - this->_storage._start_expanding_capacity(new_max_size); + this->_ok_to_expand = this->_storage._start_expanding_capacity(new_max_size); } /** End the storage expansion phase, cleaning up the old storage memory. */ ~ExpandCapacityContext() { this->_storage._finish_expanding_capacity(); } @@ -183,8 +183,21 @@ public: */ uint32_t copy_field(uint32_t old_offset, uint32_t len); + /** Indicate whether the expansion should proceed. + * Return whether @a _storage indicates that the new max size is valid for + * expanding the storage. If not, the expansion should not proceed. + * + * @return true if the new size is valid, false otherwise. + */ + bool + ok_to_expand() const + { + return this->_ok_to_expand; + } + private: XpackDynamicTableStorage &_storage; + bool _ok_to_expand = false; }; class XpackDynamicTable diff --git a/src/proxy/hdrs/XPACK.cc b/src/proxy/hdrs/XPACK.cc index 6898a36fa9..6d0cde9643 100644 --- a/src/proxy/hdrs/XPACK.cc +++ b/src/proxy/hdrs/XPACK.cc @@ -485,8 +485,11 @@ void XpackDynamicTable::_expand_storage_size(uint32_t new_storage_size) { ExpandCapacityContext context{this->_storage, new_storage_size}; - uint32_t i = this->_calc_index(this->_entries_tail, 1); - uint32_t end = this->_calc_index(this->_entries_head, 1); + if (!context.ok_to_expand()) { + return; + } + uint32_t i = this->_calc_index(this->_entries_tail, 1); + uint32_t end = this->_calc_index(this->_entries_head, 1); for (; i != end; i = this->_calc_index(i, 1)) { auto &entry = this->_entries[i]; entry.offset = context.copy_field(entry.offset, entry.name_len + entry.value_len); diff --git a/src/proxy/hdrs/unit_tests/test_XPACK.cc b/src/proxy/hdrs/unit_tests/test_XPACK.cc index a0eace5a5f..a7f3da0e92 100644 --- a/src/proxy/hdrs/unit_tests/test_XPACK.cc +++ b/src/proxy/hdrs/unit_tests/test_XPACK.cc @@ -460,6 +460,7 @@ TEST_CASE("XpackDynamicTableStorage", "[xpack]") uint32_t reoffset2 = 0, reoffset3 = 0, reoffset4 = 0; { ExpandCapacityContext context{storage, 200}; + REQUIRE(context.ok_to_expand()); reoffset2 = context.copy_field(offset2, 50); // Note that the offsets will now be shifted, starting from 0 now. REQUIRE(reoffset2 == 0); @@ -478,4 +479,10 @@ TEST_CASE("XpackDynamicTableStorage", "[xpack]") storage.read(reoffset4, &name, 25, &value, 25); REQUIRE(memcmp(name, name4.data(), 25) == 0); REQUIRE(memcmp(value, value4.data(), 25) == 0); + + // Test shrinking capacity. This should be rejected via ok_to_expand(). + { + ExpandCapacityContext context{storage, 0}; + REQUIRE(!context.ok_to_expand()); + } // context goes out of scope and finishes the expansion phase. }
