Merge origin/master after #784; bound test 102's crawls

Every other network crawl runs under a deadline through local-crawl.sh. This
test drives its own FTP fixture directly, so a wedge would hang the job until
CI cancels it and discards the log (#796).

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

src/htsback.c

@@ -340,12 +340,61 @@ static int back_index_ready(httrackp * opt, struct_back * sback, const char *adr
 }
 
 static int slot_can_be_cached_on_disk(const lien_back * back) {
+  /* A pending backup or spool means the slot is not finalized, and the swap
+     would unlink it through back_clear_entry() (#771). */
+  if (back->tmpfile != NULL && back->tmpfile[0] != '\0')
+    return 0;
   return (back->status == STATUS_READY && back->locked == 0
           && back->url_sav[0] != '\0'
           && strcmp(back->url_sav, BACK_ADD_TEST) != 0);
   /* Note: not checking !IS_DELAYED_EXT(back->url_sav) or it will quickly cause the slots to be filled! */
 }
 
+int back_selftest_slot_swap(void) {
+  lien_back back;
+  int err = 0;
+
+#define CHECK(want, why)                                                       \
+  do {                                                                         \
+    if (slot_can_be_cached_on_disk(&back) != (want)) {                         \
+      fprintf(stderr, "backswap: expected %d for %s\n", (want), (why));        \
+      err = 1;                                                                 \
+    }                                                                          \
+  } while (0)
+
+  memset(&back, 0, sizeof(back));
+  back.status = STATUS_READY;
+  strcpybuff(back.url_sav, "/tmp/httrack-selftest.bin");
+  CHECK(1, "a plain ready slot");
+
+  back.tmpfile = back.tmpfile_buffer;
+  strcpybuff(back.tmpfile_buffer, "/tmp/httrack-selftest.bin.bak");
+  CHECK(0, "a slot still holding a re-fetch backup");
+
+  /* Callers clear a spent temporary by emptying the name, not the pointer. */
+  back.tmpfile_buffer[0] = '\0';
+  CHECK(1, "a slot whose temporary was already dropped");
+
+  back.tmpfile = NULL;
+  back.locked = 1;
+  CHECK(0, "a locked slot");
+  back.locked = 0;
+
+  back.status = STATUS_TRANSFER;
+  CHECK(0, "a slot still transferring");
+  back.status = STATUS_READY;
+
+  back.url_sav[0] = '\0';
+  CHECK(0, "a slot with no save name");
+
+  strcpybuff(back.url_sav, BACK_ADD_TEST);
+  CHECK(0, "the dummy test slot");
+#undef CHECK
+
+  printf("backswap self-test: %s\n", err ? "FAIL" : "OK");
+  return err;
+}
+
 /* Put all backing entries that are ready in the storage hashtable to spare space and CPU */
 int back_cleanup_background(httrackp * opt, cache_back * cache,
                             struct_back * sback) {
@@ -585,6 +634,29 @@ static int create_back_tmpfile(httrackp *opt, lien_back *const back,
   return 0;
 }
 
+/* Note: utf-8 */
+void back_refetch_backup(httrackp *opt, lien_back *const back) {
+  back->tmpfile = NULL;
+  if (fexist_utf8(back->url_sav)) {
+    hts_boolean saved = HTS_FALSE;
+
+    if (create_back_tmpfile(opt, back, "bak") == 0) {
+      /* clobber a .bak a killed run left behind, or the guard stays off for
+         good (#758) */
+      if (fexist_utf8(back->tmpfile))
+        hts_log_print(opt, LOG_WARNING, "replacing leftover backup %s",
+                      back->tmpfile);
+      saved = hts_rename_over(back->url_sav, back->tmpfile);
+    }
+    if (!saved) {
+      hts_log_print(opt, LOG_WARNING | LOG_ERRNO,
+                    "could not back up %s; an aborted re-fetch will lose it",
+                    back->url_sav);
+      back->tmpfile = NULL;
+    }
+  }
+}
+
 /* Did the fetch fail to produce a response, as opposed to the engine
    deliberately passing the resource over? Only the latter may be purged. */
 static hts_boolean back_transfer_failed(const int statuscode) {
@@ -3164,36 +3236,7 @@ void back_wait(struct_back * sback, httrackp * opt, cache_back * cache,
                                           back[i].url_sav, 1, 1,
                                           back[i].r.notmodified);
                               back[i].r.compressed = 0;
-                              /* Re-fetch over an existing file (#77 follow-up):
-                                 move the good copy aside before truncating it
-                                 so an aborted transfer can restore it. url_sav
-                                 is still written normally (file list intact).
-                               */
-                              back[i].tmpfile = NULL;
-                              if (fexist_utf8(back[i].url_sav)) {
-                                hts_boolean saved = HTS_FALSE;
-
-                                if (create_back_tmpfile(opt, &back[i], "bak") ==
-                                    0) {
-                                  /* clobber a .bak a killed run left behind,
-                                     or the guard stays off for good (#758) */
-                                  if (fexist_utf8(back[i].tmpfile))
-                                    hts_log_print(
-                                        opt, LOG_WARNING,
-                                        "replacing leftover backup %s",
-                                        back[i].tmpfile);
-                                  saved = hts_rename_over(back[i].url_sav,
-                                                          back[i].tmpfile);
-                                }
-                                if (!saved) {
-                                  hts_log_print(
-                                      opt, LOG_WARNING | LOG_ERRNO,
-                                      "could not back up %s; an aborted "
-                                      "re-fetch will lose it",
-                                      back[i].url_sav);
-                                  back[i].tmpfile = NULL;
-                                }
-                              }
+                              back_refetch_backup(opt, &back[i]);
                               if ((back[i].r.out =
                                    filecreate(&opt->state.strc,
                                               back[i].url_sav)) == NULL) {

src/htsback.h

@@ -139,6 +139,12 @@ int back_trylive(httrackp * opt, cache_back * cache, struct_back * sback,
                  const int p);
 int back_finalize(httrackp * opt, cache_back * cache, struct_back * sback,
                   const int p);
+/* Move the previous copy of back->url_sav to back->tmpfile so back_finalize()
+   can put it back when the re-fetch fails (#77 follow-up). Call right before
+   truncating url_sav; tmpfile stays NULL when there is nothing to save. */
+void back_refetch_backup(httrackp *opt, lien_back *const back);
+/* -#test=backswap: slots eligible for the on-disk ready table. */
+int back_selftest_slot_swap(void);
 void back_info(struct_back * sback, int i, int j, FILE * fp);
 void back_infostr(struct_back *sback, int i, int j, char *s, size_t size);
 LLint back_transferred(LLint add, struct_back * sback);

src/htsftp.c

@@ -624,6 +624,9 @@ int run_launch_ftp(FTPDownloadStruct * pStruct) {
           } else {
             file_notify(opt, back->url_adr, back->url_fil, back->url_sav, 1, 1,
                         0);
+            /* Every failure exit below would else leave the mirror truncated
+               (#771); the resume branch appends and needs no backup. */
+            back_refetch_backup(opt, back);
             back->r.fp = filecreate(&opt->state.strc, back->url_sav);
           }
           strcpybuff(back->info, "receiving");

src/htsselftest.c

@@ -6366,6 +6366,13 @@ static void threadwait_gated_thread(void *arg) {
   hts_mutexrelease(&threadwait_lock);
 }
 
+static int st_backswap(httrackp *opt, int argc, char **argv) {
+  (void) opt;
+  (void) argc;
+  (void) argv;
+  return back_selftest_slot_swap();
+}
+
 static int st_threadwait(httrackp *opt, int argc, char **argv) {
   int err = 0;
   int i, round;
@@ -6688,6 +6695,8 @@ static const struct selftest_entry {
      st_changes_race},
     {"threadwait", "", "htsthread_wait() joins threads spawned just before it",
      st_threadwait},
+    {"backswap", "", "which backlog slots may be swapped to the ready table",
+     st_backswap},
     {"pause", "", "randomized inter-file pause target self-test", st_pause},
     {"relative", "<link> <curr-file>", "relative link between two paths",
      st_relative},

tests/01_engine-backswap.test

@@ -0,0 +1,8 @@
+#!/bin/bash
+#
+
+set -euo pipefail
+
+# A ready slot still owning a temporary must stay in memory: swapping it out
+# clears the entry, which unlinks the re-fetch backup (#771).
+httrack -O /dev/null -#test=backswap | grep -q "backswap self-test: OK"

tests/102_local-ftp-refetch.test

@@ -0,0 +1,163 @@
+#!/bin/bash
+#
+# An FTP re-fetch used to truncate the mirror before the transfer, so a failed
+# one destroyed the previous copy (#771). keep.bin is cut short and empty.bin
+# gets nothing; both must keep their old bytes. stay.bin takes the same path and
+# completes, so a fix that merely stopped writing would fail here.
+
+set -euo pipefail
+
+: "${top_srcdir:=..}"
+testdir=$(cd "$(dirname "$0")" && pwd)
+# shellcheck source=tests/testlib.sh
+. "${testdir}/testlib.sh"
+
+python=$(find_python) || ! echo "python3 not found; skipping" >&2 || exit 77
+command -v httrack >/dev/null || {
+    echo "could not find httrack" >&2
+    exit 1
+}
+
+server=$(nativepath "${testdir}/ftp-server.py")
+tmpdir=$(mktemp -d "${TMPDIR:-/tmp}/httrack_ftp.XXXXXX")
+serverpid=
+cleanup() {
+    stop_server "$serverpid"
+    rm -rf "$tmpdir"
+}
+trap cleanup EXIT HUP INT QUIT PIPE TERM
+
+root="${tmpdir}/root"
+out="${tmpdir}/crawl"
+mode="${tmpdir}/mode"
+report="${out}/hts-changes.json"
+mkdir -p "$root" "$out"
+
+# The two generations differ in length, or a re-fetch that resumed at EOF
+# instead of truncating would leave the same bytes and pass for the wrong reason.
+write_bodies() {
+    local gen="$1" fill="$2" name
+    for name in keep empty stay; do
+        "$python" -c 'import sys; sys.stdout.buffer.write(
+            ("%s-FTP-%s " % (sys.argv[1].upper(), sys.argv[2])).encode()
+            + sys.argv[2][-1].encode() * int(sys.argv[3]))' "$name" "$gen" "$fill" \
+            >"${root}/${name}.bin"
+    done
+}
+write_bodies V1 4096
+: >"$mode"
+
+serverlog="${tmpdir}/server.out"
+"$python" "$server" --root "$(nativepath "$root")" \
+    --mode-file "$(nativepath "$mode")" >"$serverlog" 2>&1 &
+serverpid=$!
+port=
+for _ in $(seq 1 300); do
+    line=$(grep -m1 '^PORT ' "$serverlog" 2>/dev/null) && port="${line#PORT }" && break
+    kill -0 "$serverpid" 2>/dev/null || {
+        echo "ftp server exited early: $(cat "$serverlog")" >&2
+        exit 1
+    }
+    sleep 0.1
+done
+test -n "$port" || {
+    echo "could not discover ftp server port: $(cat "$serverlog")" >&2
+    exit 1
+}
+host="127.0.0.1_${port}"
+urls=()
+for name in keep empty stay; do
+    urls+=("ftp://127.0.0.1:${port}/${name}.bin")
+done
+# -c1: a parallel FTP crawl loses whole transfers to a separate, older bug in
+# the backlog slot swap (#797), which would flake this test on a loaded runner.
+common=(--quiet --disable-security-limits --robots=0 --timeout=20 --max-time=120
+    --retries=1 -c1 --changes)
+
+fail() {
+    echo "FAIL: $*" >&2
+    test ! -f "$report" || cat "$report" >&2
+    exit 1
+}
+ok() { echo "OK: $*"; }
+size_of() { wc -c <"$1" | tr -d '[:space:]'; }
+# Files listed under a report bucket, one "file:size" per line.
+listed() {
+    "$python" - "$report" "$1" <<'EOF' | tr -d '\r'
+import json
+import sys
+
+with open(sys.argv[1], encoding="utf-8") as fp:
+    report = json.load(fp)
+for entry in report[sys.argv[2]]:
+    print("%s:%s" % (entry["file"], entry.get("size", "none")))
+EOF
+}
+
+# --- pass 1: a healthy mirror ------------------------------------------------
+# Bounded like every local-crawl.sh pass: a wedged crawl must fail the test, not
+# hang the job until CI cancels it and discards the log (#796).
+run_with_timeout 300 httrack "${urls[@]}" -O "$out" "${common[@]}" \
+    >"${tmpdir}/log1" 2>&1
+for name in keep empty stay; do
+    test -s "${out}/${host}/${name}.bin" || fail "pass 1 did not mirror ${name}.bin"
+done
+mkdir "${tmpdir}/snap"
+cp "${out}/${host}"/*.bin "${tmpdir}/snap/"
+ok "pass 1 mirrored three files"
+
+# --- pass 2: the same three re-fetched, two of them failing -------------------
+# norest throughout: an accepted REST sends the client down the append path,
+# which is not the one that truncates.
+write_bodies V2 6000
+cat >"$mode" <<'EOF'
+/keep.bin truncate norest
+/empty.bin empty norest
+/stay.bin norest
+EOF
+run_with_timeout 300 httrack "${urls[@]}" -O "$out" "${common[@]}" --update \
+    >"${tmpdir}/log2" 2>&1
+
+# Both failures have to be the transfer dying mid-body: a connect or lookup
+# failure never opens the file, and everything below would pass vacuously.
+for name in keep empty; do
+    grep -aqE "Error:.*\"FTP file incomplete\".*${name}\.bin" "${out}/hts-log.txt" ||
+        fail "${name}.bin did not fail during the transfer"
+done
+ok "both re-fetches reached the body and failed there"
+
+for name in keep empty; do
+    cmp -s "${out}/${host}/${name}.bin" "${tmpdir}/snap/${name}.bin" ||
+        fail "${name}.bin differs from the copy pass 1 left ($(size_of \
+            "${out}/${host}/${name}.bin") bytes now)"
+done
+ok "a failed FTP transfer left the previously mirrored bytes alone"
+
+grep -aq "FTP-V2 " "${out}/${host}/stay.bin" ||
+    fail "stay.bin was not refreshed; the re-fetch writes nothing at all now"
+ok "a successful FTP re-fetch still replaces the file"
+
+leftover=$(find "${out}/${host}" -name '*.bak')
+test -z "$leftover" || fail "backup left behind: ${leftover}"
+ok "no backup temporary survived the pass"
+
+# --purge-old is on by default: a kept file the run never replaced must still be
+# in new.lst, or it is deleted as gone (#746).
+for name in keep empty stay; do
+    test -f "${out}/${host}/${name}.bin" || fail "${name}.bin was purged"
+done
+ok "the kept copies survived the update purge"
+
+# --- the change report is the mirror's own account of the pass ----------------
+test -s "$report" || fail "pass 2 wrote no ${report}"
+test -z "$(listed gone)" || fail "the report calls something gone: $(listed gone)"
+for name in keep empty; do
+    listed unchanged | grep -qx "${host}/${name}.bin:$(size_of "${tmpdir}/snap/${name}.bin")" ||
+        fail "${name}.bin is not reported unchanged at its previous size"
+    if listed changed | grep -q "^${host}/${name}.bin:"; then
+        fail "${name}.bin is reported changed as well"
+    fi
+done
+listed changed | grep -qx "${host}/stay.bin:$(size_of "${out}/${host}/stay.bin")" ||
+    fail "stay.bin is not reported changed"
+ok "the change report calls the kept files unchanged and the refreshed one changed"

tests/Makefile.am

@@ -5,7 +5,8 @@ EXTRA_DIST = $(TESTS) renamefail.c crawl-test.sh run-all-tests.sh check-network.
 	proxy-https-server.py socks5-server.py proxy-connect-server.py \
 	proxytestlib.py tls-stall-server.py warc-validate.py wacz-validate.py \
 	pty-resize.py \
-	local-crawl.sh local-server.py testlib.sh server.crt server.key \
+	local-crawl.sh local-server.py ftp-server.py testlib.sh \
+	server.crt server.key \
 	server-root/simple/basic.html server-root/simple/link.html \
 	server-root/stripquery/index.html server-root/stripquery/a.html \
 	server-root/fraglink/index.html server-root/fraglink/target.html \
@@ -58,6 +59,7 @@ TESTS = \
 	01_engine-filterdual.test \
 	01_engine-ftp-line.test \
 	01_engine-ftp-userpass.test \
+	01_engine-backswap.test \
 	01_engine-cacheindex.test \
 	01_engine-hashtable.test \
 	01_engine-header.test \
@@ -214,6 +216,7 @@ TESTS = \
 	98_local-warc-segments.test \
 	99_local-robots-error.test \
 	100_local-purge-longpath.test \
-	101_local-update-stale-bak.test
+	101_local-update-stale-bak.test \
+	102_local-ftp-refetch.test
 
 CLEANFILES = check-network_sh.cache

tests/ftp-server.py

@@ -0,0 +1,227 @@
+#!/usr/bin/env python3
+"""Minimal FTP server for the crawl tests: PASV only, binary RETR, LIST.
+
+Prints "PORT <n>" once bound, like local-server.py. --mode-file is re-read
+before every transfer, so a test can flip one path's behaviour between passes
+without restarting the server and moving the port (which names the mirror
+directory). Each line is "<path> <mode>...", path "*" matching everything:
+
+  truncate  send a prefix of the body, then drop the data connection
+  empty     open the data connection and send nothing
+  norest    answer REST with 500, so the client re-fetches from scratch
+"""
+
+import argparse
+import os
+import socket
+import sys
+import threading
+
+
+def reply(conn, text):
+    conn.sendall((text + "\r\n").encode("utf-8", "replace"))
+
+
+class Session(threading.Thread):
+    def __init__(self, conn, root, mode_file, log):
+        threading.Thread.__init__(self, daemon=True)
+        self.conn = conn
+        self.root = root
+        self.mode_file = mode_file
+        self.log = log
+        self.pasv = None
+        self.rest = 0
+        self.path = "/"  # named by SIZE/RETR; REST carries no path
+
+    def modes(self):
+        if not self.mode_file:
+            return set()
+        found = set()
+        try:
+            with open(self.mode_file, encoding="utf-8") as fp:
+                for line in fp:
+                    words = line.split()
+                    if words and words[0] in ("*", self.path):
+                        found |= set(words[1:])
+        except OSError:
+            pass
+        return found
+
+    # Resolve an FTP path argument under the root, refusing escapes.
+    def resolve(self, arg):
+        arg = arg.strip().strip('"')
+        self.path = "/" + arg.lstrip("/")
+        path = os.path.normpath(os.path.join(self.root, arg.lstrip("/")))
+        if path != self.root and not path.startswith(self.root + os.sep):
+            return None
+        return path
+
+    def open_pasv(self):
+        srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        srv.bind(("127.0.0.1", 0))
+        srv.listen(1)
+        self.pasv = srv
+        port = srv.getsockname()[1]
+        return "227 Entering Passive Mode (127,0,0,1,%d,%d)" % (port >> 8, port & 0xFF)
+
+    def accept_data(self):
+        if self.pasv is None:
+            return None
+        self.pasv.settimeout(30)
+        try:
+            data, _ = self.pasv.accept()
+        except (OSError, socket.timeout):
+            data = None
+        self.pasv.close()
+        self.pasv = None
+        return data
+
+    def send_body(self, body, modes):
+        data = self.accept_data()
+        if data is None:
+            reply(self.conn, "425 no data connection")
+            return
+        try:
+            if "empty" in modes:
+                pass
+            elif "truncate" in modes:
+                data.sendall(body[: max(1, len(body) // 8)])
+            else:
+                data.sendall(body)
+        except OSError:
+            pass
+        data.close()
+        if modes & {"empty", "truncate"}:
+            reply(self.conn, "426 transfer aborted")
+        else:
+            reply(self.conn, "226 Transfer complete")
+
+    def do_retr(self, arg):
+        path = self.resolve(arg)
+        if path is None or not os.path.isfile(path):
+            reply(self.conn, "550 no such file")
+            return
+        with open(path, "rb") as fp:
+            body = fp.read()
+        modes = self.modes()
+        reply(self.conn, "150 opening binary mode")
+        self.send_body(body[self.rest :], modes)
+        self.rest = 0
+
+    def do_list(self, arg):
+        if arg.startswith("-A"):
+            arg = arg[2:].strip()
+        path = self.resolve(arg or "/")
+        if path is None or not os.path.isdir(path):
+            reply(self.conn, "550 no such directory")
+            return
+        lines = []
+        for name in sorted(os.listdir(path)):
+            full = os.path.join(path, name)
+            if os.path.isdir(full):
+                lines.append("drwxr-xr-x 2 ftp ftp 4096 Jan 01 00:00 %s" % name)
+            else:
+                lines.append(
+                    "-rw-r--r-- 1 ftp ftp %d Jan 01 00:00 %s"
+                    % (os.path.getsize(full), name)
+                )
+        reply(self.conn, "150 opening ASCII mode")
+        self.send_body(("\r\n".join(lines) + "\r\n").encode("utf-8"), set())
+
+    def dispatch(self, verb, arg):
+        conn = self.conn
+        if verb in ("USER", "PASS", "TYPE", "NOOP"):
+            reply(conn, "200 ok")
+        elif verb == "SYST":
+            reply(conn, "215 UNIX Type: L8")
+        elif verb == "PWD":
+            reply(conn, '257 "/"')
+        elif verb == "CWD":
+            reply(conn, "250 ok")
+        elif verb == "PASV":
+            reply(conn, self.open_pasv())
+        elif verb == "EPSV":
+            reply(conn, "500 not supported")
+        elif verb == "SIZE":
+            path = self.resolve(arg)
+            if path and os.path.isfile(path):
+                reply(conn, "213 %d" % os.path.getsize(path))
+            else:
+                reply(conn, "550 no such file")
+        elif verb == "REST":
+            if "norest" in self.modes():
+                reply(conn, "500 REST not understood")
+            else:
+                try:
+                    self.rest = int(arg)
+                except ValueError:
+                    self.rest = 0
+                reply(conn, "350 restarting")
+        elif verb == "RETR":
+            self.do_retr(arg)
+        elif verb in ("LIST", "NLST"):
+            self.do_list(arg)
+        elif verb == "QUIT":
+            reply(conn, "221 bye")
+            return False
+        else:
+            reply(conn, "500 unknown command")
+        return True
+
+    def run(self):
+        conn = self.conn
+        buf = b""
+        try:
+            reply(conn, "220 httrack test ftp")
+            while True:
+                while b"\r\n" not in buf:
+                    chunk = conn.recv(4096)
+                    if not chunk:
+                        return
+                    buf += chunk
+                line, buf = buf.split(b"\r\n", 1)
+                line = line.decode("utf-8", "replace")
+                self.log(line)
+                verb, _, arg = line.partition(" ")
+                if not self.dispatch(verb.upper(), arg):
+                    return
+        except OSError:
+            return
+        finally:
+            if self.pasv is not None:
+                self.pasv.close()
+            conn.close()
+
+
+def main():
+    ap = argparse.ArgumentParser()
+    ap.add_argument("--root", required=True)
+    ap.add_argument("--mode-file")
+    ap.add_argument("--log")
+    args = ap.parse_args()
+
+    logfp = open(args.log, "a", encoding="utf-8") if args.log else None
+    log_lock = threading.Lock()
+
+    def log(line):
+        if logfp:
+            with log_lock:
+                logfp.write(line + "\n")
+                logfp.flush()
+
+    srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    srv.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+    srv.bind(("127.0.0.1", 0))
+    srv.listen(16)
+    sys.stdout.reconfigure(newline="\n")  # the launcher parses PORT, CRLF breaks it
+    sys.stdout.write("PORT %d\n" % srv.getsockname()[1])
+    sys.stdout.flush()
+
+    root = os.path.abspath(args.root)
+    while True:
+        conn, _ = srv.accept()
+        Session(conn, root, args.mode_file, log).start()
+
+
+if __name__ == "__main__":
+    main()