On Wed, 2016-11-30 at 07:44 -0700, Alex Rousskov wrote:
> On 11/30/2016 04:44 AM, Garri Djavadyan wrote:
> >
> > * If-Modified-Since header is ignored if If-None-Match header
> > exists
> > (RFC7232 compliance)
>
> >
> > if (r.header.has(Http::HdrType::IF_NONE_MATCH)) {
> > + if (e->hasIfNoneMatchEtag(r)) {
> > + // RFC 7232: If-None-Match recipient MUST ignore IMS
> > r.flags.ims = false;
> > r.ims = -1;
> > r.imslen = 0;
> > r.header.delById(Http::HdrType::IF_MODIFIED_SINCE);
> >
> > sendNotModifiedOrPreconditionFailedError();
> > return true;
> > }
> >
> > + // If-None-Match did not match; treat as an unconditional
> > hit
> > + return false;
> > }
>
>
> Can you explain why we are not clearing various IMS flags and headers
> when hasIfNoneMatchEtag() returns false? Does not RFC7232 compliance
> require equal treatment of IMS in both true and false cases?
>
>
> Thank you,
>
> Alex.
Sorry, Alex you are right. I overlooked it. I've attached fixed
version.
On Thu, 2016-12-01 at 01:20 +1300, Amos Jeffries wrote:
> I would really like the LogTags flag to be an actual flag, but I can
> do that as a followup or later when I convert the others to flags.
Should I revert TCP_INM_HIT to TCP_IMS_HIT at the moment?
Thanks,
Garri
=== modified file 'src/LogTags.cc'
--- src/LogTags.cc 2016-10-08 22:19:44 +0000
+++ src/LogTags.cc 2016-11-30 09:46:54 +0000
@@ -21,6 +21,7 @@
"TCP_REFRESH",
"TCP_CLIENT_REFRESH_MISS",
"TCP_IMS_HIT",
+ "TCP_INM_HIT",
"TCP_SWAPFAIL_MISS",
"TCP_NEGATIVE_HIT",
"TCP_MEM_HIT",
@@ -73,6 +74,7 @@
return
(oldType == LOG_TCP_HIT) ||
(oldType == LOG_TCP_IMS_HIT) ||
+ (oldType == LOG_TCP_INM_HIT) ||
(oldType == LOG_TCP_REFRESH_FAIL_OLD) ||
(oldType == LOG_TCP_REFRESH_UNMODIFIED) ||
(oldType == LOG_TCP_NEGATIVE_HIT) ||
=== modified file 'src/LogTags.h'
--- src/LogTags.h 2016-10-08 22:19:44 +0000
+++ src/LogTags.h 2016-11-30 07:11:46 +0000
@@ -28,6 +28,7 @@
LOG_TCP_REFRESH, // refresh from origin started, but still pending
LOG_TCP_CLIENT_REFRESH_MISS,
LOG_TCP_IMS_HIT,
+ LOG_TCP_INM_HIT,
LOG_TCP_SWAPFAIL_MISS,
LOG_TCP_NEGATIVE_HIT,
LOG_TCP_MEM_HIT,
=== modified file 'src/client_side.cc'
--- src/client_side.cc 2016-11-18 18:08:30 +0000
+++ src/client_side.cc 2016-11-30 07:13:38 +0000
@@ -233,6 +233,10 @@
statCounter.client_http.nearMissSvcTime.count(svc_time);
break;
+ case LOG_TCP_INM_HIT:
+ statCounter.client_http.nearMissSvcTime.count(svc_time);
+ break;
+
case LOG_TCP_HIT:
case LOG_TCP_MEM_HIT:
=== modified file 'src/client_side_reply.cc'
--- src/client_side_reply.cc 2016-11-11 04:28:06 +0000
+++ src/client_side_reply.cc 2016-11-30 14:57:51 +0000
@@ -796,40 +796,27 @@
return true;
}
- bool matchedIfNoneMatch = false;
if (r.header.has(Http::HdrType::IF_NONE_MATCH)) {
- if (!e->hasIfNoneMatchEtag(r)) {
- // RFC 2616: ignore IMS if If-None-Match did not match
- r.flags.ims = false;
- r.ims = -1;
- r.imslen = 0;
- r.header.delById(Http::HdrType::IF_MODIFIED_SINCE);
- http->logType = LOG_TCP_MISS;
- sendMoreData(result);
- return true;
- }
+ // RFC 7232: If-None-Match recipient MUST ignore IMS
+ r.flags.ims = false;
+ r.ims = -1;
+ r.imslen = 0;
+ r.header.delById(Http::HdrType::IF_MODIFIED_SINCE);
- if (!r.flags.ims) {
- // RFC 2616: if If-None-Match matched and there is no IMS,
- // reply with 304 Not Modified or 412 Precondition Failed
+ if (e->hasIfNoneMatchEtag(r)) {
sendNotModifiedOrPreconditionFailedError();
return true;
}
- // otherwise check IMS below to decide if we reply with 304 or 412
- matchedIfNoneMatch = true;
+ // If-None-Match did not match; treat as an unconditional hit
+ return false;
}
if (r.flags.ims) {
// handle If-Modified-Since requests from the client
if (e->modifiedSince(r.ims, r.imslen)) {
- http->logType = LOG_TCP_IMS_HIT;
- sendMoreData(result);
-
- } else if (matchedIfNoneMatch) {
- // If-None-Match matched, reply with 304 Not Modified or
- // 412 Precondition Failed
- sendNotModifiedOrPreconditionFailedError();
+ // object modified; treat as an unconditional hit
+ return false;
} else {
// otherwise reply with 304 Not Modified
@@ -1989,7 +1976,12 @@
StoreEntry *e = http->storeEntry();
const time_t timestamp = e->timestamp;
HttpReply *const temprep = e->getReply()->make304();
- http->logType = LOG_TCP_IMS_HIT;
+ // log as TCP_INM_HIT if code 304 generated for
+ // If-None-Match request
+ if (!http->request->flags.ims)
+ http->logType = LOG_TCP_INM_HIT;
+ else
+ http->logType = LOG_TCP_IMS_HIT;
removeClientStoreReference(&sc, http);
createStoreEntry(http->request->method, RequestFlags());
e = http->storeEntry();
_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev
