MEDIUM: cache: Remove cache entry in case of POST on the same resource

In case of successful unsafe method on a stored resource, the cached entry
must be invalidated (see RFC7234#4.4).
A "non-error response" is one with a 2xx (Successful) or 3xx (Redirection)
status code.
This implies that the primary hash must now be calculated on requests
that have an unsafe method (POST or PUT for instance) so that we can
disable the corresponding entries when we process the response.

diff --git a/reg-tests/cache/post_on_entry.vtc b/reg-tests/cache/post_on_entry.vtc
new file mode 100644 (file)
index 0000000..2b807f6
--- /dev/null
+++ b/reg-tests/cache/post_on_entry.vtc
@@ -0,0 +1,66 @@
+varnishtest "A successful unsafe method (POST for instance) on a cached entry must disable it."
+
+#REQUIRE_VERSION=2.3
+
+feature ignore_unknown_macro
+
+server s1 {
+       rxreq
+       expect req.url == "/cached"
+       txresp -hdr "Cache-Control: max-age=5" \
+           -bodylen 150
+
+       rxreq
+       expect req.url == "/cached"
+       expect req.method == "POST"
+       txresp
+
+       rxreq
+       expect req.url == "/cached"
+       txresp -hdr "Cache-Control: max-age=5" \
+           -bodylen 100
+
+} -start
+
+haproxy h1 -conf {
+       defaults
+               mode http
+               ${no-htx} option http-use-htx
+               timeout connect 1s
+               timeout client  1s
+               timeout server  1s
+
+       frontend fe
+               bind "fd@${fe}"
+               default_backend test
+
+       backend test
+               http-request cache-use my_cache
+               server www ${s1_addr}:${s1_port}
+               http-response cache-store my_cache
+               http-response set-header X-Cache-Hit %[res.cache_hit]
+
+       cache my_cache
+               total-max-size 3
+               max-age 20
+               max-object-size 3072
+} -start
+
+
+client c1 -connect ${h1_fe_sock} {
+       txreq -url "/cached"
+       rxresp
+       expect resp.status == 200
+       expect resp.bodylen == 150
+
+       txreq -method "POST" -url "/cached" -bodylen 100
+       rxresp
+       expect resp.status == 200
+       expect resp.http.X-Cache-Hit == 0
+
+       txreq -url "/cached"
+       rxresp
+       expect resp.status == 200
+       expect resp.bodylen == 100
+       expect resp.http.X-Cache-Hit == 0
+} -run

diff --git a/src/cache.c b/src/cache.c
index 59a590cb7ceab432a1b09b007d81930452b3e1be..e0560e57e188e90ca78ad623da4fe730cb1bdc13 100644 (file)
--- a/src/cache.c
+++ b/src/cache.c
@@ -753,8 +753,34 @@ enum act_return http_action_store_cache(struct act_rule *rule, struct proxy *px,
                goto out;
 
        /* cache only GET method */
-       if (txn->meth != HTTP_METH_GET)
+       if (txn->meth != HTTP_METH_GET) {
+               /* In case of successful unsafe method on a stored resource, the
+                * cached entry must be invalidated (see RFC7234#4.4).
+                * A "non-error response" is one with a 2xx (Successful) or 3xx
+                * (Redirection) status code. */
+               if (txn->status >= 200 && txn->status < 400) {
+                       switch (txn->meth) {
+                       case HTTP_METH_OPTIONS:
+                       case HTTP_METH_GET:
+                       case HTTP_METH_HEAD:
+                       case HTTP_METH_TRACE:
+                               break;
+
+                       default: /* Any unsafe method */
+                               /* Discard any corresponding entry in case of sucessful
+                                * unsafe request (such as PUT, POST or DELETE). */
+                               shctx_lock(shctx);
+
+                               old = entry_exist(cconf->c.cache, txn->cache_hash);
+                               if (old) {
+                                       eb32_delete(&old->eb);
+                                       old->eb.key = 0;
+                               }
+                               shctx_unlock(shctx);
+                       }
+               }
                goto out;
+       }
 
        /* cache key was not computed */
        if (!key)
@@ -1329,15 +1355,6 @@ int sha1_hosturi(struct stream *s)
        trash = get_trash_chunk();
        ctx.blk = NULL;
 
-       switch (txn->meth) {
-       case HTTP_METH_HEAD:
-       case HTTP_METH_GET:
-               chunk_memcat(trash, "GET", 3);
-               break;
-       default:
-               return 0;
-       }
-
        sl = http_get_stline(htx);
        uri = htx_sl_req_uri(sl); // whole uri
        if (!uri.len)
@@ -1475,10 +1492,14 @@ enum act_return http_action_req_cache_use(struct act_rule *rule, struct proxy *p
 
        http_check_request_for_cacheability(s, &s->req);
 
-       if ((s->txn->flags & (TX_CACHE_IGNORE|TX_CACHEABLE)) == TX_CACHE_IGNORE)
+       /* The request's hash has to be calculated for all requests, even POSTs
+        * or PUTs for instance because RFC7234 specifies that a sucessful
+        * "unsafe" method on a stored resource must invalidate it
+        * (see RFC7234#4.4). */
+       if (!sha1_hosturi(s))
                return ACT_RET_CONT;
 
-       if (!sha1_hosturi(s))
+       if ((s->txn->flags & (TX_CACHE_IGNORE|TX_CACHEABLE)) == TX_CACHE_IGNORE)
                return ACT_RET_CONT;
 
        if (s->txn->flags & TX_CACHE_IGNORE)