squid 3.5.26: latest patches (14169-14182)

Signed-off-by: Matthias Fischer <matthias.fischer@ipfire.org>
Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
This commit is contained in:
Matthias Fischer
2017-08-13 15:41:07 +02:00
committed by Michael Tremer
parent f539ff6d5d
commit 6edc270abc
15 changed files with 2375 additions and 0 deletions

View File

@@ -70,6 +70,20 @@ $(subst %,%_MD5,$(objects)) :
$(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
@$(PREBUILD)
@rm -rf $(DIR_APP) && cd $(DIR_SRC) && tar xaf $(DIR_DL)/$(DL_FILE)
cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14169.patch
cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14170.patch
cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14171.patch
cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14172.patch
cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14173.patch
cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14174.patch
cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14175.patch
cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14176.patch
cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14177.patch
cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14178.patch
cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14179.patch
cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14180.patch
cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14181.patch
cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14182.patch
cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid-3.5.26-fix-max-file-descriptors.patch
cd $(DIR_APP) && autoreconf -vfi

View File

@@ -0,0 +1,881 @@
------------------------------------------------------------
revno: 14169
revision-id: squid3@treenet.co.nz-20170614213720-3qmiohlx4zr2jnqq
parent: squid3@treenet.co.nz-20170601134753-6u64sl2rzmbfs67l
fixes bug: http://bugs.squid-cache.org/show_bug.cgi?id=2833
author: Eduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Thu 2017-06-15 09:37:20 +1200
message:
Bug 2833 pt2: Collapse internal revalidation requests (SMP-unaware caches), again.
The security fix in v5 r14979 had a negative effect on collapsed
forwarding. All "private" entries were considered automatically
non-shareable among collapsed clients. However this is not true: there
are many situations when collapsed forwarding should work despite of
"private" entry status: 304/5xx responses are good examples of that.
This patch fixes that by means of a new StoreEntry::shareableWhenPrivate
flag.
The suggested fix is not complete: To cover all possible situations, we
need to decide whether StoreEntry::shareableWhenPrivate is true or not
for all contexts where StoreEntry::setPrivateKey() is used. This patch
fixes only few important cases inside http.cc, making CF (as well
collapsed revalidation) work for some [non-cacheable] response status
codes, including 3xx, 5xx and some others.
The original support for internal revalidation requests collapsing
was in trink r14755 and referred to Squid bugs 2833, 4311, and 4471.
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20170614213720-3qmiohlx4zr2jnqq
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: 9e248e2e9d2f1defe1070eb808177df978fb4146
# timestamp: 2017-06-14 21:51:05 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20170601134753-\
# 6u64sl2rzmbfs67l
#
# Begin patch
=== modified file 'src/HttpHdrCc.cc'
--- src/HttpHdrCc.cc 2017-01-01 00:16:45 +0000
+++ src/HttpHdrCc.cc 2017-06-14 21:37:20 +0000
@@ -262,8 +262,8 @@
case CC_PUBLIC:
break;
case CC_PRIVATE:
- if (Private().size())
- packerPrintf(p, "=\"" SQUIDSTRINGPH "\"", SQUIDSTRINGPRINT(Private()));
+ if (private_.size())
+ packerPrintf(p, "=\"" SQUIDSTRINGPH "\"", SQUIDSTRINGPRINT(private_));
break;
case CC_NO_CACHE:
=== modified file 'src/MemStore.cc'
--- src/MemStore.cc 2017-01-01 00:16:45 +0000
+++ src/MemStore.cc 2017-06-14 21:37:20 +0000
@@ -299,7 +299,7 @@
e.ping_status = PING_NONE;
EBIT_CLR(e.flags, RELEASE_REQUEST);
- EBIT_CLR(e.flags, KEY_PRIVATE);
+ e.clearPrivate();
EBIT_SET(e.flags, ENTRY_VALIDATED);
MemObject::MemCache &mc = e.mem_obj->memCache;
=== modified file 'src/Store.h'
--- src/Store.h 2017-01-01 00:16:45 +0000
+++ src/Store.h 2017-06-14 21:37:20 +0000
@@ -95,15 +95,19 @@
void abort();
void unlink();
void makePublic(const KeyScope keyScope = ksDefault);
- void makePrivate();
+ void makePrivate(const bool shareable);
+ /// A low-level method just resetting "private key" flags.
+ /// To avoid key inconsistency please use forcePublicKey()
+ /// or similar instead.
+ void clearPrivate();
void setPublicKey(const KeyScope keyScope = ksDefault);
/// Resets existing public key to a public key with default scope,
/// releasing the old default-scope entry (if any).
/// Does nothing if the existing public key already has default scope.
void clearPublicKeyScope();
- void setPrivateKey();
+ void setPrivateKey(const bool shareable);
void expireNow();
- void releaseRequest();
+ void releaseRequest(const bool shareable = false);
void negativeCache();
void cacheNegatively(); /** \todo argh, why both? */
void invokeHandlers();
@@ -230,7 +234,13 @@
/// update last reference timestamp and related Store metadata
void touch();
- virtual void release();
+ virtual void release(const bool shareable = false);
+
+ /// May the caller commit to treating this [previously locked]
+ /// entry as a cache hit?
+ bool mayStartHitting() const {
+ return !EBIT_TEST(flags, KEY_PRIVATE) || shareableWhenPrivate;
+ }
#if USE_ADAPTATION
/// call back producer when more buffer space is available
@@ -252,6 +262,13 @@
unsigned short lock_count; /* Assume < 65536! */
+ /// Nobody can find/lock KEY_PRIVATE entries, but some transactions
+ /// (e.g., collapsed requests) find/lock a public entry before it becomes
+ /// private. May such transactions start using the now-private entry
+ /// they previously locked? This member should not affect transactions
+ /// that already started reading from the entry.
+ bool shareableWhenPrivate;
+
#if USE_ADAPTATION
/// producer callback registered with deferProducer
AsyncCall::Pointer deferredProducer;
@@ -259,6 +276,8 @@
bool validLength() const;
bool hasOneOfEtags(const String &reqETags, const bool allowWeakMatch) const;
+
+ friend std::ostream &operator <<(std::ostream &os, const StoreEntry &e);
};
std::ostream &operator <<(std::ostream &os, const StoreEntry &e);
=== modified file 'src/client_side_reply.cc'
--- src/client_side_reply.cc 2017-05-29 13:15:55 +0000
+++ src/client_side_reply.cc 2017-06-14 21:37:20 +0000
@@ -396,8 +396,8 @@
if (result.flags.error && !EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED))
return;
- if (collapsedRevalidation == crSlave && EBIT_TEST(http->storeEntry()->flags, KEY_PRIVATE)) {
- debugs(88, 3, "CF slave hit private " << *http->storeEntry() << ". MISS");
+ if (collapsedRevalidation == crSlave && !http->storeEntry()->mayStartHitting()) {
+ debugs(88, 3, "CF slave hit private non-shareable " << *http->storeEntry() << ". MISS");
// restore context to meet processMiss() expectations
restoreState();
http->logType = LOG_TCP_MISS;
@@ -530,7 +530,7 @@
// The previously identified hit suddenly became unsharable!
// This is common for collapsed forwarding slaves but might also
// happen to regular hits because we are called asynchronously.
- if (EBIT_TEST(e->flags, KEY_PRIVATE)) {
+ if (!e->mayStartHitting()) {
debugs(88, 3, "unsharable " << *e << ". MISS");
http->logType = LOG_TCP_MISS;
processMiss();
=== modified file 'src/fs/rock/RockSwapDir.cc'
--- src/fs/rock/RockSwapDir.cc 2017-01-01 00:16:45 +0000
+++ src/fs/rock/RockSwapDir.cc 2017-06-14 21:37:20 +0000
@@ -149,7 +149,7 @@
e.ping_status = PING_NONE;
EBIT_CLR(e.flags, RELEASE_REQUEST);
- EBIT_CLR(e.flags, KEY_PRIVATE);
+ e.clearPrivate();
EBIT_SET(e.flags, ENTRY_VALIDATED);
e.swap_dirn = index;
=== modified file 'src/fs/ufs/UFSSwapDir.cc'
--- src/fs/ufs/UFSSwapDir.cc 2017-01-01 00:16:45 +0000
+++ src/fs/ufs/UFSSwapDir.cc 2017-06-14 21:37:20 +0000
@@ -809,7 +809,7 @@
e->refcount = refcount;
e->flags = newFlags;
EBIT_CLR(e->flags, RELEASE_REQUEST);
- EBIT_CLR(e->flags, KEY_PRIVATE);
+ e->clearPrivate();
e->ping_status = PING_NONE;
EBIT_CLR(e->flags, ENTRY_VALIDATED);
mapBitSet(e->swap_filen);
=== modified file 'src/http.cc'
--- src/http.cc 2017-01-01 00:16:45 +0000
+++ src/http.cc 2017-06-14 21:37:20 +0000
@@ -290,7 +290,9 @@
(Config.onoff.surrogate_is_remote
&& sctusable->noStoreRemote())) {
surrogateNoStore = true;
- entry->makePrivate();
+ // Be conservative for now and make it non-shareable because
+ // there is no enough information here to make the decision.
+ entry->makePrivate(false);
}
/* The HttpHeader logic cannot tell if the header it's parsing is a reply to an
@@ -315,12 +317,13 @@
}
}
-int
-HttpStateData::cacheableReply()
+HttpStateData::ReuseDecision::Answers
+HttpStateData::reusableReply(HttpStateData::ReuseDecision &decision)
{
HttpReply const *rep = finalReply();
HttpHeader const *hdr = &rep->header;
const char *v;
+
#if USE_HTTP_VIOLATIONS
const RefreshPattern *R = NULL;
@@ -337,24 +340,19 @@
#define REFRESH_OVERRIDE(flag) 0
#endif
- if (EBIT_TEST(entry->flags, RELEASE_REQUEST)) {
- debugs(22, 3, "NO because " << *entry << " has been released.");
- return 0;
- }
+ if (EBIT_TEST(entry->flags, RELEASE_REQUEST))
+ return decision.make(ReuseDecision::reuseNot, "the entry has been released");
// RFC 7234 section 4: a cache MUST use the most recent response
// (as determined by the Date header field)
- if (sawDateGoBack) {
- debugs(22, 3, "NO because " << *entry << " has an older date header.");
- return 0;
- }
+ // TODO: whether such responses could be shareable?
+ if (sawDateGoBack)
+ return decision.make(ReuseDecision::reuseNot, "the response has an older date header");
// Check for Surrogate/1.0 protocol conditions
// NP: reverse-proxy traffic our parent server has instructed us never to cache
- if (surrogateNoStore) {
- debugs(22, 3, HERE << "NO because Surrogate-Control:no-store");
- return 0;
- }
+ if (surrogateNoStore)
+ return decision.make(ReuseDecision::reuseNot, "Surrogate-Control:no-store");
// RFC 2616: HTTP/1.1 Cache-Control conditions
if (!ignoreCacheControl) {
@@ -363,11 +361,10 @@
// for now we are not reliably doing that so we waste CPU re-checking request CC
// RFC 2616 section 14.9.2 - MUST NOT cache any response with request CC:no-store
- if (request && request->cache_control && request->cache_control->noStore() &&
- !REFRESH_OVERRIDE(ignore_no_store)) {
- debugs(22, 3, HERE << "NO because client request Cache-Control:no-store");
- return 0;
- }
+ if (request && request->cache_control && request->cache_control->hasNoStore() &&
+ !REFRESH_OVERRIDE(ignore_no_store))
+ return decision.make(ReuseDecision::reuseNot,
+ "client request Cache-Control:no-store");
// NP: request CC:no-cache only means cache READ is forbidden. STORE is permitted.
if (rep->cache_control && rep->cache_control->hasNoCache() && rep->cache_control->noCache().size() > 0) {
@@ -376,19 +373,18 @@
* successfully (ie, must revalidate AND these headers are prohibited on stale replies).
* That is a bit tricky for squid right now so we avoid caching entirely.
*/
- debugs(22, 3, HERE << "NO because server reply Cache-Control:no-cache has parameters");
- return 0;
+ return decision.make(ReuseDecision::reuseNot,
+ "server reply Cache-Control:no-cache has parameters");
}
// NP: request CC:private is undefined. We ignore.
// NP: other request CC flags are limiters on HIT/MISS. We don't care about here.
// RFC 2616 section 14.9.2 - MUST NOT cache any response with CC:no-store
- if (rep->cache_control && rep->cache_control->noStore() &&
- !REFRESH_OVERRIDE(ignore_no_store)) {
- debugs(22, 3, HERE << "NO because server reply Cache-Control:no-store");
- return 0;
- }
+ if (rep->cache_control && rep->cache_control->hasNoStore() &&
+ !REFRESH_OVERRIDE(ignore_no_store))
+ return decision.make(ReuseDecision::reuseNot,
+ "server reply Cache-Control:no-store");
// RFC 2616 section 14.9.1 - MUST NOT cache any response with CC:private in a shared cache like Squid.
// CC:private overrides CC:public when both are present in a response.
@@ -401,27 +397,25 @@
* successfully (ie, must revalidate AND these headers are prohibited on stale replies).
* That is a bit tricky for squid right now so we avoid caching entirely.
*/
- debugs(22, 3, HERE << "NO because server reply Cache-Control:private");
- return 0;
+ return decision.make(ReuseDecision::reuseNot,
+ "server reply Cache-Control:private");
}
}
// RFC 2068, sec 14.9.4 - MUST NOT cache any response with Authentication UNLESS certain CC controls are present
// allow HTTP violations to IGNORE those controls (ie re-block caching Auth)
if (request && (request->flags.auth || request->flags.authSent) && !REFRESH_OVERRIDE(ignore_auth)) {
- if (!rep->cache_control) {
- debugs(22, 3, HERE << "NO because Authenticated and server reply missing Cache-Control");
- return 0;
- }
+ if (!rep->cache_control)
+ return decision.make(ReuseDecision::reuseNot,
+ "authenticated and server reply missing Cache-Control");
- if (ignoreCacheControl) {
- debugs(22, 3, HERE << "NO because Authenticated and ignoring Cache-Control");
- return 0;
- }
+ if (ignoreCacheControl)
+ return decision.make(ReuseDecision::reuseNot,
+ "authenticated and ignoring Cache-Control");
bool mayStore = false;
// HTTPbis pt6 section 3.2: a response CC:public is present
- if (rep->cache_control->Public()) {
+ if (rep->cache_control->hasPublic()) {
debugs(22, 3, HERE << "Authenticated but server reply Cache-Control:public");
mayStore = true;
@@ -441,15 +435,13 @@
#endif
// HTTPbis pt6 section 3.2: a response CC:s-maxage is present
- } else if (rep->cache_control->sMaxAge()) {
+ } else if (rep->cache_control->hasSMaxAge()) {
debugs(22, 3, HERE << "Authenticated but server reply Cache-Control:s-maxage");
mayStore = true;
}
- if (!mayStore) {
- debugs(22, 3, HERE << "NO because Authenticated transaction");
- return 0;
- }
+ if (!mayStore)
+ return decision.make(ReuseDecision::reuseNot, "authenticated transaction");
// NP: response CC:no-cache is equivalent to CC:must-revalidate,max-age=0. We MAY cache, and do so.
// NP: other request CC flags are limiters on HIT/MISS/REFRESH. We don't care about here.
@@ -460,12 +452,26 @@
* probably should not be cachable
*/
if ((v = hdr->getStr(HDR_CONTENT_TYPE)))
- if (!strncasecmp(v, "multipart/x-mixed-replace", 25)) {
- debugs(22, 3, HERE << "NO because Content-Type:multipart/x-mixed-replace");
- return 0;
- }
+ if (!strncasecmp(v, "multipart/x-mixed-replace", 25))
+ return decision.make(ReuseDecision::reuseNot, "Content-Type:multipart/x-mixed-replace");
+
+ // TODO: if possible, provide more specific message for each status code
+ static const char *shareableError = "shareable error status code";
+ static const char *nonShareableError = "non-shareable error status code";
+ ReuseDecision::Answers statusAnswer = ReuseDecision::reuseNot;
+ const char *statusReason = nonShareableError;
switch (rep->sline.status()) {
+
+ /* There are several situations when a non-cacheable response may be
+ * still shareable (e.g., among collapsed clients). We assume that these
+ * are 3xx and 5xx responses, indicating server problems and some of
+ * 4xx responses, common for all clients with a given cache key (e.g.,
+ * 404 Not Found or 414 URI Too Long). On the other hand, we should not
+ * share non-cacheable client-specific errors, such as 400 Bad Request
+ * or 406 Not Acceptable.
+ */
+
/* Responses that are cacheable */
case Http::scOkay:
@@ -482,112 +488,90 @@
* Don't cache objects that need to be refreshed on next request,
* unless we know how to refresh it.
*/
+ if (refreshIsCachable(entry) || REFRESH_OVERRIDE(store_stale))
+ decision.make(ReuseDecision::cachePositively, "refresh check returned cacheable");
+ else
+ decision.make(ReuseDecision::doNotCacheButShare, "refresh check returned non-cacheable");
- if (!refreshIsCachable(entry) && !REFRESH_OVERRIDE(store_stale)) {
- debugs(22, 3, "NO because refreshIsCachable() returned non-cacheable..");
- return 0;
- } else {
- debugs(22, 3, HERE << "YES because HTTP status " << rep->sline.status());
- return 1;
- }
- /* NOTREACHED */
break;
/* Responses that only are cacheable if the server says so */
case Http::scFound:
case Http::scTemporaryRedirect:
- if (rep->date <= 0) {
- debugs(22, 3, HERE << "NO because HTTP status " << rep->sline.status() << " and Date missing/invalid");
- return 0;
- }
- if (rep->expires > rep->date) {
- debugs(22, 3, HERE << "YES because HTTP status " << rep->sline.status() << " and Expires > Date");
- return 1;
- } else {
- debugs(22, 3, HERE << "NO because HTTP status " << rep->sline.status() << " and Expires <= Date");
- return 0;
- }
- /* NOTREACHED */
+
+ if (rep->date <= 0)
+ decision.make(ReuseDecision::doNotCacheButShare, "Date is missing/invalid");
+ else if (rep->expires > rep->date)
+ decision.make(ReuseDecision::cachePositively, "Expires > Date");
+ else
+ decision.make(ReuseDecision::doNotCacheButShare, "Expires <= Date");
break;
- /* Errors can be negatively cached */
-
+ /* These responses can be negatively cached. Most can also be shared. */
case Http::scNoContent:
-
case Http::scUseProxy:
-
- case Http::scBadRequest:
-
case Http::scForbidden:
-
case Http::scNotFound:
-
case Http::scMethodNotAllowed:
-
case Http::scUriTooLong:
-
case Http::scInternalServerError:
-
case Http::scNotImplemented:
-
case Http::scBadGateway:
-
case Http::scServiceUnavailable:
-
case Http::scGatewayTimeout:
case Http::scMisdirectedRequest:
-
- debugs(22, 3, "MAYBE because HTTP status " << rep->sline.status());
- return -1;
-
- /* NOTREACHED */
+ statusAnswer = ReuseDecision::doNotCacheButShare;
+ statusReason = shareableError;
+ // fall through to the actual decision making below
+
+ case Http::scBadRequest: // no sharing; perhaps the server did not like something specific to this request
+
+#if USE_HTTP_VIOLATIONS
+ if (Config.negativeTtl > 0)
+ decision.make(ReuseDecision::cacheNegatively, "Config.negativeTtl > 0");
+ else
+#endif
+ decision.make(statusAnswer, statusReason);
break;
- /* Some responses can never be cached */
-
- case Http::scPartialContent: /* Not yet supported */
-
+ /* these responses can never be cached, some
+ of them can be shared though */
case Http::scSeeOther:
-
case Http::scNotModified:
-
case Http::scUnauthorized:
-
case Http::scProxyAuthenticationRequired:
-
- case Http::scInvalidHeader: /* Squid header parsing error */
-
- case Http::scHeaderTooLarge:
-
case Http::scPaymentRequired:
+ case Http::scInsufficientStorage:
+ // TODO: use more specific reason for non-error status codes
+ decision.make(ReuseDecision::doNotCacheButShare, shareableError);
+ break;
+
+ case Http::scPartialContent: /* Not yet supported. TODO: make shareable for suitable ranges */
case Http::scNotAcceptable:
- case Http::scRequestTimeout:
- case Http::scConflict:
+ case Http::scRequestTimeout: // TODO: is this shareable?
+ case Http::scConflict: // TODO: is this shareable?
case Http::scLengthRequired:
case Http::scPreconditionFailed:
case Http::scPayloadTooLarge:
case Http::scUnsupportedMediaType:
case Http::scUnprocessableEntity:
- case Http::scLocked:
+ case Http::scLocked: // TODO: is this shareable?
case Http::scFailedDependency:
- case Http::scInsufficientStorage:
case Http::scRequestedRangeNotSatisfied:
case Http::scExpectationFailed:
-
- debugs(22, 3, HERE << "NO because HTTP status " << rep->sline.status());
- return 0;
-
+ case Http::scInvalidHeader: /* Squid header parsing error */
+ case Http::scHeaderTooLarge:
+ decision.make(ReuseDecision::reuseNot, nonShareableError);
+ break;
default:
/* RFC 2616 section 6.1.1: an unrecognized response MUST NOT be cached. */
- debugs (11, 3, HERE << "NO because unknown HTTP status code " << rep->sline.status());
- return 0;
- /* NOTREACHED */
+ decision.make(ReuseDecision::reuseNot, "unknown status code");
break;
}
- /* NOTREACHED */
+ return decision.answer;
}
/// assemble a variant key (vary-mark) from the given Vary header and HTTP request
@@ -898,11 +882,12 @@
Ctx ctx = ctx_enter(entry->mem_obj->urlXXX());
HttpReply *rep = finalReply();
+ const Http::StatusCode statusCode = rep->sline.status();
entry->timestampsSet();
/* Check if object is cacheable or not based on reply code */
- debugs(11, 3, "HTTP CODE: " << rep->sline.status());
+ debugs(11, 3, "HTTP CODE: " << statusCode);
if (const StoreEntry *oldEntry = findPreviouslyCachedEntry(entry))
sawDateGoBack = rep->olderThan(oldEntry->getReply());
@@ -919,7 +904,9 @@
const SBuf vary(httpMakeVaryMark(request, rep));
if (vary.isEmpty()) {
- entry->makePrivate();
+ // TODO: check whether such responses are shareable.
+ // Do not share for now.
+ entry->makePrivate(false);
if (!fwd->reforwardableStatus(rep->sline.status()))
EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT);
varyFailure = true;
@@ -942,30 +929,31 @@
if (!fwd->reforwardableStatus(rep->sline.status()))
EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT);
- switch (cacheableReply()) {
-
- case 1:
+ ReuseDecision decision(entry, statusCode);
+
+ switch (reusableReply(decision)) {
+
+ case ReuseDecision::reuseNot:
+ entry->makePrivate(false);
+ break;
+
+ case ReuseDecision::cachePositively:
entry->makePublic();
break;
- case 0:
- entry->makePrivate();
+ case ReuseDecision::cacheNegatively:
+ entry->cacheNegatively();
break;
- case -1:
-
-#if USE_HTTP_VIOLATIONS
- if (Config.negativeTtl > 0)
- entry->cacheNegatively();
- else
-#endif
- entry->makePrivate();
+ case ReuseDecision::doNotCacheButShare:
+ entry->makePrivate(true);
break;
default:
assert(0);
break;
}
+ debugs(11, 3, "decided: " << decision);
}
if (!ignoreCacheControl) {
@@ -2429,3 +2417,29 @@
mustStop(reason);
}
+HttpStateData::ReuseDecision::ReuseDecision(const StoreEntry *e, const Http::StatusCode code)
+ : answer(HttpStateData::ReuseDecision::reuseNot), reason(nullptr), entry(e), statusCode(code) {}
+
+HttpStateData::ReuseDecision::Answers
+HttpStateData::ReuseDecision::make(const HttpStateData::ReuseDecision::Answers ans, const char *why)
+{
+ answer = ans;
+ reason = why;
+ return answer;
+}
+
+std::ostream &operator <<(std::ostream &os, const HttpStateData::ReuseDecision &d)
+{
+ static const char *ReuseMessages[] = {
+ "do not cache and do not share", // reuseNot
+ "cache positively and share", // cachePositively
+ "cache negatively and share", // cacheNegatively
+ "do not cache but share" // doNotCacheButShare
+ };
+
+ assert(d.answer >= HttpStateData::ReuseDecision::reuseNot &&
+ d.answer <= HttpStateData::ReuseDecision::doNotCacheButShare);
+ return os << ReuseMessages[d.answer] << " because " << d.reason <<
+ "; HTTP status " << d.statusCode << " " << *(d.entry);
+}
+
=== modified file 'src/http.h'
--- src/http.h 2017-01-01 00:16:45 +0000
+++ src/http.h 2017-06-14 21:37:20 +0000
@@ -22,6 +22,23 @@
{
public:
+
+ /// assists in making and relaying entry caching/sharing decision
+ class ReuseDecision
+ {
+ public:
+ enum Answers { reuseNot = 0, cachePositively, cacheNegatively, doNotCacheButShare };
+
+ ReuseDecision(const StoreEntry *e, const Http::StatusCode code);
+ /// stores the corresponding decision
+ Answers make(const Answers ans, const char *why);
+
+ Answers answer; ///< the decision id
+ const char *reason; ///< the decision reason
+ const StoreEntry *entry; ///< entry for debugging
+ const Http::StatusCode statusCode; ///< HTTP status for debugging
+ };
+
HttpStateData(FwdState *);
~HttpStateData();
@@ -39,8 +56,8 @@
void readReply(const CommIoCbParams &io);
virtual void maybeReadVirginBody(); // read response data from the network
- // Determine whether the response is a cacheable representation
- int cacheableReply();
+ // Checks whether the response is cacheable/shareable.
+ ReuseDecision::Answers reusableReply(ReuseDecision &decision);
CachePeer *_peer; /* CachePeer request made to */
int eof; /* reached end-of-object? */
@@ -119,6 +136,8 @@
CBDATA_CLASS2(HttpStateData);
};
+std::ostream &operator <<(std::ostream &os, const HttpStateData::ReuseDecision &d);
+
int httpCachable(const HttpRequestMethod&);
void httpStart(FwdState *);
SBuf httpMakeVaryMark(HttpRequest * request, HttpReply const * reply);
=== modified file 'src/store.cc'
--- src/store.cc 2017-01-01 00:16:45 +0000
+++ src/store.cc 2017-06-14 21:37:20 +0000
@@ -171,11 +171,18 @@
}
void
-StoreEntry::makePrivate()
+StoreEntry::makePrivate(const bool shareable)
{
/* This object should never be cached at all */
expireNow();
- releaseRequest(); /* delete object when not used */
+ releaseRequest(shareable); /* delete object when not used */
+}
+
+void
+StoreEntry::clearPrivate()
+{
+ EBIT_CLR(flags, KEY_PRIVATE);
+ shareableWhenPrivate = false;
}
void
@@ -365,7 +372,8 @@
ping_status(PING_NONE),
store_status(STORE_PENDING),
swap_status(SWAPOUT_NONE),
- lock_count(0)
+ lock_count(0),
+ shareableWhenPrivate(false)
{
debugs(20, 5, "StoreEntry constructed, this=" << this);
}
@@ -504,14 +512,14 @@
}
void
-StoreEntry::releaseRequest()
+StoreEntry::releaseRequest(const bool shareable)
{
if (EBIT_TEST(flags, RELEASE_REQUEST))
return;
setReleaseFlag(); // makes validToSend() false, preventing future hits
- setPrivateKey();
+ setPrivateKey(shareable);
}
int
@@ -623,12 +631,16 @@
* concept'.
*/
void
-StoreEntry::setPrivateKey()
+StoreEntry::setPrivateKey(const bool shareable)
{
const cache_key *newkey;
- if (key && EBIT_TEST(flags, KEY_PRIVATE))
- return; /* is already private */
+ if (key && EBIT_TEST(flags, KEY_PRIVATE)) {
+ // The entry is already private, but it may be still shareable.
+ if (!shareable)
+ shareableWhenPrivate = false;
+ return;
+ }
if (key) {
setReleaseFlag(); // will markForUnlink(); all caches/workers will know
@@ -649,6 +661,7 @@
assert(hash_lookup(store_table, newkey) == NULL);
EBIT_SET(flags, KEY_PRIVATE);
+ shareableWhenPrivate = shareable;
hashInsert(newkey);
}
@@ -705,14 +718,17 @@
if (StoreEntry *e2 = (StoreEntry *)hash_lookup(store_table, newkey)) {
assert(e2 != this);
debugs(20, 3, "Making old " << *e2 << " private.");
- e2->setPrivateKey();
- e2->release();
+
+ // TODO: check whether there is any sense in keeping old entry
+ // shareable here. Leaving it non-shareable for now.
+ e2->setPrivateKey(false);
+ e2->release(false);
}
if (key)
hashDelete();
- EBIT_CLR(flags, KEY_PRIVATE);
+ clearPrivate();
hashInsert(newkey);
@@ -830,7 +846,7 @@
e->lock("storeCreateEntry");
if (neighbors_do_private_keys || !flags.hierarchical)
- e->setPrivateKey();
+ e->setPrivateKey(false);
else
e->setPublicKey();
@@ -1264,7 +1280,7 @@
/* release an object from a cache */
void
-StoreEntry::release()
+StoreEntry::release(const bool shareable)
{
PROF_start(storeRelease);
debugs(20, 3, "releasing " << *this << ' ' << getMD5Text());
@@ -1274,7 +1290,7 @@
if (locked()) {
expireNow();
debugs(20, 3, "storeRelease: Only setting RELEASE_REQUEST bit");
- releaseRequest();
+ releaseRequest(shareable);
PROF_stop(storeRelease);
return;
}
@@ -1282,7 +1298,7 @@
Store::Root().memoryUnlink(*this);
if (StoreController::store_dirs_rebuilding && swap_filen > -1) {
- setPrivateKey();
+ setPrivateKey(shareable);
if (swap_filen > -1) {
// lock the entry until rebuilding is done
@@ -2181,7 +2197,11 @@
if (EBIT_TEST(e.flags, REFRESH_REQUEST)) os << 'F';
if (EBIT_TEST(e.flags, ENTRY_REVALIDATE_STALE)) os << 'E';
if (EBIT_TEST(e.flags, ENTRY_DISPATCHED)) os << 'D';
- if (EBIT_TEST(e.flags, KEY_PRIVATE)) os << 'I';
+ if (EBIT_TEST(e.flags, KEY_PRIVATE)) {
+ os << 'I';
+ if (e.shareableWhenPrivate)
+ os << 'H';
+ }
if (EBIT_TEST(e.flags, ENTRY_FWD_HDR_WAIT)) os << 'W';
if (EBIT_TEST(e.flags, ENTRY_NEGCACHED)) os << 'N';
if (EBIT_TEST(e.flags, ENTRY_VALIDATED)) os << 'V';
=== modified file 'src/tests/stub_store.cc'
--- src/tests/stub_store.cc 2017-01-01 00:16:45 +0000
+++ src/tests/stub_store.cc 2017-06-14 21:37:20 +0000
@@ -43,11 +43,11 @@
void StoreEntry::abort() STUB
void StoreEntry::unlink() STUB
void StoreEntry::makePublic(const KeyScope keyScope) STUB
-void StoreEntry::makePrivate() STUB
+void StoreEntry::makePrivate(const bool shareable) STUB
void StoreEntry::setPublicKey(const KeyScope keyScope) STUB
-void StoreEntry::setPrivateKey() STUB
+void StoreEntry::setPrivateKey(const bool shareable) STUB
void StoreEntry::expireNow() STUB
-void StoreEntry::releaseRequest() STUB
+void StoreEntry::releaseRequest(const bool shareable) STUB
void StoreEntry::negativeCache() STUB
void StoreEntry::cacheNegatively() STUB
void StoreEntry::purgeMem() STUB
@@ -99,7 +99,7 @@
int64_t StoreEntry::contentLen() const STUB_RETVAL(0)
void StoreEntry::lock(const char *) STUB
void StoreEntry::touch() STUB
-void StoreEntry::release() STUB
+void StoreEntry::release(const bool shareable) STUB
NullStoreEntry *NullStoreEntry::getInstance() STUB_RETVAL(NULL)
const char *NullStoreEntry::getMD5Text() const STUB_RETVAL(NULL)
=== modified file 'src/tests/testStoreController.cc'
--- src/tests/testStoreController.cc 2017-01-01 00:16:45 +0000
+++ src/tests/testStoreController.cc 2017-06-14 21:37:20 +0000
@@ -116,7 +116,7 @@
e->lastModified(squid_curtime);
e->refcount = 1;
EBIT_CLR(e->flags, RELEASE_REQUEST);
- EBIT_CLR(e->flags, KEY_PRIVATE);
+ e->clearPrivate();
e->ping_status = PING_NONE;
EBIT_CLR(e->flags, ENTRY_VALIDATED);
e->hashInsert((const cache_key *)name.termedBuf()); /* do it after we clear KEY_PRIVATE */
=== modified file 'src/tests/testStoreHashIndex.cc'
--- src/tests/testStoreHashIndex.cc 2017-01-01 00:16:45 +0000
+++ src/tests/testStoreHashIndex.cc 2017-06-14 21:37:20 +0000
@@ -97,7 +97,7 @@
e->lastModified(squid_curtime);
e->refcount = 1;
EBIT_CLR(e->flags, RELEASE_REQUEST);
- EBIT_CLR(e->flags, KEY_PRIVATE);
+ e->clearPrivate();
e->ping_status = PING_NONE;
EBIT_CLR(e->flags, ENTRY_VALIDATED);
e->hashInsert((const cache_key *)name.termedBuf()); /* do it after we clear KEY_PRIVATE */

View File

@@ -0,0 +1,85 @@
------------------------------------------------------------
revno: 14170
revision-id: squid3@treenet.co.nz-20170614215906-ly36sobvlr2pt0u6
parent: squid3@treenet.co.nz-20170614213720-3qmiohlx4zr2jnqq
fixes bug: http://bugs.squid-cache.org/show_bug.cgi?id=2833
author: Eduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Thu 2017-06-15 09:59:06 +1200
message:
Bug 2833 pt3: Do not respond with HTTP/304 to unconditional requests
... after internal revalidation. The original unconditional HttpRequest
was still marked (and processed) as conditional after internal
revalidation because the original (clear) Last-Modified and ETag values
were not restored (cleared) after the internal revalidation abused them.
TODO: Isolate the code converting the request into conditional one _and_
the code that undoes that conversion, to keep both actions in sync.
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20170614215906-ly36sobvlr2pt0u6
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: 0991e2d39b3bcebcf18cba3db0e3b57aabf23b8b
# timestamp: 2017-06-14 22:22:43 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20170614213720-\
# 3qmiohlx4zr2jnqq
#
# Begin patch
=== modified file 'src/client_side_reply.cc'
--- src/client_side_reply.cc 2017-06-14 21:37:20 +0000
+++ src/client_side_reply.cc 2017-06-14 21:59:06 +0000
@@ -72,8 +72,8 @@
HTTPMSGUNLOCK(reply);
}
-clientReplyContext::clientReplyContext(ClientHttpRequest *clientContext) : http (cbdataReference(clientContext)), old_entry (NULL), old_sc(NULL), deleting(false),
- collapsedRevalidation(crNone)
+clientReplyContext::clientReplyContext(ClientHttpRequest *clientContext) : http (cbdataReference(clientContext)), old_entry (NULL),
+ old_sc(NULL), old_lastmod(-1), deleting(false), collapsedRevalidation(crNone)
{}
/** Create an error in the store awaiting the client side to read it.
@@ -185,6 +185,8 @@
debugs(88, 3, "clientReplyContext::saveState: saving store context");
old_entry = http->storeEntry();
old_sc = sc;
+ old_lastmod = http->request->lastmod;
+ old_etag = http->request->etag;
old_reqsize = reqsize;
tempBuffer.offset = reqofs;
/* Prevent accessing the now saved entries */
@@ -204,9 +206,13 @@
sc = old_sc;
reqsize = old_reqsize;
reqofs = tempBuffer.offset;
+ http->request->lastmod = old_lastmod;
+ http->request->etag = old_etag;
/* Prevent accessed the old saved entries */
old_entry = NULL;
old_sc = NULL;
+ old_lastmod = -1;
+ old_etag.clean();
old_reqsize = 0;
tempBuffer.offset = 0;
}
=== modified file 'src/client_side_reply.h'
--- src/client_side_reply.h 2017-01-01 00:16:45 +0000
+++ src/client_side_reply.h 2017-06-14 21:59:06 +0000
@@ -130,7 +130,11 @@
void sendNotModifiedOrPreconditionFailedError();
StoreEntry *old_entry;
- store_client *old_sc; /* ... for entry to be validated */
+ /* ... for entry to be validated */
+ store_client *old_sc;
+ time_t old_lastmod;
+ String old_etag;
+
bool deleting;
typedef enum {

View File

@@ -0,0 +1,45 @@
------------------------------------------------------------
revno: 14171
revision-id: squidadm@squid-cache.org-20170615001633-wgrl5w8isv15o7gg
parent: squid3@treenet.co.nz-20170614215906-ly36sobvlr2pt0u6
committer: Source Maintenance <squidadm@squid-cache.org>
branch nick: 3.5
timestamp: Thu 2017-06-15 00:16:33 +0000
message:
SourceFormat Enforcement
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squidadm@squid-cache.org-20170615001633-\
# wgrl5w8isv15o7gg
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: 237182ac5eed6aca7e9aca295a90057f3a8cf10b
# timestamp: 2017-06-15 00:51:05 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20170614215906-\
# ly36sobvlr2pt0u6
#
# Begin patch
=== modified file 'src/http.cc'
--- src/http.cc 2017-06-14 21:37:20 +0000
+++ src/http.cc 2017-06-15 00:16:33 +0000
@@ -523,7 +523,7 @@
case Http::scMisdirectedRequest:
statusAnswer = ReuseDecision::doNotCacheButShare;
statusReason = shareableError;
- // fall through to the actual decision making below
+ // fall through to the actual decision making below
case Http::scBadRequest: // no sharing; perhaps the server did not like something specific to this request
@@ -2438,8 +2438,8 @@
};
assert(d.answer >= HttpStateData::ReuseDecision::reuseNot &&
- d.answer <= HttpStateData::ReuseDecision::doNotCacheButShare);
+ d.answer <= HttpStateData::ReuseDecision::doNotCacheButShare);
return os << ReuseMessages[d.answer] << " because " << d.reason <<
- "; HTTP status " << d.statusCode << " " << *(d.entry);
+ "; HTTP status " << d.statusCode << " " << *(d.entry);
}

View File

@@ -0,0 +1,40 @@
------------------------------------------------------------
revno: 14172
revision-id: squid3@treenet.co.nz-20170621195439-l63xfsad58ghhhfu
parent: squidadm@squid-cache.org-20170615001633-wgrl5w8isv15o7gg
fixes bug: http://bugs.squid-cache.org/show_bug.cgi?id=4671
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Thu 2017-06-22 07:54:39 +1200
message:
Bug 4671 pt2: GCC 7: raise FTP Gateway CTRL channel buffer to 16KB
Fixes
error: %s directive output may be truncated writing up to 8191 bytes
into a region of size 1019
note: snprintf output between 8 and 8199 bytes into a destination of
size 1024
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20170621195439-l63xfsad58ghhhfu
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: eeb32b45efe5504eebeaae89088d4a81d807807c
# timestamp: 2017-06-21 20:50:58 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squidadm@squid-cache.org-20170615001633-\
# wgrl5w8isv15o7gg
#
# Begin patch
=== modified file 'src/clients/FtpGateway.cc'
--- src/clients/FtpGateway.cc 2017-05-29 04:37:41 +0000
+++ src/clients/FtpGateway.cc 2017-06-21 19:54:39 +0000
@@ -192,7 +192,7 @@
#define FTP_LOGIN_NOT_ESCAPED 0
-#define CTRL_BUFLEN 1024
+#define CTRL_BUFLEN 16*1024
static char cbuf[CTRL_BUFLEN];
/*

View File

@@ -0,0 +1,254 @@
------------------------------------------------------------
revno: 14173
revision-id: squid3@treenet.co.nz-20170621201248-ezpvykg0b307ix61
parent: squid3@treenet.co.nz-20170621195439-l63xfsad58ghhhfu
fixes bug: http://bugs.squid-cache.org/show_bug.cgi?id=4671
author: Alex Rousskov <rousskov@measurement-factory.com>
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Thu 2017-06-22 08:12:48 +1200
message:
Replace new/delete operators using modern C++ rules.
This change was motivated by "Mismatched free()/delete/delete[]" errors
reported by valgrind and mused about in Squid source code.
I speculate that the old new/delete replacement code was the result of
slow accumulation of working hacks to accomodate various environments,
as compiler support for the feature evolved. The cumulative result does
not actually work well (see the above paragraph), and the replacement
functions had the following visible coding problems according to [1,2]:
a) Declared with non-standard profiles that included throw specifiers.
b) Declared inline. C++ says that the results of inline declarations
have unspecified effects. In Squid, they probably necessitated
complex compiler-specific "extern inline" workarounds.
c) Defined in the header file. C++ says that defining replacements "in
any source file" is enough and that multiple replacements per
program (which is what a header file definition produces) result in
"undefined behavior".
d) Declared inconsistently (only 2 out of 4 flavors). Declaring one base
flavor should be sufficient, but if we declare more, we should
declare all of them.
[1] http://en.cppreference.com/w/cpp/memory/new/operator_new
[2] http://en.cppreference.com/w/cpp/memory/new/operator_delete
The replacements were not provided to clang (trunk r13219), but there
was no explanation why. This patch does not change that exclusion.
I have no idea whether any of the old hacks are still necessary in some
cases. However, I suspect that either we do not care much if the
replacements are not enabled on some poorly supported platforms OR we
can disable them (or make them work) using much simpler hacks for the
platforms we do care about.
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20170621201248-ezpvykg0b307ix61
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: 4f15c23326e4e4fe2ca2a6c7a13333e01677a0b0
# timestamp: 2017-06-21 20:51:02 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20170621195439-\
# l63xfsad58ghhhfu
#
# Begin patch
=== modified file 'compat/os/macosx.h'
--- compat/os/macosx.h 2017-01-01 00:16:45 +0000
+++ compat/os/macosx.h 2017-06-21 20:12:48 +0000
@@ -28,11 +28,6 @@
#include "compat/cmsg.h"
-// MacOS GCC 4.0.1 and 4.2.1 supply __GNUC_GNU_INLINE__ but do not actually define __attribute__((gnu_inline))
-#if defined(__cplusplus) && !defined(_SQUID_EXTERNNEW_)
-#define _SQUID_EXTERNNEW_ extern inline
-#endif
-
#endif /* _SQUID_APPLE_ */
#endif /* SQUID_OS_MACOSX_H */
=== modified file 'compat/os/sgi.h'
--- compat/os/sgi.h 2017-01-01 00:16:45 +0000
+++ compat/os/sgi.h 2017-06-21 20:12:48 +0000
@@ -25,15 +25,6 @@
#define _ABI_SOURCE
#endif /* USE_ASYNC_IO */
-#if defined(__cplusplus) && !defined(_SQUID_EXTERNNEW_) && !defined(_GNUC_)
-/*
- * The gcc compiler treats extern inline functions as being extern,
- * while the SGI MIPSpro compilers treat them as inline. To get equivalent
- * behavior, remove the inline keyword.
- */
-#define _SQUID_EXTERNNEW_ extern
-#endif
-
#endif /* _SQUID_SGI_ */
#endif /* SQUID_OS_SGI_H */
=== modified file 'compat/os/solaris.h'
--- compat/os/solaris.h 2017-01-01 00:16:45 +0000
+++ compat/os/solaris.h 2017-06-21 20:12:48 +0000
@@ -59,13 +59,6 @@
#endif
/*
- * SunPro CC handles extern inline as inline, PLUS extern symbols.
- */
-#if !defined(_SQUID_EXTERNNEW_) && defined(__SUNPRO_CC)
-#define _SQUID_EXTERNNEW_ extern
-#endif
-
-/*
* SunStudio CC does not define C++ portability API __FUNCTION__
*/
#if defined(__SUNPRO_CC) && !defined(__FUNCTION__)
=== removed file 'include/SquidNew.h'
--- include/SquidNew.h 2017-01-01 00:16:45 +0000
+++ include/SquidNew.h 1970-01-01 00:00:00 +0000
@@ -1,41 +0,0 @@
-/*
- * Copyright (C) 1996-2017 The Squid Software Foundation and contributors
- *
- * Squid software is distributed under GPLv2+ license and includes
- * contributions from numerous individuals and organizations.
- * Please see the COPYING and CONTRIBUTORS files for details.
- */
-
-#ifndef SQUID_NEW_H
-#define SQUID_NEW_H
-
-#if !defined(__SUNPRO_CC) && !defined(__clang__)
-/* Any code using libstdc++ must have externally resolvable overloads
- * for void * operator new - which means in the .o for the binary,
- * or in a shared library. static libs don't propogate the symbol
- * so, look in the translation unit containing main() in squid
- * for the extern version in squid
- */
-#include <new>
-
-_SQUID_EXTERNNEW_ void *operator new(size_t size) throw (std::bad_alloc)
-{
- return xmalloc(size);
-}
-_SQUID_EXTERNNEW_ void operator delete (void *address) throw()
-{
- xfree(address);
-}
-_SQUID_EXTERNNEW_ void *operator new[] (size_t size) throw (std::bad_alloc)
-{
- return xmalloc(size);
-}
-_SQUID_EXTERNNEW_ void operator delete[] (void *address) throw()
-{
- xfree(address);
-}
-
-#endif /* !__SUNPRO_CC && !__clang__*/
-
-#endif /* SQUID_NEW_H */
-
=== modified file 'include/util.h'
--- include/util.h 2017-01-01 00:16:45 +0000
+++ include/util.h 2017-06-21 20:12:48 +0000
@@ -19,23 +19,6 @@
SQUIDCEXTERN int tvSubUsec(struct timeval, struct timeval);
SQUIDCEXTERN double tvSubDsec(struct timeval, struct timeval);
SQUIDCEXTERN void Tolower(char *);
-#if defined(__cplusplus)
-/*
- * Any code using libstdc++ must have externally resolvable overloads
- * for void * operator new - which means in the .o for the binary,
- * or in a shared library. static libs don't propogate the symbol
- * so, look in the translation unit containing main() in squid
- * for the extern version in squid
- */
-#if !defined(_SQUID_EXTERNNEW_)
-#if defined(__GNUC_STDC_INLINE__) || defined(__GNUC_GNU_INLINE__)
-#define _SQUID_EXTERNNEW_ extern inline __attribute__((gnu_inline))
-#else
-#define _SQUID_EXTERNNEW_ extern inline
-#endif
-#endif
-#include "SquidNew.h"
-#endif
SQUIDCEXTERN time_t parse_iso3307_time(const char *buf);
=== modified file 'src/SquidNew.cc'
--- src/SquidNew.cc 2017-01-01 00:16:45 +0000
+++ src/SquidNew.cc 2017-06-21 20:12:48 +0000
@@ -8,29 +8,45 @@
/* DEBUG: none Memory Allocation */
-#define _SQUID_EXTERNNEW_
-
#include "squid.h"
-#ifdef __SUNPRO_CC
+#if !defined(__clang__)
#include <new>
-void *operator new(size_t size) throw (std::bad_alloc)
-{
- return xmalloc(size);
-}
-void operator delete (void *address) throw()
-{
- xfree (address);
-}
-void *operator new[] (size_t size) throw (std::bad_alloc)
-{
- return xmalloc(size);
-}
-void operator delete[] (void *address) throw()
-{
- xfree (address);
-}
-
-#endif /* __SUNPRO_CC */
+
+void *operator new(size_t size)
+{
+ return xmalloc(size);
+}
+void operator delete(void *address)
+{
+ xfree(address);
+}
+void *operator new[](size_t size)
+{
+ return xmalloc(size);
+}
+void operator delete[](void *address)
+{
+ xfree(address);
+}
+
+void *operator new(size_t size, const std::nothrow_t &tag)
+{
+ return xmalloc(size);
+}
+void operator delete(void *address, const std::nothrow_t &tag)
+{
+ xfree(address);
+}
+void *operator new[](size_t size, const std::nothrow_t &tag)
+{
+ return xmalloc(size);
+}
+void operator delete[](void *address, const std::nothrow_t &tag)
+{
+ xfree(address);
+}
+
+#endif /* !defined(__clang__) */

View File

@@ -0,0 +1,274 @@
------------------------------------------------------------
revno: 14174
revision-id: squid3@treenet.co.nz-20170622153146-nxo8vl6a9r8z03v4
parent: squid3@treenet.co.nz-20170621201248-ezpvykg0b307ix61
fixes bug: http://bugs.squid-cache.org/show_bug.cgi?id=4671
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Fri 2017-06-23 03:31:46 +1200
message:
Bug 4671 pt3: various GCC 7 compile errors
Also, remove limit on FTP realm strings
Convert ftpRealm() from generating char* to SBuf. This fixes issues identified
by GCC 7 where the realm string may be longer than the available buffer and
gets truncated.
The size of the buffer was making the occurance rather rare, but it is still
possible.
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20170622153146-nxo8vl6a9r8z03v4
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: b54e1a339544443ed9b34a094717b2781c746c76
# timestamp: 2017-06-22 15:50:59 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20170621201248-\
# ezpvykg0b307ix61
#
# Begin patch
=== modified file 'src/DiskIO/DiskThreads/aiops.cc'
--- src/DiskIO/DiskThreads/aiops.cc 2017-01-01 00:16:45 +0000
+++ src/DiskIO/DiskThreads/aiops.cc 2017-06-22 15:31:46 +0000
@@ -290,7 +290,7 @@
/* Create threads and get them to sit in their wait loop */
squidaio_thread_pool = memPoolCreate("aio_thread", sizeof(squidaio_thread_t));
- assert(NUMTHREADS);
+ assert(NUMTHREADS != 0);
for (i = 0; i < NUMTHREADS; ++i) {
threadp = (squidaio_thread_t *)squidaio_thread_pool->alloc();
=== modified file 'src/clients/FtpGateway.cc'
--- src/clients/FtpGateway.cc 2017-06-21 19:54:39 +0000
+++ src/clients/FtpGateway.cc 2017-06-22 15:31:46 +0000
@@ -153,8 +153,8 @@
virtual void timeout(const CommTimeoutCbParams &io);
void ftpAcceptDataConnection(const CommAcceptCbParams &io);
- static HttpReply *ftpAuthRequired(HttpRequest * request, const char *realm);
- const char *ftpRealm(void);
+ static HttpReply *ftpAuthRequired(HttpRequest * request, SBuf &realm);
+ SBuf ftpRealm();
void loginFailed(void);
virtual void haveParsedReplyHeaders();
@@ -1189,7 +1189,8 @@
{
if (!checkAuth(&request->header)) {
/* create appropriate reply */
- HttpReply *reply = ftpAuthRequired(request, ftpRealm());
+ SBuf realm(ftpRealm()); // local copy so SBuf wont disappear too early
+ HttpReply *reply = ftpAuthRequired(request, realm);
entry->replaceHttpReply(reply);
serverComplete();
return;
@@ -1290,7 +1291,9 @@
#if HAVE_AUTH_MODULE_BASIC
/* add Authenticate header */
- newrep->header.putAuth("Basic", ftpRealm());
+ // XXX: performance regression. c_str() may reallocate
+ SBuf realm(ftpRealm()); // local copy so SBuf wont disappear too early
+ newrep->header.putAuth("Basic", realm.c_str());
#endif
// add it to the store entry for response....
@@ -1298,18 +1301,19 @@
serverComplete();
}
-const char *
+SBuf
Ftp::Gateway::ftpRealm()
{
- static char realm[8192];
+ SBuf realm;
/* This request is not fully authenticated */
- if (!request) {
- snprintf(realm, 8192, "FTP %s unknown", user);
- } else if (request->port == 21) {
- snprintf(realm, 8192, "FTP %s %s", user, request->GetHost());
- } else {
- snprintf(realm, 8192, "FTP %s %s port %d", user, request->GetHost(), request->port);
+ realm.appendf("FTP %s ", user);
+ if (!request)
+ realm.append("unknown", 7);
+ else {
+ realm.append(request->GetHost());
+ if (request->port != 21)
+ realm.appendf(" port %d", request->port);
}
return realm;
}
@@ -2673,13 +2677,14 @@
}
HttpReply *
-Ftp::Gateway::ftpAuthRequired(HttpRequest * request, const char *realm)
+Ftp::Gateway::ftpAuthRequired(HttpRequest * request, SBuf &realm)
{
ErrorState err(ERR_CACHE_ACCESS_DENIED, Http::scUnauthorized, request);
HttpReply *newrep = err.BuildHttpReply();
#if HAVE_AUTH_MODULE_BASIC
/* add Authenticate header */
- newrep->header.putAuth("Basic", realm);
+ // XXX: performance regression. c_str() may reallocate
+ newrep->header.putAuth("Basic", realm.c_str());
#endif
return newrep;
}
=== modified file 'src/fde.cc'
--- src/fde.cc 2017-01-01 00:16:45 +0000
+++ src/fde.cc 2017-06-22 15:31:46 +0000
@@ -85,15 +85,15 @@
char const *
fde::remoteAddr() const
{
- LOCAL_ARRAY(char, buf, MAX_IPSTRLEN );
+ static char buf[MAX_IPSTRLEN+7]; // 7 = length of ':port' strings
if (type != FD_SOCKET)
return null_string;
if ( *ipaddr )
- snprintf( buf, MAX_IPSTRLEN, "%s:%d", ipaddr, (int)remote_port);
+ snprintf(buf, sizeof(buf), "%s:%u", ipaddr, remote_port);
else
- local_addr.toUrl(buf,MAX_IPSTRLEN); // toHostStr does not include port.
+ local_addr.toUrl(buf, sizeof(buf)); // toHostStr does not include port.
return buf;
}
=== modified file 'src/fs/ufs/RebuildState.cc'
--- src/fs/ufs/RebuildState.cc 2017-01-01 00:16:45 +0000
+++ src/fs/ufs/RebuildState.cc 2017-06-22 15:31:46 +0000
@@ -444,7 +444,7 @@
}
if (0 == in_dir) { /* we need to read in a new directory */
- snprintf(fullpath, MAXPATHLEN, "%s/%02X/%02X",
+ snprintf(fullpath, sizeof(fullpath), "%s/%02X/%02X",
sd->path,
curlvl1, curlvl2);
@@ -489,7 +489,7 @@
continue;
}
- snprintf(fullfilename, MAXPATHLEN, "%s/%s",
+ snprintf(fullfilename, sizeof(fullfilename), "%s/%s",
fullpath, entry->d_name);
debugs(47, 3, HERE << "Opening " << fullfilename);
fd = file_open(fullfilename, O_RDONLY | O_BINARY);
=== modified file 'src/fs/ufs/RebuildState.h'
--- src/fs/ufs/RebuildState.h 2017-01-01 00:16:45 +0000
+++ src/fs/ufs/RebuildState.h 2017-06-22 15:31:46 +0000
@@ -54,7 +54,7 @@
dirent_t *entry;
DIR *td;
char fullpath[MAXPATHLEN];
- char fullfilename[MAXPATHLEN];
+ char fullfilename[MAXPATHLEN*2];
StoreRebuildData counts;
=== modified file 'src/gopher.cc'
--- src/gopher.cc 2017-01-01 00:16:45 +0000
+++ src/gopher.cc 2017-06-22 15:31:46 +0000
@@ -820,7 +820,7 @@
* This will be called when request write is complete. Schedule read of reply.
*/
static void
-gopherSendComplete(const Comm::ConnectionPointer &conn, char *buf, size_t size, Comm::Flag errflag, int xerrno, void *data)
+gopherSendComplete(const Comm::ConnectionPointer &conn, char *, size_t size, Comm::Flag errflag, int xerrno, void *data)
{
GopherStateData *gopherState = (GopherStateData *) data;
StoreEntry *entry = gopherState->entry;
@@ -840,10 +840,6 @@
err->url = xstrdup(entry->url());
gopherState->fwd->fail(err);
gopherState->serverConn->close();
-
- if (buf)
- memFree(buf, MEM_4K_BUF); /* Allocated by gopherSendRequest. */
-
return;
}
@@ -885,9 +881,6 @@
AsyncCall::Pointer call = commCbCall(5,5, "gopherReadReply",
CommIoCbPtrFun(gopherReadReply, gopherState));
entry->delayAwareRead(conn, gopherState->replybuf, BUFSIZ, call);
-
- if (buf)
- memFree(buf, MEM_4K_BUF); /* Allocated by gopherSendRequest. */
}
/**
@@ -898,32 +891,31 @@
gopherSendRequest(int fd, void *data)
{
GopherStateData *gopherState = (GopherStateData *)data;
- char *buf = (char *)memAllocate(MEM_4K_BUF);
+ MemBuf mb;
+ mb.init();
if (gopherState->type_id == GOPHER_CSO) {
const char *t = strchr(gopherState->request, '?');
- if (t != NULL)
+ if (t)
++t; /* skip the ? */
else
t = "";
- snprintf(buf, 4096, "query %s\r\nquit\r\n", t);
- } else if (gopherState->type_id == GOPHER_INDEX) {
- char *t = strchr(gopherState->request, '?');
-
- if (t != NULL)
- *t = '\t';
-
- snprintf(buf, 4096, "%s\r\n", gopherState->request);
+ mb.Printf("query %s\r\nquit", t);
} else {
- snprintf(buf, 4096, "%s\r\n", gopherState->request);
+ if (gopherState->type_id == GOPHER_INDEX) {
+ if (char *t = strchr(gopherState->request, '?'))
+ *t = '\t';
+ }
+ mb.append(gopherState->request, strlen(gopherState->request));
}
+ mb.append("\r\n", 2);
- debugs(10, 5, HERE << gopherState->serverConn);
+ debugs(10, 5, gopherState->serverConn);
AsyncCall::Pointer call = commCbCall(5,5, "gopherSendComplete",
CommIoCbPtrFun(gopherSendComplete, gopherState));
- Comm::Write(gopherState->serverConn, buf, strlen(buf), call, NULL);
+ Comm::Write(gopherState->serverConn, &mb, call);
gopherState->entry->makePublic();
}
=== modified file 'src/icmp/Makefile.am'
--- src/icmp/Makefile.am 2017-01-01 00:16:45 +0000
+++ src/icmp/Makefile.am 2017-06-22 15:31:46 +0000
@@ -59,7 +59,8 @@
pinger_LDFLAGS = $(LIBADD_DL)
pinger_LDADD=\
libicmp-core.la \
- ../ip/libip.la \
+ $(top_builddir)/src/ip/libip.la \
+ $(top_builddir)/src/base/libbase.la \
$(COMPAT_LIB) \
$(XTRA_LIBS)

View File

@@ -0,0 +1,34 @@
------------------------------------------------------------
revno: 14175
revision-id: squid3@treenet.co.nz-20170629125627-socq6szqysvm9ifa
parent: squid3@treenet.co.nz-20170622153146-nxo8vl6a9r8z03v4
fixes bug: http://bugs.squid-cache.org/show_bug.cgi?id=4112
author: Sven Eisenberg <sven.eisenberg@lairdtech.com>
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Fri 2017-06-30 00:56:27 +1200
message:
Bug 4112: ssl_engine does not accept cryptodev
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20170629125627-socq6szqysvm9ifa
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: c74e6941e5b6df8e36d26dd5c02389ae2955bc21
# timestamp: 2017-06-29 13:51:04 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20170622153146-\
# nxo8vl6a9r8z03v4
#
# Begin patch
=== modified file 'src/ssl/support.cc'
--- src/ssl/support.cc 2017-01-27 16:14:19 +0000
+++ src/ssl/support.cc 2017-06-29 12:56:27 +0000
@@ -737,6 +737,7 @@
#if HAVE_OPENSSL_ENGINE_H
if (Config.SSL.ssl_engine) {
+ ENGINE_load_builtin_engines();
ENGINE *e;
if (!(e = ENGINE_by_id(Config.SSL.ssl_engine)))
fatalf("Unable to find SSL engine '%s'\n", Config.SSL.ssl_engine);

View File

@@ -0,0 +1,42 @@
------------------------------------------------------------
revno: 14176
revision-id: squid3@treenet.co.nz-20170701073514-uzowexcysowqostf
parent: squid3@treenet.co.nz-20170629125627-socq6szqysvm9ifa
fixes bug: http://bugs.squid-cache.org/show_bug.cgi?id=4687
author: Lubos Uhliarik <luhliari@redhat.com>
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Sat 2017-07-01 19:35:14 +1200
message:
Bug 4687: Wrong names of components in man page, section SEE ALSO
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20170701073514-uzowexcysowqostf
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: 9099c98de3cb8fc125dd9952373de42c079b0ccc
# timestamp: 2017-07-01 07:45:05 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20170629125627-\
# socq6szqysvm9ifa
#
# Begin patch
=== modified file 'src/squid.8.in'
--- src/squid.8.in 2017-01-01 00:16:45 +0000
+++ src/squid.8.in 2017-07-01 07:35:14 +0000
@@ -265,11 +265,11 @@
.SH SEE ALSO
.if !'po4a'hide' .B cachemgr.cgi "(8), "
.if !'po4a'hide' .B squidclient "(1), "
-.if !'po4a'hide' .B pam_auth "(8), "
-.if !'po4a'hide' .B squid_ldap_auth "(8), "
-.if !'po4a'hide' .B squid_ldap_group "(8), "
+.if !'po4a'hide' .B basic_pam_auth "(8), "
+.if !'po4a'hide' .B basic_ldap_auth "(8), "
+.if !'po4a'hide' .B ext_ldap_group_acl "(8), "
.if !'po4a'hide' .B ext_session_acl "(8), "
-.if !'po4a'hide' .B squid_unix_group "(8), "
+.if !'po4a'hide' .B ext_unix_group_acl "(8), "
.br
The Squid FAQ wiki
.if !'po4a'hide' http://wiki.squid-cache.org/SquidFaq

View File

@@ -0,0 +1,52 @@
------------------------------------------------------------
revno: 14177
revision-id: squid3@treenet.co.nz-20170701073754-4x1i6p5s1gzk73co
parent: squid3@treenet.co.nz-20170701073514-uzowexcysowqostf
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Sat 2017-07-01 19:37:54 +1200
message:
basic_ncsa_auth: fix hash listing wrap in man(8) page
'*' list bullet points must be indented with whitespace.
If they are not the list is treated as a single wrapped paragraph.
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20170701073754-4x1i6p5s1gzk73co
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: ffd783ab19a438c56affcdc6c1d106fa00403f4f
# timestamp: 2017-07-01 07:45:09 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20170701073514-\
# uzowexcysowqostf
#
# Begin patch
=== modified file 'helpers/basic_auth/NCSA/basic_ncsa_auth.8'
--- helpers/basic_auth/NCSA/basic_ncsa_auth.8 2017-01-01 00:16:45 +0000
+++ helpers/basic_auth/NCSA/basic_ncsa_auth.8 2017-07-01 07:37:54 +0000
@@ -18,15 +18,15 @@
.PP
This authenticator accepts:
.BR
-* Blowfish - for passwords 72 characters or less in length
-.BR
-* SHA256 - with salting and magic strings
-.BR
-* SHA512 - with salting and magic strings
-.BR
-* MD5 - with optional salt and magic strings
-.BR
-* DES - for passwords 8 characters or less in length
+ * Blowfish \- for passwords 72 characters or less in length.
+.BR
+ * SHA256 \- with salting and magic strings.
+.BR
+ * SHA512 \- with salting and magic strings.
+.BR
+ * MD5 \- with optional salt and magic strings.
+.BR
+ * DES \- for passwords 8 characters or less in length.
.
NOTE: Blowfish and SHA algorithms require system-specific support.
.

View File

@@ -0,0 +1,36 @@
------------------------------------------------------------
revno: 14178
revision-id: squid3@treenet.co.nz-20170701081116-xekwolj1wdkeaxqv
parent: squid3@treenet.co.nz-20170701073754-4x1i6p5s1gzk73co
author: Alex Rousskov <rousskov@measurement-factory.com>
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Sat 2017-07-01 20:11:16 +1200
message:
Fix message packing error handling in mgr and snmp SMP Forwarders.
A missing "return" resulted in Forwarders sending garbage (or worse) to
Coordinator.
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20170701081116-xekwolj1wdkeaxqv
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: a593abc992a79d4539dede76b4f63e013f96d33a
# timestamp: 2017-07-01 08:51:30 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20170701073754-\
# 4x1i6p5s1gzk73co
#
# Begin patch
=== modified file 'src/ipc/Forwarder.cc'
--- src/ipc/Forwarder.cc 2017-01-01 00:16:45 +0000
+++ src/ipc/Forwarder.cc 2017-07-01 08:11:16 +0000
@@ -62,6 +62,7 @@
// assume the pack() call failed because the message did not fit
// TODO: add a more specific exception?
handleError();
+ return;
}
SendMessage(Ipc::Port::CoordinatorAddr(), message);

View File

@@ -0,0 +1,105 @@
------------------------------------------------------------
revno: 14179
revision-id: squid3@treenet.co.nz-20170701095916-wknqmneq2w0mxt6a
parent: squid3@treenet.co.nz-20170701081116-xekwolj1wdkeaxqv
author: Alex Rousskov <rousskov@measurement-factory.com>
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Sat 2017-07-01 21:59:16 +1200
message:
Fix mgr query handoff from the original recipient to Coordinator.
This bug has already been fixed once, in trunk r11164.1.61, but that fix
was accidentally undone shortly after, during significant cross-branch
merging activity combined with the Forwarder class split. The final
merge importing the associated code (trunk r11730) was buggy.
The bug (explained in r11164.1.61) leads to a race condition between
* Store notifying Server classes about the entry completion (which might
trigger a bogus error message sent to the cache manager client while
Coordinator sends its own valid response on the same connection!) and
* post-cleanup() connection closure handlers of Server classes silently
closing everything (and leaving Coordinator the only responding
process on that shared connection).
The bug probably was not noticed for so long because, evidently, the
latter actions tend to win in the current code.
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20170701095916-wknqmneq2w0mxt6a
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: c7e89c80468c7f388f7e09ad2d68a245789db50d
# timestamp: 2017-07-01 10:51:12 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20170701081116-\
# xekwolj1wdkeaxqv
#
# Begin patch
=== modified file 'src/ipc/Forwarder.h'
--- src/ipc/Forwarder.h 2017-01-01 00:16:45 +0000
+++ src/ipc/Forwarder.h 2017-07-01 09:59:16 +0000
@@ -47,12 +47,14 @@
virtual void handleError();
virtual void handleTimeout();
virtual void handleException(const std::exception& e);
- virtual void handleRemoteAck();
private:
static void RequestTimedOut(void* param);
void requestTimedOut();
void removeTimeoutEvent();
+
+ void handleRemoteAck();
+
static AsyncCall::Pointer DequeueRequest(unsigned int requestId);
protected:
=== modified file 'src/mgr/Forwarder.cc'
--- src/mgr/Forwarder.cc 2017-01-01 00:16:45 +0000
+++ src/mgr/Forwarder.cc 2017-07-01 09:59:16 +0000
@@ -102,17 +102,6 @@
mustStop("commClosed");
}
-/// called when Coordinator starts processing the request
-void
-Mgr::Forwarder::handleRemoteAck()
-{
- Ipc::Forwarder::handleRemoteAck();
-
- Must(entry != NULL);
- EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT);
- entry->complete();
-}
-
/// send error page
void
Mgr::Forwarder::sendError(ErrorState *error)
=== modified file 'src/mgr/Forwarder.h'
--- src/mgr/Forwarder.h 2017-01-01 00:16:45 +0000
+++ src/mgr/Forwarder.h 2017-07-01 09:59:16 +0000
@@ -40,7 +40,6 @@
virtual void handleError();
virtual void handleTimeout();
virtual void handleException(const std::exception& e);
- virtual void handleRemoteAck();
private:
void noteCommClosed(const CommCloseCbParams& params);
=== modified file 'src/tests/stub_libmgr.cc'
--- src/tests/stub_libmgr.cc 2017-01-01 00:16:45 +0000
+++ src/tests/stub_libmgr.cc 2017-07-01 09:59:16 +0000
@@ -100,7 +100,6 @@
void Mgr::Forwarder::handleError() STUB
void Mgr::Forwarder::handleTimeout() STUB
void Mgr::Forwarder::handleException(const std::exception& e) STUB
-void Mgr::Forwarder::handleRemoteAck() STUB
#include "mgr/FunAction.h"
Mgr::Action::Pointer Mgr::FunAction::Create(const CommandPointer &cmd, OBJH *aHandler) STUB_RETVAL(dummyAction)

View File

@@ -0,0 +1,448 @@
------------------------------------------------------------
revno: 14180
revision-id: squid3@treenet.co.nz-20170701120848-q2xznzfvxb4kwvb6
parent: squid3@treenet.co.nz-20170701095916-wknqmneq2w0mxt6a
fixes bug: http://bugs.squid-cache.org/show_bug.cgi?id=4464
author: Christos Tsantilas <chtsanti@users.sourceforge.net>
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Sun 2017-07-02 00:08:48 +1200
message:
Bug 4464: Reduce "!Comm::MonitorsRead(serverConnection->fd)" assertions.
* Protect Squid Client classes from new requests that compete with
ongoing pinned connection use and
* resume dealing with new requests when those Client classes are done
using the pinned connection.
Replaced primary ConnStateData::pinConnection() calls with a pair of
pinBusyConnection() and notePinnedConnectionBecameIdle() calls,
depending on the pinned connection state ("busy" or "idle").
Removed pinConnection() parameters that were not longer used or could be computed from the remaining parameters.
Removed ConnStateData::httpsPeeked() code "hiding" the originating
request and connection peer details while entering the first "idle"
state. The old (trunk r11880.1.6) bump-server-first code used a pair of
NULLs because "Intercepted connections do not have requests at the
connection pinning stage", but that limitation no longer applicable
because Squid always fakes (when intercepting) or parses (a CONNECT)
request now, even during SslBump step1.
The added XXX and TODOs are not directly related to this fix. They
were added to document problems discovered while working on this fix.
In v3.5 code, the same problems manifest as Read.cc
"fd_table[conn->fd].halfClosedReader != NULL" assertions.
This is a Measurement Factory project
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20170701120848-q2xznzfvxb4kwvb6
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: e4f432eed8a845431d4bbbf023de04d682adeaff
# timestamp: 2017-07-01 12:32:26 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20170701095916-\
# wknqmneq2w0mxt6a
#
# Begin patch
=== modified file 'src/FwdState.cc'
--- src/FwdState.cc 2017-01-01 00:16:45 +0000
+++ src/FwdState.cc 2017-07-01 12:08:48 +0000
@@ -246,7 +246,7 @@
#if USE_OPENSSL
if (request->flags.sslPeek && request->clientConnectionManager.valid()) {
CallJobHere1(17, 4, request->clientConnectionManager, ConnStateData,
- ConnStateData::httpsPeeked, Comm::ConnectionPointer(NULL));
+ ConnStateData::httpsPeeked, ConnStateData::PinnedIdleContext(Comm::ConnectionPointer(nullptr), request));
}
#endif
} else {
@@ -952,7 +952,7 @@
#if USE_OPENSSL
if (request->flags.sslPeek) {
CallJobHere1(17, 4, request->clientConnectionManager, ConnStateData,
- ConnStateData::httpsPeeked, serverConnection());
+ ConnStateData::httpsPeeked, ConnStateData::PinnedIdleContext(serverConnection(), request));
unregister(serverConn); // async call owns it now
complete(); // destroys us
return;
=== modified file 'src/base/RefCount.h'
--- src/base/RefCount.h 2017-01-01 00:16:45 +0000
+++ src/base/RefCount.h 2017-07-01 12:08:48 +0000
@@ -54,9 +54,7 @@
C & operator * () const {return *const_cast<C *>(p_); }
- C const * getRaw() const {return p_; }
-
- C * getRaw() {return const_cast<C *>(p_); }
+ C * getRaw() const { return const_cast<C *>(p_); }
bool operator == (const RefCount& p) const {
return p.p_ == p_;
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2017-05-29 13:15:55 +0000
+++ src/client_side.cc 2017-07-01 12:08:48 +0000
@@ -836,6 +836,7 @@
assert(areAllContextsForThisConnection());
freeAllContexts();
+ // XXX: Closing pinned conn is too harsh: The Client may want to continue!
unpinConnection(true);
if (Comm::IsConnOpen(clientConnection))
@@ -1559,6 +1560,13 @@
debugs(33, 3, HERE << "ConnnStateData(" << conn->clientConnection << "), Context(" << clientConnection << ")");
connIsFinished();
+ conn->kick();
+}
+
+void
+ConnStateData::kick()
+{
+ ConnStateData * conn = this; // XXX: Remove this diff minimization hack
if (conn->pinning.pinned && !Comm::IsConnOpen(conn->pinning.serverConnection)) {
debugs(33, 2, HERE << conn->clientConnection << " Connection was pinned but server side gone. Terminating client connection");
@@ -3240,6 +3248,13 @@
if (in.buf.isEmpty())
break;
+ // Prohibit concurrent requests when using a pinned to-server connection
+ // because our Client classes do not support request pipelining.
+ if (pinning.pinned && !pinning.readHandler) {
+ debugs(33, 3, clientConnection << " waits for busy " << pinning.serverConnection);
+ break;
+ }
+
/* Limit the number of concurrent requests */
if (concurrentRequestQueueFilled())
break;
@@ -4434,22 +4449,19 @@
}
void
-ConnStateData::httpsPeeked(Comm::ConnectionPointer serverConnection)
+ConnStateData::httpsPeeked(PinnedIdleContext pic)
{
Must(sslServerBump != NULL);
+ Must(sslServerBump->request == pic.request);
+ Must(currentobject == NULL || currentobject->http == NULL || currentobject->http->request == pic.request.getRaw());
- if (Comm::IsConnOpen(serverConnection)) {
- pinConnection(serverConnection, NULL, NULL, false);
+ if (Comm::IsConnOpen(pic.connection)) {
+ notePinnedConnectionBecameIdle(pic);
debugs(33, 5, HERE << "bumped HTTPS server: " << sslConnectHostOrIp);
- } else {
+ } else
debugs(33, 5, HERE << "Error while bumping: " << sslConnectHostOrIp);
- // copy error detail from bump-server-first request to CONNECT request
- if (currentobject != NULL && currentobject->http != NULL && currentobject->http->request)
- currentobject->http->request->detailError(sslServerBump->request->errType, sslServerBump->request->errDetail);
- }
-
getSslContextStart();
}
@@ -4952,19 +4964,35 @@
}
void
-ConnStateData::pinConnection(const Comm::ConnectionPointer &pinServer, HttpRequest *request, CachePeer *aPeer, bool auth, bool monitor)
-{
- if (!Comm::IsConnOpen(pinning.serverConnection) ||
- pinning.serverConnection->fd != pinServer->fd)
- pinNewConnection(pinServer, request, aPeer, auth);
-
- if (monitor)
- startPinnedConnectionMonitoring();
-}
-
-void
-ConnStateData::pinNewConnection(const Comm::ConnectionPointer &pinServer, HttpRequest *request, CachePeer *aPeer, bool auth)
-{
+ConnStateData::pinBusyConnection(const Comm::ConnectionPointer &pinServer, const HttpRequest::Pointer &request)
+{
+ pinConnection(pinServer, request);
+}
+
+void
+ConnStateData::notePinnedConnectionBecameIdle(PinnedIdleContext pic)
+{
+ Must(pic.connection != NULL);
+ Must(pic.request != NULL);
+ pinConnection(pic.connection, pic.request);
+
+ // monitor pinned server connection for remote-end closures.
+ startPinnedConnectionMonitoring();
+
+ if (!currentobject)
+ kick(); // in case clientParseRequests() was blocked by a busy pic.connection
+}
+
+/// Forward future client requests using the given server connection.
+void
+ConnStateData::pinConnection(const Comm::ConnectionPointer &pinServer, const HttpRequest::Pointer &request)
+{
+ if (Comm::IsConnOpen(pinning.serverConnection) &&
+ pinning.serverConnection->fd == pinServer->fd) {
+ debugs(33, 3, "already pinned" << pinServer);
+ return;
+ }
+
unpinConnection(true); // closes pinned connection, if any, and resets fields
pinning.serverConnection = pinServer;
@@ -4973,23 +5001,21 @@
Must(pinning.serverConnection != NULL);
- // when pinning an SSL bumped connection, the request may be NULL
const char *pinnedHost = "[unknown]";
- if (request) {
+ if (request != NULL) {
pinning.host = xstrdup(request->GetHost());
pinning.port = request->port;
pinnedHost = pinning.host;
+ pinning.auth = request->flags.connectionAuth;
} else {
pinning.port = pinServer->remote.port();
}
pinning.pinned = true;
- if (aPeer)
- pinning.peer = cbdataReference(aPeer);
- pinning.auth = auth;
+ pinning.peer = cbdataReference(pinServer->getPeer());
char stmp[MAX_IPSTRLEN];
char desc[FD_DESC_SZ];
snprintf(desc, FD_DESC_SZ, "%s pinned connection for %s (%d)",
- (auth || !aPeer) ? pinnedHost : aPeer->name,
+ (pinning.auth || !pinning.peer) ? pinnedHost : pinning.peer->name,
clientConnection->remote.toUrl(stmp,MAX_IPSTRLEN),
clientConnection->fd);
fd_note(pinning.serverConnection->fd, desc);
@@ -5164,3 +5190,8 @@
* connection has gone away */
}
+std::ostream &
+operator <<(std::ostream &os, const ConnStateData::PinnedIdleContext &pic)
+{
+ return os << pic.connection << ", request=" << pic.request;
+}
=== modified file 'src/client_side.h'
--- src/client_side.h 2017-01-01 00:16:45 +0000
+++ src/client_side.h 2017-07-01 12:08:48 +0000
@@ -26,6 +26,8 @@
#include "ssl/support.h"
#endif
+#include <iosfwd>
+
class ConnStateData;
class ClientHttpRequest;
class clientStreamNode;
@@ -188,6 +190,11 @@
/// Traffic parsing
bool clientParseRequests();
void readNextRequest();
+
+ // In v3.5, usually called via ClientSocketContext::keepaliveNextRequest().
+ /// try to make progress on a transaction or read more I/O
+ void kick();
+
ClientSocketContext::Pointer getCurrentContext() const;
void addContextToQueue(ClientSocketContext * context);
int getConcurrentRequestCount() const;
@@ -287,9 +294,21 @@
bool handleReadData();
bool handleRequestBodyData();
- /// Forward future client requests using the given server connection.
- /// Optionally, monitor pinned server connection for remote-end closures.
- void pinConnection(const Comm::ConnectionPointer &pinServerConn, HttpRequest *request, CachePeer *peer, bool auth, bool monitor = true);
+ /// parameters for the async notePinnedConnectionBecameIdle() call
+ class PinnedIdleContext
+ {
+ public:
+ PinnedIdleContext(const Comm::ConnectionPointer &conn, const HttpRequest::Pointer &req): connection(conn), request(req) {}
+
+ Comm::ConnectionPointer connection; ///< to-server connection to be pinned
+ HttpRequest::Pointer request; ///< to-server request that initiated serverConnection
+ };
+
+ /// Called when a pinned connection becomes available for forwarding the next request.
+ void notePinnedConnectionBecameIdle(PinnedIdleContext pic);
+ /// Forward future client requests using the given to-server connection.
+ /// The connection is still being used by the current client request.
+ void pinBusyConnection(const Comm::ConnectionPointer &pinServerConn, const HttpRequest::Pointer &request);
/// Undo pinConnection() and, optionally, close the pinned connection.
void unpinConnection(const bool andClose);
/// Returns validated pinnned server connection (and stops its monitoring).
@@ -345,7 +364,7 @@
/// generated
void doPeekAndSpliceStep();
/// called by FwdState when it is done bumping the server
- void httpsPeeked(Comm::ConnectionPointer serverConnection);
+ void httpsPeeked(PinnedIdleContext pic);
/// Start to create dynamic SSL_CTX for host or uses static port SSL context.
void getSslContextStart();
@@ -449,7 +468,7 @@
void clientAfterReadingRequests();
bool concurrentRequestQueueFilled() const;
- void pinNewConnection(const Comm::ConnectionPointer &pinServer, HttpRequest *request, CachePeer *aPeer, bool auth);
+ void pinConnection(const Comm::ConnectionPointer &pinServerConn, const HttpRequest::Pointer &request);
/* PROXY protocol functionality */
bool proxyProtocolValidateClient();
@@ -516,5 +535,7 @@
void clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *context, const HttpRequestMethod& method, Http::ProtocolVersion http_ver);
void clientPostHttpsAccept(ConnStateData *connState);
+std::ostream &operator <<(std::ostream &os, const ConnStateData::PinnedIdleContext &pic);
+
#endif /* SQUID_CLIENTSIDE_H */
=== modified file 'src/clients/FtpRelay.cc'
--- src/clients/FtpRelay.cc 2017-01-01 00:16:45 +0000
+++ src/clients/FtpRelay.cc 2017-07-01 12:08:48 +0000
@@ -210,9 +210,10 @@
mgr->unpinConnection(false);
ctrl.close();
} else {
- mgr->pinConnection(ctrl.conn, fwd->request,
- ctrl.conn->getPeer(),
- fwd->request->flags.connectionAuth);
+ CallJobHere1(9, 4, mgr,
+ ConnStateData,
+ notePinnedConnectionBecameIdle,
+ ConnStateData::PinnedIdleContext(ctrl.conn, fwd->request));
ctrl.forget();
}
}
=== modified file 'src/http.cc'
--- src/http.cc 2017-06-15 00:16:33 +0000
+++ src/http.cc 2017-07-01 12:08:48 +0000
@@ -1383,9 +1383,6 @@
void
HttpStateData::processReplyBody()
{
- Ip::Address client_addr;
- bool ispinned = false;
-
if (!flags.headers_parsed) {
flags.do_next_read = true;
maybeReadVirginBody();
@@ -1435,35 +1432,49 @@
}
break;
- case COMPLETE_PERSISTENT_MSG:
+ case COMPLETE_PERSISTENT_MSG: {
debugs(11, 5, "processReplyBody: COMPLETE_PERSISTENT_MSG from " << serverConnection);
- /* yes we have to clear all these! */
+
+ // TODO: Remove serverConnectionSaved but preserve exception safety.
+
commUnsetConnTimeout(serverConnection);
flags.do_next_read = false;
comm_remove_close_handler(serverConnection->fd, closeHandler);
closeHandler = NULL;
- fwd->unregister(serverConnection);
+ Ip::Address client_addr; // XXX: Remove as unused. Why was it added?
if (request->flags.spoofClientIp)
client_addr = request->client_addr;
+ Comm::ConnectionPointer serverConnectionSaved = serverConnection;
+ fwd->unregister(serverConnection);
+ serverConnection = nullptr;
+
+ bool ispinned = false; // TODO: Rename to isOrShouldBePinned
if (request->flags.pinned) {
ispinned = true;
} else if (request->flags.connectionAuth && request->flags.authSent) {
ispinned = true;
}
- if (ispinned && request->clientConnectionManager.valid()) {
- request->clientConnectionManager->pinConnection(serverConnection, request, _peer,
- (request->flags.connectionAuth));
+ if (ispinned) {
+ if (request->clientConnectionManager.valid()) {
+ CallJobHere1(11, 4, request->clientConnectionManager,
+ ConnStateData,
+ notePinnedConnectionBecameIdle,
+ ConnStateData::PinnedIdleContext(serverConnectionSaved, request));
+ } else {
+ // must not pool/share ispinned connections, even orphaned ones
+ serverConnectionSaved->close();
+ }
} else {
- fwd->pconnPush(serverConnection, request->GetHost());
+ fwd->pconnPush(serverConnectionSaved, request->GetHost());
}
- serverConnection = NULL;
serverComplete();
return;
+ }
case COMPLETE_NONPERSISTENT_MSG:
debugs(11, 5, "processReplyBody: COMPLETE_NONPERSISTENT_MSG from " << serverConnection);
=== modified file 'src/servers/FtpServer.cc'
--- src/servers/FtpServer.cc 2017-02-26 11:09:42 +0000
+++ src/servers/FtpServer.cc 2017-07-01 12:08:48 +0000
@@ -301,12 +301,8 @@
Must(http != NULL);
HttpRequest *const request = http->request;
Must(request != NULL);
-
- // this is not an idle connection, so we do not want I/O monitoring
- const bool monitor = false;
-
// make FTP peer connection exclusive to our request
- pinConnection(conn, request, conn->getPeer(), false, monitor);
+ pinBusyConnection(conn, request);
}
void
=== modified file 'src/tests/stub_client_side.cc'
--- src/tests/stub_client_side.cc 2017-01-01 00:16:45 +0000
+++ src/tests/stub_client_side.cc 2017-07-01 12:08:48 +0000
@@ -60,7 +60,8 @@
void ConnStateData::noteBodyConsumerAborted(BodyPipe::Pointer) STUB
bool ConnStateData::handleReadData() STUB_RETVAL(false)
bool ConnStateData::handleRequestBodyData() STUB_RETVAL(false)
-void ConnStateData::pinConnection(const Comm::ConnectionPointer &pinServerConn, HttpRequest *request, CachePeer *peer, bool auth, bool monitor) STUB
+void ConnStateData::pinBusyConnection(const Comm::ConnectionPointer &, const HttpRequest::Pointer &) STUB
+void ConnStateData::notePinnedConnectionBecameIdle(PinnedIdleContext) STUB
void ConnStateData::unpinConnection(const bool andClose) STUB
const Comm::ConnectionPointer ConnStateData::validatePinnedConnection(HttpRequest *request, const CachePeer *peer) STUB_RETVAL(NULL)
void ConnStateData::clientPinnedConnectionClosed(const CommCloseCbParams &io) STUB
@@ -70,7 +71,7 @@
void ConnStateData::swanSong() STUB
void ConnStateData::quitAfterError(HttpRequest *request) STUB
#if USE_OPENSSL
-void ConnStateData::httpsPeeked(Comm::ConnectionPointer serverConnection) STUB
+void ConnStateData::httpsPeeked(PinnedIdleContext) STUB
void ConnStateData::getSslContextStart() STUB
void ConnStateData::getSslContextDone(SSL_CTX * sslContext, bool isNew) STUB
void ConnStateData::sslCrtdHandleReplyWrapper(void *data, const Helper::Reply &reply) STUB

View File

@@ -0,0 +1,30 @@
------------------------------------------------------------
revno: 14181
revision-id: squidadm@squid-cache.org-20170701121615-ktx76udds2mzmc6c
parent: squid3@treenet.co.nz-20170701120848-q2xznzfvxb4kwvb6
committer: Source Maintenance <squidadm@squid-cache.org>
branch nick: 3.5
timestamp: Sat 2017-07-01 12:16:15 +0000
message:
SourceFormat Enforcement
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squidadm@squid-cache.org-20170701121615-\
# ktx76udds2mzmc6c
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: d7942d1260def31f62ccc820a44bb769381beae2
# timestamp: 2017-07-01 12:32:29 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20170701120848-\
# q2xznzfvxb4kwvb6
#
# Begin patch
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2017-07-01 12:08:48 +0000
+++ src/client_side.cc 2017-07-01 12:16:15 +0000
@@ -5195,3 +5195,4 @@
{
return os << pic.connection << ", request=" << pic.request;
}
+

View File

@@ -0,0 +1,35 @@
------------------------------------------------------------
revno: 14182
revision-id: squid3@treenet.co.nz-20170701232253-3beaysa03xf5c67p
parent: squidadm@squid-cache.org-20170701121615-ktx76udds2mzmc6c
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Sun 2017-07-02 11:22:53 +1200
message:
Fix build on FreeBSD after rev.14180
RefCount<> does not support assignment from nullptr with C++03
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20170701232253-3beaysa03xf5c67p
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: d5ecf68c60c022783f69311e9049e546be8fa1a0
# timestamp: 2017-07-01 23:50:58 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squidadm@squid-cache.org-20170701121615-\
# ktx76udds2mzmc6c
#
# Begin patch
=== modified file 'src/http.cc'
--- src/http.cc 2017-07-01 12:08:48 +0000
+++ src/http.cc 2017-07-01 23:22:53 +0000
@@ -1449,7 +1449,7 @@
Comm::ConnectionPointer serverConnectionSaved = serverConnection;
fwd->unregister(serverConnection);
- serverConnection = nullptr;
+ serverConnection = NULL;
bool ispinned = false; // TODO: Rename to isOrShouldBePinned
if (request->flags.pinned) {