Report a clipped .arc header block, and tighten the test around the real cap

Review follow-ups on the header-block bound. Truncating a cached header block
was silent, so a short record downstream had nothing pointing back at it; the
writer now names the URL on stderr, the way the readers already report a
corrupted entry. The test's own cap assertion allowed 8192 where the block
tops out at 8191, so a one-byte overflow would have passed it. runlen() had
been copied from the long-fields test and now lives in testlib.sh, and the
comment on the four reserved bytes says what depends on them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>

src/proxy/store.c

@@ -2372,11 +2372,28 @@ typedef struct PT_SaveCache__Arc_t {
   char md5[32 + 2];
 } PT_SaveCache__Arc_t;
 
+/* Append src to an .arc header block of capacity size, clipping what does not
+   fit: the values come from a cache entry, so shortening beats dropping it.
+   HTS_FALSE when it had to clip. */
+static hts_boolean arc_headers_cat(char *headers, size_t size,
+                                   const char *src) {
+  const size_t used = strlen(headers);
+  const size_t left = used < size - 1 ? size - used - 1 : 0;
+
+  if (left != 0) {
+    strlncatbuff(headers, src, size, left);
+  }
+  return strlen(src) <= left ? HTS_TRUE : HTS_FALSE;
+}
+
 static int PT_SaveCache__Arc_Fun(void *arg, const char *url, PT_Element element) {
   PT_SaveCache__Arc_t *st = (PT_SaveCache__Arc_t *) arg;
   FILE *const fp = st->fp;
   struct tm *tm = convert_time_rfc822(&st->buff, element->lastmodified);
   struct tm unknown_date;
+  /* the two strcatbuff calls closing the block rely on these 4 bytes */
+  const size_t room = sizeof(st->headers) - 4;
+  hts_boolean fit;
   int size_headers;
 
   /* a cached entry with no parseable Last-Modified must not take the writer
@@ -2388,34 +2405,42 @@ static int PT_SaveCache__Arc_Fun(void *arg, const char *url, PT_Element element)
     tm = &unknown_date;
   }
 
-  sprintf(st->headers,
-          "HTTP/1.0 %d %s"
-          "\r\n"
-          "X-Server: ProxyTrack " PROXYTRACK_VERSION "\r\n"
-          "Content-type: %s%s%s%s"
-          "\r\n"
-          "Last-modified: %s"
-          "\r\n"
-          "Content-length: %d"
-          "\r\n",
-          element->statuscode, element->msg,
-          /**/ hts_effective_mime(element->contenttype),
-          (element->charset[0] ? "; charset=\"" : ""),
-          (element->charset[0] ? element->charset : ""),
-          (element->charset[0] ? "\"" : ""), /**/ element->lastmodified,
-          (int) element->size);
+  fit = slprintfbuff(st->headers, room,
+                     "HTTP/1.0 %d %s"
+                     "\r\n"
+                     "X-Server: ProxyTrack " PROXYTRACK_VERSION "\r\n"
+                     "Content-type: %s%s%s%s"
+                     "\r\n"
+                     "Last-modified: %s"
+                     "\r\n"
+                     "Content-length: %d"
+                     "\r\n",
+                     element->statuscode, element->msg,
+                     /**/ hts_effective_mime(element->contenttype),
+                     (element->charset[0] ? "; charset=\"" : ""),
+                     (element->charset[0] ? element->charset : ""),
+                     (element->charset[0] ? "\"" : ""),
+                     /**/ element->lastmodified, (int) element->size);
   if (element->location != NULL && element->location[0] != '\0') {
-    sprintf(st->headers + strlen(st->headers), "Location: %s" "\r\n",
-            element->location);
-  }
-  if (element->headers != NULL) {
-    if (strlen(element->headers) <
-        sizeof(st->headers) - strlen(element->headers) - 1) {
-      strcat(st->headers, element->headers);
+    if (!arc_headers_cat(st->headers, room, "Location: ") ||
+        !arc_headers_cat(st->headers, room, element->location) ||
+        !arc_headers_cat(st->headers, room, "\r\n")) {
+      fit = HTS_FALSE;
     }
   }
-  strcat(st->headers, "\r\n");
+  if (element->headers != NULL &&
+      !arc_headers_cat(st->headers, room, element->headers)) {
+    fit = HTS_FALSE;
+  }
+  /* a clip landing mid-line must still end it, or the body reads as a header */
+  if (hts_lastchar(st->headers) != '\n') {
+    strcatbuff(st->headers, "\r\n");
+  }
+  strcatbuff(st->headers, "\r\n");
   size_headers = (int) strlen(st->headers);
+  if (!fit) {
+    fprintf(stderr, "Headers of %s clipped to %d bytes" LF, url, size_headers);
+  }
 
   /* doc == <nl><URL-record><nl><network_doc> */
 

tests/118_local-proxytrack-arcwrite.test

@@ -0,0 +1,123 @@
+#!/bin/bash
+# The .arc writer builds each entry's headers in a fixed 8192-byte block, and
+# its one bound ignored what the block already held: a long cached header
+# string wrapped the subtraction and smashed it.
+
+set -euo pipefail
+export LC_ALL=C
+
+testdir=$(cd "$(dirname "$0")" && pwd)
+# shellcheck source=tests/testlib.sh
+. "${testdir}/testlib.sh"
+
+dir=$(mktemp -d)
+cleanup() { rm -rf "$dir"; }
+trap 'set +e; cleanup' EXIT
+trap 'set +e; cleanup; exit 1' HUP INT TERM
+
+# sizeof(PT_SaveCache__Arc_t.headers)
+HDRMAX=8192
+BODY=BODYMARKER
+
+fail() {
+    echo "FAIL: $1" >&2
+    exit 1
+}
+
+pad() { printf "%${1}s" '' | tr ' ' "$2"; }
+
+P1000=$(pad 1000 P)
+
+# Header lines contributing exactly $1 bytes to the entry's stored headers: the
+# reader rebuilds each as "name: value\r\n", and its line buffer holds 2048.
+gen_headers() {
+    local want=$1 c
+    while [ "$want" -gt 0 ]; do
+        c=$want
+        if [ "$c" -gt 1008 ]; then
+            c=1008
+            test "$((want - c))" -ge 9 || c=$((c - 9))
+        fi
+        printf 'X-P0: %s\r\n' "${P1000:0:$((c - 8))}"
+        want=$((want - c))
+    done
+}
+
+# $1 reason phrase, $2 Location value, $3 bytes of passthrough headers.
+build_arc() {
+    {
+        printf 'HTTP/1.1 200 %s\r\n' "$1"
+        printf 'Content-Type: text/html\r\n'
+        printf 'Last-Modified: Wed, 01 Jan 2025 00:00:00 GMT\r\n'
+        printf 'Content-Length: %d\r\n' "${#BODY}"
+        test -z "$2" || printf 'Location: %s\r\n' "$2"
+        test "$3" -eq 0 || gen_headers "$3"
+        printf '\r\n'
+    } >"$dir/hdr"
+    printf '%s' "$BODY" >"$dir/body"
+    local alen
+    alen=$(($(wc -c <"$dir/hdr") + ${#BODY}))
+    {
+        printf 'filedesc://t.arc 0.0.0.0 20250101000000 text/plain 200 - - 0 t.arc 9\n'
+        printf '2 0 test\n'
+        printf '\n\n'
+        printf 'http://example.com/p.html 0.0.0.0 20250101000000 text/html 200 - - 0 t.arc %d\n' "$alen"
+        cat "$dir/hdr" "$dir/body"
+    } >"$dir/in.arc"
+}
+
+# Convert $dir/in.arc and check the emitted record, $1 naming the case. Sets
+# last_alen to the record's declared length.
+convert_check() {
+    local rec off alen total dataoff
+    proxytrack --convert "$dir/out.arc" "$dir/in.arc" >"$dir/log" 2>&1 ||
+        fail "$1: proxytrack died writing the .arc"
+    rec=$(grep -a -m1 '^http://example.com/p.html ' "$dir/out.arc") ||
+        fail "$1: entry dropped from the .arc"
+    off=$(grep -abo -m1 '^http://example.com/p.html ' "$dir/out.arc" | cut -d: -f1)
+    alen=${rec##* }
+    total=$(wc -c <"$dir/out.arc")
+    dataoff=$((off + ${#rec} + 1))
+    test "$((total - dataoff))" -eq "$alen" ||
+        fail "$1: record declares $alen bytes, wrote $((total - dataoff))"
+    test "$((alen - ${#BODY}))" -lt "$HDRMAX" ||
+        fail "$1: header block is $((alen - ${#BODY})) bytes, cap is $((HDRMAX - 1))"
+    # a clip landing mid-line must still close it, or the body reads as headers
+    tail -c $((${#BODY} + 4)) "$dir/out.arc" >"$dir/got.tail"
+    printf '\r\n\r\n%s' "$BODY" >"$dir/want.tail"
+    cmp -s "$dir/got.tail" "$dir/want.tail" ||
+        fail "$1: header block does not end on a blank line"
+    last_alen=$alen
+}
+
+# Control: nothing overshoots, so a broken probe cannot pass as a clean bound.
+last_alen=0
+build_arc OK "" 0
+convert_check "short entry"
+# what the writer adds on its own, so the sweep below can bracket the cap
+fixed=$((last_alen - ${#BODY}))
+
+# The reported bug: 12000 bytes wrap "sizeof - strlen - 1", so the guard passes.
+build_arc OK "" 12000
+convert_check "over-long cached headers"
+grep -q 'clipped to' "$dir/log" ||
+    fail "a clipped header block was not reported on stderr"
+
+# Three sources at once, each with its own limit: the reason phrase is cut at
+# msg[1024], the shorter location not at all, the passthrough takes what is
+# left. One all-fields bound gets at least one of them wrong.
+build_arc "$(pad 1200 M)" "http://example.com/$(pad 1500 L)" 5000
+convert_check "long reason, location and headers"
+got=$(runlen "$dir/out.arc" M)
+test "${got:-0}" = 1023 || fail "reason phrase kept ${got:-0} bytes, expected 1023"
+got=$(runlen "$dir/out.arc" L)
+test "${got:-0}" = 1500 || fail "location kept ${got:-0} bytes, expected 1500"
+
+# An off-by-one in the room held back for the closing blank line shows up only
+# on the exact boundary, so walk the block across its cap byte by byte.
+for n in $(seq $((HDRMAX - fixed - 32)) $((HDRMAX - fixed + 32))); do
+    build_arc OK "" "$n"
+    convert_check "passthrough of $n bytes"
+done
+
+echo "OK: the .arc writer clips its header block instead of overflowing it"

tests/86_local-proxytrack-cache-longfields.test

@@ -13,11 +13,6 @@ trap 'set +e; rm -rf "$dir"' EXIT
 
 pad() { printf "%${1}s" '' | tr ' ' "$2"; }
 
-# Longest surviving run of char $2 in file $1, or 0.
-runlen() {
-    grep -ao "$2\\+" "$1" | awk '{ print length($0) }' | sort -rn | head -n1 || true
-}
-
 # --- ARC reader (HTTP_READFIELD_STRING), no python needed -------------------
 # Each header overshoots its own destination, so a per-field clip is the only
 # way to get every expected length right at once.

tests/Makefile.am

@@ -226,6 +226,7 @@ TESTS = \
 	102_local-ftp-refetch.test \
 	103_teardown-status.test \
 	104_engine-warc-longurl.test \
-	105_suite-timeout.test
+	105_suite-timeout.test \
+	118_local-proxytrack-arcwrite.test
 
 CLEANFILES = check-network_sh.cache

tests/testlib.sh

@@ -24,6 +24,12 @@ nativepath() {
     fi
 }
 
+# Longest surviving run of char $2 in file $1, or 0: the length a field was
+# clipped to, read back out of a binary artifact.
+runlen() {
+    grep -ao "$2\\+" "$1" | awk '{ print length($0) }' | sort -rn | head -n1 || true
+}
+
 is_windows() {
     case "$(uname -s)" in
     MINGW* | MSYS* | CYGWIN*) return 0 ;;