warc: force rotation in the segment test and guard its vacuity

--warc-max-size 2000 never rotated under the harness's --robots=0, so the
segment test was checking a single-file archive; a mutant that renamed
only segment 0 passed it. 600 rotates, and --archive-min-files fails the
test if a shrinking crawl ever stops producing the segments it checks.

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

src/htscore.c

@@ -1980,10 +1980,9 @@ int httpmirror(char *url1, httrackp * opt) {
           }
 
           // ATTENTION C'EST ICI QU'ON SAUVE LE FICHIER!!
-          // An empty body must not overwrite the file when the transfer failed
-          // (statuscode <= 0, e.g. an -M hard-stop): it would truncate a good
-          // copy to 0 (#77 follow-up).
-          if (r.adr != NULL || (r.size == 0 && r.statuscode > 0)) {
+          // A failed transfer has no body: r.adr holds debris from the aborted
+          // read, which would destroy the copy being re-fetched (#748).
+          if (r.statuscode > 0 && (r.adr != NULL || r.size == 0)) {
             file_notify(opt, urladr(), urlfil(), savename(), 1, 1, r.notmodified);
             if (filesave(opt, r.adr, (int) r.size, savename(), urladr(), urlfil()) !=
                 0) {
@@ -2693,7 +2692,11 @@ HTSEXT_API int structcheck(const char *path) {
       if (!S_ISDIR(st.st_mode)) {
 #if HTS_REMOVE_ANNOYING_INDEX
         if (S_ISREG(st.st_mode)) {      /* Regular file in place ; move it and create directory */
-          sprintf(tmpbuf, "%s.txt", file);
+          /* bounded here, not by the path-length guard far above */
+          if (!sprintfbuff(tmpbuf, "%s.txt", file)) {
+            errno = ENAMETOOLONG;
+            return -1;
+          }
           if (rename(file, tmpbuf) != 0) {      /* Can't rename regular file */
             return -1;
           }
@@ -2801,7 +2804,11 @@ HTSEXT_API int structcheck_utf8(const char *path) {
       if (!S_ISDIR(st.st_mode)) {
 #if HTS_REMOVE_ANNOYING_INDEX
         if (S_ISREG(st.st_mode)) {      /* Regular file in place ; move it and create directory */
-          sprintf(tmpbuf, "%s.txt", file);
+          /* bounded here, not by the path-length guard far above */
+          if (!sprintfbuff(tmpbuf, "%s.txt", file)) {
+            errno = ENAMETOOLONG;
+            return -1;
+          }
           if (RENAME(file, tmpbuf) != 0) {      /* Can't rename regular file */
             return -1;
           }
@@ -3821,11 +3828,12 @@ int htsAddLink(htsmoduleStruct * str, char *link) {
         strcpybuff(codebase, heap(ptr)->fil);
       else
         strcpybuff(codebase, heap(heap(ptr)->precedent)->fil);
-      a = codebase + strlen(codebase) - 1;
+      // empty codebase has no last char; codebase-1 would underflow
+      a = codebase[0] != '\0' ? codebase + strlen(codebase) - 1 : codebase;
       while((*a) && (*a != '/') && (a > codebase))
         a--;
       if (*a == '/')
-        *(a + 1) = '\0';        // couper
+        *(a + 1) = '\0';        // cut
     } else {                    // couper http:// éventuel
       if (strfield(codebase, "http://")) {
         char BIGSTK tempo[HTS_URLMAXSIZE * 2];

src/htsselftest.c

@@ -43,6 +43,7 @@ Please visit our Website: http://www.httrack.com
 
 #include "htsglobal.h"
 #include "htscore.h"
+#include "htsmodules.h"
 #include "htsback.h"
 #include "htsdefines.h"
 #include "htslib.h"
@@ -2621,6 +2622,68 @@ static int st_savename(httrackp *opt, int argc, char **argv) {
   return 0;
 }
 
+/* an empty fil started htsAddLink's codebase walk before the buffer (#730) */
+static int st_addlink(httrackp *opt, int argc, char **argv) {
+  htsmoduleStruct BIGSTK str;
+  cache_back cache;
+  struct_back *sback;
+  hash_struct hash;
+  int ptr = 0;
+  int i;
+
+  (void) argc;
+  (void) argv;
+
+  memset(&cache, 0, sizeof(cache));
+  cache.hashtable = (void *) coucal_new(0);
+  sback = back_new(opt, opt->maxsoc * 32 + 1024);
+  /* same wiring as hts_mirror (htscore.c) */
+  hash_init(opt, &hash, opt->urlhack);
+  hash.liens = (const lien_url *const *const *) &opt->liens;
+  opt->hash = &hash;
+  hts_record_init(opt);
+
+  memset(&str, 0, sizeof(str));
+  str.opt = opt;
+  str.sback = sback;
+  str.cache = &cache;
+  str.hashptr = &hash;
+  str.ptr_ = &ptr;
+  str.addLink = htsAddLink;
+
+  /* [0] is the underflow; [1] and [2] are controls that the trim is unchanged.
+     A query-only link is the one that notices the trim at all: for the others
+     ident_url_relatif() re-derives the directory from the path it is given. */
+  for (i = 0; i < 3; i++) {
+    static const char *const fil[3] = {"", "/dir/page.html", "/dir/page.html"};
+    static const char *const lnk[3] = {"sub/page.html", "sub/page.html",
+                                       "?x=1"};
+    static const char *const want[3] = {
+        "untouched", "http://www.example.com/dir/sub/page.html",
+        "http://www.example.com/dir/?x=1"};
+    char BIGSTK loc[HTS_URLMAXSIZE * 2];
+    char BIGSTK link[HTS_URLMAXSIZE];
+
+    strcpybuff(loc, "untouched");
+    strcpybuff(link, lnk[i]);
+    str.localLink = loc;
+    str.localLinkSize = (int) sizeof(loc);
+    if (!hts_record_link(opt, "www.example.com", fil[i], "", "", "", ""))
+      return 1;
+    ptr = heap_top_index();
+    str.url_host = heap(ptr)->adr;
+    str.url_file = heap(ptr)->fil;
+    assertf(htsAddLink(&str, link) == 0); /* refused by the wizard either way */
+    if (strcmp(loc, want[i]) != 0) {
+      fprintf(stderr, "addlink[%d]: got '%s' want '%s'\n", i, loc, want[i]);
+      return 1;
+    }
+  }
+
+  printf("addlink self-test OK\n");
+  return 0;
+}
+
 static int st_cache(httrackp *opt, int argc, char **argv) {
   int err;
 
@@ -3262,6 +3325,81 @@ static int st_topindex(httrackp *opt, int argc, char **argv) {
   return 0;
 }
 
+/* Build a path of exactly len chars under base; returns that length. */
+static size_t st_structcheck_longpath(char *dst, size_t dstsize,
+                                      const char *base, size_t len) {
+  size_t n = strlen(base);
+
+  assertf(len < dstsize && n + 2 <= len);
+  memmove(dst, base, n);
+  while (n < len) {
+    size_t seg = len - n - 1;
+
+    if (seg > 200) /* stay under the usual 255-byte component limit */
+      seg = len - n == 202 ? 199 : 200; /* never leave a bare separator */
+    dst[n++] = '/';
+    memset(dst + n, 'x', seg);
+    n += seg;
+  }
+  dst[n] = '\0';
+  return n;
+}
+
+/* The path guard, and the <name>.txt rename structcheck() performs when a
+   regular file sits where a directory has to go (#745). */
+static int st_structcheck(httrackp *opt, int argc, char **argv) {
+  char BIGSTK path[HTS_URLMAXSIZE * 2];
+  char BIGSTK target[HTS_URLMAXSIZE * 2];
+  FILE *fp;
+
+  (void) opt;
+  if (argc < 1) {
+    fprintf(stderr, "usage: -#test=structcheck <writable directory>\n");
+    return 1;
+  }
+
+  /* over the guard: refused before a single directory is created */
+  st_structcheck_longpath(path, sizeof(path), argv[0], HTS_URLMAXSIZE + 1);
+  errno = 0;
+  assertf(structcheck(path) == -1);
+  assertf(errno == EINVAL);
+  errno = 0;
+  assertf(structcheck_utf8(path) == -1);
+  assertf(errno == EINVAL);
+  {
+    char *const sep = strchr(path + strlen(argv[0]) + 1, '/');
+
+    assertf(sep != NULL);
+    sep[1] = '\0'; /* the outermost component it would have created */
+    assertf(!dir_exists(path));
+  }
+
+  /* a regular file where a directory belongs is renamed to <name>.txt */
+  snprintf(path, sizeof(path), "%s/sc", argv[0]);
+  fp = fopen(path, "wb");
+  assertf(fp != NULL);
+  fclose(fp);
+  snprintf(path, sizeof(path), "%s/sc/sub/", argv[0]);
+  assertf(structcheck(path) == 0);
+  assertf(dir_exists(path));
+  snprintf(target, sizeof(target), "%s/sc.txt", argv[0]);
+  assertf(fexist(target));
+
+  /* the utf-8 entry point carries the same rename */
+  snprintf(path, sizeof(path), "%s/u8", argv[0]);
+  fp = FOPEN(path, "wb");
+  assertf(fp != NULL);
+  fclose(fp);
+  snprintf(path, sizeof(path), "%s/u8/sub/", argv[0]);
+  assertf(structcheck_utf8(path) == 0);
+  assertf(dir_exists(path));
+  snprintf(target, sizeof(target), "%s/u8.txt", argv[0]);
+  assertf(fexist_utf8(target));
+
+  printf("structcheck self-test OK\n");
+  return 0;
+}
+
 /* Each inplace_escape_*() must equal escape_*() on a copy. */
 static int st_inplace_escape(httrackp *opt, int argc, char **argv) {
   /* >255 bytes forces the helper's malloct path, not the stack buffer */
@@ -6398,6 +6536,8 @@ static const struct selftest_entry {
     {"fsize", "<dir>", "file size past the 2GB signed-32-bit wrap", st_fsize},
     {"growsize", "", "buffer capacity for a 64-bit file size (no int wrap)",
      st_growsize},
+    {"addlink", "", "htsAddLink codebase walk over an empty current path",
+     st_addlink},
     {"cache", "<dir>", "cache read/write round-trip self-test", st_cache},
     {"cacheindex", "", "cache-index (.ndx) parse must stay in bounds",
      st_cacheindex},
@@ -6421,6 +6561,9 @@ static const struct selftest_entry {
     {"useragent", "", "default User-Agent self-test", st_useragent},
     {"makeindex", "[dir]", "hts_finish_makeindex footer/refresh self-test",
      st_makeindex},
+    {"structcheck", "<dir>",
+     "structcheck path guard and the <name>.txt rename it performs",
+     st_structcheck},
     {"topindex", "[dir]",
      "hts_buildtopindex charset handling of a non-ASCII project dir",
      st_topindex},

src/htswarc.c

@@ -57,6 +57,9 @@ Please visit our Website: http://www.httrack.com
 /* opt->state.warc value meaning "open failed once, do not retry". */
 #define WARC_DISABLED ((void *) ~(uintptr_t) 0)
 
+/* Suffix of the in-progress archive when a previous one must survive it. */
+#define WARC_TMP_SUFFIX ".tmp"
+
 struct warc_writer {
   FILE *f;
   httrackp *opt;   /* kept for close-time logging (warc_wacz_package) */
@@ -86,6 +89,11 @@ struct warc_writer {
   char *base_path;      /* resolved archive path minus .warc[.gz] suffix */
   const char *base_ext; /* ".warc.gz" or ".warc" (static) */
   char *arc_path;    /* full single-file archive path (NULL under rotation) */
+  /* A re-run must not destroy an archive it cannot replace (#759). */
+  hts_boolean protect_prev;   /* previous archive present: build in a temp */
+  hts_boolean opened;         /* open completed; a failed one swaps nothing */
+  hts_boolean failed;         /* a record or segment was lost: swap nothing */
+  uint64_t unbacked_revisits; /* revisits whose payload no file here holds */
   char **page_lines; /* one JSON page line per 200 text/html response, owned */
   size_t page_count;
   size_t page_cap;
@@ -1129,13 +1137,17 @@ static int warc_emit(warc_writer *w, const char *type, const char *content_type,
      never split a record, and never rotate a warcinfo (it opens a segment). */
   if (w->max_size > 0 && w->seg_base != NULL && w->offset >= w->max_size &&
       strcmp(type, "warcinfo") != 0) {
-    if (warc_rotate(w) != 0)
+    if (warc_rotate(w) != 0) {
+      w->failed = HTS_TRUE;
       return -1;
+    }
   }
 
   /* F4: overflow-safe block length; http_hdr_len+sep is provably small. */
-  if (payload > (size_t) -1 - http_hdr_len - sep)
+  if (payload > (size_t) -1 - http_hdr_len - sep) {
+    w->failed = HTS_TRUE;
     return -1;
+  }
   block_len = http_hdr_len + sep + payload;
 
   memset(&hdr, 0, sizeof(hdr));
@@ -1241,11 +1253,23 @@ static int warc_emit(warc_writer *w, const char *type, const char *content_type,
   rc = 0;
 done:
   wbuf_free(&hdr);
+  if (rc != 0)
+    w->failed = HTS_TRUE; /* a truncated run must not replace a whole one */
   return rc;
 }
 
 /* ---- segment rotation (--warc-max-size) ---- */
 
+/* Path to open for the segment whose final path is `final`: that path itself,
+   or a sibling temp while a previous archive must survive until close. */
+static const char *warc_open_path(warc_writer *w, const char *final, char *buf,
+                                  size_t bufsz) {
+  if (!w->protect_prev)
+    return final;
+  snprintf(buf, bufsz, "%s" WARC_TMP_SUFFIX, final);
+  return buf;
+}
+
 /* Emit the warcinfo that heads a segment; sets w->info_id for its records. */
 static int warc_write_warcinfo_record(warc_writer *w) {
   w->info_id[0] = '\0'; /* warcinfo itself carries no WARC-Warcinfo-ID */
@@ -1257,17 +1281,23 @@ static int warc_write_warcinfo_record(warc_writer *w) {
 
 static int warc_rotate(warc_writer *w) {
   char namebuf[HTS_URLMAXSIZE * 2];
+  char openbuf[HTS_URLMAXSIZE * 2 + sizeof(WARC_TMP_SUFFIX)];
   char catbuff[CATBUFF_SIZE];
-  if (w->f != NULL) {
-    fclose(w->f);
-    w->f = NULL;
-  }
-  w->seg++;
-  snprintf(namebuf, sizeof(namebuf), "%s-%05u%s", w->seg_base, w->seg,
+  const unsigned next = w->seg + 1;
+  FILE *f;
+  snprintf(namebuf, sizeof(namebuf), "%s-%05u%s", w->seg_base, next,
            w->seg_ext);
-  w->f = FOPEN(fconv(catbuff, sizeof(catbuff), namebuf), "wb");
-  if (w->f == NULL)
+  /* Open before advancing: w->seg must only ever name a segment that exists,
+     or close-time packaging and swapping would work on a missing file. */
+  f = FOPEN(fconv(catbuff, sizeof(catbuff),
+                  warc_open_path(w, namebuf, openbuf, sizeof(openbuf))),
+            "wb");
+  if (f == NULL)
     return -1;
+  if (w->f != NULL)
+    fclose(w->f);
+  w->f = f;
+  w->seg = next;
   w->offset = 0;
   if (w->cdx_on) {
     freet(w->cur_seg);
@@ -1324,6 +1354,7 @@ void warc_adopt_rawspool(htsblk *r, const char *tmpfile_path) {
 warc_writer *warc_open(httrackp *opt, const char *path) {
   warc_writer *w;
   char namebuf[HTS_URLMAXSIZE * 2];
+  char openbuf[HTS_URLMAXSIZE * 2 + sizeof(WARC_TMP_SUFFIX)];
   char catbuff[CATBUFF_SIZE];
   wbuf info;
   const char *robots;
@@ -1474,35 +1505,105 @@ warc_writer *warc_open(httrackp *opt, const char *path) {
     path = namebuf;
   }
 
-  w->f = FOPEN(fconv(catbuff, sizeof(catbuff), path), "wb");
+  if (w->max_size == 0 && (w->arc_path = strdupt(path)) == NULL) {
+    warc_close(w);
+    return NULL;
+  }
+  /* Set only once arc_path is recorded, so a half-built writer never swaps. */
+  w->protect_prev = fsize_utf8(path) > 0 ? HTS_TRUE : HTS_FALSE;
+  w->f = FOPEN(fconv(catbuff, sizeof(catbuff),
+                     warc_open_path(w, path, openbuf, sizeof(openbuf))),
+               "wb");
   if (w->f == NULL) {
     warc_close(w);
     return NULL;
   }
   if (w->cdx_on)
     w->cur_seg = path_basename_dup(path);
-  if (w->wacz_on && w->max_size == 0)
-    w->arc_path = strdupt(path); /* single-file: package this exact path */
 
   if (warc_write_warcinfo_record(w) != 0) {
     warc_close(w);
     return NULL;
   }
+  w->opened = HTS_TRUE;
   return w;
 }
 
+/* Final path of segment s (the run's only archive when rotation is off). */
+static const char *warc_seg_path(warc_writer *w, unsigned s, char *buf,
+                                 size_t bufsz) {
+  if (w->max_size == 0)
+    return w->arc_path;
+  snprintf(buf, bufsz, "%s-%05u%s", w->seg_base, s, w->seg_ext);
+  return buf;
+}
+
+/* Swap this run's archive into place, unless it only holds revisits naming
+   bodies the previous archive still has and this one does not (#759).
+   HTS_FALSE: the previous archive was kept, so leave its .cdx and .wacz too. */
+static hts_boolean warc_commit(warc_writer *w) {
+  char finalbuf[HTS_URLMAXSIZE * 2];
+  char tmpbuf[HTS_URLMAXSIZE * 2 + sizeof(WARC_TMP_SUFFIX)];
+  char catbuff[CATBUFF_SIZE];
+  const unsigned nseg = (w->max_size > 0) ? w->seg + 1 : 1;
+  hts_boolean swap;
+  unsigned s;
+
+  if (!w->protect_prev)
+    return HTS_TRUE; /* nothing was there to lose: written in place */
+
+  /* hts_rename_over unlinks its destination when the source is missing, so
+     every segment has to be on disk before the first rename. */
+  swap = w->opened && !w->failed && w->unbacked_revisits == 0;
+  for (s = 0; s < nseg && swap; s++) {
+    snprintf(tmpbuf, sizeof(tmpbuf), "%s" WARC_TMP_SUFFIX,
+             warc_seg_path(w, s, finalbuf, sizeof(finalbuf)));
+    swap = fsize_utf8(tmpbuf) > 0;
+  }
+
+  if (swap) {
+    for (s = 0; s < nseg; s++) {
+      const char *final = warc_seg_path(w, s, finalbuf, sizeof(finalbuf));
+      snprintf(tmpbuf, sizeof(tmpbuf), "%s" WARC_TMP_SUFFIX, final);
+      if (!hts_rename_over(tmpbuf, final)) {
+        hts_log_print(w->opt, LOG_ERROR | LOG_ERRNO,
+                      "WARC: could not replace %s", final);
+        return HTS_FALSE;
+      }
+    }
+    return HTS_TRUE;
+  }
+
+  for (s = 0; s < nseg; s++) {
+    snprintf(tmpbuf, sizeof(tmpbuf), "%s" WARC_TMP_SUFFIX,
+             warc_seg_path(w, s, finalbuf, sizeof(finalbuf)));
+    (void) UNLINK(fconv(catbuff, sizeof(catbuff), tmpbuf));
+  }
+  if (w->unbacked_revisits > 0)
+    hts_log_print(
+        w->opt, LOG_ERROR,
+        "WARC: this pass revisited %llu URL(s) without re-downloading them, so "
+        "its archive would name bodies no file holds; kept the previous %s "
+        "(re-run with -C0, or --warc-file with a name of its own)",
+        (unsigned long long) w->unbacked_revisits,
+        warc_seg_path(w, 0, finalbuf, sizeof(finalbuf)));
+  return HTS_FALSE;
+}
+
 void warc_close(warc_writer *w) {
   size_t i;
   if (w == NULL)
     return;
-  warc_cdx_flush(w); /* sort + write <base>.cdx before tearing down */
   if (w->f != NULL)
     fclose(w->f);
   w->f = NULL;
+  if (warc_commit(w)) {
+    warc_cdx_flush(w); /* sort + write <base>.cdx beside the archive */
 #if HTS_USEOPENSSL
-  if (w->wacz_on) /* package once the segment(s) + .cdx are closed on disk */
-    warc_wacz_package(w);
+    if (w->wacz_on) /* package once the segment(s) + .cdx are closed on disk */
+      warc_wacz_package(w);
 #endif
+  }
   if (w->seen != NULL)
     coucal_delete(&w->seen);
   for (i = 0; i < w->cdx_count; i++)
@@ -1583,6 +1684,8 @@ int warc_write_transaction(warc_writer *w, const char *target_uri,
   if (is_update_unchanged) {
     is_revisit = 1;
     profile = "http://netpreserve.org/warc/1.1/revisit/server-not-modified";
+    /* Served from cache: the payload sits in the previous archive, not here. */
+    w->unbacked_revisits++;
   } else if (have_pdig && w->seen != NULL) {
     void *prev = NULL;
     if (coucal_read_pvoid(w->seen, pdig, &prev) && prev != NULL) {

src/htsweb.c

@@ -101,8 +101,8 @@ static void htsweb_sig_brpipe(int code) {
   /* ignore */
 }
 
-/* Number of background threads */
-static int background_threads = 0;
+/* Threads that never return; no wait may count on them draining. */
+static int nonjoinable_threads = 0;
 
 /* Server/client ping handling */
 static htsmutex pingMutex = HTSMUTEX_INIT;
@@ -297,15 +297,19 @@ int main(int argc, char *argv[]) {
 
   /* pinger */
   if (parentPid > 0) {
-    hts_newthread(client_ping, (void *) (uintptr_t) parentPid);
-    background_threads++; /* Do not wait for this thread! */
+    if (hts_newthread(client_ping, (void *) (uintptr_t) parentPid) == 0) {
+#ifndef _WIN32
+      nonjoinable_threads++; /* client_ping() only ever leaves through exit() */
+#endif
+    }
     smallserver_setpinghandler(pingHandler, NULL);
   }
 
   /* launch */
   ret = help_server(argv[1], defaultPort, bindAddr);
 
-  htsthread_wait_n(background_threads - 1);
+  /* Drain everything a mirror may still have in flight, the pinger aside. */
+  htsthread_wait_n(nonjoinable_threads);
   hts_uninit();
 
 #ifdef _WIN32
@@ -380,7 +384,6 @@ void webhttrack_main(char *cmd) {
   commandRunning = 1;
   DEBUG(fprintf(stderr, "commandRunning=1\n"));
   hts_newthread(back_launch_cmd, (void *) strdup(cmd));
-  background_threads++; /* Do not wait for this thread! */
 }
 
 void webhttrack_lock(void) {
@@ -421,8 +424,8 @@ static int webhttrack_runmain(httrackp * opt, int argc, char **argv) {
   /* Rock'in! */
   ret = hts_main2(argc, argv, opt);
 
-  /* Wait for pending threads to finish */
-  htsthread_wait_n(background_threads);
+  /* Wait for pending threads to finish; the pinger and this thread stay. */
+  htsthread_wait_n(nonjoinable_threads + 1);
 
   return ret;
 }

tests/01_engine-addlink.test

@@ -0,0 +1,7 @@
+#!/bin/bash
+#
+# an empty current path underflowed htsAddLink's codebase walk (#730).
+
+set -euo pipefail
+
+httrack -O /dev/null -#test=addlink | grep -q "addlink self-test OK"

tests/01_engine-structcheck.test

@@ -0,0 +1,12 @@
+#!/bin/bash
+#
+# structcheck(): the path-length guard, and the <name>.txt rename that once
+# built its target with an unbounded sprintf (#745).
+
+set -euo pipefail
+
+dir=$(mktemp -d)
+trap 'rm -rf "$dir"' EXIT
+
+httrack -O /dev/null -#test=structcheck "$dir" |
+    grep -q "structcheck self-test OK"

tests/65_port-siblings.test

@@ -79,6 +79,29 @@ done
 # 65535 is a valid port the old "< 65535" bound refused
 web_accepted 65535
 
+# --- htsserver returns from main() instead of blocking at exit -------------
+# The exit wait must exclude exactly the threads that never return (#753).
+# $tmp has no lang.def, so the server fails right after announcing and main()
+# reaches the wait on its own; rc, not EXITED, carries the verdict.
+
+web_exits() {
+    local rc=0
+
+    : >"$tmp/exit.log"
+    run_with_timeout 30 htsserver "$tmp" "$@" >"$tmp/exit.log" 2>&1 || rc=$?
+    grep -q "^EXITED" "$tmp/exit.log" ||
+        ! echo "FAIL: #753: htsserver ${*:-(no options)} never finished serving" || exit 1
+    # exactly 1, not merely "not 124": a crash or an assertf abort also escapes
+    # the wait, and would otherwise read as a pass
+    test "$rc" -eq 1 ||
+        ! echo "FAIL: #753: htsserver ${*:-(no options)} exited $rc, want 1" || exit 1
+}
+
+# no pinger: the excluded count went negative
+web_exits
+# a pinger, which never returns and so must stay excluded
+web_exits --ppid $$
+
 # --- proxytrack <proxy-addr:port> <ICP-addr:port> --------------------------
 # A bad argument falls through to the usage screen; it had no range check at
 # all, so 65616 quietly listened on port 80. A valid one binds and blocks.

tests/73_local-warc.test

@@ -1,11 +1,14 @@
 #!/bin/bash
 #
-# A --warc crawl writes a standards-conformant WARC/1.1 archive, and an
-# --update re-crawl of an unchanged (all-304) site emits revisit records.
-# The stdlib validator (no warcio) is the real gate: it byte-compares the
-# fresh page.html response body against what the server served, asserts the
-# encoding headers were stripped and the payload digest matches, then checks
-# the update pass turned the unchanged assets into revisits (no full response).
+# A --warc crawl writes a standards-conformant WARC/1.1 archive, and a re-crawl
+# of an unchanged (all-304) site emits revisit records. The stdlib validator (no
+# warcio) is the real gate: it byte-compares the fresh page.html response body
+# against what the server served, asserts the encoding headers were stripped and
+# the payload digest matches, then checks the second pass turned the unchanged
+# assets into revisits (no full response).
+#
+# That second pass archives under a name of its own, which is also the recovery
+# the engine points at when it refuses to strand a previous archive (#759).
 
 set -eu
 
@@ -14,8 +17,10 @@ set -eu
 # page.html body served by tests/local-server.py (route_mini304_page).
 export WARC_VALIDATE_BODY="page.html=3c68746d6c3e3c626f64793e74696e7920636163686561626c6520706167653c2f626f64793e3c2f68746d6c3e0a"
 export WARC_VALIDATE_NORESP="index.html page.html"
+export WARC_VALIDATE_UPDATE="warc-update.warc.gz"
 
-bash "$top_srcdir/tests/local-crawl.sh" --errors 0 --rerun --warc-validate \
+bash "$top_srcdir/tests/local-crawl.sh" --errors 0 --warc-validate \
+    --rerun-args '--warc-file warc-update' \
     --log-found 'no files updated' \
     --found 'mini304/index.html' --found 'mini304/page.html' \
     httrack 'BASEURL/mini304/index.html' --warc-file warc-out

tests/74_local-warc-wacz.test

@@ -6,9 +6,10 @@
 # the datapackage-digest chain (plus py-wacz/pywb when importable). The crawl
 # skips cleanly on a build without OpenSSL (no conformant SHA-256 -> no package).
 #
-# The cacheless second pass repackages over the .wacz the first one left: the
-# clobber, not the happy rename, is what breaks when a platform refuses to
-# rename onto an existing file (#726).
+# The cacheless (-C0) second pass repackages over the .wacz the first one left:
+# the clobber, not the happy rename, is what breaks when a platform refuses to
+# rename onto an existing file (#726). A cache-served pass keeps the previous
+# package instead, which is 97_local-warc-update-keep.
 
 set -eu
 

tests/93_local-changes.test

@@ -94,10 +94,7 @@ expect_counts_match() {
     echo "OK"
 }
 lines() { printf '%s\n' "$@" | sort; }
-# A connection killed before the status line surfaces differently per platform
-# (macOS truncates the file, #748; Linux leaves it), so reset.bin is asserted on
-# its own and kept out of the exact lists.
-listed_but_reset() { listed "$1" | grep -v '/reset\.bin$' | sort; }
+digest() { cksum <"$1" | tr -d '[:space:]'; }
 
 # --- pass 1: nothing to compare against --------------------------------------
 httrack "${common[@]}" -O "$out" --purge-old=0 "${base}/changes/index.html" \
@@ -120,6 +117,7 @@ grep -aq "first crawl" "${out}/hts-log.txt" || {
 }
 
 host="127.0.0.1_${port}"
+resetdigest=$(digest "${out}/${host}/changes/reset.bin")
 # A leftover from some earlier failed attempt, at a name pass 2 fetches for the
 # first time: on disk, but never part of the previous mirror, so it is new.
 printf 'leftover junk' >"${out}/${host}/changes/fresh.html"
@@ -144,21 +142,19 @@ expect "gone is doomed.html and the old redirect target" \
 expect "changed is exactly the four moved resources" \
     "$(lines "${host}/changes/coded.bin" "${host}/changes/index.html" \
         "${host}/changes/moved.bin" "${host}/changes/moved.html")" \
-    "$(listed_but_reset changed)"
+    "$(listed changed | sort)"
 # Re-served byte for byte, 200, no Last-Modified, no ETag. flaky.bin gets there
 # through a failed transfer and a retry, so its file is notified twice;
 # redirtarget.html arrives behind a 302. sized.html has a byte-identical
 # payload, but the rewritten link to the renamed redirect target makes the file
-# on disk change length.
-expect "unchanged is exactly the six stable resources" \
+# on disk change length. reset.bin never completes a transfer (#746), so its
+# previous copy stands.
+expect "unchanged is exactly the seven stable resources" \
     "$(lines "${host}/changes/codedstable.bin" "${host}/changes/flaky.bin" \
-        "${host}/changes/redirtarget.html" "${host}/changes/sized.html" \
-        "${host}/changes/stable.bin" "${host}/changes/stable.html")" \
-    "$(listed_but_reset unchanged)"
-# reset.bin never completes a transfer, so it drops out of new.lst with its
-# mirrored copy still there. Whatever else it is, it is not a deletion.
-expect "a failed re-fetch is never reported gone" "" \
-    "$(listed gone | grep '/reset\.bin$' || true)"
+        "${host}/changes/redirtarget.html" "${host}/changes/reset.bin" \
+        "${host}/changes/sized.html" "${host}/changes/stable.bin" \
+        "${host}/changes/stable.html")" \
+    "$(listed unchanged | sort)"
 expect_counts_match "pass 2 counts match its lists"
 
 # Every mirrored file appears once and only once, across all four buckets.
@@ -192,19 +188,15 @@ test -f "${out}/${host}/changes/doomed.html" || {
     exit 1
 }
 echo "OK"
-printf '[a failed transfer keeps its file] ..\t'
-test -f "${out}/${host}/changes/reset.bin" || {
-    echo "FAIL: reset.bin lost its previous copy"
-    exit 1
-}
-echo "OK"
+expect "a failed transfer keeps its bytes" "$resetdigest" \
+    "$(digest "${out}/${host}/changes/reset.bin")"
 
 # --- pass 3: the same run with purging on ------------------------------------
 httrack "${common[@]}" -O "$out" --update "${base}/changes/index.html" \
     >"${tmpdir}/log3" 2>&1
 expect "the report says files were purged" "true" "$(field purged)"
 expect "gone is the page that only pass 2 linked" \
-    "${host}/changes/transient.html" "$(listed_but_reset gone)"
+    "${host}/changes/transient.html" "$(listed gone | sort)"
 printf '[a purged file is off disk] ..\t'
 test ! -f "${out}/${host}/changes/transient.html" || {
     echo "FAIL: transient.html survived --purge-old"

tests/96_local-refetch-keep.test

@@ -0,0 +1,28 @@
+#!/bin/bash
+#
+# A re-fetch cut mid-header must leave the mirrored file alone (#748): unfixed,
+# the aborted read's leftover buffer lands on it. err.bin is the control, an
+# HTTP 500 on the same resource already keeping its copy; stay.bin answers
+# normally with a new body, so a fix that stopped overwriting anything fails.
+#
+# Purging is off: an update purge deletes both files for an unrelated reason
+# (#746), which would hide what this asserts.
+
+set -euo pipefail
+
+: "${top_srcdir:=..}"
+
+bash "$top_srcdir/tests/local-crawl.sh" \
+    --rerun-args '--update --purge-old=0' \
+    --log-found 'after 2 retries at link .*keep/page\.html' \
+    --log-found 'after 2 retries at link .*keep/data\.bin' \
+    --file-matches 'keep/page.html' 'KEEP-PAGE-V1' \
+    --file-not-matches 'keep/page.html' 'HTTP/1\.0 200' \
+    --file-matches 'keep/data.bin' 'KEEP-BIN-V1' \
+    --file-not-matches 'keep/data.bin' 'HTTP/1\.0 200' \
+    --file-min-bytes 'keep/data.bin' 2048 \
+    --file-matches 'keep/err.bin' 'KEEP-ERR-V1' \
+    --file-min-bytes 'keep/err.bin' 2048 \
+    --file-matches 'keep/stay.bin' 'KEEP-STAY-V2' \
+    --file-min-bytes 'keep/stay.bin' 2048 \
+    httrack 'BASEURL/keep/index.html' --retries=2

tests/97_local-warc-update-keep.test

@@ -0,0 +1,15 @@
+#!/bin/bash
+#
+# A cache-served second pass fetches no bodies, so it must leave the WARC, CDXJ
+# and WACZ of the first pass untouched (#759). The kept package is re-validated
+# to prove it still replays, not just that a same-named file is still there.
+# The one Error: line is the keep notice; a second means the pass also failed.
+
+set -eu
+
+: "${top_srcdir:=..}"
+
+bash "$top_srcdir/tests/local-crawl.sh" --errors 1 --archive-kept-on-rerun \
+    --rerun --wacz-validate --log-found 'WARC: this pass revisited' \
+    --found 'mini304/index.html' --found 'mini304/page.html' \
+    httrack 'BASEURL/mini304/index.html' --warc-file warc-out --wacz

tests/98_local-warc-segments.test

@@ -0,0 +1,24 @@
+#!/bin/bash
+#
+# The same keep/replace rule under --warc-max-size, where the archive is several
+# segments: a cache-served pass must leave every one alone, a cacheless pass must
+# swap every one in. Half a swap is the corruption one rename loop can produce,
+# and only a rotated archive exposes it (#759). --archive-min-files pins the two
+# segments plus the .cdx, so a shrinking crawl cannot quietly stop rotating.
+
+set -eu
+
+: "${top_srcdir:=..}"
+
+# The one Error: line is the keep notice; a second means the pass also failed.
+bash "$top_srcdir/tests/local-crawl.sh" --errors 1 --archive-kept-on-rerun \
+    --archive-min-files 3 --rerun --log-found 'WARC: this pass revisited' \
+    --found 'mini304/index.html' --found 'mini304/page.html' \
+    httrack 'BASEURL/mini304/index.html' --warc-file warc-out --warc-cdx \
+    --warc-max-size 600
+
+bash "$top_srcdir/tests/local-crawl.sh" --errors 0 --archive-replaced-on-rerun \
+    --archive-min-files 3 --rerun-args '-C0' \
+    --found 'mini304/index.html' --found 'mini304/page.html' \
+    httrack 'BASEURL/mini304/index.html' --warc-file warc-out --warc-cdx \
+    --warc-max-size 600

tests/Makefile.am

@@ -30,6 +30,7 @@ TEST_EXTENSIONS = .test
 TEST_LOG_COMPILER = $(BASH)
 TESTS = \
 	00_runnable.test \
+	01_engine-addlink.test \
 	01_engine-changes.test \
 	01_engine-charset.test \
 	01_engine-cmdline.test \
@@ -81,6 +82,7 @@ TESTS = \
 	01_engine-status.test \
 	01_engine-stripquery.test \
 	01_engine-strsafe.test \
+	01_engine-structcheck.test \
 	01_engine-syscharset.test \
 	01_engine-threadwait.test \
 	01_engine-topindex.test \
@@ -195,6 +197,9 @@ TESTS = \
 	92_local-proxytrack-ndx-fields.test \
 	93_local-changes.test \
 	94_local-single-file.test \
-	95_local-sitemap.test
+	95_local-sitemap.test \
+	96_local-refetch-keep.test \
+	97_local-warc-update-keep.test \
+	98_local-warc-segments.test
 
 CLEANFILES = check-network_sh.cache

tests/local-crawl.sh

@@ -34,6 +34,12 @@
 # httrack via --cookies-file, to exercise preloaded cookies.
 # --rerun-dead re-runs with the server stopped: the no-data rollback must
 # restore the previous hts-cache generation byte-identical.
+# --archive-kept-on-rerun: the second pass must leave the first pass's
+# .warc[.gz]/.cdx/.wacz byte-identical, having no bodies to replace them (#759).
+# --archive-replaced-on-rerun is its mirror: every one of them must have been
+# rewritten. Both also require no *.tmp left behind, and take an optional
+# --archive-min-files N guarding against a scenario that silently stopped
+# producing the segments it means to check.
 
 set -u
 
@@ -54,6 +60,9 @@ outdir_intl=
 rerun=
 rerun_args=
 rerun_dead=
+archive_kept=
+archive_replaced=
+archive_min_files=0
 tmpdir=
 serverpid=
 crawlpid=
@@ -118,6 +127,13 @@ while test "$pos" -lt "$nargs"; do
     --debug) verbose=1 ;;
     --rerun) rerun=1 ;;           # run httrack a second time (update pass) before auditing
     --rerun-dead) rerun_dead=1 ;; # re-run with the server stopped (cache rollback)
+    # the second pass must leave the first pass's archive files untouched
+    --archive-kept-on-rerun) archive_kept=1 ;;
+    --archive-replaced-on-rerun) archive_replaced=1 ;; # ...or rewrite all of them
+    --archive-min-files)
+        pos=$((pos + 1))
+        archive_min_files="${args[$pos]}"
+        ;;
     # validate the produced .warc.gz (see the validation block near the end)
     --warc-validate) warc_validate=1 ;;
     # validate the produced .wacz package (stdlib, plus py-wacz/pywb if present)
@@ -274,10 +290,26 @@ if test -n "$warc_validate"; then
     test -z "$w1" || cp "$w1" "${tmpdir}/warc-pass1.gz"
 fi
 
+# Snapshot the archive files the second pass must keep (or must replace).
+declare -a kept_files=()
+if test -n "${archive_kept}${archive_replaced}"; then
+    while read -r f; do
+        test -n "$f" || continue
+        cp "$f" "${tmpdir}/kept-${#kept_files[@]}" || die "could not snapshot $f"
+        kept_files+=("$f")
+    done < <(find "$mirrorroot" -maxdepth 2 \
+        \( -name '*.warc.gz' -o -name '*.warc' -o -name '*.cdx' -o -name '*.wacz' \) \
+        2>/dev/null | sort)
+    test "${#kept_files[@]}" -gt 0 ||
+        die "the first pass produced no archive to compare against"
+    test "${#kept_files[@]}" -ge "$archive_min_files" ||
+        die "only ${#kept_files[@]} archive file(s), wanted $archive_min_files: ${kept_files[*]}"
+fi
+
 # Poison the first-pass .wacz when a second pass follows: repackaging moves the
 # new archive over it, so the marker must be gone afterwards (#726). Poisoning
 # beats comparing the two packages, which can come out byte-identical.
-if test -n "$wacz_validate" && test -n "${rerun}${rerun_args}"; then
+if test -z "$archive_kept" && test -n "$wacz_validate" && test -n "${rerun}${rerun_args}"; then
     wacz_poisoned=$(find "$mirrorroot" -maxdepth 2 -name '*.wacz' 2>/dev/null | sort | tail -n1)
     test -z "$wacz_poisoned" || echo "$wacz_poison" >"$wacz_poisoned"
 fi
@@ -326,6 +358,29 @@ if test -n "$rerun_args"; then
     result "OK (second pass)"
 fi
 
+# --- optional: did the second pass keep, or replace, the whole archive? ------
+if test "${#kept_files[@]}" -gt 0; then
+    i=0
+    for f in "${kept_files[@]}"; do
+        if test -n "$archive_kept"; then
+            info "checking the second pass kept $(basename "$f")"
+            cmp -s "${tmpdir}/kept-${i}" "$f" ||
+                die "$(basename "$f") was rewritten: the previous archive was destroyed"
+        else
+            info "checking the second pass replaced $(basename "$f")"
+            cmp -s "${tmpdir}/kept-${i}" "$f" &&
+                die "$(basename "$f") still holds the first pass's bytes"
+        fi
+        result "OK"
+        i=$((i + 1))
+    done
+    # A leftover in-progress file is as bad: the next pass would silently eat it.
+    info "checking no in-progress archive was left behind"
+    leftover=$(find "$mirrorroot" -maxdepth 2 \( -name '*.warc.gz.tmp' -o -name '*.warc.tmp' \) 2>/dev/null | head -n1)
+    test -z "$leftover" || die "left behind $leftover"
+    result "OK"
+fi
+
 # --- optional dead pass: server stopped, the cache must survive the rollback --
 if test -n "$rerun_dead"; then
     zip="${out}/hts-cache/new.zip"
@@ -397,14 +452,18 @@ if test -n "$warc_validate"; then
         die "fresh WARC validation failed"
     result "OK"
 
-    # Final file: after an update pass the unchanged assets must be revisits.
-    if test -n "$rerun"; then
+    # After an update pass the unchanged assets must be revisits, in an archive
+    # of the pass's own (WARC_VALIDATE_UPDATE): a revisit-only pass never
+    # replaces the archive holding the bodies it would strand (#759).
+    if test -n "${WARC_VALIDATE_UPDATE:-}"; then
+        upd=$(find "$mirrorroot" -maxdepth 2 -name "$WARC_VALIDATE_UPDATE" 2>/dev/null | head -n1)
+        test -n "$upd" || die "no $WARC_VALIDATE_UPDATE produced under $mirrorroot"
         declare -a revargs=(--expect-revisit)
         for sub in ${WARC_VALIDATE_NORESP:-}; do
             revargs+=(--no-response-for "$sub")
         done
         info "validating update WARC (revisits)"
-        "$python" "$validator" "$(nativepath "$warc")" "${revargs[@]}" >&2 ||
+        "$python" "$validator" "$(nativepath "$upd")" "${revargs[@]}" >&2 ||
             die "update WARC validation failed"
         result "OK"
     fi

tests/local-server.py

@@ -1000,6 +1000,62 @@ class Handler(SimpleHTTPRequestHandler):
         v = 1 if self.refetch_pass() == 1 else 2
         self.send_raw(b"<html><body><p>STAY-V%d</p></body></html>" % v, "text/html")
 
+    # --- re-fetch cut mid-header, so nothing is stored (#746, #748) ---------
+    KEEP_PAGE = b"<html><body><p>KEEP-PAGE-V1</p></body></html>"
+    KEEP_BIN = b"KEEP-BIN-V1\n" + b"\x51\x52\x53\x54" * 512
+    KEEP_ERR = b"KEEP-ERR-V1\n" + b"\x61\x62\x63\x64" * 512
+
+    def send_cut_headers(self):
+        """Hang up mid-header: the response never becomes parseable."""
+        self.close_connection = True
+        try:
+            self.wfile.write(b"HTTP/1.0 200 OK\r\nContent-Ty")
+            self.wfile.flush()
+        except OSError:
+            pass
+        self.connection.close()
+
+    def route_keep_index(self):
+        self.refetch_pass()
+        self.send_html(
+            '\t<a href="page.html">page</a>\n'
+            '\t<a href="data.bin">data</a>\n'
+            '\t<a href="err.bin">err</a>\n'
+            '\t<a href="stay.bin">stay</a>\n'
+        )
+
+    def route_keep_page(self):
+        if self.refetch_pass() == 1:
+            self.send_raw(self.KEEP_PAGE, "text/html")
+        else:
+            self.send_cut_headers()
+
+    def route_keep_data(self):
+        if self.refetch_pass() == 1:
+            self.send_raw(self.KEEP_BIN, "application/octet-stream")
+        else:
+            self.send_cut_headers()
+
+    # Control: an HTTP error on the same resource already keeps the copy, so
+    # the cut-header routes above must end up indistinguishable from it.
+    def route_keep_err(self):
+        if self.refetch_pass() == 1:
+            self.send_raw(self.KEEP_ERR, "application/octet-stream")
+        else:
+            self.send_response(500)
+            self.send_header("Content-Type", "text/html")
+            self.send_header("Content-Length", "0")
+            self.end_headers()
+
+    # Control: answers normally, with a new body on pass 2, so a fix that
+    # stopped overwriting mirrored files altogether would be caught.
+    def route_keep_stay(self):
+        v = 1 if self.refetch_pass() == 1 else 2
+        self.send_raw(
+            b"KEEP-STAY-V%d\n" % v + b"\x71\x72\x73\x74" * 512,
+            "application/octet-stream",
+        )
+
     # Echo what httrack advertised, so a crawl can assert the header.
     def route_codec_ae(self):
         self.send_raw(
@@ -1950,6 +2006,11 @@ class Handler(SimpleHTTPRequestHandler):
         "/uptrunc/page.html": route_uptrunc_page,
         "/uptrunc/file.bin": route_uptrunc_file,
         "/uptrunc/stay.html": route_uptrunc_stay,
+        "/keep/index.html": route_keep_index,
+        "/keep/page.html": route_keep_page,
+        "/keep/data.bin": route_keep_data,
+        "/keep/err.bin": route_keep_err,
+        "/keep/stay.bin": route_keep_stay,
         "/types/index.html": route_types_index,
         "/types/control.php": route_types,
         "/types/photo.png": route_types,