mod_cache_disk: preserve long cache metadata lines

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]

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,14 +710,46 @@ static int remove_url(cache_handle_t *h, request_rec *r)
     return OK;
 }
 
+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;
+    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(buf, buf_size, file);
+        if (rv != APR_SUCCESS) {
+            ap_varbuf_free(&vb);
+            return rv;
+        }
+        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;
+}
+
 static apr_status_t read_array(request_rec *r, apr_array_header_t* arr,
                                apr_file_t *file)
 {
-    char w[MAX_STRING_LEN];
+    char buf[MAX_STRING_LEN];
+    char *w;
     apr_size_t p;
     apr_status_t rv;
 
     while (1) {
+        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)
@@ -725,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';
@@ -776,14 +818,16 @@ 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 buf[MAX_STRING_LEN];
+    char *w;
     char *l;
     apr_size_t p;
     apr_status_t rv;
 
     while (1) {
 
         /* ### What about APR_EOF? */
+        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)
@@ -794,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.

test/pytest_suite/t/conf/cache.conf.in

@@ -20,6 +20,10 @@
             CacheDirLength 1
 
    </IfModule>
+   <IfModule mod_headers.c>
+            Header echo ^TS
+            Header set Cache-Control "public, max-age=600"
+   </IfModule>
    DocumentRoot @SERVERROOT@/htdocs/modules/cache
    </VirtualHost>
 </IfModule>

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"