Signed-off-by: Matthias Fischer matthias.fischer@ipfire.org --- lfs/squid | 6 + src/patches/squid/squid-3.5-14085.patch | 39 + src/patches/squid/squid-3.5-14086.patch | 51 ++ src/patches/squid/squid-3.5-14087.patch | 250 +++++++ src/patches/squid/squid-3.5-14088.patch | 1212 +++++++++++++++++++++++++++++++ src/patches/squid/squid-3.5-14089.patch | 69 ++ src/patches/squid/squid-3.5-14090.patch | 513 +++++++++++++ 7 files changed, 2140 insertions(+) create mode 100644 src/patches/squid/squid-3.5-14085.patch create mode 100644 src/patches/squid/squid-3.5-14086.patch create mode 100644 src/patches/squid/squid-3.5-14087.patch create mode 100644 src/patches/squid/squid-3.5-14088.patch create mode 100644 src/patches/squid/squid-3.5-14089.patch create mode 100644 src/patches/squid/squid-3.5-14090.patch
diff --git a/lfs/squid b/lfs/squid index 7ea4ce7..c47bd6f 100644 --- a/lfs/squid +++ b/lfs/squid @@ -73,6 +73,12 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects)) cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14082.patch cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14083.patch cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14084.patch + cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14085.patch + cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14086.patch + cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14087.patch + cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14088.patch + cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14089.patch + cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-3.5-14090.patch cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid-3.5.21-fix-max-file-descriptors.patch
cd $(DIR_APP) && autoreconf -vfi diff --git a/src/patches/squid/squid-3.5-14085.patch b/src/patches/squid/squid-3.5-14085.patch new file mode 100644 index 0000000..447435c --- /dev/null +++ b/src/patches/squid/squid-3.5-14085.patch @@ -0,0 +1,39 @@ +------------------------------------------------------------ +revno: 14085 +revision-id: squid3@treenet.co.nz-20160923111148-pjwuzgfvac43phk4 +parent: squid3@treenet.co.nz-20160921020936-2tn38o3ed7ssydfw +author: Alex Rousskov rousskov@measurement-factory.com +committer: Amos Jeffries squid3@treenet.co.nz +branch nick: 3.5 +timestamp: Fri 2016-09-23 23:11:48 +1200 +message: + Do reset $HOME if needed after r13435. Minimize putenv() memory leaks. + + While r13435 is attributed to me, the wrong condition was not mine. This + is a good example of why "cmp() op 0" pattern is usually safer because + the "==" or "!=" operator "tells" us whether the strings are equal, + unlike "!cmp()" that is often misread as "not equal". +------------------------------------------------------------ +# Bazaar merge directive format 2 (Bazaar 0.90) +# revision_id: squid3@treenet.co.nz-20160923111148-pjwuzgfvac43phk4 +# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# testament_sha1: 1ee13663f00dfee525ba77da72d97b65ff8fcf40 +# timestamp: 2016-09-23 11:32:15 +0000 +# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# base_revision_id: squid3@treenet.co.nz-20160921020936-\ +# 2tn38o3ed7ssydfw +# +# Begin patch +=== modified file 'src/cache_cf.cc' +--- src/cache_cf.cc 2016-01-01 00:14:27 +0000 ++++ src/cache_cf.cc 2016-09-23 11:11:48 +0000 +@@ -849,7 +849,7 @@ + if (pwd->pw_dir && *pwd->pw_dir) { + // putenv() leaks by design; avoid leaks when nothing changes + static SBuf lastDir; +- if (lastDir.isEmpty() || !lastDir.cmp(pwd->pw_dir)) { ++ if (lastDir.isEmpty() || lastDir.cmp(pwd->pw_dir) != 0) { + lastDir = pwd->pw_dir; + int len = strlen(pwd->pw_dir) + 6; + char *env_str = (char *)xcalloc(len, 1); + diff --git a/src/patches/squid/squid-3.5-14086.patch b/src/patches/squid/squid-3.5-14086.patch new file mode 100644 index 0000000..84e30e4 --- /dev/null +++ b/src/patches/squid/squid-3.5-14086.patch @@ -0,0 +1,51 @@ +------------------------------------------------------------ +revno: 14086 +revision-id: squid3@treenet.co.nz-20160923112248-1qvteivc0t3hgxnw +parent: squid3@treenet.co.nz-20160923111148-pjwuzgfvac43phk4 +author: Alex Rousskov rousskov@measurement-factory.com +committer: Amos Jeffries squid3@treenet.co.nz +branch nick: 3.5 +timestamp: Fri 2016-09-23 23:22:48 +1200 +message: + Do not leak url_rewrite_extras and store_id_extras on reconfigure/shutdown. + + TODO: We should not create unneeded extras either. +------------------------------------------------------------ +# Bazaar merge directive format 2 (Bazaar 0.90) +# revision_id: squid3@treenet.co.nz-20160923112248-1qvteivc0t3hgxnw +# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# testament_sha1: 859b1493e4e63328bbe29f45093ea8b71442f0e8 +# timestamp: 2016-09-23 11:32:18 +0000 +# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# base_revision_id: squid3@treenet.co.nz-20160923111148-\ +# pjwuzgfvac43phk4 +# +# Begin patch +=== modified file 'src/redirect.cc' +--- src/redirect.cc 2016-01-01 00:14:27 +0000 ++++ src/redirect.cc 2016-09-23 11:22:48 +0000 +@@ -369,11 +369,13 @@ + } + + if (Config.redirector_extras) { ++ delete redirectorExtrasFmt; + redirectorExtrasFmt = new ::Format::Format("url_rewrite_extras"); + (void)redirectorExtrasFmt->parse(Config.redirector_extras); + } + + if (Config.storeId_extras) { ++ delete storeIdExtrasFmt; + storeIdExtrasFmt = new ::Format::Format("store_id_extras"); + (void)storeIdExtrasFmt->parse(Config.storeId_extras); + } +@@ -388,9 +390,6 @@ + * When and if needed for more helpers a separated shutdown + * method will be added for each of them. + */ +- if (!storeIds && !redirectors) +- return; +- + if (redirectors) + helperShutdown(redirectors); + + diff --git a/src/patches/squid/squid-3.5-14087.patch b/src/patches/squid/squid-3.5-14087.patch new file mode 100644 index 0000000..5d5beca --- /dev/null +++ b/src/patches/squid/squid-3.5-14087.patch @@ -0,0 +1,250 @@ +------------------------------------------------------------ +revno: 14087 +revision-id: squid3@treenet.co.nz-20160923124044-i1road76n1108syf +parent: squid3@treenet.co.nz-20160923112248-1qvteivc0t3hgxnw +fixes bug: http://bugs.squid-cache.org/show_bug.cgi?id=3819 +author: Alex Rousskov rousskov@measurement-factory.com +committer: Amos Jeffries squid3@treenet.co.nz +branch nick: 3.5 +timestamp: Sat 2016-09-24 00:40:44 +1200 +message: + Bug 3819: "fd >= 0" assertion in file_write() during reconfiguration + + Other possible assertions include "NumberOfUFSDirs" in UFSSwapDir and + "fd >= 0" in file_close(). And Fs::Ufs::UFSStoreState::drainWriteQueue() + may segfault while dereferencing nil theFile, although I am not sure + that bug is caused specifically by reconfiguration. + + Since trunk r9181.3.1, reconfiguration is done in at least two steps: + First, mainReconfigureStart() closes/cleans/disables all modules + supporting hot reconfiguration and then, at the end of the event loop + iteration, mainReconfigureFinish() opens/configures/enables them again. + UFS code hits assertions if it has to log entries or rebuild swap.state + between those two steps. The tiny assertion opportunity window hides + these bugs from most proxies that are not doing enough disk I/O or are + not being reconfigured frequently enough. + + Asynchronous UFS cache_dirs such as diskd were the most exposed, but + even blocking UFS caching code could probably hit [rebuild] assertions. + + The swap.state rebuilding (always initiated at startup) probably did not + work as intended if reconfiguration happened during the rebuild time + because reconfiguration closed the swap.state file being rebuilt. We now + protect that swap.state file and delay rebuilding progress until + reconfiguration is over. + + TODO: To squash more similar bugs, we need to move hot reconfiguration + support into the modules so that each module can reconfigure instantly. +------------------------------------------------------------ +# Bazaar merge directive format 2 (Bazaar 0.90) +# revision_id: squid3@treenet.co.nz-20160923124044-i1road76n1108syf +# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# testament_sha1: 78eb315dda916767cc6addf75f78289161a2226c +# timestamp: 2016-09-23 12:51:02 +0000 +# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# base_revision_id: squid3@treenet.co.nz-20160923112248-\ +# 1qvteivc0t3hgxnw +# +# Begin patch +=== modified file 'src/fs/ufs/RebuildState.cc' +--- src/fs/ufs/RebuildState.cc 2016-01-01 00:14:27 +0000 ++++ src/fs/ufs/RebuildState.cc 2016-09-23 12:40:44 +0000 +@@ -74,9 +74,11 @@ + Fs::Ufs::RebuildState::RebuildStep(void *data) + { + RebuildState *rb = (RebuildState *)data; +- rb->rebuildStep(); ++ if (!reconfiguring) ++ rb->rebuildStep(); + +- if (!rb->isDone()) ++ // delay storeRebuildComplete() when reconfiguring to protect storeCleanup() ++ if (!rb->isDone() || reconfiguring) + eventAdd("storeRebuild", RebuildStep, rb, 0.01, 1); + else { + -- StoreController::store_dirs_rebuilding; + +=== modified file 'src/fs/ufs/UFSStoreState.cc' +--- src/fs/ufs/UFSStoreState.cc 2016-01-01 00:14:27 +0000 ++++ src/fs/ufs/UFSStoreState.cc 2016-09-23 12:40:44 +0000 +@@ -421,7 +421,7 @@ + if (flags.write_draining) + return; + +- if (!theFile->canWrite()) ++ if (!theFile || !theFile->canWrite()) + return; + + flags.write_draining = true; + +=== modified file 'src/fs/ufs/UFSSwapDir.cc' +--- src/fs/ufs/UFSSwapDir.cc 2016-01-01 00:14:27 +0000 ++++ src/fs/ufs/UFSSwapDir.cc 2016-09-23 12:40:44 +0000 +@@ -316,7 +316,8 @@ + currentIOOptions(new ConfigOptionVector()), + ioType(xstrdup(anIOType)), + cur_size(0), +- n_disk_objects(0) ++ n_disk_objects(0), ++ rebuilding_(false) + { + /* modulename is only set to disk modules that are built, by configure, + * so the Find call should never return NULL here. +@@ -723,6 +724,15 @@ + void + Fs::Ufs::UFSSwapDir::openLog() + { ++ assert(NumberOfUFSDirs || !UFSDirToGlobalDirMapping); ++ ++NumberOfUFSDirs; ++ assert(NumberOfUFSDirs <= Config.cacheSwap.n_configured); ++ ++ if (rebuilding_) { // we did not close the temporary log used for rebuilding ++ assert(swaplog_fd >= 0); ++ return; ++ } ++ + char *logPath; + logPath = logFile(); + swaplog_fd = file_open(logPath, O_WRONLY | O_CREAT | O_BINARY); +@@ -733,13 +743,6 @@ + } + + debugs(50, 3, HERE << "Cache Dir #" << index << " log opened on FD " << swaplog_fd); +- +- if (0 == NumberOfUFSDirs) +- assert(NULL == UFSDirToGlobalDirMapping); +- +- ++NumberOfUFSDirs; +- +- assert(NumberOfUFSDirs <= Config.cacheSwap.n_configured); + } + + void +@@ -748,18 +751,19 @@ + if (swaplog_fd < 0) /* not open */ + return; + +- file_close(swaplog_fd); +- +- debugs(47, 3, "Cache Dir #" << index << " log closed on FD " << swaplog_fd); +- +- swaplog_fd = -1; +- + --NumberOfUFSDirs; +- + assert(NumberOfUFSDirs >= 0); +- +- if (0 == NumberOfUFSDirs) ++ if (!NumberOfUFSDirs) + safe_free(UFSDirToGlobalDirMapping); ++ ++ if (rebuilding_) // we cannot close the temporary log used for rebuilding ++ return; ++ ++ file_close(swaplog_fd); ++ ++ debugs(47, 3, "Cache Dir #" << index << " log closed on FD " << swaplog_fd); ++ ++ swaplog_fd = -1; + } + + bool +@@ -839,6 +843,9 @@ + void + Fs::Ufs::UFSSwapDir::closeTmpSwapLog() + { ++ assert(rebuilding_); ++ rebuilding_ = false; ++ + char *swaplog_path = xstrdup(logFile(NULL)); // where the swaplog should be + char *tmp_path = xstrdup(logFile(".new")); // the temporary file we have generated + int fd; +@@ -864,6 +871,8 @@ + FILE * + Fs::Ufs::UFSSwapDir::openTmpSwapLog(int *clean_flag, int *zero_flag) + { ++ assert(!rebuilding_); ++ + char *swaplog_path = xstrdup(logFile(NULL)); + char *clean_path = xstrdup(logFile(".last-clean")); + char *new_path = xstrdup(logFile(".new")); +@@ -897,6 +906,7 @@ + } + + swaplog_fd = fd; ++ rebuilding_ = true; + + { + const StoreSwapLogHeader header; +@@ -1053,18 +1063,17 @@ + cleanLog = NULL; + } + +-void +-Fs::Ufs::UFSSwapDir::CleanEvent(void *unused) ++/// safely cleans a few unused files if possible ++int ++Fs::Ufs::UFSSwapDir::HandleCleanEvent() + { + static int swap_index = 0; + int i; + int j = 0; + int n = 0; +- /* +- * Assert that there are UFS cache_dirs configured, otherwise +- * we should never be called. +- */ +- assert(NumberOfUFSDirs); ++ ++ if (!NumberOfUFSDirs) ++ return 0; // probably in the middle of reconfiguration + + if (NULL == UFSDirToGlobalDirMapping) { + SwapDir *sd; +@@ -1106,6 +1115,13 @@ + ++swap_index; + } + ++ return n; ++} ++ ++void ++Fs::Ufs::UFSSwapDir::CleanEvent(void *) ++{ ++ const int n = HandleCleanEvent(); + eventAdd("storeDirClean", CleanEvent, NULL, + 15.0 * exp(-0.25 * n), 1); + } +@@ -1283,6 +1299,11 @@ + void + Fs::Ufs::UFSSwapDir::logEntry(const StoreEntry & e, int op) const + { ++ if (swaplog_fd < 0) { ++ debugs(36, 5, "cannot log " << e << " in the middle of reconfiguration"); ++ return; ++ } ++ + StoreSwapLogData *s = new StoreSwapLogData; + s->op = (char) op; + s->swap_filen = e.swap_filen; + +=== modified file 'src/fs/ufs/UFSSwapDir.h' +--- src/fs/ufs/UFSSwapDir.h 2016-01-01 00:14:27 +0000 ++++ src/fs/ufs/UFSSwapDir.h 2016-09-23 12:40:44 +0000 +@@ -145,6 +145,7 @@ + bool pathIsDirectory(const char *path)const; + int swaplog_fd; + static EVH CleanEvent; ++ static int HandleCleanEvent(); + /** Verify that the the CacheDir exists + * + * If this returns < 0, then Squid exits, complains about swap +@@ -164,6 +165,7 @@ + char const *ioType; + uint64_t cur_size; ///< currently used space in the storage area + uint64_t n_disk_objects; ///< total number of objects stored ++ bool rebuilding_; ///< whether RebuildState is writing the new swap.state + }; + + } //namespace Ufs + diff --git a/src/patches/squid/squid-3.5-14088.patch b/src/patches/squid/squid-3.5-14088.patch new file mode 100644 index 0000000..83b295c --- /dev/null +++ b/src/patches/squid/squid-3.5-14088.patch @@ -0,0 +1,1212 @@ +------------------------------------------------------------ +revno: 14088 +revision-id: squid3@treenet.co.nz-20160923152842-wstun879sjafjg2p +parent: squid3@treenet.co.nz-20160923124044-i1road76n1108syf +fixes bugs: http://bugs.squid-cache.org/show_bug.cgi?id=4311 http://bugs.squid-cache.org/show_bug.cgi?id=2833 http://bugs.squid-cache.org/show_bug.cgi?id=4471 +author: Eduard Bagdasaryan eduard.bagdasaryan@measurement-factory.com +committer: Amos Jeffries squid3@treenet.co.nz +branch nick: 3.5 +timestamp: Sat 2016-09-24 03:28:42 +1200 +message: + Bug 2833: Collapse internal revalidation requests (SMP-unaware caches) + + ... also address Bug 4311 and partially address Bug 2833 and Bug 4471. + + Extend collapsed_forwarding functionality to internal revalidation + requests. This implementation does not support Vary-controlled cache + objects and is limited to SMP-unaware caching environments, where each + Squid worker knows nothing about requests and caches handled by other + workers. However, it also lays critical groundwork for future SMP-aware + collapsed revalidation support. + + Prior to these changes, multiple concurrent HTTP requests for the same + stale cached object always resulted in multiple internal revalidation + requests sent by Squid to the origin server. Those internal requests + were likely to result in multiple competing Squid cache updates, causing + cache misses and/or more internal revalidation requests, negating + collapsed forwarding savings. + + Internal cache revalidation requests are collapsed if and only if + collapsed_forwarding is enabled. There is no option to control just + revalidation collapsing because there is no known use case for it. + + * Public revalidation keys + + Each Store entry has a unique key. Keys are used to find entries in the + Store (both already cached/swapped_out entries and not). Public keys are + normally tied to the request method and target URI. Same request + properties normally lead to the same public key, making cache hits + possible. If we were to calculate a public key for an internal + revalidation request, it would have been the same as the public key of + the stale cache entry being revalidated. Adding a revalidation response + to Store would have purged that being-revalidated cached entry, even if + the revalidation response itself was not cachable. + + To avoid purging being-revalidated cached entries, the old code used + private keys for internal revalidation requests. Private keys are always + unique and cannot accidentally purge a public entry. On the other hand, + for concurrent [revalidation] requests to find the store entry to + collapse on, that store entry has to have a public key! + + We resolved this conflict by adding "scope" to public keys: + + * Regular/old public keys have default empty scope (that does not affect + key generation). The code not dealing with collapsed revalidation + continues to work as before. All entries stored in caches continue to + have the same keys (with an empty scope). + + * When collapsed forwarding is enabled, collapsable internal + revalidation requests get public keys with a "revalidation" scope + (that is fed to the MD5 hash when the key is generated). Such a + revalidation request can find a matching store entry created by + another revalidation request (and collapse on it), but cannot clash + with the entry being revalidated (because that entry key is using a + different [empty] scope). + + This change not only enables collapsing of internal revalidation + requests within one worker, but opens the way for SMP-aware workers to + share information about collapsed revalidation requests, similar to how + those workers already share information about being-swapped-out cache + entries. + + + After receiving the revalidation response, each collapsed revalidation + request may call updateOnNotModified() to update the stale entry [with + the same revalidation response!]. Concurrent entry updates would have + wasted many resources, especially for disk-cached entries that support + header updates, and may have purged being-revalidated entries due to + locking conflicts among updating transactions. To minimize these + problems, we adjusted header and entry metadata updating logic to skip + the update if nothing have changed since the last update. + + + Also fixed Bug 4311: Collapsed forwarding deadlocks for SMP Squids using + SMP-unaware caches. Collapsed transactions stalled without getting a + response because they were waiting for the shared "transients" table + updates. The table was created by Store::Controller but then abandoned (not + updated) by SMP-unaware caches. Now, the transients table is not created + at all unless SMP-aware caches are present. This fix should also address + complaints about shared memory being used for Squid instances without + SMP-aware caches. + + A combination of SMP-aware and SMP-unaware caches is still not supported + and is likely to stall collapsed transactions if they are enabled. Note + that, by default, the memory cache is SMP-aware in SMP environments. +------------------------------------------------------------ +# Bazaar merge directive format 2 (Bazaar 0.90) +# revision_id: squid3@treenet.co.nz-20160923152842-wstun879sjafjg2p +# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# testament_sha1: 9bec994af031236daece9809ab5b219a0e7edde5 +# timestamp: 2016-09-23 15:51:02 +0000 +# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# base_revision_id: squid3@treenet.co.nz-20160923124044-\ +# i1road76n1108syf +# +# Begin patch +=== modified file 'src/HttpHeader.cc' +--- src/HttpHeader.cc 2016-01-01 00:14:27 +0000 ++++ src/HttpHeader.cc 2016-09-23 15:28:42 +0000 +@@ -450,7 +450,7 @@ + HttpHeader::HttpHeader(const HttpHeader &other): owner(other.owner), len(other.len), conflictingContentLength_(false) + { + httpHeaderMaskInit(&mask, 0); +- update(&other, NULL); // will update the mask as well ++ update(&other); // will update the mask as well + } + + HttpHeader::~HttpHeader() +@@ -465,7 +465,7 @@ + // we do not really care, but the caller probably does + assert(owner == other.owner); + clean(); +- update(&other, NULL); // will update the mask as well ++ update(&other); // will update the mask as well + len = other.len; + conflictingContentLength_ = other.conflictingContentLength_; + } +@@ -535,26 +535,71 @@ + } + } + ++/// check whether the fresh header has any new/changed updatable fields ++bool ++HttpHeader::needUpdate(HttpHeader const *fresh) const ++{ ++ for (unsigned int i = 0; i < fresh->entries.size(); ++i) { ++ const HttpHeaderEntry *e = fresh->entries[i]; ++ if (!e || skipUpdateHeader(e->id)) ++ continue; ++ String value; ++ const char *name = e->name.termedBuf(); ++ if (!getByNameIfPresent(name, value) || ++ (value != fresh->getByName(name))) ++ return true; ++ } ++ return false; ++} ++ + /* use fresh entries to replace old ones */ + void + httpHeaderUpdate(HttpHeader * old, const HttpHeader * fresh, const HttpHeaderMask * denied_mask) + { + assert (old); +- old->update (fresh, denied_mask); ++ old->update(fresh); + } + + void +-HttpHeader::update (HttpHeader const *fresh, HttpHeaderMask const *denied_mask) ++HttpHeader::updateWarnings() + { +- const HttpHeaderEntry *e; ++ int count = 0; + HttpHeaderPos pos = HttpHeaderInitPos; ++ ++ // RFC 7234, section 4.3.4: delete 1xx warnings and retain 2xx warnings ++ while (HttpHeaderEntry *e = getEntry(&pos)) { ++ if (e->id == HDR_WARNING && (e->getInt()/100 == 1) ) ++ delAt(pos, count); ++ } ++} ++ ++bool ++HttpHeader::skipUpdateHeader(const http_hdr_type id) const ++{ ++ // RFC 7234, section 4.3.4: use fields other from Warning for update ++ return id == HDR_WARNING; ++} ++ ++bool ++HttpHeader::update(HttpHeader const *fresh) ++{ + assert(fresh); + assert(this != fresh); + ++ // Optimization: Finding whether a header field changed is expensive ++ // and probably not worth it except for collapsed revalidation needs. ++ if (Config.onoff.collapsed_forwarding && !needUpdate(fresh)) ++ return false; ++ ++ updateWarnings(); ++ ++ const HttpHeaderEntry *e; ++ HttpHeaderPos pos = HttpHeaderInitPos; ++ + while ((e = fresh->getEntry(&pos))) { + /* deny bad guys (ok to check for HDR_OTHER) here */ + +- if (denied_mask && CBIT_TEST(*denied_mask, e->id)) ++ if (skipUpdateHeader(e->id)) + continue; + + if (e->id != HDR_OTHER) +@@ -567,13 +612,14 @@ + while ((e = fresh->getEntry(&pos))) { + /* deny bad guys (ok to check for HDR_OTHER) here */ + +- if (denied_mask && CBIT_TEST(*denied_mask, e->id)) ++ if (skipUpdateHeader(e->id)) + continue; + + debugs(55, 7, "Updating header '" << HeadersAttrs[e->id].name << "' in cached entry"); + + addEntry(e->clone()); + } ++ return true; + } + + /* just handy in parsing: resets and returns false */ +@@ -1720,7 +1766,6 @@ + HttpHeaderEntry::getInt() const + { + assert_eid (id); +- assert (Headers[id].type == ftInt); + int val = -1; + int ok = httpHeaderParseInt(value.termedBuf(), &val); + httpHeaderNoteParsedEntry(id, value, !ok); +@@ -1734,7 +1779,6 @@ + HttpHeaderEntry::getInt64() const + { + assert_eid (id); +- assert (Headers[id].type == ftInt64); + int64_t val = -1; + int ok = httpHeaderParseOffset(value.termedBuf(), &val); + httpHeaderNoteParsedEntry(id, value, !ok); + +=== modified file 'src/HttpHeader.h' +--- src/HttpHeader.h 2016-01-01 00:14:27 +0000 ++++ src/HttpHeader.h 2016-09-23 15:28:42 +0000 +@@ -217,7 +217,7 @@ + /* Interface functions */ + void clean(); + void append(const HttpHeader * src); +- void update (HttpHeader const *fresh, HttpHeaderMask const *denied_mask); ++ bool update(HttpHeader const *fresh); + void compact(); + int reset(); + int parse(const char *header_start, const char *header_end); +@@ -278,6 +278,9 @@ + protected: + /** \deprecated Public access replaced by removeHopByHopEntries() */ + void removeConnectionHeaderEntries(); ++ bool needUpdate(const HttpHeader *fresh) const; ++ bool skipUpdateHeader(const http_hdr_type id) const; ++ void updateWarnings(); + + private: + HttpHeaderEntry *findLastEntry(http_hdr_type id) const; + +=== modified file 'src/HttpReply.cc' +--- src/HttpReply.cc 2016-03-23 15:36:45 +0000 ++++ src/HttpReply.cc 2016-09-23 15:28:42 +0000 +@@ -24,39 +24,6 @@ + #include "Store.h" + #include "StrList.h" + +-/* local constants */ +- +-/* If we receive a 304 from the origin during a cache revalidation, we must +- * update the headers of the existing entry. Specifically, we need to update all +- * end-to-end headers and not any hop-by-hop headers (rfc2616 13.5.3). +- * +- * This is not the whole story though: since it is possible for a faulty/malicious +- * origin server to set headers it should not in a 304, we must explicitly ignore +- * these too. Specifically all entity-headers except those permitted in a 304 +- * (rfc2616 10.3.5) must be ignored. +- * +- * The list of headers we don't update is made up of: +- * all hop-by-hop headers +- * all entity-headers except Expires and Content-Location +- */ +-static HttpHeaderMask Denied304HeadersMask; +-static http_hdr_type Denied304HeadersArr[] = { +- // hop-by-hop headers +- HDR_CONNECTION, HDR_KEEP_ALIVE, HDR_PROXY_AUTHENTICATE, HDR_PROXY_AUTHORIZATION, +- HDR_TE, HDR_TRAILER, HDR_TRANSFER_ENCODING, HDR_UPGRADE, +- // entity headers +- HDR_ALLOW, HDR_CONTENT_ENCODING, HDR_CONTENT_LANGUAGE, HDR_CONTENT_LENGTH, +- HDR_CONTENT_MD5, HDR_CONTENT_RANGE, HDR_CONTENT_TYPE, HDR_LAST_MODIFIED +-}; +- +-/* module initialization */ +-void +-httpReplyInitModule(void) +-{ +- assert(Http::scNone == 0); // HttpReply::parse() interface assumes that +- httpHeaderMaskInit(&Denied304HeadersMask, 0); +- httpHeaderCalcMask(&Denied304HeadersMask, Denied304HeadersArr, countof(Denied304HeadersArr)); +-} + + HttpReply::HttpReply() : HttpMsg(hoReply), date (0), last_modified (0), + expires (0), surrogate_control (NULL), content_range (NULL), keep_alive (0), +@@ -271,20 +238,23 @@ + return 1; + } + +-void ++bool + HttpReply::updateOnNotModified(HttpReply const * freshRep) + { + assert(freshRep); + ++ /* update raw headers */ ++ if (!header.update(&freshRep->header)) ++ return false; ++ + /* clean cache */ + hdrCacheClean(); +- /* update raw headers */ +- header.update(&freshRep->header, +- (const HttpHeaderMask *) &Denied304HeadersMask); + + header.compact(); + /* init cache */ + hdrCacheInit(); ++ ++ return true; + } + + /* internal routines */ + +=== modified file 'src/HttpReply.h' +--- src/HttpReply.h 2016-01-01 00:14:27 +0000 ++++ src/HttpReply.h 2016-09-23 15:28:42 +0000 +@@ -72,7 +72,7 @@ + + virtual bool inheritProperties(const HttpMsg *aMsg); + +- void updateOnNotModified(HttpReply const *other); ++ bool updateOnNotModified(HttpReply const *other); + + /** set commonly used info with one call */ + void setHeaders(Http::StatusCode status, + +=== modified file 'src/MemStore.h' +--- src/MemStore.h 2016-01-01 00:14:27 +0000 ++++ src/MemStore.h 2016-09-23 15:28:42 +0000 +@@ -63,6 +63,7 @@ + virtual void maintain(); + virtual bool anchorCollapsed(StoreEntry &collapsed, bool &inSync); + virtual bool updateCollapsed(StoreEntry &collapsed); ++ virtual bool smpAware() const { return true; } + + static int64_t EntryLimit(); + + +=== modified file 'src/Store.h' +--- src/Store.h 2016-01-01 00:14:27 +0000 ++++ src/Store.h 2016-09-23 15:28:42 +0000 +@@ -24,6 +24,7 @@ + #include "Range.h" + #include "RemovalPolicy.h" + #include "StoreIOBuffer.h" ++#include "store_key_md5.h" + #include "StoreStats.h" + + #if USE_SQUID_ESI +@@ -93,9 +94,13 @@ + + void abort(); + void unlink(); +- void makePublic(); ++ void makePublic(const KeyScope keyScope = ksDefault); + void makePrivate(); +- void setPublicKey(); ++ 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 expireNow(); + void releaseRequest(); +@@ -130,7 +135,7 @@ + void registerAbort(STABH * cb, void *); + void reset(); + void setMemStatus(mem_status_t); +- void timestampsSet(); ++ bool timestampsSet(); + void unregisterAbort(); + void destroyMemObject(); + int checkTooSmall(); +@@ -228,6 +233,9 @@ + + private: + bool checkTooBig() const; ++ void forcePublicKey(const cache_key *newkey); ++ void adjustVary(); ++ const cache_key *calcPublicKey(const KeyScope keyScope); + + static MemAllocator *pool; + +@@ -430,6 +438,9 @@ + /// update a local collapsed entry with fresh info from this cache (if any) + virtual bool updateCollapsed(StoreEntry &collapsed) { return false; } + ++ /// whether this storage is capable of serving multiple workers ++ virtual bool smpAware() const = 0; ++ + private: + static RefCount<Store> CurrentRoot; + }; +@@ -450,10 +461,10 @@ + StoreEntry *storeGetPublic(const char *uri, const HttpRequestMethod& method); + + /// \ingroup StoreAPI +-StoreEntry *storeGetPublicByRequest(HttpRequest * request); ++StoreEntry *storeGetPublicByRequest(HttpRequest * request, const KeyScope keyScope = ksDefault); + + /// \ingroup StoreAPI +-StoreEntry *storeGetPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod& method); ++StoreEntry *storeGetPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod& method, const KeyScope keyScope = ksDefault); + + /// \ingroup StoreAPI + /// Like storeCreatePureEntry(), but also locks the entry and sets entry key. + +=== modified file 'src/StoreHashIndex.h' +--- src/StoreHashIndex.h 2016-01-01 00:14:27 +0000 ++++ src/StoreHashIndex.h 2016-09-23 15:28:42 +0000 +@@ -59,6 +59,8 @@ + + virtual StoreSearch *search(String const url, HttpRequest *); + ++ virtual bool smpAware() const; ++ + private: + /* migration logic */ + StorePointer store(int const x) const; + +=== modified file 'src/SwapDir.h' +--- src/SwapDir.h 2016-01-01 00:14:27 +0000 ++++ src/SwapDir.h 2016-09-23 15:28:42 +0000 +@@ -51,6 +51,7 @@ + virtual void memoryDisconnect(StoreEntry &e); + virtual void allowCollapsing(StoreEntry *e, const RequestFlags &reqFlags, const HttpRequestMethod &reqMethod); + virtual void syncCollapsed(const sfileno xitIndex); ++ virtual bool smpAware() const; + + virtual void init(); + +@@ -158,6 +159,7 @@ + virtual void getStats(StoreInfoStats &stats) const; + virtual void stat (StoreEntry &anEntry) const; + virtual StoreSearch *search(String const url, HttpRequest *) = 0; ++ virtual bool smpAware() const { return false; } + + /* migrated from store_dir.cc */ + bool objectSizeIsAcceptable(int64_t objsize) const; + +=== modified file 'src/Transients.cc' +--- src/Transients.cc 2016-01-01 00:14:27 +0000 ++++ src/Transients.cc 2016-09-23 15:28:42 +0000 +@@ -197,7 +197,8 @@ + e->mem_obj->xitTable.io = MemObject::ioReading; + e->mem_obj->xitTable.index = index; + +- e->setPublicKey(); ++ // TODO: Support collapsed revalidation for SMP-aware caches ++ e->setPublicKey(ksDefault); + assert(e->key); + + // How do we know its SMP- and not just locally-collapsed? A worker gets + +=== modified file 'src/Transients.h' +--- src/Transients.h 2016-01-01 00:14:27 +0000 ++++ src/Transients.h 2016-09-23 15:28:42 +0000 +@@ -72,6 +72,7 @@ + virtual bool dereference(StoreEntry &, bool); + virtual void markForUnlink(StoreEntry &e); + virtual void maintain(); ++ virtual bool smpAware() const { return true; } + + static int64_t EntryLimit(); + + +=== modified file 'src/adaptation/History.cc' +--- src/adaptation/History.cc 2016-01-01 00:14:27 +0000 ++++ src/adaptation/History.cc 2016-09-23 15:28:42 +0000 +@@ -150,9 +150,9 @@ + void Adaptation::History::recordMeta(const HttpHeader *lm) + { + lastMeta.clean(); +- lastMeta.update(lm, NULL); ++ lastMeta.update(lm); + +- allMeta.update(lm, NULL); ++ allMeta.update(lm); + allMeta.compact(); + } + + +=== modified file 'src/cf.data.pre' +--- src/cf.data.pre 2016-09-16 11:53:28 +0000 ++++ src/cf.data.pre 2016-09-23 15:28:42 +0000 +@@ -6105,17 +6105,29 @@ + LOC: Config.onoff.collapsed_forwarding + DEFAULT: off + DOC_START +- This option controls whether Squid is allowed to merge multiple +- potentially cachable requests for the same URI before Squid knows +- whether the response is going to be cachable. +- +- This feature is disabled by default: Enabling collapsed forwarding +- needlessly delays forwarding requests that look cachable (when they are +- collapsed) but then need to be forwarded individually anyway because +- they end up being for uncachable content. However, in some cases, such +- as accelleration of highly cachable content with periodic or groupped +- expiration times, the gains from collapsing [large volumes of +- simultenous refresh requests] outweigh losses from such delays. ++ When enabled, instead of forwarding each concurrent request for ++ the same URL, Squid just sends the first of them. The other, so ++ called "collapsed" requests, wait for the response to the first ++ request and, if it happens to be cachable, use that response. ++ Here, "concurrent requests" means "received after the first ++ request headers were parsed and before the corresponding response ++ headers were parsed". ++ ++ This feature is disabled by default: enabling collapsed ++ forwarding needlessly delays forwarding requests that look ++ cachable (when they are collapsed) but then need to be forwarded ++ individually anyway because they end up being for uncachable ++ content. However, in some cases, such as acceleration of highly ++ cachable content with periodic or grouped expiration times, the ++ gains from collapsing [large volumes of simultaneous refresh ++ requests] outweigh losses from such delays. ++ ++ Squid collapses two kinds of requests: regular client requests ++ received on one of the listening ports and internal "cache ++ revalidation" requests which are triggered by those regular ++ requests hitting a stale cached object. Revalidation collapsing ++ is currently disabled for Squid instances containing SMP-aware ++ disk or memory caches and for Vary-controlled cached objects. + DOC_END + + COMMENT_START + +=== modified file 'src/client_side_reply.cc' +--- src/client_side_reply.cc 2016-08-17 05:48:29 +0000 ++++ src/client_side_reply.cc 2016-09-23 15:28:42 +0000 +@@ -72,7 +72,8 @@ + HTTPMSGUNLOCK(reply); + } + +-clientReplyContext::clientReplyContext(ClientHttpRequest *clientContext) : http (cbdataReference(clientContext)), old_entry (NULL), old_sc(NULL), deleting(false) ++clientReplyContext::clientReplyContext(ClientHttpRequest *clientContext) : http (cbdataReference(clientContext)), old_entry (NULL), old_sc(NULL), deleting(false), ++collapsedRevalidation(crNone) + {} + + /** Create an error in the store awaiting the client side to read it. +@@ -245,7 +246,6 @@ + clientReplyContext::processExpired() + { + const char *url = storeId(); +- StoreEntry *entry = NULL; + debugs(88, 3, "clientReplyContext::processExpired: '" << http->uri << "'"); + assert(http->storeEntry()->lastmod >= 0); + /* +@@ -267,9 +267,36 @@ + #endif + /* Prepare to make a new temporary request */ + saveState(); +- entry = storeCreateEntry(url, +- http->log_uri, http->request->flags, http->request->method); +- /* NOTE, don't call StoreEntry->lock(), storeCreateEntry() does it */ ++ ++ // TODO: support collapsed revalidation of Vary-controlled entries ++ const bool collapsingAllowed = Config.onoff.collapsed_forwarding && ++ !Store::Root().smpAware() && ++ http->request->vary_headers.isEmpty(); ++ ++ StoreEntry *entry = nullptr; ++ if (collapsingAllowed) { ++ if ((entry = storeGetPublicByRequest(http->request, ksRevalidation))) ++ entry->lock("clientReplyContext::processExpired#alreadyRevalidating"); ++ } ++ ++ if (entry) { ++ debugs(88, 5, "collapsed on existing revalidation entry: " << *entry); ++ collapsedRevalidation = crSlave; ++ } else { ++ entry = storeCreateEntry(url, ++ http->log_uri, http->request->flags, http->request->method); ++ /* NOTE, don't call StoreEntry->lock(), storeCreateEntry() does it */ ++ ++ if (collapsingAllowed) { ++ debugs(88, 5, "allow other revalidation requests to collapse on " << *entry); ++ Store::Root().allowCollapsing(entry, http->request->flags, ++ http->request->method); ++ collapsedRevalidation = crInitiator; ++ } else { ++ collapsedRevalidation = crNone; ++ } ++ } ++ + sc = storeClientListAdd(entry, this); + #if USE_DELAY_POOLS + /* delay_id is already set on original store client */ +@@ -289,12 +316,14 @@ + assert(http->out.offset == 0); + assert(http->request->clientConnectionManager == http->getConn()); + +- /* +- * A refcounted pointer so that FwdState stays around as long as +- * this clientReplyContext does +- */ +- Comm::ConnectionPointer conn = http->getConn() != NULL ? http->getConn()->clientConnection : NULL; +- FwdState::Start(conn, http->storeEntry(), http->request, http->al); ++ if (collapsedRevalidation != crSlave) { ++ /* ++ * A refcounted pointer so that FwdState stays around as long as ++ * this clientReplyContext does ++ */ ++ Comm::ConnectionPointer conn = http->getConn() != NULL ? http->getConn()->clientConnection : NULL; ++ FwdState::Start(conn, http->storeEntry(), http->request, http->al); ++ } + + /* Register with storage manager to receive updates when data comes in. */ + +@@ -408,6 +437,10 @@ + // forward response from origin + http->logType = LOG_TCP_REFRESH_MODIFIED; + debugs(88, 3, "handleIMSReply: origin replied " << status << ", replacing existing entry and forwarding to client"); ++ ++ if (collapsedRevalidation) ++ http->storeEntry()->clearPublicKeyScope(); ++ + sendClientUpstreamResponse(); + } + + +=== modified file 'src/client_side_reply.h' +--- src/client_side_reply.h 2016-01-01 00:14:27 +0000 ++++ src/client_side_reply.h 2016-09-23 15:28:42 +0000 +@@ -133,6 +133,14 @@ + store_client *old_sc; /* ... for entry to be validated */ + bool deleting; + ++ typedef enum { ++ crNone = 0, ///< collapsed revalidation is not allowed for this context ++ crInitiator, ///< we initiated collapsed revalidation request ++ crSlave ///< we collapsed on the existing revalidation request ++ } CollapsedRevalidation; ++ ++ CollapsedRevalidation collapsedRevalidation; ++ + CBDATA_CLASS2(clientReplyContext); + }; + + +=== modified file 'src/client_side_request.cc' +--- src/client_side_request.cc 2016-08-17 02:58:28 +0000 ++++ src/client_side_request.cc 2016-09-23 15:28:42 +0000 +@@ -336,7 +336,7 @@ + * correctness. + */ + if (header) +- request->header.update(header, NULL); ++ request->header.update(header); + + http->log_uri = xstrdup(urlCanonicalClean(request)); + + +=== modified file 'src/esi/Include.cc' +--- src/esi/Include.cc 2016-01-01 00:14:27 +0000 ++++ src/esi/Include.cc 2016-09-23 15:28:42 +0000 +@@ -279,7 +279,7 @@ + void + ESIInclude::prepareRequestHeaders(HttpHeader &tempheaders, ESIVarState *vars) + { +- tempheaders.update (&vars->header(), NULL); ++ tempheaders.update (&vars->header()); + tempheaders.removeHopByHopEntries(); + } + + +=== modified file 'src/fs/rock/RockSwapDir.h' +--- src/fs/rock/RockSwapDir.h 2016-01-01 00:14:27 +0000 ++++ src/fs/rock/RockSwapDir.h 2016-09-23 15:28:42 +0000 +@@ -48,6 +48,7 @@ + virtual void swappedOut(const StoreEntry &e); + virtual void create(); + virtual void parse(int index, char *path); ++ virtual bool smpAware() const { return true; } + + // temporary path to the shared memory map of first slots of cached entries + SBuf inodeMapPath() const; + +=== modified file 'src/fs/ufs/UFSSwapDir.h' +--- src/fs/ufs/UFSSwapDir.h 2016-09-23 12:40:44 +0000 ++++ src/fs/ufs/UFSSwapDir.h 2016-09-23 15:28:42 +0000 +@@ -90,6 +90,7 @@ + virtual void swappedOut(const StoreEntry &e); + virtual uint64_t currentSize() const { return cur_size; } + virtual uint64_t currentCount() const { return n_disk_objects; } ++ virtual bool smpAware() const { return false; } + + void unlinkFile(sfileno f); + // move down when unlink is a virtual method + +=== modified file 'src/main.cc' +--- src/main.cc 2016-03-02 09:12:10 +0000 ++++ src/main.cc 2016-09-23 15:28:42 +0000 +@@ -1069,8 +1069,6 @@ + + httpHeaderInitModule(); /* must go before any header processing (e.g. the one in errorInitialize) */ + +- httpReplyInitModule(); /* must go before accepting replies */ +- + errorInitialize(); + + accessLogInit(); + +=== modified file 'src/store.cc' +--- src/store.cc 2016-09-07 15:58:59 +0000 ++++ src/store.cc 2016-09-23 15:28:42 +0000 +@@ -162,12 +162,12 @@ + } + + void +-StoreEntry::makePublic() ++StoreEntry::makePublic(const KeyScope scope) + { + /* This object can be cached for a long time */ + + if (!EBIT_TEST(flags, RELEASE_REQUEST)) +- setPublicKey(); ++ setPublicKey(scope); + } + + void +@@ -585,19 +585,19 @@ + } + + StoreEntry * +-storeGetPublicByRequestMethod(HttpRequest * req, const HttpRequestMethod& method) ++storeGetPublicByRequestMethod(HttpRequest * req, const HttpRequestMethod& method, const KeyScope keyScope) + { +- return Store::Root().get(storeKeyPublicByRequestMethod(req, method)); ++ return Store::Root().get(storeKeyPublicByRequestMethod(req, method, keyScope)); + } + + StoreEntry * +-storeGetPublicByRequest(HttpRequest * req) ++storeGetPublicByRequest(HttpRequest * req, const KeyScope keyScope) + { +- StoreEntry *e = storeGetPublicByRequestMethod(req, req->method); ++ StoreEntry *e = storeGetPublicByRequestMethod(req, req->method, keyScope); + + if (e == NULL && req->method == Http::METHOD_HEAD) + /* We can generate a HEAD reply from a cached GET object */ +- e = storeGetPublicByRequestMethod(req, Http::METHOD_GET); ++ e = storeGetPublicByRequestMethod(req, Http::METHOD_GET, keyScope); + + return e; + } +@@ -653,10 +653,8 @@ + } + + void +-StoreEntry::setPublicKey() ++StoreEntry::setPublicKey(const KeyScope scope) + { +- const cache_key *newkey; +- + if (key && !EBIT_TEST(flags, KEY_PRIVATE)) + return; /* is already public */ + +@@ -680,80 +678,35 @@ + + assert(!EBIT_TEST(flags, RELEASE_REQUEST)); + +- if (mem_obj->request) { +- HttpRequest *request = mem_obj->request; +- +- if (mem_obj->vary_headers.isEmpty()) { +- /* First handle the case where the object no longer varies */ +- request->vary_headers.clear(); +- } else { +- 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 +- */ +- 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.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.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); +- /* We are allowed to do this typecast */ +- HttpReply *rep = new HttpReply; +- rep->setHeaders(Http::scOkay, "Internal marker object", "x-squid-internal/vary", -1, -1, squid_curtime + 100000); +- vary = mem_obj->getReply()->header.getList(HDR_VARY); +- +- if (vary.size()) { +- /* Again, we own this structure layout */ +- rep->header.putStr(HDR_VARY, vary.termedBuf()); +- vary.clean(); +- } +- +-#if X_ACCELERATOR_VARY +- vary = mem_obj->getReply()->header.getList(HDR_X_ACCELERATOR_VARY); +- +- if (vary.size() > 0) { +- /* Again, we own this structure layout */ +- rep->header.putStr(HDR_X_ACCELERATOR_VARY, vary.termedBuf()); +- vary.clean(); +- } +- +-#endif +- pe->replaceHttpReply(rep, false); // no write until key is public +- +- pe->timestampsSet(); +- +- pe->makePublic(); +- +- pe->startWriting(); // after makePublic() +- +- pe->complete(); +- +- pe->unlock("StoreEntry::setPublicKey+Vary"); +- } +- +- newkey = storeKeyPublicByRequest(mem_obj->request); +- } else +- newkey = storeKeyPublic(mem_obj->storeId(), mem_obj->method); +- ++ adjustVary(); ++ forcePublicKey(calcPublicKey(scope)); ++} ++ ++void ++StoreEntry::clearPublicKeyScope() ++{ ++ if (!key || EBIT_TEST(flags, KEY_PRIVATE)) ++ return; // probably the old public key was deleted or made private ++ ++ // TODO: adjustVary() when collapsed revalidation supports that ++ ++ const cache_key *newKey = calcPublicKey(ksDefault); ++ if (!storeKeyHashCmp(key, newKey)) ++ return; // probably another collapsed revalidation beat us to this change ++ ++ forcePublicKey(newKey); ++} ++ ++/// Unconditionally sets public key for this store entry. ++/// Releases the old entry with the same public key (if any). ++void ++StoreEntry::forcePublicKey(const cache_key *newkey) ++{ + if (StoreEntry *e2 = (StoreEntry *)hash_lookup(store_table, newkey)) { ++ assert(e2 != this); + debugs(20, 3, "Making old " << *e2 << " private."); + e2->setPrivateKey(); + e2->release(); +- +- if (mem_obj->request) +- newkey = storeKeyPublicByRequest(mem_obj->request); +- else +- newkey = storeKeyPublic(mem_obj->storeId(), mem_obj->method); + } + + if (key) +@@ -767,6 +720,84 @@ + storeDirSwapLog(this, SWAP_LOG_ADD); + } + ++/// Calculates correct public key for feeding forcePublicKey(). ++/// Assumes adjustVary() has been called for this entry already. ++const cache_key *StoreEntry::calcPublicKey(const KeyScope keyScope) { ++ assert(mem_obj); ++ return mem_obj->request ? storeKeyPublicByRequest(mem_obj->request, keyScope) : ++ storeKeyPublic(mem_obj->storeId(), mem_obj->method, keyScope); ++} ++ ++/// Updates mem_obj->request->vary_headers to reflect the current Vary. ++/// The vary_headers field is used to calculate the Vary marker key. ++/// Releases the old Vary marker with an outdated key (if any). ++void StoreEntry::adjustVary() { ++ assert(mem_obj); ++ ++ if (!mem_obj->request) ++ return; ++ ++ HttpRequest *request = mem_obj->request; ++ ++ if (mem_obj->vary_headers.isEmpty()) { ++ /* First handle the case where the object no longer varies */ ++ request->vary_headers.clear(); ++ } else { ++ 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 ++ */ ++ 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.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.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); ++ /* We are allowed to do this typecast */ ++ HttpReply *rep = new HttpReply; ++ rep->setHeaders(Http::scOkay, "Internal marker object", "x-squid-internal/vary", -1, -1, squid_curtime + 100000); ++ vary = mem_obj->getReply()->header.getList(HDR_VARY); ++ ++ if (vary.size()) { ++ /* Again, we own this structure layout */ ++ rep->header.putStr(HDR_VARY, vary.termedBuf()); ++ vary.clean(); ++ } ++ ++#if X_ACCELERATOR_VARY ++ vary = mem_obj->getReply()->header.getList(HDR_X_ACCELERATOR_VARY); ++ ++ if (vary.size() > 0) { ++ /* Again, we own this structure layout */ ++ rep->header.putStr(HDR_X_ACCELERATOR_VARY, vary.termedBuf()); ++ vary.clean(); ++ } ++ ++#endif ++ pe->replaceHttpReply(rep, false); // no write until key is public ++ ++ pe->timestampsSet(); ++ ++ pe->makePublic(); ++ ++ pe->startWriting(); // after makePublic() ++ ++ pe->complete(); ++ ++ pe->unlock("StoreEntry::forcePublicKey+Vary"); ++ } ++} ++ + StoreEntry * + storeCreatePureEntry(const char *url, const char *log_url, const RequestFlags &flags, const HttpRequestMethod& method) + { +@@ -1549,7 +1580,7 @@ + return 1; + } + +-void ++bool + StoreEntry::timestampsSet() + { + const HttpReply *reply = getReply(); +@@ -1587,14 +1618,22 @@ + served_date -= (squid_curtime - request_sent); + } + ++ time_t exp = 0; + if (reply->expires > 0 && reply->date > -1) +- expires = served_date + (reply->expires - reply->date); ++ exp = served_date + (reply->expires - reply->date); + else +- expires = reply->expires; ++ exp = reply->expires; ++ ++ if (lastmod == reply->last_modified && timestamp == served_date && expires == exp) ++ return false; ++ ++ expires = exp; + + lastmod = reply->last_modified; + + timestamp = served_date; ++ ++ return true; + } + + void + +=== modified file 'src/store_dir.cc' +--- src/store_dir.cc 2016-01-01 00:14:27 +0000 ++++ src/store_dir.cc 2016-09-23 15:28:42 +0000 +@@ -77,7 +77,7 @@ + debugs(47, DBG_IMPORTANT, "Using Least Load store dir selection"); + } + +- if (UsingSmp() && IamWorkerProcess() && Config.onoff.collapsed_forwarding) { ++ if (UsingSmp() && IamWorkerProcess() && Config.onoff.collapsed_forwarding && smpAware()) { + transients = new Transients; + transients->init(); + } +@@ -916,7 +916,8 @@ + StoreController::allowCollapsing(StoreEntry *e, const RequestFlags &reqFlags, + const HttpRequestMethod &reqMethod) + { +- e->makePublic(); // this is needed for both local and SMP collapsing ++ const KeyScope keyScope = reqFlags.refresh ? ksRevalidation : ksDefault; ++ e->makePublic(keyScope); // this is needed for both local and SMP collapsing + if (transients) + transients->startWriting(e, reqFlags, reqMethod); + debugs(20, 3, "may " << (transients && e->mem_obj->xitTable.index >= 0 ? +@@ -1003,6 +1004,12 @@ + return found; + } + ++bool ++StoreController::smpAware() const ++{ ++ return memStore || (swapDir.getRaw() && swapDir->smpAware()); ++} ++ + StoreHashIndex::StoreHashIndex() + { + if (store_table) +@@ -1267,6 +1274,19 @@ + return new StoreSearchHashIndex (this); + } + ++bool ++StoreHashIndex::smpAware() const ++{ ++ for (int i = 0; i < Config.cacheSwap.n_configured; ++i) { ++ // A mix is not supported, but we conservatively check every ++ // dir because features like collapsed revalidation should ++ // currently be disabled if any dir is SMP-aware ++ if (dir(i).smpAware()) ++ return true; ++ } ++ return false; ++} ++ + CBDATA_CLASS_INIT(StoreSearchHashIndex); + + StoreSearchHashIndex::StoreSearchHashIndex(RefCount<StoreHashIndex> aSwapDir) : +@@ -1345,4 +1365,3 @@ + ++bucket; + debugs(47,3, "got entries: " << entries.size()); + } +- + +=== modified file 'src/store_key_md5.cc' +--- src/store_key_md5.cc 2016-04-01 06:15:31 +0000 ++++ src/store_key_md5.cc 2016-09-23 15:28:42 +0000 +@@ -96,7 +96,7 @@ + } + + const cache_key * +-storeKeyPublic(const char *url, const HttpRequestMethod& method) ++storeKeyPublic(const char *url, const HttpRequestMethod& method, const KeyScope keyScope) + { + static cache_key digest[SQUID_MD5_DIGEST_LENGTH]; + unsigned char m = (unsigned char) method.id(); +@@ -104,18 +104,20 @@ + SquidMD5Init(&M); + SquidMD5Update(&M, &m, sizeof(m)); + SquidMD5Update(&M, (unsigned char *) url, strlen(url)); ++ if (keyScope) ++ SquidMD5Update(&M, &keyScope, sizeof(keyScope)); + SquidMD5Final(digest, &M); + return digest; + } + + const cache_key * +-storeKeyPublicByRequest(HttpRequest * request) ++storeKeyPublicByRequest(HttpRequest * request, const KeyScope keyScope) + { +- return storeKeyPublicByRequestMethod(request, request->method); ++ return storeKeyPublicByRequestMethod(request, request->method, keyScope); + } + + const cache_key * +-storeKeyPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod& method) ++storeKeyPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod& method, const KeyScope keyScope) + { + static cache_key digest[SQUID_MD5_DIGEST_LENGTH]; + unsigned char m = (unsigned char) method.id(); +@@ -125,6 +127,9 @@ + SquidMD5Update(&M, &m, sizeof(m)); + SquidMD5Update(&M, (unsigned char *) url, strlen(url)); + ++ if (keyScope) ++ SquidMD5Update(&M, &keyScope, sizeof(keyScope)); ++ + 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_key_md5.h' +--- src/store_key_md5.h 2016-01-01 00:14:27 +0000 ++++ src/store_key_md5.h 2016-09-23 15:28:42 +0000 +@@ -17,14 +17,19 @@ + class HttpRequestMethod; + class HttpRequest; + ++typedef enum { ++ ksDefault, ++ ksRevalidation ++} KeyScope; ++ + cache_key *storeKeyDup(const cache_key *); + cache_key *storeKeyCopy(cache_key *, const cache_key *); + void storeKeyFree(const cache_key *); + const cache_key *storeKeyScan(const char *); + const char *storeKeyText(const cache_key *); +-const cache_key *storeKeyPublic(const char *, const HttpRequestMethod&); +-const cache_key *storeKeyPublicByRequest(HttpRequest *); +-const cache_key *storeKeyPublicByRequestMethod(HttpRequest *, const HttpRequestMethod&); ++const cache_key *storeKeyPublic(const char *, const HttpRequestMethod&, const KeyScope keyScope = ksDefault); ++const cache_key *storeKeyPublicByRequest(HttpRequest *, const KeyScope keyScope = ksDefault); ++const cache_key *storeKeyPublicByRequestMethod(HttpRequest *, const HttpRequestMethod&, const KeyScope keyScope = ksDefault); + const cache_key *storeKeyPrivate(const char *, const HttpRequestMethod&, int); + int storeKeyHashBuckets(int); + int storeKeyNull(const cache_key *); + +=== modified file 'src/tests/stub_store.cc' +--- src/tests/stub_store.cc 2016-01-01 00:14:27 +0000 ++++ src/tests/stub_store.cc 2016-09-23 15:28:42 +0000 +@@ -42,9 +42,9 @@ + void StoreEntry::trimMemory(const bool preserveSwappable) STUB + void StoreEntry::abort() STUB + void StoreEntry::unlink() STUB +-void StoreEntry::makePublic() STUB ++void StoreEntry::makePublic(const KeyScope keyScope) STUB + void StoreEntry::makePrivate() STUB +-void StoreEntry::setPublicKey() STUB ++void StoreEntry::setPublicKey(const KeyScope keyScope) STUB + void StoreEntry::setPrivateKey() STUB + void StoreEntry::expireNow() STUB + void StoreEntry::releaseRequest() STUB +@@ -67,7 +67,7 @@ + void StoreEntry::registerAbort(STABH * cb, void *) STUB + void StoreEntry::reset() STUB + void StoreEntry::setMemStatus(mem_status_t) STUB +-void StoreEntry::timestampsSet() STUB ++bool StoreEntry::timestampsSet() STUB_RETVAL(false) + void StoreEntry::unregisterAbort() STUB + void StoreEntry::destroyMemObject() STUB + int StoreEntry::checkTooSmall() STUB_RETVAL(0) +@@ -125,8 +125,8 @@ + size_t storeEntryInUse() STUB_RETVAL(0) + void storeEntryReplaceObject(StoreEntry *, HttpReply *) STUB + StoreEntry *storeGetPublic(const char *uri, const HttpRequestMethod& method) STUB_RETVAL(NULL) +-StoreEntry *storeGetPublicByRequest(HttpRequest * request) STUB_RETVAL(NULL) +-StoreEntry *storeGetPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod& method) STUB_RETVAL(NULL) ++StoreEntry *storeGetPublicByRequest(HttpRequest * request, const KeyScope keyScope) STUB_RETVAL(NULL) ++StoreEntry *storeGetPublicByRequestMethod(HttpRequest * request, const HttpRequestMethod& method, const KeyScope keyScope) STUB_RETVAL(NULL) + StoreEntry *storeCreateEntry(const char *, const char *, const RequestFlags &, const HttpRequestMethod&) STUB_RETVAL(NULL) + StoreEntry *storeCreatePureEntry(const char *storeId, const char *logUrl, const RequestFlags &, const HttpRequestMethod&) STUB_RETVAL(NULL) + void storeInit(void) STUB + +=== modified file 'src/tests/testRock.cc' +--- src/tests/testRock.cc 2016-02-23 15:52:04 +0000 ++++ src/tests/testRock.cc 2016-09-23 15:28:42 +0000 +@@ -143,8 +143,6 @@ + + httpHeaderInitModule(); /* must go before any header processing (e.g. the one in errorInitialize) */ + +- httpReplyInitModule(); /* must go before accepting replies */ +- + mem_policy = createRemovalPolicy(Config.replPolicy); + + inited = true; + +=== modified file 'src/tests/testStore.h' +--- src/tests/testStore.h 2016-01-01 00:14:27 +0000 ++++ src/tests/testStore.h 2016-09-23 15:28:42 +0000 +@@ -76,6 +76,8 @@ + virtual bool dereference(StoreEntry &, bool) { return true; } + + virtual StoreSearch *search(String const url, HttpRequest *); ++ ++ virtual bool smpAware() const { return false; } + }; + + typedef RefCount<TestStore> TestStorePointer; + +=== modified file 'src/tests/testUfs.cc' +--- src/tests/testUfs.cc 2016-01-01 00:14:27 +0000 ++++ src/tests/testUfs.cc 2016-09-23 15:28:42 +0000 +@@ -75,8 +75,6 @@ + + httpHeaderInitModule(); /* must go before any header processing (e.g. the one in errorInitialize) */ + +- httpReplyInitModule(); /* must go before accepting replies */ +- + inited = true; + } + + diff --git a/src/patches/squid/squid-3.5-14089.patch b/src/patches/squid/squid-3.5-14089.patch new file mode 100644 index 0000000..56d2672 --- /dev/null +++ b/src/patches/squid/squid-3.5-14089.patch @@ -0,0 +1,69 @@ +------------------------------------------------------------ +revno: 14089 +revision-id: squidadm@squid-cache.org-20160923181424-i8cbpi71d8mz4tzo +parent: squid3@treenet.co.nz-20160923152842-wstun879sjafjg2p +committer: Source Maintenance squidadm@squid-cache.org +branch nick: 3.5 +timestamp: Fri 2016-09-23 18:14:24 +0000 +message: + SourceFormat Enforcement +------------------------------------------------------------ +# Bazaar merge directive format 2 (Bazaar 0.90) +# revision_id: squidadm@squid-cache.org-20160923181424-\ +# i8cbpi71d8mz4tzo +# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# testament_sha1: 4eaa97c34097f02baac996cedd2b2c5b4bda120e +# timestamp: 2016-09-23 18:50:59 +0000 +# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# base_revision_id: squid3@treenet.co.nz-20160923152842-\ +# wstun879sjafjg2p +# +# Begin patch +=== modified file 'src/HttpReply.cc' +--- src/HttpReply.cc 2016-09-23 15:28:42 +0000 ++++ src/HttpReply.cc 2016-09-23 18:14:24 +0000 +@@ -24,7 +24,6 @@ + #include "Store.h" + #include "StrList.h" + +- + HttpReply::HttpReply() : HttpMsg(hoReply), date (0), last_modified (0), + expires (0), surrogate_control (NULL), content_range (NULL), keep_alive (0), + protoPrefix("HTTP/"), bodySizeMax(-2) + +=== modified file 'src/Store.h' +--- src/Store.h 2016-09-23 15:28:42 +0000 ++++ src/Store.h 2016-09-23 18:14:24 +0000 +@@ -23,8 +23,8 @@ + #include "MemObject.h" + #include "Range.h" + #include "RemovalPolicy.h" ++#include "store_key_md5.h" + #include "StoreIOBuffer.h" +-#include "store_key_md5.h" + #include "StoreStats.h" + + #if USE_SQUID_ESI + +=== modified file 'src/client_side_reply.cc' +--- src/client_side_reply.cc 2016-09-23 15:28:42 +0000 ++++ src/client_side_reply.cc 2016-09-23 18:14:24 +0000 +@@ -73,7 +73,7 @@ + } + + clientReplyContext::clientReplyContext(ClientHttpRequest *clientContext) : http (cbdataReference(clientContext)), old_entry (NULL), old_sc(NULL), deleting(false), +-collapsedRevalidation(crNone) ++ collapsedRevalidation(crNone) + {} + + /** Create an error in the store awaiting the client side to read it. + +=== modified file 'src/store_dir.cc' +--- src/store_dir.cc 2016-09-23 15:28:42 +0000 ++++ src/store_dir.cc 2016-09-23 18:14:24 +0000 +@@ -1365,3 +1365,4 @@ + ++bucket; + debugs(47,3, "got entries: " << entries.size()); + } ++ + diff --git a/src/patches/squid/squid-3.5-14090.patch b/src/patches/squid/squid-3.5-14090.patch new file mode 100644 index 0000000..e54857d --- /dev/null +++ b/src/patches/squid/squid-3.5-14090.patch @@ -0,0 +1,513 @@ +------------------------------------------------------------ +revno: 14090 +revision-id: squid3@treenet.co.nz-20160923204924-28rxvvbloccrma6a +parent: squidadm@squid-cache.org-20160923181424-i8cbpi71d8mz4tzo +fixes bug: http://bugs.squid-cache.org/show_bug.cgi?id=4471 +author: Eduard Bagdasaryan eduard.bagdasaryan@measurement-factory.com +committer: Amos Jeffries squid3@treenet.co.nz +branch nick: 3.5 +timestamp: Sat 2016-09-24 08:49:24 +1200 +message: + Bug 4471: revalidation doesn't work when expired cached object lacks Last-Modified. + + The bug was caused by the fact that Squid used only Last-Modified header + value for evaluating entry's last modification time while making an + internal revalidation request. So, without Last-Modified it was not + possible to correctly fill If-Modified-Since header value. As a result, + the revalidation request was not initiated when it should be. + + To fix this problem, we should generate If-Modified-Since based on other + data, showing how "old" the cached response is, such as Date and Age + header values. Both Date and Age header values are utilized while + calculating entry's timestamp. Therefore, we should use the timestamp if + Last-Modified is unavailable. + + TODO: HttpRequest::imslen support looks broken/deprecated. Using this + field inside StoreEntry::modifiedSince() leads to several useless checks + and probably may cause other [hidden] problems. +------------------------------------------------------------ +# Bazaar merge directive format 2 (Bazaar 0.90) +# revision_id: squid3@treenet.co.nz-20160923204924-28rxvvbloccrma6a +# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# testament_sha1: 6178f67a2e1f914666b59cb8d3126062164e4141 +# timestamp: 2016-09-23 20:50:56 +0000 +# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 +# base_revision_id: squidadm@squid-cache.org-20160923181424-\ +# i8cbpi71d8mz4tzo +# +# Begin patch +=== modified file 'src/MemStore.cc' +--- src/MemStore.cc 2016-04-01 06:15:31 +0000 ++++ src/MemStore.cc 2016-09-23 20:49:24 +0000 +@@ -281,7 +281,7 @@ + e.lastref = basics.lastref; + e.timestamp = basics.timestamp; + e.expires = basics.expires; +- e.lastmod = basics.lastmod; ++ e.lastModified(basics.lastmod); + e.refcount = basics.refcount; + e.flags = basics.flags; + + +=== modified file 'src/Store.h' +--- src/Store.h 2016-09-23 18:14:24 +0000 ++++ src/Store.h 2016-09-23 20:49:24 +0000 +@@ -143,7 +143,16 @@ + void delayAwareRead(const Comm::ConnectionPointer &conn, char *buf, int len, AsyncCall::Pointer callback); + + void setNoDelay (bool const); +- bool modifiedSince(HttpRequest * request) const; ++ void lastModified(const time_t when) { lastModified_ = when; } ++ /// \returns entry's 'effective' modification time ++ time_t lastModified() const { ++ // may still return -1 if timestamp is not set ++ return lastModified_ < 0 ? timestamp : lastModified_; ++ } ++ /// \returns a formatted string with entry's timestamps ++ const char *describeTimestamps() const; ++ // TODO: consider removing currently unsupported imslen parameter ++ bool modifiedSince(const time_t ims, const int imslen = -1) const; + /// has ETag matching at least one of the If-Match etags + bool hasIfMatchEtag(const HttpRequest &request) const; + /// has ETag matching at least one of the If-None-Match etags +@@ -160,7 +169,9 @@ + time_t timestamp; + time_t lastref; + time_t expires; +- time_t lastmod; ++private: ++ time_t lastModified_; ///< received Last-Modified value or -1; use lastModified() ++public: + uint64_t swap_file_sz; + uint16_t refcount; + uint16_t flags; + +=== modified file 'src/client_side.cc' +--- src/client_side.cc 2016-09-16 11:53:28 +0000 ++++ src/client_side.cc 2016-09-23 20:49:24 +0000 +@@ -1237,7 +1237,7 @@ + + /* got modification time? */ + if (spec.time >= 0) { +- return http->storeEntry()->lastmod <= spec.time; ++ return !http->storeEntry()->modifiedSince(spec.time); + } + + assert(0); /* should not happen */ + +=== modified file 'src/client_side_reply.cc' +--- src/client_side_reply.cc 2016-09-23 18:14:24 +0000 ++++ src/client_side_reply.cc 2016-09-23 20:49:24 +0000 +@@ -247,7 +247,8 @@ + { + const char *url = storeId(); + debugs(88, 3, "clientReplyContext::processExpired: '" << http->uri << "'"); +- assert(http->storeEntry()->lastmod >= 0); ++ const time_t lastmod = http->storeEntry()->lastModified(); ++ assert(lastmod >= 0); + /* + * check if we are allowed to contact other servers + * @?@: Instead of a 504 (Gateway Timeout) reply, we may want to return +@@ -303,7 +304,7 @@ + sc->setDelayId(DelayId::DelayClient(http)); + #endif + +- http->request->lastmod = old_entry->lastmod; ++ http->request->lastmod = lastmod; + + if (!http->request->header.has(HDR_IF_NONE_MATCH)) { + ETag etag = {NULL, -1}; // TODO: make that a default ETag constructor +@@ -311,7 +312,7 @@ + http->request->etag = etag.str; + } + +- debugs(88, 5, "clientReplyContext::processExpired : lastmod " << entry->lastmod ); ++ debugs(88, 5, "lastmod " << entry->lastModified()); + http->storeEntry(entry); + assert(http->out.offset == 0); + assert(http->request->clientConnectionManager == http->getConn()); +@@ -420,7 +421,7 @@ + + // if client sent IMS + +- if (http->request->flags.ims && !old_entry->modifiedSince(http->request)) { ++ if (http->request->flags.ims && !old_entry->modifiedSince(http->request->ims, http->request->imslen)) { + // forward the 304 from origin + debugs(88, 3, "handleIMSReply: origin replied 304, revalidating existing entry and forwarding 304 to client"); + sendClientUpstreamResponse(); +@@ -596,11 +597,12 @@ + */ + r->flags.needValidation = true; + +- if (e->lastmod < 0) { +- debugs(88, 3, "validate HIT object? NO. Missing Last-Modified header. Do MISS."); ++ if (e->lastModified() < 0) { ++ debugs(88, 3, "validate HIT object? NO. Can't calculate entry modification time. Do MISS."); + /* +- * Previous reply didn't have a Last-Modified header, +- * we cannot revalidate it. ++ * We cannot revalidate entries without knowing their ++ * modification time. ++ * XXX: BUG 1890 objects without Date do not get one added. + */ + http->logType = LOG_TCP_MISS; + processMiss(); +@@ -789,7 +791,7 @@ + + if (r.flags.ims) { + // handle If-Modified-Since requests from the client +- if (e->modifiedSince(&r)) { ++ if (e->modifiedSince(r.ims, r.imslen)) { + http->logType = LOG_TCP_IMS_HIT; + sendMoreData(result); + return; + +=== modified file 'src/fs/rock/RockSwapDir.cc' +--- src/fs/rock/RockSwapDir.cc 2016-01-01 00:14:27 +0000 ++++ src/fs/rock/RockSwapDir.cc 2016-09-23 20:49:24 +0000 +@@ -134,7 +134,7 @@ + e.lastref = basics.lastref; + e.timestamp = basics.timestamp; + e.expires = basics.expires; +- e.lastmod = basics.lastmod; ++ e.lastModified(basics.lastmod); + e.refcount = basics.refcount; + e.flags = basics.flags; + + +=== modified file 'src/fs/ufs/RebuildState.cc' +--- src/fs/ufs/RebuildState.cc 2016-09-23 12:40:44 +0000 ++++ src/fs/ufs/RebuildState.cc 2016-09-23 20:49:24 +0000 +@@ -201,7 +201,7 @@ + tmpe.expires, + tmpe.timestamp, + tmpe.lastref, +- tmpe.lastmod, ++ tmpe.lastModified(), + tmpe.refcount, /* refcount */ + tmpe.flags, /* flags */ + (int) flags.clean)); +@@ -328,7 +328,7 @@ + currentEntry()->lastref = swapData.timestamp; + currentEntry()->timestamp = swapData.timestamp; + currentEntry()->expires = swapData.expires; +- currentEntry()->lastmod = swapData.lastmod; ++ currentEntry()->lastModified(swapData.lastmod); + currentEntry()->flags = swapData.flags; + currentEntry()->refcount += swapData.refcount; + sd->dereference(*currentEntry(), false); + +=== modified file 'src/fs/ufs/UFSSwapDir.cc' +--- src/fs/ufs/UFSSwapDir.cc 2016-09-23 12:40:44 +0000 ++++ src/fs/ufs/UFSSwapDir.cc 2016-09-23 20:49:24 +0000 +@@ -87,7 +87,7 @@ + s.timestamp = e.timestamp; + s.lastref = e.lastref; + s.expires = e.expires; +- s.lastmod = e.lastmod; ++ s.lastmod = e.lastModified(); + s.swap_file_sz = e.swap_file_sz; + s.refcount = e.refcount; + s.flags = e.flags; +@@ -805,7 +805,7 @@ + e->lastref = lastref; + e->timestamp = timestamp; + e->expires = expires; +- e->lastmod = lastmod; ++ e->lastModified(lastmod); + e->refcount = refcount; + e->flags = newFlags; + EBIT_CLR(e->flags, RELEASE_REQUEST); +@@ -1310,7 +1310,7 @@ + s->timestamp = e.timestamp; + s->lastref = e.lastref; + s->expires = e.expires; +- s->lastmod = e.lastmod; ++ s->lastmod = e.lastModified(); + s->swap_file_sz = e.swap_file_sz; + s->refcount = e.refcount; + s->flags = e.flags; + +=== modified file 'src/htcp.cc' +--- src/htcp.cc 2016-01-01 00:14:27 +0000 ++++ src/htcp.cc 2016-09-23 20:49:24 +0000 +@@ -886,8 +886,8 @@ + if (e && e->expires > -1) + hdr.putTime(HDR_EXPIRES, e->expires); + +- if (e && e->lastmod > -1) +- hdr.putTime(HDR_LAST_MODIFIED, e->lastmod); ++ if (e && e->lastModified() > -1) ++ hdr.putTime(HDR_LAST_MODIFIED, e->lastModified()); + + hdr.packInto(&p); + + +=== modified file 'src/ipc/StoreMap.cc' +--- src/ipc/StoreMap.cc 2016-01-01 00:14:27 +0000 ++++ src/ipc/StoreMap.cc 2016-09-23 20:49:24 +0000 +@@ -491,7 +491,7 @@ + basics.timestamp = from.timestamp; + basics.lastref = from.lastref; + basics.expires = from.expires; +- basics.lastmod = from.lastmod; ++ basics.lastmod = from.lastModified(); + basics.swap_file_sz = from.swap_file_sz; + basics.refcount = from.refcount; + basics.flags = from.flags; + +=== modified file 'src/peer_digest.cc' +--- src/peer_digest.cc 2016-07-23 07:16:20 +0000 ++++ src/peer_digest.cc 2016-09-23 20:49:24 +0000 +@@ -359,7 +359,7 @@ + /* set lastmod to trigger IMS request if possible */ + + if (old_e) +- e->lastmod = old_e->lastmod; ++ e->lastModified(old_e->lastModified()); + + /* push towards peer cache */ + debugs(72, 3, "peerDigestRequest: forwarding to fwdStart..."); +@@ -942,8 +942,8 @@ + debugs(72, 3, "peerDigestFetchFinish: expires: " << + (long int) fetch->expires << " (" << std::showpos << + (int) (fetch->expires - squid_curtime) << "), lmt: " << +- std::noshowpos << (long int) fetch->entry->lastmod << " (" << +- std::showpos << (int) (fetch->entry->lastmod - squid_curtime) << ++ std::noshowpos << (long int) fetch->entry->lastModified() << " (" << ++ std::showpos << (int) (fetch->entry->lastModified() - squid_curtime) << + ")"); + + } + +=== modified file 'src/refresh.cc' +--- src/refresh.cc 2016-09-07 18:14:33 +0000 ++++ src/refresh.cc 2016-09-23 20:49:24 +0000 +@@ -186,13 +186,8 @@ + } + + // 3. If there is a Last-Modified header, try the last-modified factor algorithm. +- if (entry->lastmod > -1 && entry->timestamp > entry->lastmod) { +- +- /* lastmod_delta is the difference between the last-modified date of the response +- * and the time we cached it. It's how "old" the response was when we got it. +- */ +- time_t lastmod_delta = entry->timestamp - entry->lastmod; +- ++ const time_t lastmod_delta = entry->timestamp - entry->lastModified(); ++ if (lastmod_delta > 0) { + /* stale_age is the age of the response when it became/becomes stale according to + * the last-modified factor algorithm. It's how long we can consider the response + * fresh from the time we cached it. +@@ -553,8 +548,8 @@ + /* Does not need refresh. This is certainly cachable */ + return true; + +- if (entry->lastmod < 0) +- /* Last modified is needed to do a refresh */ ++ if (entry->lastModified() < 0) ++ /* We should know entry's modification time to do a refresh */ + return false; + + if (entry->mem_obj == NULL) + +=== modified file 'src/stat.cc' +--- src/stat.cc 2016-09-07 15:58:59 +0000 ++++ src/stat.cc 2016-09-23 20:49:24 +0000 +@@ -77,7 +77,6 @@ + + /* LOCALS */ + static const char *describeStatuses(const StoreEntry *); +-static const char *describeTimestamps(const StoreEntry *); + static void statAvgTick(void *notused); + static void statAvgDump(StoreEntry *, int minutes, int hours); + #if STAT_GRAPHS +@@ -328,18 +327,6 @@ + return buf; + } + +-static const char * +-describeTimestamps(const StoreEntry * entry) +-{ +- LOCAL_ARRAY(char, buf, 256); +- snprintf(buf, 256, "LV:%-9d LU:%-9d LM:%-9d EX:%-9d", +- (int) entry->timestamp, +- (int) entry->lastref, +- (int) entry->lastmod, +- (int) entry->expires); +- return buf; +-} +- + static void + statStoreEntry(MemBuf * mb, StoreEntry * e) + { +@@ -347,7 +334,7 @@ + mb->Printf("KEY %s\n", e->getMD5Text()); + mb->Printf("\t%s\n", describeStatuses(e)); + mb->Printf("\t%s\n", storeEntryFlags(e)); +- mb->Printf("\t%s\n", describeTimestamps(e)); ++ mb->Printf("\t%s\n", e->describeTimestamps()); + mb->Printf("\t%d locks, %d clients, %d refs\n", + (int) e->locks(), + storePendingNClients(e), + +=== modified file 'src/store.cc' +--- src/store.cc 2016-09-23 15:28:42 +0000 ++++ src/store.cc 2016-09-23 20:49:24 +0000 +@@ -355,7 +355,7 @@ + timestamp(-1), + lastref(-1), + expires(-1), +- lastmod(-1), ++ lastModified_(-1), + swap_file_sz(0), + refcount(0), + flags(0), +@@ -1624,12 +1624,17 @@ + else + exp = reply->expires; + +- if (lastmod == reply->last_modified && timestamp == served_date && expires == exp) +- return false; ++ if (timestamp == served_date && expires == exp) { ++ // if the reply lacks LMT, then we now know that our effective ++ // LMT (i.e., timestamp) will stay the same, otherwise, old and ++ // new modification times must match ++ if (reply->last_modified < 0 || reply->last_modified == lastModified()) ++ return false; // nothing has changed ++ } + + expires = exp; + +- lastmod = reply->last_modified; ++ lastModified_ = reply->last_modified; + + timestamp = served_date; + +@@ -1664,7 +1669,7 @@ + debugs(20, l, "StoreEntry->timestamp: " << timestamp); + debugs(20, l, "StoreEntry->lastref: " << lastref); + debugs(20, l, "StoreEntry->expires: " << expires); +- debugs(20, l, "StoreEntry->lastmod: " << lastmod); ++ debugs(20, l, "StoreEntry->lastModified_: " << lastModified_); + debugs(20, l, "StoreEntry->swap_file_sz: " << swap_file_sz); + debugs(20, l, "StoreEntry->refcount: " << refcount); + debugs(20, l, "StoreEntry->flags: " << storeEntryFlags(this)); +@@ -1795,7 +1800,7 @@ + mem_obj->reset(); + HttpReply *rep = (HttpReply *) getReply(); // bypass const + rep->reset(); +- expires = lastmod = timestamp = -1; ++ expires = lastModified_ = timestamp = -1; + } + + /* +@@ -2005,13 +2010,10 @@ + } + + bool +-StoreEntry::modifiedSince(HttpRequest * request) const ++StoreEntry::modifiedSince(const time_t ims, const int imslen) const + { + int object_length; +- time_t mod_time = lastmod; +- +- if (mod_time < 0) +- mod_time = timestamp; ++ const time_t mod_time = lastModified(); + + debugs(88, 3, "modifiedSince: '" << url() << "'"); + +@@ -2026,16 +2028,16 @@ + if (object_length < 0) + object_length = contentLen(); + +- if (mod_time > request->ims) { ++ if (mod_time > ims) { + debugs(88, 3, "--> YES: entry newer than client"); + return true; +- } else if (mod_time < request->ims) { ++ } else if (mod_time < ims) { + debugs(88, 3, "--> NO: entry older than client"); + return false; +- } else if (request->imslen < 0) { ++ } else if (imslen < 0) { + debugs(88, 3, "--> NO: same LMT, no client length"); + return false; +- } else if (request->imslen == object_length) { ++ } else if (imslen == object_length) { + debugs(88, 3, "--> NO: same LMT, same length"); + return false; + } else { +@@ -2132,6 +2134,18 @@ + return true; + } + ++const char * ++StoreEntry::describeTimestamps() const ++{ ++ LOCAL_ARRAY(char, buf, 256); ++ snprintf(buf, 256, "LV:%-9d LU:%-9d LM:%-9d EX:%-9d", ++ static_cast<int>(timestamp), ++ static_cast<int>(lastref), ++ static_cast<int>(lastModified_), ++ static_cast<int>(expires)); ++ return buf; ++} ++ + std::ostream &operator <<(std::ostream &os, const StoreEntry &e) + { + os << "e:"; + +=== modified file 'src/store_rebuild.cc' +--- src/store_rebuild.cc 2016-01-01 00:14:27 +0000 ++++ src/store_rebuild.cc 2016-09-23 20:49:24 +0000 +@@ -254,7 +254,7 @@ + what->timestamp = tmp->timestamp; + what->lastref = tmp->lastref; + what->expires = tmp->expires; +- what->lastmod = tmp->lastmod; ++ what->lastModified(tmp->lastmod); + what->swap_file_sz = tmp->swap_file_sz; + what->refcount = tmp->refcount; + what->flags = tmp->flags; + +=== modified file 'src/tests/stub_store.cc' +--- src/tests/stub_store.cc 2016-09-23 15:28:42 +0000 ++++ src/tests/stub_store.cc 2016-09-23 20:49:24 +0000 +@@ -73,7 +73,7 @@ + int StoreEntry::checkTooSmall() STUB_RETVAL(0) + void StoreEntry::delayAwareRead(const Comm::ConnectionPointer&, char *buf, int len, AsyncCall::Pointer callback) STUB + void StoreEntry::setNoDelay (bool const) STUB +-bool StoreEntry::modifiedSince(HttpRequest * request) const STUB_RETVAL(false) ++bool StoreEntry::modifiedSince(const time_t ims, const int imslen) const STUB_RETVAL(false); + bool StoreEntry::hasIfMatchEtag(const HttpRequest &request) const STUB_RETVAL(false) + bool StoreEntry::hasIfNoneMatchEtag(const HttpRequest &request) const STUB_RETVAL(false) + RefCount<SwapDir> StoreEntry::store() const STUB_RETVAL(NULL) + +=== modified file 'src/tests/testStoreController.cc' +--- src/tests/testStoreController.cc 2016-01-01 00:14:27 +0000 ++++ src/tests/testStoreController.cc 2016-09-23 20:49:24 +0000 +@@ -113,7 +113,7 @@ + e->lastref = squid_curtime; + e->timestamp = squid_curtime; + e->expires = squid_curtime; +- e->lastmod = squid_curtime; ++ e->lastModified(squid_curtime); + e->refcount = 1; + EBIT_CLR(e->flags, RELEASE_REQUEST); + EBIT_CLR(e->flags, KEY_PRIVATE); + +=== modified file 'src/tests/testStoreHashIndex.cc' +--- src/tests/testStoreHashIndex.cc 2016-01-01 00:14:27 +0000 ++++ src/tests/testStoreHashIndex.cc 2016-09-23 20:49:24 +0000 +@@ -94,7 +94,7 @@ + e->lastref = squid_curtime; + e->timestamp = squid_curtime; + e->expires = squid_curtime; +- e->lastmod = squid_curtime; ++ e->lastModified(squid_curtime); + e->refcount = 1; + EBIT_CLR(e->flags, RELEASE_REQUEST); + EBIT_CLR(e->flags, KEY_PRIVATE); +