A stale .bak silently disables the re-fetch backup on Windows

src/htsback.c

@@ -3171,10 +3171,26 @@ void back_wait(struct_back * sback, httrackp * opt, cache_back * cache,
                                */
                               back[i].tmpfile = NULL;
                               if (fexist_utf8(back[i].url_sav)) {
-                                if (create_back_tmpfile(opt, &back[i], "bak") !=
-                                        0 ||
-                                    RENAME(back[i].url_sav, back[i].tmpfile) !=
-                                        0) {
+                                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;
                                 }
                               }

src/htscache.c

@@ -855,13 +855,6 @@ static htsblk cache_readex_new(httrackp * opt, cache_back * cache,
   return r;
 }
 
-// lecture d'un fichier dans le cache
-// si save==null alors test unqiquement
-static int hts_rename(httrackp * opt, const char *a, const char *b) {
-  hts_log_print(opt, LOG_DEBUG, "Cache: rename %s -> %s (%p %p)", a, b, a, b);
-  return RENAME(a, b);
-}
-
 /* Open the cache ZIP via hts_fopen_utf8 so a non-ASCII path_log isn't mangled
    to ANSI (#630); 64-bit funcs keep multi-GB caches whole on Windows LLP64. */
 static voidpf ZCALLBACK hts_zip_fopen_utf8(voidpf opaque, const void *filename,
@@ -991,29 +984,15 @@ void cache_init(cache_back * cache, httrackp * opt) {
               OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt),
               StringBuff(opt->path_log),
               "hts-cache/new.zip")))) { // a previous cache exists.. rename it
-        /* Remove OLD cache */
-        if (fexist_utf8(fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt),
-                                StringBuff(opt->path_log),
-                                "hts-cache/old.zip"))) {
-          if (UNLINK(fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt),
-                             StringBuff(opt->path_log), "hts-cache/old.zip")) !=
-              0) {
-            hts_log_print(opt, LOG_WARNING | LOG_ERRNO,
-                          "Cache: error while moving previous cache");
-          }
-        }
-
-        /* Rename */
-        if (hts_rename
-            (opt,
-             fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), StringBuff(opt->path_log),
-                     "hts-cache/new.zip"), fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt),
-                                                   StringBuff(opt->path_log),
-                                                   "hts-cache/old.zip")) != 0) {
+        if (!hts_rename_over(
+                fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt),
+                        StringBuff(opt->path_log), "hts-cache/new.zip"),
+                fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt),
+                        StringBuff(opt->path_log), "hts-cache/old.zip"))) {
           hts_log_print(opt, LOG_WARNING | LOG_ERRNO,
                         "Cache: error while moving previous cache");
         } else {
-          hts_log_print(opt, LOG_DEBUG, "Cache: successfully renamed");
+          hts_log_print(opt, LOG_DEBUG, "Cache: rotated new.zip to old.zip");
         }
       }
     } else {

tests/101_local-update-stale-bak.test

@@ -0,0 +1,27 @@
+#!/bin/bash
+#
+# A .bak left behind by a killed run must not disable the #77 re-fetch backup
+# (#758): a leftover file (slow.bin.bak) has to be clobbered so the -M abort can
+# still restore the pass-1 copy, and a leftover the engine cannot clobber
+# (fast.bin.bak, planted as a directory) has to be reported instead of silently
+# truncating with no safety net. Only the Windows leg arms the first half:
+# POSIX rename() clobbers on its own.
+
+set -euo pipefail
+
+: "${top_srcdir:=..}"
+
+bash "$top_srcdir/tests/local-crawl.sh" \
+    --plant-file bigtrunc/slow.bin.bak \
+    --plant-dir bigtrunc/fast.bin.bak \
+    --rerun-args '--update -M400000' \
+    --log-found 'More than 400000 bytes have been transferred.. giving up' \
+    --log-found 'replacing leftover backup .*slow\.bin\.bak' \
+    --log-found 'could not back up .*fast\.bin' \
+    --log-not-found 'could not back up .*slow\.bin' \
+    --log-not-found 'could not restore' \
+    --found bigtrunc/slow.bin \
+    --found bigtrunc/fast.bin \
+    --file-min-bytes bigtrunc/slow.bin 655360 \
+    --file-not-matches bigtrunc/slow.bin 'stale-leftover' \
+    httrack 'BASEURL/bigtrunc/index.html' -c2

tests/37_local-cache-outage.test

@@ -1,12 +1,14 @@
 #!/bin/bash
 #
 # An update run against a dead server must not destroy the cache: the no-data
-# rollback restores the previous hts-cache generation (zip caches lost it).
+# rollback restores the previous hts-cache generation (zip caches lost it). The
+# live update pass leaves an old.zip behind, so the dead pass rotates onto an
+# existing generation instead of into empty space.
 
 set -eu
 
 : "${top_srcdir:=..}"
 
-bash "$top_srcdir/tests/local-crawl.sh" --errors 0 --rerun-dead \
+bash "$top_srcdir/tests/local-crawl.sh" --errors 0 --rerun --rerun-dead \
     --found 'simple/basic.html' \
     httrack 'BASEURL/simple/basic.html'

tests/Makefile.am

@@ -200,6 +200,7 @@ TESTS = \
 	95_local-sitemap.test \
 	96_local-refetch-keep.test \
 	97_local-warc-update-keep.test \
-	98_local-warc-segments.test
+	98_local-warc-segments.test \
+	101_local-update-stale-bak.test
 
 CLEANFILES = check-network_sh.cache

tests/local-crawl.sh

@@ -40,6 +40,9 @@
 # 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.
+# --plant-file/--plant-dir drop a regular file (holding $plant_poison) or a
+# directory at PATH under the host root between the passes, to hand the second
+# pass leftovers a killed run would have left (#758).
 
 set -u
 
@@ -68,6 +71,7 @@ serverpid=
 crawlpid=
 wacz_poisoned=
 wacz_poison="stale-wacz-that-a-second-pass-must-replace"
+plant_poison="stale-leftover-that-a-second-pass-must-clobber"
 
 function warning {
     echo "** $*" >&2
@@ -96,6 +100,18 @@ function cleanup {
     fi
 }
 
+hostroot=
+function find_hostroot {
+    local cand
+    for cand in "${mirrorroot}/127.0.0.1_${port}" "${mirrorroot}/127.0.0.1"; do
+        if test -d "$cand"; then
+            hostroot="$cand"
+            return 0
+        fi
+    done
+    die "could not find host root under $out"
+}
+
 function assert_equals {
     info "$1"
     if test ! "$2" == "$3"; then
@@ -118,6 +134,7 @@ tmpdir=$(mktemp -d "${tmptopdir}/httrack_local.XXXXXX") || die "could not create
 # --- parse leading control flags --------------------------------------------
 declare -a audit=()
 declare -a cookies=()
+declare -a plants=()
 scheme=http
 pos=0
 args=("$@")
@@ -157,6 +174,10 @@ while test "$pos" -lt "$nargs"; do
         pos=$((pos + 1))
         cookies+=("${args[$pos]}")
         ;;
+    --plant-file | --plant-dir)
+        plants+=("${args[$pos]}" "${args[$((pos + 1))]}")
+        pos=$((pos + 1))
+        ;;
     --rerun-args)
         pos=$((pos + 1))
         rerun_args="${args[$pos]}"
@@ -314,6 +335,23 @@ if test -z "$archive_kept" && test -n "$wacz_validate" && test -n "${rerun}${rer
     test -z "$wacz_poisoned" || echo "$wacz_poison" >"$wacz_poisoned"
 fi
 
+# --- plant leftovers the second pass has to deal with ------------------------
+if test "${#plants[@]}" -gt 0; then
+    find_hostroot
+    i=0
+    while test "$i" -lt "${#plants[@]}"; do
+        path="${hostroot}/${plants[$((i + 1))]}"
+        info "planting ${plants[$i]} ${plants[$((i + 1))]}"
+        if test "${plants[$i]}" = "--plant-dir"; then
+            mkdir -p "$path" || die "could not create $path"
+        else
+            echo "$plant_poison" >"$path" || die "could not write $path"
+        fi
+        result "OK"
+        i=$((i + 2))
+    done
+fi
+
 # --- optional second pass: re-mirror into the same dir (cache/update path) ----
 if test -n "$rerun"; then
     info "re-running httrack (update pass)"
@@ -418,14 +456,7 @@ if test -n "$rerun_dead"; then
 fi
 
 # --- discover the single host root (127.0.0.1_<port> or 127.0.0.1) -----------
-hostroot=
-for cand in "${mirrorroot}/127.0.0.1_${port}" "${mirrorroot}/127.0.0.1"; do
-    if test -d "$cand"; then
-        hostroot="$cand"
-        break
-    fi
-done
-test -n "$hostroot" || die "could not find host root under $out"
+find_hostroot
 debug "host root: $hostroot"
 
 # --- optional WARC validation (stdlib validator, no warcio) ------------------