review: trim comments to the one-line default

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>

src/htsback.c

@@ -572,9 +572,12 @@ int back_finalize(httrackp * opt, cache_back * cache, struct_back * sback,
         && back[p].r.size != back[p].r.totalsize && !opt->tolerant) {
       if (back[p].status == STATUS_READY) {
         hts_log_print(opt, LOG_WARNING,
-                      "file not stored in cache due to bogus state (broken size, expected "
-                      LLintP " got " LLintP "): %s%s", back[p].r.totalsize,
-                      back[p].r.size, back[p].url_adr, back[p].url_fil);
+                      "incomplete transfer (expected " LLintP
+                      " bytes, got " LLintP
+                      "): file not cached, will be retried on the next update"
+                      " (use -%%B to cache anyway): %s%s",
+                      back[p].r.totalsize, back[p].r.size, back[p].url_adr,
+                      back[p].url_fil);
       } else {
         hts_log_print(opt, LOG_INFO,
                       "incomplete file not yet stored in cache (expected "
@@ -879,11 +882,12 @@ int back_finalize(httrackp * opt, cache_back * cache, struct_back * sback,
                          back[p].url_fil, NULL);
           } else {
             /* Partial file, but marked as "ok" ? */
-            hts_log_print(opt, LOG_WARNING,
-                          "file not stored in cache due to bogus state (incomplete type with %s (%d), size "
-                          LLintP "): %s%s", back[p].r.msg, back[p].r.statuscode,
-                          (LLint) back[p].r.size, back[p].url_adr,
-                          back[p].url_fil);
+            hts_log_print(
+                opt, LOG_WARNING,
+                "file with unresolved type not cached (%s (%d), size " LLintP
+                "): %s%s",
+                back[p].r.msg, back[p].r.statuscode, (LLint) back[p].r.size,
+                back[p].url_adr, back[p].url_fil);
           }
         }
 

src/htscache.c

@@ -220,23 +220,38 @@ struct cache_back_zip_entry {
 	} \
 } while(0)
 
-/* A cache (new.zip) write failed: storage is gone (disk full / dropped share),
-   so the mirror is doomed too. Abort it via exit_xh, don't crash as assertf
-   did. */
+/* Consecutive entry write failures before the cache stream is declared dead. */
+#define CACHE_MAX_WRITE_FAILURES 8
+
+/* Cache write failed: a fatal errno or a failure streak aborts the mirror
+   (exit_xh); an isolated failure only drops the current entry. */
 static void cache_zip_write_failed(httrackp *opt, cache_back *cache,
-                                   const char *what, int zErr) {
-  if (!cache->zipWriteFailed) {
-    cache->zipWriteFailed = HTS_TRUE;
-    if (check_fatal_io_errno()) {
-      hts_log_print(opt, LOG_ERROR,
-                    "Mirror aborted: disk full or filesystem problems");
-    } else {
-      hts_log_print(opt, LOG_ERROR,
-                    "Mirror aborted: cache write failed (%s): %s", what,
-                    hts_get_zerror(zErr));
+                                   const char *what, int zErr,
+                                   hts_boolean entry_open, const char *url_adr,
+                                   const char *url_fil) {
+  const int fatal_errno = zErr == ZIP_ERRNO && check_fatal_io_errno();
+
+  cache->zipWriteFailures++;
+  if (fatal_errno || cache->zipWriteFailures >= CACHE_MAX_WRITE_FAILURES) {
+    if (!cache->zipWriteFailed) {
+      cache->zipWriteFailed = HTS_TRUE;
+      if (fatal_errno) {
+        hts_log_print(opt, LOG_ERROR,
+                      "Mirror aborted: disk full or filesystem problems");
+      } else {
+        hts_log_print(opt, LOG_ERROR,
+                      "Mirror aborted: cache write failed (%s): %s", what,
+                      hts_get_zerror(zErr));
+      }
     }
+    opt->state.exit_xh = -1; /* fatal: stop the mirror, exit non-zero */
+  } else {
+    if (entry_open)
+      zipCloseFileInZip((zipFile) cache->zipOutput); /* abandon, best-effort */
+    hts_log_print(opt, LOG_WARNING,
+                  "cache write failed (%s: %s), entry not cached: %s%s", what,
+                  hts_get_zerror(zErr), url_adr, url_fil);
   }
-  opt->state.exit_xh = -1; /* fatal: stop the mirror, exit non-zero */
 }
 
 /* Ajout d'un fichier en cache */
@@ -286,10 +301,19 @@ void cache_add(httrackp * opt, cache_back * cache, const htsblk * r,
   if (r->size < 0)              // error
     return;
 
-  // data in cache
-  if (dataincache) {
-    assertf(((int) r->size) == r->size);
-    //entryBodySize = (int) r->size;
+  // data in cache: the body must fit the 32-bit zip write API
+  if (dataincache && (LLint) (int) r->size != r->size) {
+    if (r->is_write && url_save != NULL && strnotempty(url_save)) {
+      hts_log_print(opt, LOG_WARNING,
+                    "file too large for the cache, storing headers only: %s%s",
+                    url_adr, url_fil);
+      dataincache = 0;
+    } else {
+      hts_log_print(opt, LOG_WARNING,
+                    "entry too large for the cache, not cached: %s%s", url_adr,
+                    url_fil);
+      return;
+    }
   }
 
   /* Fields */
@@ -369,7 +393,8 @@ void cache_add(httrackp * opt, cache_back * cache, const htsblk * r,
                                    */
                                   headers, (uInt) strlen(headers), NULL, 0, NULL,       /* comment */
                                   Z_DEFLATED, Z_DEFAULT_COMPRESSION)) != Z_OK) {
-    cache_zip_write_failed(opt, cache, "opening a cache entry", zErr);
+    cache_zip_write_failed(opt, cache, "opening a cache entry", zErr, HTS_FALSE,
+                           url_adr, url_fil);
     return;
   }
 
@@ -380,7 +405,8 @@ void cache_add(httrackp * opt, cache_back * cache, const htsblk * r,
         if ((zErr =
              zipWriteInFileInZip((zipFile) cache->zipOutput, r->adr,
                                  (int) r->size)) != Z_OK) {
-          cache_zip_write_failed(opt, cache, "writing to the cache", zErr);
+          cache_zip_write_failed(opt, cache, "writing to the cache", zErr,
+                                 HTS_TRUE, url_adr, url_fil);
           return;
         }
       }
@@ -402,8 +428,8 @@ void cache_add(httrackp * opt, cache_back * cache, const htsblk * r,
               if ((zErr =
                    zipWriteInFileInZip((zipFile) cache->zipOutput, buff,
                                        (int) nl)) != Z_OK) {
-                cache_zip_write_failed(opt, cache, "writing to the cache",
-                                       zErr);
+                cache_zip_write_failed(opt, cache, "writing to the cache", zErr,
+                                       HTS_TRUE, url_adr, url_fil);
                 fclose(fp);
                 return;
               }
@@ -419,15 +445,19 @@ void cache_add(httrackp * opt, cache_back * cache, const htsblk * r,
 
   /* Close */
   if ((zErr = zipCloseFileInZip((zipFile) cache->zipOutput)) != Z_OK) {
-    cache_zip_write_failed(opt, cache, "closing a cache entry", zErr);
+    cache_zip_write_failed(opt, cache, "closing a cache entry", zErr, HTS_FALSE,
+                           url_adr, url_fil);
     return;
   }
 
   /* Flush */
   if ((zErr = zipFlush((zipFile) cache->zipOutput)) != 0) {
-    cache_zip_write_failed(opt, cache, "flushing the cache", zErr);
+    cache_zip_write_failed(opt, cache, "flushing the cache", zErr, HTS_FALSE,
+                           url_adr, url_fil);
     return;
   }
+
+  cache->zipWriteFailures = 0; /* entry stored: reset the failure streak */
 }
 
 #else

src/htscache_selftest.c

@@ -48,6 +48,7 @@ Please visit our Website: http://www.httrack.com
 #include "htszlib.h"
 
 #include <errno.h>
+#include <limits.h>
 #include <stdio.h>
 #include <string.h>
 
@@ -321,6 +322,7 @@ typedef struct {
   size_t budget;  /**< bytes allowed through before writes start failing */
   int fail_errno; /**< errno set on the failing write (ENOSPC, EIO, ...) */
   int writes;     /**< zwrite call count, to detect re-entry into the stream */
+  int fail_once;  /**< recover (unlimited budget) after the first failure */
 } writefail_inject;
 
 /* zwrite that copies until the budget runs out, then fails with inj->fail_errno
@@ -335,6 +337,8 @@ static uLong selftest_failing_zwrite(voidpf opaque, voidpf stream,
     inj->budget -= (size_t) size;
     return (uLong) fwrite(buf, 1, (size_t) size, (FILE *) stream);
   }
+  if (inj->fail_once)
+    inj->budget = (size_t) -1; /* the backend recovers after this failure */
   errno = inj->fail_errno;
   return 0; /* short write -> the minizip op returns an error */
 }
@@ -373,9 +377,50 @@ static void writefail_store(httrackp *opt, cache_back *cache, const char *fil,
   freet(bodycopy);
 }
 
-/* #174/#219: a failing cache write used to crash via assertf(); it must instead
-   stop the mirror (exit_xh = -1) without crashing. Assert that, plus the cache
-   is flagged and a sibling write doesn't re-enter the broken stream. */
+/* Store an entry claiming a >2GB body; the degrade path never reads data. */
+static void writefail_store_oversized(httrackp *opt, cache_back *cache,
+                                      const char *fil, int is_write) {
+  htsblk r;
+  char locbuf[4];
+
+  hts_init_htsblk(&r);
+  r.statuscode = 200;
+  r.size = (LLint) INT_MAX + 1;
+  strcpybuff(r.msg, "OK");
+  strcpybuff(r.contenttype, "application/octet-stream");
+  locbuf[0] = '\0';
+  r.location = locbuf;
+  r.is_write = (short int) is_write;
+  cache_add(opt, cache, &r, "example.com", fil, "example.com/big.bin", 1, NULL);
+}
+
+/* Read back `entryname`: extra field (cached headers) and body. Returns the
+   body length, or -1 if the entry is absent or unreadable. */
+static int writefail_read_entry(const char *path, const char *entryname,
+                                char *extra, size_t extralen, char *body,
+                                size_t bodylen) {
+  unzFile z = unzOpen(path);
+  int n = -1;
+
+  if (z == NULL)
+    return -1;
+  if (unzLocateFile(z, entryname, 1) == UNZ_OK &&
+      unzOpenCurrentFile(z) == UNZ_OK) {
+    const int elen = unzGetLocalExtrafield(z, extra, (unsigned) (extralen - 1));
+
+    if (elen >= 0) {
+      extra[elen] = '\0';
+      n = unzReadCurrentFile(z, body, (unsigned) bodylen);
+    }
+    unzCloseCurrentFile(z);
+  }
+  unzClose(z);
+  return n;
+}
+
+/* Cache write-failure policy (#174/#219): fatal errno or a failure streak
+   stops the mirror (exit_xh=-1, no crash); isolated/oversized drops the entry.
+ */
 int cache_write_failure_selftest(httrackp *opt, const char *dir) {
   int fail = 0;
   char path[HTS_URLMAXSIZE];
@@ -388,9 +433,8 @@ int cache_write_failure_selftest(httrackp *opt, const char *dir) {
   gen_body(body, body_len, 1 /* incompressible */);
   fconcat(path, sizeof(path), dir, "/wfail.zip");
 
-  /* phase 0: fail on the body write, fatal errno (ENOSPC, the disk-full
-     branch). phase 1: fail on the open, non-fatal errno (EIO, dropped-share
-     branch). Both must abort the mirror. */
+  /* phase 0: fatal errno (ENOSPC) aborts at once; phase 1: persistent EIO
+     drops entries until the streak caps out, then aborts. */
   for (phase = 0; phase < 2; phase++) {
     cache_back cache;
     writefail_inject inj;
@@ -399,6 +443,7 @@ int cache_write_failure_selftest(httrackp *opt, const char *dir) {
     inj.budget = (phase == 0) ? 4096 : 0;
     inj.fail_errno = (phase == 0) ? ENOSPC : EIO;
     inj.writes = 0;
+    inj.fail_once = 0;
     memset(&cache, 0, sizeof(cache));
     cache.type = 1;
     cache.log = stderr;
@@ -412,7 +457,25 @@ int cache_write_failure_selftest(httrackp *opt, const char *dir) {
     }
 
     opt->state.exit_xh = 0; /* clear; the failing write must set it to -1 */
-    writefail_store(opt, &cache, "/blob.bin", body, body_len);
+    if (phase == 0) {
+      writefail_store(opt, &cache, "/blob.bin", body, body_len);
+    } else {
+      /* the abort must land exactly on the 8th consecutive failure */
+      int i;
+
+      for (i = 0; i < 7; i++) {
+        char fil[32];
+
+        snprintf(fil, sizeof(fil), "/b%d.bin", i);
+        writefail_store(opt, &cache, fil, body, 16);
+      }
+      if (cache.zipWriteFailed) {
+        fprintf(stderr, "cache-writefail: phase 1: aborted before the "
+                        "8th consecutive failure\n");
+        fail++;
+      }
+      writefail_store(opt, &cache, "/b7.bin", body, 16);
+    }
     if (!cache.zipWriteFailed) {
       fprintf(stderr, "cache-writefail: phase %d: write error not caught\n",
               phase);
@@ -443,6 +506,136 @@ int cache_write_failure_selftest(httrackp *opt, const char *dir) {
     }
   }
 
+  /* failures with successes in between reset the streak: never aborts */
+  {
+    cache_back cache;
+    writefail_inject inj;
+    int i;
+
+    inj.budget = (size_t) -1;
+    inj.fail_errno = EIO;
+    inj.writes = 0;
+    inj.fail_once = 0;
+    memset(&cache, 0, sizeof(cache));
+    cache.type = 1;
+    cache.log = stderr;
+    cache.errlog = stderr;
+    cache.hashtable = coucal_new(0);
+    cache.zipOutput = selftest_open_failing_zip(path, &inj);
+    opt->state.exit_xh = 0;
+
+    for (i = 0; i < 10; i++) {
+      char fil[32];
+
+      inj.budget = 0; /* this store fails */
+      snprintf(fil, sizeof(fil), "/s%d.bin", i);
+      writefail_store(opt, &cache, fil, body, 16);
+      inj.budget = (size_t) -1; /* this one succeeds and resets the streak */
+      snprintf(fil, sizeof(fil), "/ok%d.bin", i);
+      writefail_store(opt, &cache, fil, body, 16);
+    }
+    if (cache.zipWriteFailed || opt->state.exit_xh != 0) {
+      fprintf(stderr,
+              "cache-writefail: scattered: non-consecutive failures aborted "
+              "the mirror (flagged=%d, exit_xh=%d)\n",
+              (int) cache.zipWriteFailed, opt->state.exit_xh);
+      fail++;
+    }
+    zipClose(cache.zipOutput, NULL);
+    cache.zipOutput = NULL;
+  }
+
+  /* isolated failure: only that entry drops; a later sibling round-trips */
+  {
+    cache_back cache;
+    writefail_inject inj;
+    char extra[8192];
+    char rbody[64];
+    int n;
+
+    inj.budget = 4096;
+    inj.fail_errno = EIO;
+    inj.writes = 0;
+    inj.fail_once = 1;
+    memset(&cache, 0, sizeof(cache));
+    cache.type = 1;
+    cache.log = stderr;
+    cache.errlog = stderr;
+    cache.hashtable = coucal_new(0);
+    cache.zipOutput = selftest_open_failing_zip(path, &inj);
+    opt->state.exit_xh = 0;
+
+    writefail_store(opt, &cache, "/blob.bin", body, body_len);
+    if (cache.zipWriteFailed || opt->state.exit_xh != 0) {
+      fprintf(stderr,
+              "cache-writefail: skip: isolated failure aborted the mirror "
+              "(flagged=%d, exit_xh=%d)\n",
+              (int) cache.zipWriteFailed, opt->state.exit_xh);
+      fail++;
+    }
+    writefail_store(opt, &cache, "/blob2.bin", body, 16);
+    zipClose(cache.zipOutput, NULL);
+    cache.zipOutput = NULL;
+    n = writefail_read_entry(path, "http://example.com/blob2.bin", extra,
+                             sizeof(extra), rbody, sizeof(rbody));
+    if (n != 16 || memcmp(rbody, body, 16) != 0) {
+      fprintf(stderr,
+              "cache-writefail: skip: sibling entry lost after a skipped "
+              "entry (%d)\n",
+              n);
+      fail++;
+    }
+  }
+
+  /* >2GB bodies: in-memory drops the entry, on-disk degrades to headers-only */
+  {
+    cache_back cache;
+    writefail_inject inj;
+    char extra[8192];
+    char rbody[64];
+    int n;
+
+    inj.budget = (size_t) -1; /* no injected failure */
+    inj.fail_errno = 0;
+    inj.writes = 0;
+    inj.fail_once = 0;
+    memset(&cache, 0, sizeof(cache));
+    cache.type = 1;
+    cache.log = stderr;
+    cache.errlog = stderr;
+    cache.hashtable = coucal_new(0);
+    cache.zipOutput = selftest_open_failing_zip(path, &inj);
+    opt->state.exit_xh = 0;
+
+    writefail_store_oversized(opt, &cache, "/bigmem.bin", 0 /* in-memory */);
+    writefail_store_oversized(opt, &cache, "/bigdisk.bin", 1 /* on-disk */);
+    zipClose(cache.zipOutput, NULL);
+    cache.zipOutput = NULL;
+
+    if (cache.zipWriteFailed || opt->state.exit_xh != 0) {
+      fprintf(stderr,
+              "cache-writefail: oversize: mirror aborted (flagged=%d, "
+              "exit_xh=%d)\n",
+              (int) cache.zipWriteFailed, opt->state.exit_xh);
+      fail++;
+    }
+    if (writefail_read_entry(path, "http://example.com/bigmem.bin", extra,
+                             sizeof(extra), rbody, sizeof(rbody)) >= 0) {
+      fprintf(stderr,
+              "cache-writefail: oversize: in-memory entry was stored\n");
+      fail++;
+    }
+    n = writefail_read_entry(path, "http://example.com/bigdisk.bin", extra,
+                             sizeof(extra), rbody, sizeof(rbody));
+    if (n != 0 || strstr(extra, "X-In-Cache: 0") == NULL) {
+      fprintf(stderr,
+              "cache-writefail: oversize: on-disk entry not stored "
+              "headers-only (%d)\n",
+              n);
+      fail++;
+    }
+  }
+
   freet(body);
   return fail;
 }

src/htscache_selftest.h

@@ -52,8 +52,8 @@ int cache_selftests(httrackp *opt, const char *dir);
    committed file, never by the test). Returns the failed-check count. */
 int cache_golden_selftest(httrackp *opt, const char *dir, int regen);
 
-/* #174/#219: assert a failing cache write aborts the mirror cleanly instead of
-   crashing. Returns the failed-check count. */
+/* Cache write-failure policy (#174/#219): abort on fatal errno or a streak,
+   drop just the entry otherwise. Returns the failed-check count. */
 int cache_write_failure_selftest(httrackp *opt, const char *dir);
 
 /* Exercise the hts_cache_reconcile() generation policies on file fixtures

src/htscore.c

@@ -2852,6 +2852,9 @@ int check_fatal_io_errno(void) {
 #endif
 #ifdef EROFS
   case EROFS:                  /* Read-only file system */
+#endif
+#ifdef EDQUOT
+  case EDQUOT: /* Disk quota exceeded */
 #endif
     return 1;
     break;

src/htscore.h

@@ -216,6 +216,7 @@ struct cache_back {
   int zipEntriesCapa;
   hts_boolean
       zipWriteFailed; /**< a cache write failed; stop touching the stream */
+  int zipWriteFailures; /**< consecutive entry write failures; reset on store */
 };
 
 #ifndef HTS_DEF_FWSTRUCT_hash_struct

tests/01_zlib-cache-writefail.test

@@ -4,10 +4,9 @@
 # POSIX /bin/sh on some platforms (e.g. macOS), so avoid bashisms and GNU-only
 # tool flags despite the #!/bin/bash above.
 
-# Cache write-failure handling (httrack -#test=cache-writefail <dir>). #174/#219.
-# A failing new.zip write (disk full) used to crash the process via assertf; it
-# must instead stop the mirror with a fatal error (exit_xh=-1), no crash. The
-# self-test asserts that; reverting the fix makes -#test=cache-writefail abort (SIGABRT) and fail.
+# Cache write-failure policy (-#test=cache-writefail <dir>). #174/#219: disk
+# full or a failure streak aborts cleanly; an isolated failure or an oversized
+# entry is only dropped.
 
 set -eu
 
@@ -22,3 +21,9 @@ printf '%s\n' "$out" | grep -qx "cache-writefail: OK" || {
     echo "expected 'cache-writefail: OK', got: $out" >&2
     exit 1
 }
+
+# A skipped entry must be warned about with its URL.
+printf '%s\n' "$out" | grep -q "entry not cached: example.com/" || {
+    echo "expected a URL-bearing skip warning" >&2
+    exit 1
+}

tests/22_local-broken-size.test

@@ -1,17 +1,19 @@
 #!/bin/bash
-# Issues #32/#41: a Content-Length that disagrees with the body warns "bogus
-# state (broken size)" and skips the cache; -%B (tolerant) accepts it.
+# Issues #32/#41: a Content-Length that disagrees with the body warns
+# "incomplete transfer" and skips the cache; -%B (tolerant) accepts it.
+
+set -euo pipefail
 
 : "${top_srcdir:=..}"
 
 # Default: warn, but the file is still written.
 bash "$top_srcdir/tests/local-crawl.sh" --errors 0 \
     --found 'size/oversize.bin' \
-    --log-found 'bogus state \(broken size' \
+    --log-found 'incomplete transfer \(expected' \
     httrack 'BASEURL/size/index.html'
 
 # -%B (tolerant): no warning, file written.
 bash "$top_srcdir/tests/local-crawl.sh" --errors 0 \
     --found 'size/oversize.bin' \
-    --log-not-found 'bogus state' \
+    --log-not-found 'incomplete transfer|not cached' \
     httrack 'BASEURL/size/index.html' '-%B'

tests/33_local-delayed.test

@@ -2,7 +2,7 @@
 #
 # Degenerate delayed-type paths (#5/#107 family): redirects that never resolve
 # a name must drop cleanly -- no .delayed leftovers (audited by local-crawl.sh),
-# no "bogus state" cache warnings, resolvable links still land correctly.
+# no "not cached" warnings, resolvable links still land correctly.
 
 set -euo pipefail
 
@@ -16,5 +16,5 @@ bash "$top_srcdir/tests/local-crawl.sh" --rerun --errors 0 \
     --not-found 'delayed/noloc.html' \
     --not-found 'delayed/selfloop.html' \
     --not-found 'delayed/chain9.pdf' \
-    --log-not-found 'bogus state' \
+    --log-not-found 'not cached' \
     httrack 'BASEURL/delayed/index.html'

tests/36_local-bigcrawl.test

@@ -44,7 +44,7 @@ bash "$top_srcdir/tests/local-crawl.sh" --rerun \
     --file-matches 'big/a/blk2.css' 'url\(blk2-bg\.png\)' \
     --file-matches 'big/p/5.html' "document\\.write\\('<a href=\"\\.\\./f5/dw\\.html\"" \
     --file-not-matches 'big/p/1.html' 'href="/big/' \
-    --log-not-found 'bogus state|[Pp]anic|assert' \
+    --log-not-found 'not cached|[Pp]anic|assert' \
     --log-found '\(404\) at link [^ ]*/big/e/404\.html' \
     --log-found '\(410\) at link [^ ]*/big/e/410\.html' \
     --log-found '\(500\) at link [^ ]*/big/e/500\.html' \

tests/local-server.py

@@ -781,7 +781,7 @@ class Handler(SimpleHTTPRequestHandler):
         self.send_raw(b"", "text/html")
 
     # broken Content-Length (#32/#41): declared size != bytes sent. httrack
-    # warns "bogus state (broken size)" and skips the cache unless -%B.
+    # warns "incomplete transfer" and skips the cache unless -%B.
     def route_size_index(self):
         self.send_html('\t<a href="oversize.bin">over</a>\n')