------------------------------------------------------------
revno: 14016
revision-id: squid3@treenet.co.nz-20160401061531-j6gfoo1umnsb27b0
parent: squid3@treenet.co.nz-20160330134757-7rf7qfe63b0vfcvz
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Fri 2016-04-01 19:15:31 +1300
message:
Convert Vary handling to SBuf
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20160401061531-j6gfoo1umnsb27b0
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: 0060adcbcb8ad9f726e9f7382c9e616efd43f93e
# timestamp: 2016-04-01 06:17:44 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20160330134757-\
# 7rf7qfe63b0vfcvz
#
# Begin patch
=== modified file 'src/HttpRequest.cc'
--- src/HttpRequest.cc 2016-01-01 00:14:27 +0000
+++ src/HttpRequest.cc 2016-04-01 06:15:31 +0000
@@ -92,7 +92,7 @@
peer_login = NULL; // not allocated/deallocated by this class
peer_domain = NULL; // not allocated/deallocated by this class
peer_host = NULL;
- vary_headers = NULL;
+ vary_headers = SBuf();
myportname = null_string;
tag = null_string;
#if USE_AUTH
@@ -124,9 +124,7 @@
auth_user_request = NULL;
#endif
safe_free(canonical);
-
- safe_free(vary_headers);
-
+ vary_headers.clear();
url.clear();
urlpath.clean();
@@ -203,7 +201,7 @@
copy->lastmod = lastmod;
copy->etag = etag;
- copy->vary_headers = vary_headers ? xstrdup(vary_headers) : NULL;
+ copy->vary_headers = vary_headers;
// XXX: what to do with copy->peer_domain?
copy->tag = tag;
=== modified file 'src/HttpRequest.h'
--- src/HttpRequest.h 2016-01-01 00:14:27 +0000
+++ src/HttpRequest.h 2016-04-01 06:15:31 +0000
@@ -179,7 +179,8 @@
time_t lastmod; /* Used on refreshes */
- const char *vary_headers; /* Used when varying entities are detected. Changes how the store key is calculated */
+ /// The variant second-stage cache key. Generated from Vary header pattern for this request.
+ SBuf vary_headers;
char *peer_domain; /* Configured peer forceddomain */
=== modified file 'src/MemObject.cc'
--- src/MemObject.cc 2016-01-01 00:14:27 +0000
+++ src/MemObject.cc 2016-04-01 06:15:31 +0000
@@ -136,8 +136,6 @@
HTTPMSGUNLOCK(request);
ctx_exit(ctx); /* must exit before we free mem->url */
-
- safe_free(vary_headers);
}
void
@@ -221,8 +219,8 @@
MemObject::stat(MemBuf * mb) const
{
mb->Printf("\t" SQUIDSBUFPH " %s\n", SQUIDSBUFPRINT(method.image()), logUri());
- if (vary_headers)
- mb->Printf("\tvary_headers: %s\n", vary_headers);
+ if (!vary_headers.isEmpty())
+ mb->Printf("\tvary_headers: " SQUIDSBUFPH "\n", SQUIDSBUFPRINT(vary_headers));
mb->Printf("\tinmem_lo: %" PRId64 "\n", inmem_lo);
mb->Printf("\tinmem_hi: %" PRId64 "\n", data_hdr.endOffset());
mb->Printf("\tswapout: %" PRId64 " bytes queued\n",
=== modified file 'src/MemObject.h'
--- src/MemObject.h 2016-01-01 00:14:27 +0000
+++ src/MemObject.h 2016-04-01 06:15:31 +0000
@@ -13,6 +13,7 @@
#include "dlink.h"
#include "HttpRequestMethod.h"
#include "RemovalPolicy.h"
+#include "SBuf.h"
#include "stmem.h"
#include "StoreIOBuffer.h"
#include "StoreIOState.h"
@@ -167,7 +168,7 @@
unsigned int chksum;
#endif
- const char *vary_headers;
+ SBuf vary_headers;
void delayRead(DeferredRead const &);
void kickReads();
=== modified file 'src/MemStore.cc'
--- src/MemStore.cc 2016-01-01 00:14:27 +0000
+++ src/MemStore.cc 2016-04-01 06:15:31 +0000
@@ -443,7 +443,7 @@
assert(e.mem_obj);
- if (e.mem_obj->vary_headers) {
+ if (!e.mem_obj->vary_headers.isEmpty()) {
// XXX: We must store/load SerialisedMetaData to cache Vary in RAM
debugs(20, 5, "Vary not yet supported: " << e.mem_obj->vary_headers);
return false;
=== modified file 'src/StoreMetaVary.cc'
--- src/StoreMetaVary.cc 2016-01-01 00:14:27 +0000
+++ src/StoreMetaVary.cc 2016-04-01 06:15:31 +0000
@@ -18,14 +18,14 @@
{
assert (getType() == STORE_META_VARY_HEADERS);
- if (!e->mem_obj->vary_headers) {
+ if (e->mem_obj->vary_headers.isEmpty()) {
/* XXX separate this mutator from the query */
/* Assume the object is OK.. remember the vary request headers */
- e->mem_obj->vary_headers = xstrdup((char *)value);
+ e->mem_obj->vary_headers.assign(static_cast<const char *>(value), length);
return true;
}
- if (strcmp(e->mem_obj->vary_headers, (char *)value) != 0)
+ if (e->mem_obj->vary_headers.cmp(static_cast<const char *>(value), length) != 0)
return false;
return true;
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2016-03-23 14:41:24 +0000
+++ src/client_side.cc 2016-04-01 06:15:31 +0000
@@ -4684,20 +4684,19 @@
int
varyEvaluateMatch(StoreEntry * entry, HttpRequest * request)
{
- const char *vary = request->vary_headers;
+ SBuf vary(request->vary_headers);
int has_vary = entry->getReply()->header.has(HDR_VARY);
#if X_ACCELERATOR_VARY
-
has_vary |=
entry->getReply()->header.has(HDR_X_ACCELERATOR_VARY);
#endif
- if (!has_vary || !entry->mem_obj->vary_headers) {
- if (vary) {
+ if (!has_vary || entry->mem_obj->vary_headers.isEmpty()) {
+ if (!vary.isEmpty()) {
/* Oops... something odd is going on here.. */
debugs(33, DBG_IMPORTANT, "varyEvaluateMatch: Oops. Not a Vary object on second attempt, '" <<
entry->mem_obj->urlXXX() << "' '" << vary << "'");
- safe_free(request->vary_headers);
+ request->vary_headers.clear();
return VARY_CANCEL;
}
@@ -4711,8 +4710,8 @@
*/
vary = httpMakeVaryMark(request, entry->getReply());
- if (vary) {
- request->vary_headers = xstrdup(vary);
+ if (!vary.isEmpty()) {
+ request->vary_headers = vary;
return VARY_OTHER;
} else {
/* Ouch.. we cannot handle this kind of variance */
@@ -4720,18 +4719,18 @@
return VARY_CANCEL;
}
} else {
- if (!vary) {
+ if (vary.isEmpty()) {
vary = httpMakeVaryMark(request, entry->getReply());
- if (vary)
- request->vary_headers = xstrdup(vary);
+ if (!vary.isEmpty())
+ request->vary_headers = vary;
}
- if (!vary) {
+ if (vary.isEmpty()) {
/* Ouch.. we cannot handle this kind of variance */
/* XXX This cannot really happen, but just to be complete */
return VARY_CANCEL;
- } else if (strcmp(vary, entry->mem_obj->vary_headers) == 0) {
+ } else if (vary.cmp(entry->mem_obj->vary_headers) == 0) {
return VARY_MATCH;
} else {
/* Oops.. we have already been here and still haven't
=== modified file 'src/client_side_reply.cc'
--- src/client_side_reply.cc 2016-03-30 13:25:34 +0000
+++ src/client_side_reply.cc 2016-04-01 06:15:31 +0000
@@ -987,9 +987,8 @@
}
/* And for Vary, release the base URI if none of the headers was included in the request */
-
- if (http->request->vary_headers
- && !strstr(http->request->vary_headers, "=")) {
+ if (!http->request->vary_headers.isEmpty()
+ && http->request->vary_headers.find('=') != SBuf::npos) {
StoreEntry *entry = storeGetPublic(urlCanonical(http->request), Http::METHOD_GET);
if (entry) {
=== modified file 'src/http.cc'
--- src/http.cc 2016-02-29 19:20:49 +0000
+++ src/http.cc 2016-04-01 06:15:31 +0000
@@ -575,9 +575,9 @@
/*
* For Vary, store the relevant request headers as
* virtual headers in the reply
- * Returns false if the variance cannot be stored
+ * Returns an empty SBuf if the variance cannot be stored
*/
-const char *
+SBuf
httpMakeVaryMark(HttpRequest * request, HttpReply const * reply)
{
String vary, hdr;
@@ -585,9 +585,9 @@
const char *item;
const char *value;
int ilen;
- static String vstr;
+ SBuf vstr;
+ static const SBuf asterisk("*");
- vstr.clean();
vary = reply->header.getList(HDR_VARY);
while (strListGetItem(&vary, ',', &item, &ilen, &pos)) {
@@ -598,11 +598,13 @@
if (strcmp(name, "*") == 0) {
/* Can not handle "Vary: *" withtout ETag support */
safe_free(name);
- vstr.clean();
+ vstr.clear();
break;
}
- strListAdd(&vstr, name, ',');
+ if (!vstr.isEmpty())
+ vstr.append(", ", 2);
+ vstr.append(name);
hdr = request->header.getByName(name);
safe_free(name);
value = hdr.termedBuf();
@@ -627,7 +629,17 @@
char *name = (char *)xmalloc(ilen + 1);
xstrncpy(name, item, ilen + 1);
Tolower(name);
- strListAdd(&vstr, name, ',');
+
+ if (strcmp(name, "*") == 0) {
+ /* Can not handle "Vary: *" withtout ETag support */
+ safe_free(name);
+ vstr.clear();
+ break;
+ }
+
+ if (!vstr.isEmpty())
+ vstr.append(", ", 2);
+ vstr.append(name);
hdr = request->header.getByName(name);
safe_free(name);
value = hdr.termedBuf();
@@ -645,8 +657,8 @@
vary.clean();
#endif
- debugs(11, 3, "httpMakeVaryMark: " << vstr);
- return vstr.termedBuf();
+ debugs(11, 3, vstr);
+ return vstr;
}
void
@@ -916,15 +928,15 @@
|| rep->header.has(HDR_X_ACCELERATOR_VARY)
#endif
) {
- const char *vary = httpMakeVaryMark(request, rep);
+ const SBuf vary(httpMakeVaryMark(request, rep));
- if (!vary) {
+ if (vary.isEmpty()) {
entry->makePrivate();
if (!fwd->reforwardableStatus(rep->sline.status()))
EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT);
varyFailure = true;
} else {
- entry->mem_obj->vary_headers = xstrdup(vary);
+ entry->mem_obj->vary_headers = vary;
}
}
=== modified file 'src/http.h'
--- src/http.h 2016-01-31 05:39:09 +0000
+++ src/http.h 2016-04-01 06:15:31 +0000
@@ -12,6 +12,7 @@
#include "clients/Client.h"
#include "comm.h"
#include "HttpStateFlags.h"
+#include "SBuf.h"
class ChunkedCodingParser;
class FwdState;
@@ -117,7 +118,7 @@
int httpCachable(const HttpRequestMethod&);
void httpStart(FwdState *);
-const char *httpMakeVaryMark(HttpRequest * request, HttpReply const * reply);
+SBuf httpMakeVaryMark(HttpRequest * request, HttpReply const * reply);
#endif /* SQUID_HTTP_H */
=== modified file 'src/store.cc'
--- src/store.cc 2016-03-29 16:44:16 +0000
+++ src/store.cc 2016-04-01 06:15:31 +0000
@@ -683,31 +683,27 @@
if (mem_obj->request) {
HttpRequest *request = mem_obj->request;
- if (!mem_obj->vary_headers) {
+ if (mem_obj->vary_headers.isEmpty()) {
/* First handle the case where the object no longer varies */
- safe_free(request->vary_headers);
+ request->vary_headers.clear();
} else {
- if (request->vary_headers && strcmp(request->vary_headers, mem_obj->vary_headers) != 0) {
+ if (!request->vary_headers.isEmpty() && request->vary_headers.cmp(mem_obj->vary_headers) != 0) {
/* Oops.. the variance has changed. Kill the base object
* to record the new variance key
*/
- safe_free(request->vary_headers); /* free old "bad" variance key */
+ request->vary_headers.clear(); /* free old "bad" variance key */
if (StoreEntry *pe = storeGetPublic(mem_obj->storeId(), mem_obj->method))
pe->release();
}
/* Make sure the request knows the variance status */
- if (!request->vary_headers) {
- const char *vary = httpMakeVaryMark(request, mem_obj->getReply());
-
- if (vary)
- request->vary_headers = xstrdup(vary);
- }
+ if (request->vary_headers.isEmpty())
+ request->vary_headers = httpMakeVaryMark(request, mem_obj->getReply());
}
// TODO: storeGetPublic() calls below may create unlocked entries.
// We should add/use storeHas() API or lock/unlock those entries.
- if (mem_obj->vary_headers && !storeGetPublic(mem_obj->storeId(), mem_obj->method)) {
+ if (!mem_obj->vary_headers.isEmpty() && !storeGetPublic(mem_obj->storeId(), mem_obj->method)) {
/* Create "vary" base object */
String vary;
StoreEntry *pe = storeCreateEntry(mem_obj->storeId(), mem_obj->logUri(), request->flags, request->method);
=== modified file 'src/store_key_md5.cc'
--- src/store_key_md5.cc 2016-01-01 00:14:27 +0000
+++ src/store_key_md5.cc 2016-04-01 06:15:31 +0000
@@ -125,8 +125,8 @@
SquidMD5Update(&M, &m, sizeof(m));
SquidMD5Update(&M, (unsigned char *) url, strlen(url));
- if (request->vary_headers) {
- SquidMD5Update(&M, (unsigned char *) request->vary_headers, strlen(request->vary_headers));
+ if (!request->vary_headers.isEmpty()) {
+ SquidMD5Update(&M, request->vary_headers.rawContent(), request->vary_headers.length());
debugs(20, 3, "updating public key by vary headers: " << request->vary_headers << " for: " << url);
}
=== modified file 'src/store_swapmeta.cc'
--- src/store_swapmeta.cc 2016-01-01 00:14:27 +0000
+++ src/store_swapmeta.cc 2016-04-01 06:15:31 +0000
@@ -40,7 +40,6 @@
tlv *TLV = NULL; /* we'll return this */
tlv **T = &TLV;
const char *url;
- const char *vary;
assert(e->mem_obj != NULL);
const int64_t objsize = e->mem_obj->expectedReplySize();
assert(e->swap_status == SWAPOUT_WRITING);
@@ -87,10 +86,12 @@
}
T = StoreMeta::Add(T, t);
- vary = e->mem_obj->vary_headers;
+ SBuf vary(e->mem_obj->vary_headers);
- if (vary) {
- t =StoreMeta::Factory(STORE_META_VARY_HEADERS, strlen(vary) + 1, vary);
+ 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());
if (!t) {
storeSwapTLVFree(TLV);
=== modified file 'src/tests/stub_MemObject.cc'
--- src/tests/stub_MemObject.cc 2016-01-01 00:14:27 +0000
+++ src/tests/stub_MemObject.cc 2016-04-01 06:15:31 +0000
@@ -38,7 +38,6 @@
id(0),
object_sz(-1),
swap_hdr_sz(0),
- vary_headers(NULL),
_reply(NULL)
{
memset(&clients, 0, sizeof(clients));
=== modified file 'src/tests/stub_http.cc'
--- src/tests/stub_http.cc 2016-01-01 00:14:27 +0000
+++ src/tests/stub_http.cc 2016-04-01 06:15:31 +0000
@@ -7,12 +7,12 @@
*/
#include "squid.h"
-
#include "HttpReply.h"
#include "HttpRequest.h"
+#include "SBuf.h"
#define STUB_API "http.cc"
#include "tests/STUB.h"
-const char * httpMakeVaryMark(HttpRequest * request, HttpReply const * reply) STUB_RETVAL(NULL)
+SBuf httpMakeVaryMark(HttpRequest *, HttpReply const *) STUB_RETVAL(SBuf())