------------------------------------------------------------

revno: 14109

revision-id: squid3@treenet.co.nz-20161111060325-yh8chavvnzuvfh3h

parent: squid3@treenet.co.nz-20161101112231-k77st4up2sekl5zx

fixes bug: http://bugs.squid-cache.org/show_bug.cgi?id=3379

author: Garri Djavadyan <garryd@comnet.uz>, Amos Jeffries <squid3@treenet.co.nz>

committer: Amos Jeffries <squid3@treenet.co.nz>

branch nick: 3.5

timestamp: Fri 2016-11-11 19:03:25 +1300

message:

  Bug 3379: Combination of If-Match and a Cache Hit result in TCP Connection Failure

------------------------------------------------------------

# Bazaar merge directive format 2 (Bazaar 0.90)

# revision_id: squid3@treenet.co.nz-20161111060325-yh8chavvnzuvfh3h

# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5

# testament_sha1: 50d66878a765925d9a64569b3c226bebdee1f736

# timestamp: 2016-11-11 06:10:37 +0000

# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5

# base_revision_id: squid3@treenet.co.nz-20161101112231-\

#   k77st4up2sekl5zx

# 

# Begin patch

=== modified file 'src/client_side_reply.cc'

--- src/client_side_reply.cc  2016-10-09 19:47:26 +0000

+++ src/client_side_reply.cc  2016-11-11 06:03:25 +0000

@@ -589,6 +589,7 @@

         debugs(88, 5, "negative-HIT");

         http->logType = LOG_TCP_NEGATIVE_HIT;

         sendMoreData(result);

+        return;

     } else if (blockedHit()) {

         debugs(88, 5, "send_hit forces a MISS");

         http->logType = LOG_TCP_MISS;

@@ -641,27 +642,29 @@

             http->logType = LOG_TCP_MISS;

             processMiss();

         }

+        return;

     } else if (r->conditional()) {

         debugs(88, 5, "conditional HIT");

-        processConditional(result);

-    } else {

-        /*

-         * plain ol' cache hit

-         */

-        debugs(88, 5, "plain old HIT");

+        if (processConditional(result))

+            return;

+    }

+

+    /*

+     * plain ol' cache hit

+     */

+    debugs(88, 5, "plain old HIT");

 #if USE_DELAY_POOLS

-        if (e->store_status != STORE_OK)

-            http->logType = LOG_TCP_MISS;

-        else

+    if (e->store_status != STORE_OK)

+        http->logType = LOG_TCP_MISS;

+    else

 #endif

-            if (e->mem_status == IN_MEMORY)

-                http->logType = LOG_TCP_MEM_HIT;

-            else if (Config.onoff.offline)

-                http->logType = LOG_TCP_OFFLINE_HIT;

+        if (e->mem_status == IN_MEMORY)

+            http->logType = LOG_TCP_MEM_HIT;

+        else if (Config.onoff.offline)

+            http->logType = LOG_TCP_OFFLINE_HIT;

-        sendMoreData(result);

-    }

+    sendMoreData(result);

 }

 /**

@@ -755,17 +758,16 @@

 }

 /// process conditional request from client

-void

+bool

 clientReplyContext::processConditional(StoreIOBuffer &result)

 {

     StoreEntry *const e = http->storeEntry();

     if (e->getReply()->sline.status() != Http::scOkay) {

-        debugs(88, 4, "clientReplyContext::processConditional: Reply code " <<

-               e->getReply()->sline.status() << " != 200");

+        debugs(88, 4, "Reply code " << e->getReply()->sline.status() << " != 200");

         http->logType = LOG_TCP_MISS;

         processMiss();

-        return;

+        return true;

     }

     HttpRequest &r = *http->request;

@@ -773,7 +775,7 @@

     if (r.header.has(HDR_IF_MATCH) && !e->hasIfMatchEtag(r)) {

         // RFC 2616: reply with 412 Precondition Failed if If-Match did not match

         sendPreconditionFailedError();

-        return;

+        return true;

     }

     bool matchedIfNoneMatch = false;

@@ -786,14 +788,14 @@

             r.header.delById(HDR_IF_MODIFIED_SINCE);

             http->logType = LOG_TCP_MISS;

             sendMoreData(result);

-            return;

+            return true;

         }

         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

             sendNotModifiedOrPreconditionFailedError();

-            return;

+            return true;

         }

         // otherwise check IMS below to decide if we reply with 304 or 412

@@ -805,19 +807,20 @@

         if (e->modifiedSince(&r)) {

             http->logType = LOG_TCP_IMS_HIT;

             sendMoreData(result);

-            return;

-        }

-        if (matchedIfNoneMatch) {

+        } else if (matchedIfNoneMatch) {

             // If-None-Match matched, reply with 304 Not Modified or

             // 412 Precondition Failed

             sendNotModifiedOrPreconditionFailedError();

-            return;

+

+        } else {

+            // otherwise reply with 304 Not Modified

+            sendNotModified();

         }

-

-        // otherwise reply with 304 Not Modified

-        sendNotModified();

+        return true;

     }

+

+    return false;

 }

 /// whether squid.conf send_hit prevents us from serving this hit

=== modified file 'src/client_side_reply.h'

--- src/client_side_reply.h 2016-09-23 15:28:42 +0000

+++ src/client_side_reply.h 2016-11-11 06:03:25 +0000

@@ -114,7 +114,7 @@

     bool alwaysAllowResponse(Http::StatusCode sline) const;

     int checkTransferDone();

     void processOnlyIfCachedMiss();

-    void processConditional(StoreIOBuffer &result);

+    bool processConditional(StoreIOBuffer &result);

     void cacheHit(StoreIOBuffer result);

     void handleIMSReply(StoreIOBuffer result);

     void sendMoreData(StoreIOBuffer result);

------------------------------------------------------------

revno: 14126

revision-id: squid3@treenet.co.nz-20161215103357-827wow3k1y3k9yql

parent: squid3@treenet.co.nz-20161215093634-ykbs6tv8pdusz7cj

fixes bug: http://bugs.squid-cache.org/show_bug.cgi?id=4169

author: Garri Djavadyan <garryd@comnet.uz>

committer: Amos Jeffries <squid3@treenet.co.nz>

branch nick: 3.5

timestamp: Thu 2016-12-15 23:33:57 +1300

message:

  Bug 4169: HIT marked as MISS when If-None-Match does not match

------------------------------------------------------------

# Bazaar merge directive format 2 (Bazaar 0.90)

# revision_id: squid3@treenet.co.nz-20161215103357-827wow3k1y3k9yql

# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5

# testament_sha1: 258cd3e400bcb137a7bcdf6e7e0240287ea581a3

# timestamp: 2016-12-15 10:34:30 +0000

# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5

# base_revision_id: squid3@treenet.co.nz-20161215093634-\

#   ykbs6tv8pdusz7cj

# 

# Begin patch

=== modified file 'src/LogTags.h'

--- src/LogTags.h 2016-10-09 19:47:26 +0000

+++ src/LogTags.h 2016-12-15 10:33:57 +0000

@@ -28,6 +28,7 @@

     LOG_TCP_REFRESH_MODIFIED,   // refresh from origin replaced existing entry

     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,

@@ -54,6 +55,7 @@

     return

         (code == LOG_TCP_HIT) ||

         (code == LOG_TCP_IMS_HIT) ||

+        (code == LOG_TCP_INM_HIT) ||

         (code == LOG_TCP_REFRESH_FAIL_OLD) ||

         (code == LOG_TCP_REFRESH_UNMODIFIED) ||

         (code == LOG_TCP_NEGATIVE_HIT) ||

=== modified file 'src/client_side.cc'

--- src/client_side.cc  2016-12-09 01:58:33 +0000

+++ src/client_side.cc  2016-12-15 10:33:57 +0000

@@ -429,6 +429,7 @@

         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;

=== modified file 'src/client_side_reply.cc'

--- src/client_side_reply.cc  2016-12-15 09:36:34 +0000

+++ src/client_side_reply.cc  2016-12-15 10:33:57 +0000

@@ -778,40 +778,27 @@

         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 = false;

-            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

@@ -1974,7 +1961,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();