From 61ceb2cd5ab0774f6dd5cddf935fe06281cebda5 Mon Sep 17 00:00:00 2001 From: Oz Kaiser <1888609+tatikoma@users.noreply.github.com> Date: Wed, 28 Oct 2026 14:42:48 +0300 Subject: [PATCH 1/2] mod_cache_disk: preserve long metadata lines Cache metadata is serialized as complete logical lines, but recall parsed each fixed-size read chunk as a separate line. Accumulate chunks through the terminating newline before parsing both header tables and Vary arrays. Add a regression covering a cached response header that spans the previous read boundary. --- changes-entries/cache-disk-long-lines.txt | 2 + modules/cache/mod_cache_disk.c | 35 ++++++++++-- test/pytest_suite/t/conf/cache.conf.in | 4 ++ .../tests/t/modules/test_cache.py | 54 +++++++++++++++++++ 4 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 changes-entries/cache-disk-long-lines.txt diff --git a/changes-entries/cache-disk-long-lines.txt b/changes-entries/cache-disk-long-lines.txt new file mode 100644 index 00000000000..9df432b9242 --- /dev/null +++ b/changes-entries/cache-disk-long-lines.txt @@ -0,0 +1,2 @@ + *) mod_cache_disk: Preserve cached response headers longer than + MAX_STRING_LEN when recalling entries. [Oz Kaiser] diff --git a/modules/cache/mod_cache_disk.c b/modules/cache/mod_cache_disk.c index 17e22a32f18..594e8e616d5 100644 --- a/modules/cache/mod_cache_disk.c +++ b/modules/cache/mod_cache_disk.c @@ -26,6 +26,7 @@ #include "util_filter.h" #include "util_script.h" #include "util_charset.h" +#include "util_varbuf.h" /* * mod_cache_disk: Disk Based HTTP 1.1 Cache. @@ -709,15 +710,41 @@ static int remove_url(cache_handle_t *h, request_rec *r) return OK; } +static apr_status_t read_line(request_rec *r, apr_file_t *file, char **line) +{ + struct ap_varbuf vb; + char chunk[MAX_STRING_LEN]; + apr_size_t len; + apr_status_t rv; + + ap_varbuf_init(r->pool, &vb, MAX_STRING_LEN); + vb.strlen = 0; + vb.buf[0] = '\0'; + + do { + rv = apr_file_gets(chunk, MAX_STRING_LEN - 1, file); + if (rv != APR_SUCCESS) { + ap_varbuf_free(&vb); + return rv; + } + len = strlen(chunk); + ap_varbuf_strmemcat(&vb, chunk, len); + } while (len == 0 || chunk[len - 1] != '\n'); + + *line = ap_varbuf_pdup(r->pool, &vb, NULL, 0, NULL, 0, NULL); + ap_varbuf_free(&vb); + return APR_SUCCESS; +} + static apr_status_t read_array(request_rec *r, apr_array_header_t* arr, apr_file_t *file) { - char w[MAX_STRING_LEN]; + char *w; apr_size_t p; apr_status_t rv; while (1) { - rv = apr_file_gets(w, MAX_STRING_LEN - 1, file); + rv = read_line(r, file, &w); if (rv != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00716) "Premature end of vary array."); @@ -776,7 +803,7 @@ static apr_status_t store_array(apr_file_t *fd, apr_array_header_t* arr) static apr_status_t read_table(cache_handle_t *handle, request_rec *r, apr_table_t *table, apr_file_t *file) { - char w[MAX_STRING_LEN]; + char *w; char *l; apr_size_t p; apr_status_t rv; @@ -784,7 +811,7 @@ static apr_status_t read_table(cache_handle_t *handle, request_rec *r, while (1) { /* ### What about APR_EOF? */ - rv = apr_file_gets(w, MAX_STRING_LEN - 1, file); + rv = read_line(r, file, &w); if (rv != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(00717) "Premature end of cache headers."); diff --git a/test/pytest_suite/t/conf/cache.conf.in b/test/pytest_suite/t/conf/cache.conf.in index 91284d13644..71e3d09b393 100644 --- a/test/pytest_suite/t/conf/cache.conf.in +++ b/test/pytest_suite/t/conf/cache.conf.in @@ -20,6 +20,10 @@ CacheDirLength 1 + + Header echo ^TS + Header set Cache-Control "public, max-age=600" + DocumentRoot @SERVERROOT@/htdocs/modules/cache diff --git a/test/pytest_suite/tests/t/modules/test_cache.py b/test/pytest_suite/tests/t/modules/test_cache.py index 76f10b107a7..6a68586c554 100644 --- a/test/pytest_suite/tests/t/modules/test_cache.py +++ b/test/pytest_suite/tests/t/modules/test_cache.py @@ -9,10 +9,32 @@ """ import os +import shutil from apache_pytest import need_min_apache_version, need_module, t_cmp +def _raw_get(http, path, headers=()): + request = [ + f"GET {path} HTTP/1.1", + f"Host: {http.hostport('mod_cache')}", + "Connection: close", + ] + request.extend(f"{name}: {value}" for name, value in headers) + + with http.vhost_socket("mod_cache") as sock: + sock.print("\r\n".join(request) + "\r\n\r\n") + response = sock.read() + + head = response.split("\r\n\r\n", 1)[0].split("\r\n") + fields = [] + for line in head[1:]: + name, separator, value = line.partition(":") + if separator: + fields.append((name.lower(), value.lstrip())) + return head[0], fields + + @need_module("cache", "cache_disk") @need_min_apache_version("2.1.9") def test_cache(http): @@ -29,3 +51,35 @@ def test_cache(http): r = http.GET("/cache/") assert t_cmp(r.status_code, 200), "cached call to index.html" + + +@need_module("cache", "cache_disk", "headers") +@need_min_apache_version("2.1.9") +def test_cache_long_response_header(http): + """A cache metadata line longer than MAX_STRING_LEN stays one header.""" + http.module("mod_cache") + + cacheroot = os.path.join(http.vars("statedir"), "cacheroot") + shutil.rmtree(cacheroot, ignore_errors=True) + os.makedirs(cacheroot, exist_ok=True) + + # Align a header-shaped tail with the old fixed-size read boundary. + tail = "X-Cache-Tail: preserved" + name = "TS" + "A" * (len(tail) - 4) + first = "a" * 4000 + second = "b" * (8190 - len(first) - 2 - len(tail)) + tail + expected = first + ", " + second + + status, prime = _raw_get( + http, + "/cache/index.html?long-response-header", + ((name, first), (name, second)), + ) + assert status.startswith("HTTP/1.1 200"), "cache prime returned 200" + assert (name.lower(), expected) in prime, "prime response kept the long header" + + status, recalled = _raw_get(http, "/cache/index.html?long-response-header") + assert status.startswith("HTTP/1.1 200"), "cached response returned 200" + assert (name.lower(), expected) in recalled, "cache recall kept the long header" + assert not any(name == "x-cache-tail" for name, _ in recalled), \ + "cache recall did not split a continuation chunk into another header" From ead061cafbc895671276d461af283ad9199cb135 Mon Sep 17 00:00:00 2001 From: Oz Kaiser <1888609+tatikoma@users.noreply.github.com> Date: Wed, 4 Nov 2026 13:03:21 +0300 Subject: [PATCH 2/2] mod_cache_disk: avoid allocation for short metadata lines --- modules/cache/mod_cache_disk.c | 42 +++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/modules/cache/mod_cache_disk.c b/modules/cache/mod_cache_disk.c index 594e8e616d5..50ef7fa3067 100644 --- a/modules/cache/mod_cache_disk.c +++ b/modules/cache/mod_cache_disk.c @@ -710,27 +710,31 @@ static int remove_url(cache_handle_t *h, request_rec *r) return OK; } -static apr_status_t read_line(request_rec *r, apr_file_t *file, char **line) +static apr_status_t read_line_tail(request_rec *r, apr_file_t *file, + char *buf, apr_size_t buf_size, + apr_size_t first_len, char **line, + apr_size_t *line_len) { struct ap_varbuf vb; - char chunk[MAX_STRING_LEN]; apr_size_t len; apr_status_t rv; ap_varbuf_init(r->pool, &vb, MAX_STRING_LEN); vb.strlen = 0; vb.buf[0] = '\0'; + ap_varbuf_strmemcat(&vb, buf, first_len); do { - rv = apr_file_gets(chunk, MAX_STRING_LEN - 1, file); + rv = apr_file_gets(buf, buf_size, file); if (rv != APR_SUCCESS) { ap_varbuf_free(&vb); return rv; } - len = strlen(chunk); - ap_varbuf_strmemcat(&vb, chunk, len); - } while (len == 0 || chunk[len - 1] != '\n'); + len = strlen(buf); + ap_varbuf_strmemcat(&vb, buf, len); + } while (len == 0 || buf[len - 1] != '\n'); + *line_len = vb.strlen; *line = ap_varbuf_pdup(r->pool, &vb, NULL, 0, NULL, 0, NULL); ap_varbuf_free(&vb); return APR_SUCCESS; @@ -739,12 +743,14 @@ static apr_status_t read_line(request_rec *r, apr_file_t *file, char **line) static apr_status_t read_array(request_rec *r, apr_array_header_t* arr, apr_file_t *file) { + char buf[MAX_STRING_LEN]; char *w; apr_size_t p; apr_status_t rv; while (1) { - rv = read_line(r, file, &w); + w = buf; + rv = apr_file_gets(w, MAX_STRING_LEN - 1, file); if (rv != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00716) "Premature end of vary array."); @@ -752,6 +758,15 @@ static apr_status_t read_array(request_rec *r, apr_array_header_t* arr, } p = strlen(w); + if (p == 0 || w[p - 1] != '\n') { + rv = read_line_tail(r, file, buf, MAX_STRING_LEN - 1, + p, &w, &p); + if (rv != APR_SUCCESS) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00716) + "Premature end of vary array."); + return rv; + } + } if (p > 0 && w[p - 1] == '\n') { if (p > 1 && w[p - 2] == CR) { w[p - 2] = '\0'; @@ -803,6 +818,7 @@ static apr_status_t store_array(apr_file_t *fd, apr_array_header_t* arr) static apr_status_t read_table(cache_handle_t *handle, request_rec *r, apr_table_t *table, apr_file_t *file) { + char buf[MAX_STRING_LEN]; char *w; char *l; apr_size_t p; @@ -811,7 +827,8 @@ static apr_status_t read_table(cache_handle_t *handle, request_rec *r, while (1) { /* ### What about APR_EOF? */ - rv = read_line(r, file, &w); + w = buf; + rv = apr_file_gets(w, MAX_STRING_LEN - 1, file); if (rv != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(00717) "Premature end of cache headers."); @@ -821,6 +838,15 @@ static apr_status_t read_table(cache_handle_t *handle, request_rec *r, /* Delete terminal (CR?)LF */ p = strlen(w); + if (p == 0 || w[p - 1] != '\n') { + rv = read_line_tail(r, file, buf, MAX_STRING_LEN - 1, + p, &w, &p); + if (rv != APR_SUCCESS) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(00717) + "Premature end of cache headers."); + return rv; + } + } /* Indeed, the host's '\n': '\012' for UNIX; '\015' for MacOS; '\025' for OS/390 -- whatever the script generates.