This commit adds support for conditional requests using ETags to the cache. Specifically the cache now is able to return a 304 Not Modified to requests if the resource is unchanged according to the validation logic of RFC 7232#3.2.
The implementation is not 100% according to spec: Only the first if-none-match value received in the request is being used for the validation logic. This is to avoid having to preserve all the if-none-match values from the request until response processing. This resolves GitHub issue #821. --- include/haproxy/applet-t.h | 1 + reg-tests/cache/if-none-match.vtc | 72 +++++++++++++++++++++++++++++++ src/cache.c | 40 +++++++++++++++-- 3 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 reg-tests/cache/if-none-match.vtc diff --git a/include/haproxy/applet-t.h b/include/haproxy/applet-t.h index 60f30c56f..7cccec977 100644 --- a/include/haproxy/applet-t.h +++ b/include/haproxy/applet-t.h @@ -113,6 +113,7 @@ struct appctx { unsigned int offset; /* start offset of remaining data relative to beginning of the next block */ unsigned int rem_data; /* Remaining bytes for the last data block (HTX only, 0 means process next block) */ struct shared_block *next; /* The next block of data to be sent for this cache entry. */ + struct ist if_none_match; /* The if-none-match request header. */ } cache; /* all entries below are used by various CLI commands, please * keep the grouped together and avoid adding new ones. diff --git a/reg-tests/cache/if-none-match.vtc b/reg-tests/cache/if-none-match.vtc new file mode 100644 index 000000000..79507ae2c --- /dev/null +++ b/reg-tests/cache/if-none-match.vtc @@ -0,0 +1,72 @@ +varnishtest "If-None-Match support" + +#REQUIRE_VERSION=2.3 + +feature ignore_unknown_macro + +server s1 { + rxreq + txresp -nolen -hdr "Transfer-Encoding: chunked" \ + -hdr "ETag: \"etag\"" + chunkedlen 1 + chunkedlen 1 + chunkedlen 2 + chunkedlen 3 + chunkedlen 5 + chunkedlen 8 + chunkedlen 13 + chunkedlen 21 + chunkedlen 34 + chunkedlen 55 + chunkedlen 89 + chunkedlen 144 + chunkedlen 233 + chunkedlen 0 +} -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 + + cache my_cache + total-max-size 3 + max-age 20 + max-object-size 3072 +} -start + + +client c1 -connect ${h1_fe_sock} { + txreq + rxresp + expect resp.status == 200 + expect resp.bodylen == 609 + txreq + rxresp + expect resp.status == 200 + expect resp.bodylen == 609 + txreq \ + -hdr "if-none-match: *" + rxresp + expect resp.status == 304 + txreq \ + -hdr "if-none-match: \"etag\"" + rxresp + expect resp.status == 304 + txreq \ + -hdr "if-none-match: W/\"etag\"" + rxresp + expect resp.status == 304 +} -run diff --git a/src/cache.c b/src/cache.c index b42262948..3c9392f8b 100644 --- a/src/cache.c +++ b/src/cache.c @@ -4,6 +4,8 @@ * Copyright 2017 HAProxy Technologies * William Lallemand <wlallem...@haproxy.com> * + * Copyright 2020 Tim Duesterhus <t...@bastelstu.be> + * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License * as published by the Free Software Foundation; either version @@ -21,6 +23,7 @@ #include <haproxy/errors.h> #include <haproxy/filters.h> #include <haproxy/hash.h> +#include <haproxy/http.h> #include <haproxy/http_ana.h> #include <haproxy/http_htx.h> #include <haproxy/http_rules.h> @@ -694,6 +697,8 @@ static void http_cache_applet_release(struct appctx *appctx) struct cache *cache = cconf->c.cache; struct shared_block *first = block_ptr(cache_ptr); + istfree(&appctx->ctx.cache.if_none_match); + shctx_lock(shctx_ptr(cache)); shctx_row_dec_hot(shctx_ptr(cache), first); shctx_unlock(shctx_ptr(cache)); @@ -899,15 +904,35 @@ static void http_cache_io_handler(struct appctx *appctx) } if (appctx->st0 == HTX_CACHE_HEADER) { + int etag_match = 0; + /* Headers must be dump at once. Otherwise it is an error */ len = first->len - sizeof(*cache_ptr) - appctx->ctx.cache.sent; ret = htx_cache_dump_msg(appctx, res_htx, len, HTX_BLK_EOH); - if (!ret || (htx_get_tail_type(res_htx) != HTX_BLK_EOH) || - !htx_cache_add_age_hdr(appctx, res_htx)) + if (!ret || (htx_get_tail_type(res_htx) != HTX_BLK_EOH)) + goto error; + + if (isttest(appctx->ctx.cache.if_none_match)) { + struct http_hdr_ctx etag_ctx = { .blk = NULL }; + if (unlikely(isteq(appctx->ctx.cache.if_none_match, ist("*")))) { + etag_match = 1; + } + else if (http_find_header(res_htx, ist("etag"), &etag_ctx, 0) && + http_etag_compare(appctx->ctx.cache.if_none_match, etag_ctx.value, 1) == 1) { + etag_match = 1; + } + } + + if (!htx_cache_add_age_hdr(appctx, res_htx)) goto error; + if (etag_match) { + http_replace_res_status(res_htx, ist("304")); + http_replace_res_reason(res_htx, ist("Not Modified")); + } + /* Skip response body for HEAD requests */ - if (si_strm(si)->txn->meth == HTTP_METH_HEAD) + if (si_strm(si)->txn->meth == HTTP_METH_HEAD || etag_match) appctx->st0 = HTX_CACHE_EOM; else appctx->st0 = HTX_CACHE_DATA; @@ -1099,9 +1124,12 @@ enum act_return http_action_req_cache_use(struct act_rule *rule, struct proxy *p { struct http_txn *txn = s->txn; + struct htx *htx = htxbuf(&s->req.buf); struct cache_entry *res; struct cache_flt_conf *cconf = rule->arg.act.p[0]; struct cache *cache = cconf->c.cache; + struct ist if_none_match = IST_NULL; + struct http_hdr_ctx hdr_ctx; /* Ignore cache for HTTP/1.0 requests and for requests other than GET * and HEAD */ @@ -1120,6 +1148,11 @@ enum act_return http_action_req_cache_use(struct act_rule *rule, struct proxy *p if (s->txn->flags & TX_CACHE_IGNORE) return ACT_RET_CONT; + hdr_ctx.blk = NULL; + if (http_find_header(htx, ist("if-none-match"), &hdr_ctx, 0)) { + if_none_match = istdup(hdr_ctx.value); + } + if (px == strm_fe(s)) _HA_ATOMIC_ADD(&px->fe_counters.p.http.cache_lookups, 1); else @@ -1138,6 +1171,7 @@ enum act_return http_action_req_cache_use(struct act_rule *rule, struct proxy *p appctx->ctx.cache.entry = res; appctx->ctx.cache.next = NULL; appctx->ctx.cache.sent = 0; + appctx->ctx.cache.if_none_match = if_none_match; if (px == strm_fe(s)) _HA_ATOMIC_ADD(&px->fe_counters.p.http.cache_hits, 1); -- 2.28.0