From bd765c1526723712361743383447ad6ba4aedf9b Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Mon, 19 Oct 2026 16:43:51 +0200 Subject: [PATCH] mod_cache_socache: Keep the body when a revalidated entry is refreshed. A 304 response updates the headers of the stored response, but store_headers() rebuilt the entry without the body recalled by open_entity(), so later requests were answered from the cache with the headers and an empty body. Copy the body into the new entry, or drop the entry when it no longer fits. PR 62493. --- .../cache-socache-revalidate-body.txt | 3 + docs/log-message-tags/next-number | 2 +- modules/cache/mod_cache_socache.c | 32 ++++ test/modules/proxy/test_08_cache_socache.py | 152 ++++++++++++++++++ 4 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 changes-entries/cache-socache-revalidate-body.txt create mode 100644 test/modules/proxy/test_08_cache_socache.py 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)