Author: Dmitry Kurochkin <dmitry.kurochkin@measurement-factory.com>

Bug 3099: Incorrect 412 on If-None-Match and If-Modified-Since

diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index afcaafaa4fa86aec6d57e0ab4064bbfc948ddc62..5e1f7ca3d4cf4e590a3627dbe48f17b3470e54df 100644 (file)
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -683,7 +683,7 @@ clientReplyContext::processOnlyIfCachedMiss()
 void
 clientReplyContext::processConditional(StoreIOBuffer &result)
 {
-    StoreEntry *e = http->storeEntry();
+    StoreEntry *const e = http->storeEntry();
 
     if (e->getReply()->sline.status != HTTP_OK) {
         debugs(88, 4, "clientReplyContext::processConditional: Reply code " <<
@@ -716,12 +716,12 @@ clientReplyContext::processConditional(StoreIOBuffer &result)
 
         if (!r.flags.ims) {
             // RFC 2616: if If-None-Match matched and there is no IMS,
-            // reply with 412 Precondition Failed
-            sendPreconditionFailedError();
+            // reply with 304 Not Modified or 412 Precondition Failed
+            sendNotModifiedOrPreconditionFailedError();
             return;
         }
 
-        // otherwise check IMS below to decide if we reply with 412
+        // otherwise check IMS below to decide if we reply with 304 or 412
         matchedIfNoneMatch = true;
     }
 
@@ -734,29 +734,14 @@ clientReplyContext::processConditional(StoreIOBuffer &result)
         }
 
         if (matchedIfNoneMatch) {
-            // If-None-Match matched, reply with 412 Precondition Failed
-            sendPreconditionFailedError();
+            // If-None-Match matched, reply with 304 Not Modified or
+            // 412 Precondition Failed
+            sendNotModifiedOrPreconditionFailedError();
             return;
         }
 
         // otherwise reply with 304 Not Modified
-        const time_t timestamp = e->timestamp;
-        HttpReply *const temprep = e->getReply()->make304();
-        http->logType = LOG_TCP_IMS_HIT;
-        removeClientStoreReference(&sc, http);
-        createStoreEntry(http->request->method, request_flags());
-        e = http->storeEntry();
-        // Copy timestamp from the original entry so the 304
-        // reply has a meaningful Age: header.
-        e->timestamp = timestamp;
-        e->replaceHttpReply(temprep);
-        e->complete();
-        /*
-         * TODO: why put this in the store and then serialise it and
-         * then parse it again. Simply mark the request complete in
-         * our context and write the reply struct to the client side.
-         */
-        triggerInitialStoreRead();
+        sendNotModified();
     }
 }
 
@@ -1865,6 +1850,42 @@ clientReplyContext::sendPreconditionFailedError()
     startError(err);
 }
 
+/// send 304 (Not Modified) to client
+void
+clientReplyContext::sendNotModified()
+{
+    StoreEntry *e = http->storeEntry();
+    const time_t timestamp = e->timestamp;
+    HttpReply *const temprep = e->getReply()->make304();
+    http->logType = LOG_TCP_IMS_HIT;
+    removeClientStoreReference(&sc, http);
+    createStoreEntry(http->request->method, request_flags());
+    e = http->storeEntry();
+    // Copy timestamp from the original entry so the 304
+    // reply has a meaningful Age: header.
+    e->timestamp = timestamp;
+    e->replaceHttpReply(temprep);
+    e->complete();
+    /*
+     * TODO: why put this in the store and then serialise it and
+     * then parse it again. Simply mark the request complete in
+     * our context and write the reply struct to the client side.
+     */
+    triggerInitialStoreRead();
+}
+
+/// send 304 (Not Modified) or 412 (Precondition Failed) to client
+/// depending on request method
+void
+clientReplyContext::sendNotModifiedOrPreconditionFailedError()
+{
+    if (http->request->method == METHOD_GET ||
+        http->request->method == METHOD_HEAD)
+        sendNotModified();
+    else
+        sendPreconditionFailedError();
+}
+
 void
 clientReplyContext::processReplyAccess ()
 {

diff --git a/src/client_side_reply.h b/src/client_side_reply.h
index c1787f736dbd651e0338c61fd26a18668577bbbc..77c023fb3f90f2694a36635085c7b94a95d851db 100644 (file)
--- a/src/client_side_reply.h
+++ b/src/client_side_reply.h
@@ -138,6 +138,8 @@ private:
 
     void sendBodyTooLargeError();
     void sendPreconditionFailedError();
+    void sendNotModified();
+    void sendNotModifiedOrPreconditionFailedError();
 
     StoreEntry *old_entry;
     store_client *old_sc;      /* ... for entry to be validated */