From 3e6ecf5f4652e88b4f0b33054b73c839fd42912e Mon Sep 17 00:00:00 2001 From: Garri Djavadyan Date: Sun, 18 Dec 2016 01:59:41 +1300 Subject: [PATCH] Bug 4169: HIT marked as MISS when If-None-Match does not match --- src/client_side.cc | 1 + src/client_side_reply.cc | 40 ++++++++++++++++------------------------ src/enums.h | 1 + src/log/access_log.cc | 3 +++ 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/client_side.cc b/src/client_side.cc index a0cf69884a..15b60a4324 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -455,6 +455,7 @@ clientUpdateStatHistCounters(log_type logType, int svc_time) statCounter.client_http.nearHitSvcTime.count(svc_time); break; + case LOG_TCP_INM_HIT: case LOG_TCP_IMS_HIT: statCounter.client_http.nearMissSvcTime.count(svc_time); break; diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc index 5e9ce9b6d7..20a38d725a 100644 --- a/src/client_side_reply.cc +++ b/src/client_side_reply.cc @@ -710,40 +710,27 @@ clientReplyContext::processConditional(StoreIOBuffer &result) return true; } - bool matchedIfNoneMatch = false; if (r.header.has(HDR_IF_NONE_MATCH)) { - if (!e->hasIfNoneMatchEtag(r)) { - // RFC 2616: ignore IMS if If-None-Match did not match - r.flags.ims = 0; - r.ims = -1; - r.imslen = 0; - r.header.delById(HDR_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(HDR_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; + // None-Match is true (no ETag matched); treat as an unconditional hit + return false; } if (r.flags.ims) { // handle If-Modified-Since requests from the client if (e->modifiedSince(&r)) { - 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(); + // Modified-Since is true; treat as an unconditional hit + return false; } else { // otherwise reply with 304 Not Modified @@ -1880,7 +1867,12 @@ clientReplyContext::sendNotModified() 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, request_flags()); e = http->storeEntry(); diff --git a/src/enums.h b/src/enums.h index fc6639641f..63a210b9aa 100644 --- a/src/enums.h +++ b/src/enums.h @@ -45,6 +45,7 @@ typedef enum { 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_INM_HIT, LOG_TCP_IMS_HIT, LOG_TCP_SWAPFAIL_MISS, LOG_TCP_NEGATIVE_HIT, diff --git a/src/log/access_log.cc b/src/log/access_log.cc index 0aa40aed16..c3d995902a 100644 --- a/src/log/access_log.cc +++ b/src/log/access_log.cc @@ -584,6 +584,9 @@ logTypeIsATcpHit(log_type code) if (code == LOG_TCP_HIT) return 1; + if (code == LOG_TCP_INM_HIT) + return 1; + if (code == LOG_TCP_IMS_HIT) return 1; -- 2.47.2