(trafficserver) branch master updated: Fixed crash in XPACK when making space for a new entry (#11657)

[bcall]

This is an automated email from the ASF dual-hosted git repository.

bcall pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 9c6ae2e070 Fixed crash in XPACK when making space for a new entry 
(#11657)
9c6ae2e070 is described below

commit 9c6ae2e07004f4606271107503cddb3c02f82c1e
Author: Bryan Call <bc...@apache.org>
AuthorDate: Wed Aug 7 08:15:41 2024 -0700

    Fixed crash in XPACK when making space for a new entry (#11657)
---
 src/proxy/hdrs/XPACK.cc                 | 18 +++++++-----------
 src/proxy/hdrs/unit_tests/test_XPACK.cc |  9 +++++++++
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/src/proxy/hdrs/XPACK.cc b/src/proxy/hdrs/XPACK.cc
index ccfbeec57b..26d4245c5d 100644
--- a/src/proxy/hdrs/XPACK.cc
+++ b/src/proxy/hdrs/XPACK.cc
@@ -496,22 +496,17 @@ XpackDynamicTable::_expand_storage_size(uint32_t 
new_storage_size)
 bool
 XpackDynamicTable::_make_space(uint64_t extra_space_needed)
 {
-  if (is_empty()) {
-    // If the table is empty, there's nothing to free.
-    return extra_space_needed == 0;
-  }
   uint32_t freed = 0;
-  uint32_t tail  = this->_calc_index(this->_entries_tail, 1);
+  uint32_t tail  = this->_entries_tail;
+
+  // Check to see if we need more space and that we have entries to evict
+  while (extra_space_needed > freed && this->_entries_head != tail) {
+    tail = this->_calc_index(tail, 1); // Move to the next entry
 
-  while (extra_space_needed > freed) {
-    if (this->_entries_head < tail) {
-      break;
-    }
     if (this->_entries[tail].ref_count) {
       break;
     }
     freed += this->_entries[tail].name_len + this->_entries[tail].value_len + 
ADDITIONAL_32_BYTES;
-    tail   = this->_calc_index(tail, 1);
   }
 
   // Evict
@@ -519,7 +514,8 @@ XpackDynamicTable::_make_space(uint64_t extra_space_needed)
     XPACKDbg("Evict entries: from %u to %u", 
this->_entries[this->_calc_index(this->_entries_tail, 1)].index,
              this->_entries[tail - 1].index);
     this->_available    += freed;
-    this->_entries_tail  = tail - 1;
+    this->_entries_tail  = tail;
+
     XPACKDbg("Available size: %u", this->_available);
   }
 
diff --git a/src/proxy/hdrs/unit_tests/test_XPACK.cc 
b/src/proxy/hdrs/unit_tests/test_XPACK.cc
index 93fc1bb1f4..a0eace5a5f 100644
--- a/src/proxy/hdrs/unit_tests/test_XPACK.cc
+++ b/src/proxy/hdrs/unit_tests/test_XPACK.cc
@@ -383,6 +383,15 @@ TEST_CASE("XPACK_String", "[xpack]")
     REQUIRE(dt.maximum_size() == 4096);
     REQUIRE(dt.is_empty());
     REQUIRE(dt.count() == 0);
+
+    // Test to insert 10k random size entries
+    for (int i = 0; i < 10000; i++) {
+      int         name_size  = rand() % 20000;
+      int         value_size = rand() % 20000;
+      std::string name       = get_long_string(name_size);
+      std::string value      = get_long_string(value_size);
+      dt.insert_entry(name, value);
+    }
   }
 }