diff --git a/changes-entries/cache-socache-revalidate-body.txt b/changes-entries/cache-socache-revalidate-body.txt new file mode 100644 index 00000000000..53eb1564558 --- /dev/null +++ b/changes-entries/cache-socache-revalidate-body.txt @@ -0,0 +1,3 @@ + *) mod_cache_socache: Keep the cached body when the headers of an entry + are refreshed after a 304 response from the origin, instead of serving + the entry without a body afterwards. PR 62493. diff --git a/docs/log-message-tags/next-number b/docs/log-message-tags/next-number index 4319db81665..16a7e7453cc 100644 --- a/docs/log-message-tags/next-number +++ b/docs/log-message-tags/next-number @@ -1 +1 @@ -10636 +10644 diff --git a/modules/cache/mod_cache_socache.c b/modules/cache/mod_cache_socache.c index 78bd3e7bb4c..0f3813c2cd1 100644 --- a/modules/cache/mod_cache_socache.c +++ b/modules/cache/mod_cache_socache.c @@ -936,6 +936,38 @@ static apr_status_t store_headers(cache_handle_t *h, request_rec *r, sobj->body_offset = slider; + /* The headers and the body share one entry, so when only the headers of + * a revalidated entry are updated the body recalled by open_entity() + * has to be carried over to the new buffer. + */ + if (sobj->body && !sobj->newbody) { + apr_off_t len; + apr_size_t flen; + + rv = apr_brigade_length(sobj->body, 1, &len); + if (rv != APR_SUCCESS || len < 0 + || (apr_size_t)len >= sobj->buffer_len - sobj->body_offset) { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r, APLOGNO(10642) + "cached body didn't fit in buffer: %s", sobj->name); + apr_pool_destroy(sobj->pool); + sobj->pool = NULL; + return APR_EGENERAL; + } + + flen = (apr_size_t)len; + rv = apr_brigade_flatten(sobj->body, + (char *)sobj->buffer + sobj->body_offset, + &flen); + if (rv != APR_SUCCESS || flen != (apr_size_t)len) { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r, APLOGNO(10643) + "cached body unreadable: %s", sobj->name); + apr_pool_destroy(sobj->pool); + sobj->pool = NULL; + return APR_EGENERAL; + } + sobj->body_length = len; + } + return APR_SUCCESS; } diff --git a/test/modules/proxy/test_08_cache_socache.py b/test/modules/proxy/test_08_cache_socache.py new file mode 100644 index 00000000000..b087c08e33c --- /dev/null +++ b/test/modules/proxy/test_08_cache_socache.py @@ -0,0 +1,152 @@ +import os +import time +import uuid + +import pytest + +from pyhttpd.conf import HttpdConf +from .env import TCPFaker + +# Size limit of one cache entry, headers and body together +MAX_ENTRY = 8192 + + +# Backend answering // with a body of SIZES[index] bytes made of +# the X-Var request header, or with 304 when the request is conditional on +# the entity tag. The 304 carries "pad" bytes of additional header fields. +class _RevalidateFaker(TCPFaker): + + def __init__(self, host, port, sizes): + super().__init__(host, port) + self.sizes = sizes + self.hits = {} + self.conditional = {} + self.pad = 0 + + def _make_response(self, data): + head = data.split(b"\r\n\r\n")[0].decode("latin-1").split("\r\n") + path = head[0].split(" ")[1] + fields = {} + for line in head[1:]: + name, _, value = line.partition(":") + fields[name.lower()] = value.strip() + variant = fields.get("x-var", "") + key = (path, variant) + validators = ["Last-Modified: Thu, 01 Jan 2026 00:00:00 GMT", + f'ETag: "t-{variant}"', + "Cache-Control: max-age=1", + "Vary: X-Var", + "Connection: close"] + if fields.get("if-none-match") == f'"t-{variant}"': + self.conditional[key] = self.conditional.get(key, 0) + 1 + pad = [f"X-Pad{i}: {'p' * 3000}" for i in range(self.pad // 3000)] + return ("\r\n".join(["HTTP/1.1 304 Not Modified"] + validators + pad) + + "\r\n\r\n").encode() + self.hits[key] = self.hits.get(key, 0) + 1 + body = (variant or "x") * self.sizes[int(path.rsplit("/", 1)[1])] + lines = ["HTTP/1.1 200 OK", + "Content-Type: application/octet-stream", + f"Content-Length: {len(body)}"] + validators + return ("\r\n".join(lines) + "\r\n\r\n" + body).encode() + + +SIZES = [284, 0, 700, 1100, 3000] + + +class TestCacheSocache: + + @pytest.fixture(autouse=True, scope='class') + def _class_scope(self, env): + faker = _RevalidateFaker("127.0.0.1", env.http_port2, SIZES) + faker.start() + conf = HttpdConf(env) + conf.start_vhost(domains=[f"test1.{env.http_tld}"], port=env.http_port, + doc_root="htdocs", with_ssl=False) + conf.add([ + f'CacheSocache shmcb:{os.path.join(env.gen_dir, "socache")}(512000)', + f'CacheSocacheMaxSize {MAX_ENTRY}', + 'CacheEnable socache "/sc/"', + 'CacheHeader on', + f"ProxyPass /sc/ http://127.0.0.1:{env.http_port2}/", + ]) + conf.end_vhost() + conf.install() + assert env.apache_restart() == 0 + TestCacheSocache.faker = faker + TestCacheSocache.run = uuid.uuid4().hex[:12] + yield + faker.stop() + + def get(self, env, index, variant=None): + url = env.mkurl("http", "test1", f"/sc/{self.run}/{index}") + options = ["-H", f"X-Var: {variant}"] if variant else [] + r = env.curl_get(url, options=options) + assert r.response["status"] == 200, f"{r}" + return r.response + + def body(self, index, variant=None): + return ((variant or "x") * SIZES[index]).encode() + + def conditional(self, index, variant=None): + return self.faker.conditional.get((f"/{self.run}/{index}", variant or ""), 0) + + def backend_hits(self, index, variant=None): + return self.faker.hits.get((f"/{self.run}/{index}", variant or ""), 0) + + # the body is kept in the cache when the headers are refreshed by a 304 + @pytest.mark.parametrize("index", [0, 1]) + def test_proxy_08_001(self, env, index): + assert self.get(env, index)["body"] == self.body(index) + time.sleep(2) + assert self.get(env, index)["body"] == self.body(index) + assert self.conditional(index) == 1, "entry was not revalidated" + r = self.get(env, index) + assert r["header"]["x-cache"].startswith("HIT") + assert r["body"] == self.body(index) + + # every variant keeps its own body when it is revalidated + def test_proxy_08_002(self, env): + index = 2 + for variant in ["a", "b"]: + assert self.get(env, index, variant)["body"] == self.body(index, variant) + time.sleep(2) + for variant in ["a", "b"]: + assert self.get(env, index, variant)["body"] == self.body(index, variant) + assert self.conditional(index, variant) == 1 + for variant in ["a", "b"]: + r = self.get(env, index, variant) + assert r["header"]["x-cache"].startswith("HIT") + assert r["body"] == self.body(index, variant) + + # the body survives any number of revalidations + def test_proxy_08_003(self, env): + index = 3 + assert self.get(env, index)["body"] == self.body(index) + for n in range(1, 4): + time.sleep(2) + assert self.get(env, index)["body"] == self.body(index) + assert self.conditional(index) == n + r = self.get(env, index) + assert r["header"]["x-cache"].startswith("HIT") + assert r["body"] == self.body(index) + assert self.backend_hits(index) == 1 + + # a refreshed entry that no longer fits is dropped, not stored truncated + def test_proxy_08_004(self, env): + index = 4 + assert self.get(env, index)["body"] == self.body(index) + time.sleep(2) + self.faker.pad = 2 * 3000 + try: + r = self.get(env, index) + finally: + self.faker.pad = 0 + assert self.conditional(index) == 1 + assert r["body"] == self.body(index) + r = self.get(env, index) + assert not r["header"]["x-cache"].startswith("HIT") + assert r["body"] == self.body(index) + assert self.backend_hits(index) == 2 + r = self.get(env, index) + assert r["header"]["x-cache"].startswith("HIT") + assert r["body"] == self.body(index)