HTTP Compliance: Reply with an error if required validation failed
RFC 2616 says that proxy MUST not use stale entries that have s-maxage,
proxy-revalidate, or must-revalidate cache-directive.
Add new fail_on_validation_err request flag to store result from
refreshCheck(). It is needed to avoid refreshLimits() recalculation in
clientReplyContext::handleIMSReply().
Split LOG_TCP_REFRESH_FAIL into LOG_TCP_REFRESH_FAIL_OLD (stale reply
sent) and LOG_TCP_REFRESH_FAIL_ERR (error forwarded). Though both logged
as TCP_REFRESH_FAIL for backward-compatibility with external scripts, etc.
Co-Advisor test cases:
test_case/rfc2616/noSrv-hit-must-reval-s-maxage-resp
test_case/rfc2616/noSrv-hit-must-reval-proxy-revalidate-resp
test_case/rfc2616/noSrv-hit-must-reval-must-revalidate-resp
-------------
Is it a good idea to log new LOG_TCP_REFRESH_FAIL_OLD (stale reply sent)
and LOG_TCP_REFRESH_FAIL_ERR (error forwarded) as
"LOG_TCP_REFRESH_FAIL"? There are probably a lot of scripts out there
that cannot easily handle new outcomes, but I do not know whether we
should be kept hostage to them as the two outcomes are rather different.
Will we have to use the old tag forever?
Thank you,
Alex.
HTTP Compliance: Reply with an error if required validation failed.
RFC 2616 says that proxy MUST not use stale entries that have s-maxage,
proxy-revalidate, or must-revalidate cache-directive.
Add new fail_on_validation_err request flag to store result from
refreshCheck(). It is needed to avoid refreshLimits() recalculation in
clientReplyContext::handleIMSReply().
Split LOG_TCP_REFRESH_FAIL into LOG_TCP_REFRESH_FAIL_OLD (stale reply sent)
and LOG_TCP_REFRESH_FAIL_ERR (error forwarded). Though both logged as
TCP_REFRESH_FAIL for backward-compatibility with external scripts, etc.
Co-Advisor test cases:
test_case/rfc2616/noSrv-hit-must-reval-s-maxage-resp
test_case/rfc2616/noSrv-hit-must-reval-proxy-revalidate-resp
test_case/rfc2616/noSrv-hit-must-reval-must-revalidate-resp
=== modified file 'src/client_side_reply.cc'
--- src/client_side_reply.cc 2010-09-13 02:25:09 +0000
+++ src/client_side_reply.cc 2010-09-19 04:19:39 +0000
@@ -332,83 +332,88 @@ clientReplyContext::handleIMSReply(Store
{
if (deleting)
return;
debugs(88, 3, "handleIMSReply: " << http->storeEntry()->url() << ", " << (long unsigned) result.length << " bytes" );
if (http->storeEntry() == NULL)
return;
if (result.flags.error && !EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED))
return;
/* update size of the request */
reqsize = result.length + reqofs;
const http_status status = http->storeEntry()->getReply()->sline.status;
// request to origin was aborted
if (EBIT_TEST(http->storeEntry()->flags, ENTRY_ABORTED)) {
debugs(88, 3, "handleIMSReply: request to origin aborted '" << http->storeEntry()->url() << "', sending old entry to client" );
- http->logType = LOG_TCP_REFRESH_FAIL;
+ http->logType = LOG_TCP_REFRESH_FAIL_OLD;
sendClientOldEntry();
}
HttpReply *old_rep = (HttpReply *) old_entry->getReply();
// origin replied 304
// TODO FIXME: old_rep2 was forcibly unshadowed, used to be old_rep. Are we sure
// that the right semantics were preserved?
if (status == HTTP_NOT_MODIFIED) {
http->logType = LOG_TCP_REFRESH_UNMODIFIED;
// update headers on existing entry
HttpReply *old_rep2 = (HttpReply *) old_entry->getReply();
old_rep2->updateOnNotModified(http->storeEntry()->getReply());
old_entry->timestampsSet();
// if client sent IMS
if (http->request->flags.ims && !old_entry->modifiedSince(http->request)) {
// forward the 304 from origin
debugs(88, 3, "handleIMSReply: origin replied 304, revalidating existing entry and forwarding 304 to client");
sendClientUpstreamResponse();
} else {
// send existing entry, it's still valid
debugs(88, 3, "handleIMSReply: origin replied 304, revalidating existing entry and sending " <<
old_rep2->sline.status << " to client");
sendClientOldEntry();
}
}
// origin replied with a non-error code
else if (status > HTTP_STATUS_NONE && status < HTTP_INTERNAL_SERVER_ERROR) {
// forward response from origin
http->logType = LOG_TCP_REFRESH_MODIFIED;
debugs(88, 3, "handleIMSReply: origin replied " << status << ", replacing existing entry and forwarding to client");
sendClientUpstreamResponse();
}
// origin replied with an error
- else {
+ else if (http->request->flags.fail_on_validation_err) {
+ http->logType = LOG_TCP_REFRESH_FAIL_ERR;
+ debugs(88, 3, "handleIMSReply: origin replied with error " << status <<
+ ", forwarding to client due to fail_on_validation_err");
+ sendClientUpstreamResponse();
+ } else {
// ignore and let client have old entry
- http->logType = LOG_TCP_REFRESH_FAIL;
+ http->logType = LOG_TCP_REFRESH_FAIL_OLD;
debugs(88, 3, "handleIMSReply: origin replied with error " <<
status << ", sending old entry (" << old_rep->sline.status << ") to client");
sendClientOldEntry();
}
}
extern "C" CSR clientGetMoreData;
extern "C" CSD clientReplyDetach;
/**
* clientReplyContext::cacheHit Should only be called until the HTTP reply headers
* have been parsed. Normally this should be a single call, but
* it might take more than one. As soon as we have the headers,
* we hand off to clientSendMoreData, processExpired, or
* processMiss.
*/
void
clientReplyContext::CacheHit(void *data, StoreIOBuffer result)
{
clientReplyContext *context = (clientReplyContext *)data;
=== modified file 'src/enums.h'
--- src/enums.h 2010-09-01 08:18:47 +0000
+++ src/enums.h 2010-09-19 04:11:16 +0000
@@ -24,41 +24,42 @@
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA.
*
*/
#ifndef SQUID_ENUMS_H
#define SQUID_ENUMS_H
#include "HttpStatusCode.h"
typedef enum {
LOG_TAG_NONE,
LOG_TCP_HIT,
LOG_TCP_MISS,
LOG_TCP_REFRESH_UNMODIFIED, // refresh from origin revalidated existing entry
- LOG_TCP_REFRESH_FAIL, // refresh from origin failed
+ LOG_TCP_REFRESH_FAIL_OLD, // refresh from origin failed, stale reply sent
+ LOG_TCP_REFRESH_FAIL_ERR, // refresh from origin failed, error forwarded
LOG_TCP_REFRESH_MODIFIED, // refresh from origin replaced existing entry
LOG_TCP_CLIENT_REFRESH_MISS,
LOG_TCP_IMS_HIT,
LOG_TCP_SWAPFAIL_MISS,
LOG_TCP_NEGATIVE_HIT,
LOG_TCP_MEM_HIT,
LOG_TCP_DENIED,
LOG_TCP_DENIED_REPLY,
LOG_TCP_OFFLINE_HIT,
#if LOG_TCP_REDIRECTS
LOG_TCP_REDIRECT,
#endif
LOG_UDP_HIT,
LOG_UDP_MISS,
LOG_UDP_DENIED,
LOG_UDP_INVALID,
LOG_UDP_MISS_NOFETCH,
LOG_ICP_QUERY,
LOG_TYPE_MAX
} log_type;
=== modified file 'src/http.cc'
--- src/http.cc 2010-09-14 07:37:38 +0000
+++ src/http.cc 2010-09-19 04:04:53 +0000
@@ -924,43 +924,43 @@ HttpStateData::haveParsedReplyHeaders()
case -1:
#if USE_HTTP_VIOLATIONS
if (Config.negativeTtl > 0)
entry->cacheNegatively();
else
#endif
entry->makePrivate();
break;
default:
assert(0);
break;
}
no_cache:
if (!ignoreCacheControl && rep->cache_control) {
- if (EBIT_TEST(rep->cache_control->mask, CC_PROXY_REVALIDATE))
- EBIT_SET(entry->flags, ENTRY_REVALIDATE);
- else if (EBIT_TEST(rep->cache_control->mask, CC_MUST_REVALIDATE))
+ if (EBIT_TEST(rep->cache_control->mask, CC_PROXY_REVALIDATE) ||
+ EBIT_TEST(rep->cache_control->mask, CC_MUST_REVALIDATE) ||
+ EBIT_TEST(rep->cache_control->mask, CC_S_MAXAGE))
EBIT_SET(entry->flags, ENTRY_REVALIDATE);
}
#if HEADERS_LOG
headersLog(1, 0, request->method, rep);
#endif
ctx_exit(ctx);
}
HttpStateData::ConnectionStatus
HttpStateData::statusIfComplete() const
{
const HttpReply *rep = virginReply();
/** \par
* If the reply wants to close the connection, it takes precedence */
if (httpHeaderHasConnDir(&rep->header, "close"))
return COMPLETE_NONPERSISTENT_MSG;
=== modified file 'src/log/access_log.cc'
--- src/log/access_log.cc 2010-09-01 08:18:47 +0000
+++ src/log/access_log.cc 2010-09-19 03:56:40 +0000
@@ -56,41 +56,42 @@
static void accessLogSquid(AccessLogEntry * al, Logfile * logfile);
static void accessLogCommon(AccessLogEntry * al, Logfile * logfile);
static void accessLogCustom(AccessLogEntry * al, customlog * log);
#if HEADERS_LOG
static Logfile *headerslog = NULL;
#endif
#if MULTICAST_MISS_STREAM
static int mcast_miss_fd = -1;
static struct sockaddr_in mcast_miss_to;
static void mcast_encode(unsigned int *, size_t, const unsigned int *);
#endif
const char *log_tags[] = {
"NONE",
"TCP_HIT",
"TCP_MISS",
"TCP_REFRESH_UNMODIFIED",
- "TCP_REFRESH_FAIL",
+ "TCP_REFRESH_FAIL", // same tag logged for LOG_TCP_REFRESH_FAIL_OLD and
+ "TCP_REFRESH_FAIL", // LOG_TCP_REFRESH_FAIL_ERR for backward-compatibility
"TCP_REFRESH_MODIFIED",
"TCP_CLIENT_REFRESH_MISS",
"TCP_IMS_HIT",
"TCP_SWAPFAIL_MISS",
"TCP_NEGATIVE_HIT",
"TCP_MEM_HIT",
"TCP_DENIED",
"TCP_DENIED_REPLY",
"TCP_OFFLINE_HIT",
#if LOG_TCP_REDIRECTS
"TCP_REDIRECT",
#endif
"UDP_HIT",
"UDP_MISS",
"UDP_DENIED",
"UDP_INVALID",
"UDP_MISS_NOFETCH",
"ICP_QUERY",
"LOG_TYPE_MAX"
};
@@ -2482,37 +2483,37 @@ accessLogFreeMemory(AccessLogEntry * aLo
HTTPMSGUNLOCK(aLogEntry->reply);
HTTPMSGUNLOCK(aLogEntry->request);
#if ICAP_CLIENT
HTTPMSGUNLOCK(aLogEntry->icap.reply);
HTTPMSGUNLOCK(aLogEntry->icap.request);
#endif
}
int
logTypeIsATcpHit(log_type code)
{
/* this should be a bitmap for better optimization */
if (code == LOG_TCP_HIT)
return 1;
if (code == LOG_TCP_IMS_HIT)
return 1;
- if (code == LOG_TCP_REFRESH_FAIL)
+ if (code == LOG_TCP_REFRESH_FAIL_OLD)
return 1;
if (code == LOG_TCP_REFRESH_UNMODIFIED)
return 1;
if (code == LOG_TCP_NEGATIVE_HIT)
return 1;
if (code == LOG_TCP_MEM_HIT)
return 1;
if (code == LOG_TCP_OFFLINE_HIT)
return 1;
return 0;
}
=== modified file 'src/refresh.cc'
--- src/refresh.cc 2010-05-14 09:47:03 +0000
+++ src/refresh.cc 2010-09-19 04:04:53 +0000
@@ -260,40 +260,41 @@ refreshCheck(const StoreEntry * entry, H
debugs(22, 3, "Staleness = " << staleness);
debugs(22, 3, "refreshCheck: Matched '" << R->pattern << " " <<
(int) R->min << " " << (int) (100.0 * R->pct) << "%% " <<
(int) R->max << "'");
debugs(22, 3, "refreshCheck: age = " << age);
debugs(22, 3, "\tcheck_time:\t" << mkrfc1123(check_time));
debugs(22, 3, "\tentry->timestamp:\t" << mkrfc1123(entry->timestamp));
if (EBIT_TEST(entry->flags, ENTRY_REVALIDATE) && staleness > -1
#if USE_HTTP_VIOLATIONS
&& !R->flags.ignore_must_revalidate
#endif
) {
debugs(22, 3, "refreshCheck: YES: Must revalidate stale response");
+ request->flags.fail_on_validation_err = 1;
return STALE_MUST_REVALIDATE;
}
/* request-specific checks */
if (request && !request->flags.ignore_cc) {
HttpHdrCc *cc = request->cache_control;
if (request->flags.ims && (R->flags.refresh_ims || Config.onoff.refresh_all_ims)) {
/* The clients no-cache header is changed into a IMS query */
debugs(22, 3, "refreshCheck: YES: refresh-ims");
return STALE_FORCED_RELOAD;
}
#if USE_HTTP_VIOLATIONS
if (!request->flags.nocache_hack) {
(void) 0;
} else if (R->flags.ignore_reload) {
/* The clients no-cache header is ignored */
debugs(22, 3, "refreshCheck: MAYBE: ignore-reload");
=== modified file 'src/structs.h'
--- src/structs.h 2010-09-14 07:37:38 +0000
+++ src/structs.h 2010-09-19 04:04:53 +0000
@@ -991,62 +991,63 @@ struct _netdbEntry {
int n_peers;
};
struct _iostats {
enum { histSize = 16 };
struct {
int reads;
int reads_deferred;
int read_hist[histSize];
int writes;
int write_hist[histSize];
}
Http, Ftp, Gopher;
};
struct request_flags {
- request_flags(): range(0),nocache(0),ims(0),auth(0),cachable(0),hierarchical(0),loopdetect(0),proxy_keepalive(0),proxying(0),refresh(0),redirected(0),need_validation(0),accelerated(0),ignore_cc(0),intercepted(0),spoof_client_ip(0),internal(0),internalclient(0),must_keepalive(0),chunked_reply(0),stream_error(0),destinationIPLookedUp_(0) {
+ request_flags(): range(0),nocache(0),ims(0),auth(0),cachable(0),hierarchical(0),loopdetect(0),proxy_keepalive(0),proxying(0),refresh(0),redirected(0),need_validation(0),fail_on_validation_err(0),accelerated(0),ignore_cc(0),intercepted(0),spoof_client_ip(0),internal(0),internalclient(0),must_keepalive(0),chunked_reply(0),stream_error(0),destinationIPLookedUp_(0) {
#if USE_HTTP_VIOLATIONS
nocache_hack = 0;
#endif
#if FOLLOW_X_FORWARDED_FOR
done_follow_x_forwarded_for = 0;
#endif /* FOLLOW_X_FORWARDED_FOR */
}
unsigned int range:1;
unsigned int nocache:1;
unsigned int ims:1;
unsigned int auth:1;
unsigned int cachable:1;
unsigned int hierarchical:1;
unsigned int loopdetect:1;
unsigned int proxy_keepalive:1;
unsigned int proxying:
1; /* this should be killed, also in httpstateflags */
unsigned int refresh:1;
unsigned int redirected:1;
unsigned int need_validation:1;
+ unsigned int fail_on_validation_err:1; ///< whether we should fail if validation fails
#if USE_HTTP_VIOLATIONS
unsigned int nocache_hack:1; /* for changing/ignoring no-cache requests */
#endif
unsigned int accelerated:1;
unsigned int ignore_cc:1;
unsigned int intercepted:1; /**< transparently intercepted request */
unsigned int spoof_client_ip:1; /**< spoof client ip if possible */
unsigned int internal:1;
unsigned int internalclient:1;
unsigned int must_keepalive:1;
unsigned int connection_auth:1; /** Request wants connection oriented auth */
unsigned int connection_auth_disabled:1; /** Connection oriented auth can not be supported */
unsigned int connection_proxy_auth:1; /** Request wants connection oriented auth */
unsigned int pinned:1; /* Request sent on a pinned connection */
unsigned int auth_sent:1; /* Authentication forwarded */
unsigned int no_direct:1; /* Deny direct forwarding unless overriden by always_direct. Used in accelerator mode */
unsigned int chunked_reply:1; /**< Reply with chunked transfer encoding */
unsigned int stream_error:1; /**< Whether stream error has occured */
// When adding new flags, please update cloneAdaptationImmune() as needed.
