[squid-users] Squid 3.5.16 and vary loop objects (bug ?)

[Amos Jeffries]

On 7/04/2016 1:20 a.m., joe wrote:
> amos
> 
> vary.length()    = 33
> vary.c_str()    = accept-encoding="gzip,%20deflate"
>  so without +1 is fine
> 

Nod. What I'm thinking is that old Squid versions saved the string
terminator byte to the cache entries, and builds with the +1 replicate
that. BuUT that is being loaded back as part of the string (counted in
length). So the compare against the new un-terminated strings now fails
due to different lengths (off-by-1) and re-saving it to disk might make
length +2 (and +3 on the next cycle etc).

Attached is a patch which I think will fix 3.5.16 (should apply fine on
4.0.8 too) without needing the cache reset. Anyone able to test it please?

Amos

-------------- next part --------------
=== modified file 'src/StoreMetaVary.cc'
--- src/StoreMetaVary.cc	2016-04-01 06:15:31 +0000
+++ src/StoreMetaVary.cc	2016-04-06 13:18:55 +0000
@@ -22,6 +22,9 @@
         /* XXX separate this mutator from the query */
         /* Assume the object is OK.. remember the vary request headers */
         e->mem_obj->vary_headers.assign(static_cast<const char *>(value), length);
+        /* entries created before SBuf vary handling may include string terminator */
+        static const SBuf nul("\0", 1);
+        e->mem_obj->vary_headers.trim(nul);
         return true;
     }
 

=== modified file 'src/store_swapmeta.cc'
--- src/store_swapmeta.cc	2016-04-01 06:15:31 +0000
+++ src/store_swapmeta.cc	2016-04-06 12:03:43 +0000
@@ -89,9 +89,7 @@
     SBuf vary(e->mem_obj->vary_headers);
 
     if (!vary.isEmpty()) {
-        // TODO: do we still need +1 here? StoreMetaVary::checkConsistency
-        //       no longer relies on nul-termination, but other things might.
-        t = StoreMeta::Factory(STORE_META_VARY_HEADERS, vary.length() + 1, vary.c_str());
+        t = StoreMeta::Factory(STORE_META_VARY_HEADERS, vary.length(), vary.c_str());
 
         if (!t) {
             storeSwapTLVFree(TLV);