Store the cache self-tests' X-Save verbatim, and feed the sniff BOM as hex

The cache self-tests built the X-Save path with fconcat(), which rewrites
every '/' to '\' on Windows -- including the path_html_utf8 prefix the
reader matches on. cache_readex then took the pre-3.40 compat branch and
re-rooted the name into a path that does not exist, so -#test=cache's
disk-fallback read and -#test=cache-corrupt's headers-only and in-memory
cases all failed on MSVC builds. The engine keeps save names in '/' form and
DOS-ifies only at the syscall boundary; the self-tests now do the same.

The sniff test passed a UTF-8 BOM as raw bytes through argv, where Windows
routes it via the ANSI codepage and U+FEFF has no mapping, so it arrived as
'?'. Use the hex: body form the self-test already accepts.

Both are self-test bugs: neither path is reachable from the crawler, which
never sniffs argv and never DOS-ifies a stored save name.

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

src/htsback.c

@@ -557,6 +557,14 @@ static int create_back_tmpfile(httrackp *opt, lien_back *const back,
   return 0;
 }
 
+/* Move src onto dst; RENAME does not clobber an existing target on Windows. */
+static hts_boolean replace_file(const char *src, const char *dst) {
+  if (RENAME(src, dst) == 0)
+    return HTS_TRUE;
+  (void) UNLINK(dst);
+  return RENAME(src, dst) == 0 ? HTS_TRUE : HTS_FALSE;
+}
+
 /* Commit or restore a re-fetch backup (#77 follow-up): a re-fetch over an
    existing file moved the good copy to back->tmpfile before truncating url_sav.
    commit keeps the new file and drops the backup; else restore it so an aborted
@@ -572,10 +580,9 @@ static void back_finalize_backup(httrackp *opt, lien_back *const back,
       fclose(back->r.out);
       back->r.out = NULL;
     }
-    (void) UNLINK(back->url_sav); /* drop the failed partial */
-    /* On rename failure keep the backup: it still holds the good copy, so
-       losing it would be worse than an orphaned temp. */
-    if (RENAME(back->tmpfile, back->url_sav) != 0)
+    /* On failure keep the backup: an orphaned temp beats losing the good copy.
+     */
+    if (!replace_file(back->tmpfile, back->url_sav))
       hts_log_print(opt, LOG_WARNING | LOG_ERRNO,
                     "could not restore %s; previous copy kept as %s",
                     back->url_sav, back->tmpfile);
@@ -676,24 +683,45 @@ int back_finalize(httrackp * opt, cache_back * cache, struct_back * sback,
               if (back[p].url_sav[0]) {
                 const hts_codec codec =
                     hts_codec_parse(back[p].r.contentencoding);
+                /* Never decode over url_sav: a failed decode would destroy the
+                   copy an --update re-fetch is supposed to refresh (#557). */
+                char BIGSTK unpacked[HTS_URLMAXSIZE * 2 + 4]; // room for ".u"
                 LLint size;
 
-                file_notify(opt, back[p].url_adr, back[p].url_fil,
-                            back[p].url_sav, 1, 1, back[p].r.notmodified);
-                filecreateempty(&opt->state.strc, back[p].url_sav);     // filenote & co
+                snprintf(unpacked, sizeof(unpacked), "%s.u", back[p].url_sav);
                 if ((size = hts_codec_unpack(codec, back[p].tmpfile,
-                                             back[p].url_sav)) >= 0) {
+                                             unpacked)) >= 0) {
                   back[p].r.size = back[p].r.totalsize = size;
-                  // fichier -> mémoire
                   if (!back[p].r.is_write) {
+                    // fichier -> mémoire ; le fichier est écrit plus tard
                     deleteaddr(&back[p].r);
-                    back[p].r.adr = readfile_utf8(back[p].url_sav);
+                    back[p].r.adr = readfile_utf8(unpacked);
                     if (!back[p].r.adr) {
                       back[p].r.statuscode = STATUSCODE_INVALID;
                       strcpybuff(back[p].r.msg,
                                  "Read error when decompressing");
                     }
-                    UNLINK(back[p].url_sav);
+                    UNLINK(unpacked);
+                  } else if (replace_file(unpacked, back[p].url_sav)) {
+                    /* The temp bypassed filecreate(), which is what chmods. */
+#ifndef _WIN32
+                    chmod(back[p].url_sav, HTS_ACCESS_FILE);
+#endif
+                    file_notify(opt, back[p].url_adr, back[p].url_fil,
+                                back[p].url_sav, 1, 1, back[p].r.notmodified);
+                    filenote(&opt->state.strc, back[p].url_sav, NULL);
+                  } else {
+                    back[p].r.statuscode = STATUSCODE_INVALID;
+                    strcpybuff(back[p].r.msg,
+                               "Write error when decompressing (can not rename "
+                               "the temporary file)");
+                    /* Keep the decoded body: the failed replace may have
+                       removed the previous copy, leaving this as the only one.
+                     */
+                    hts_log_print(
+                        opt, LOG_WARNING | LOG_ERRNO,
+                        "could not replace %s; decoded copy kept as %s",
+                        back[p].url_sav, unpacked);
                   }
                 } else {
                   back[p].r.statuscode = STATUSCODE_INVALID;
@@ -702,12 +730,18 @@ int back_finalize(httrackp * opt, cache_back * cache, struct_back * sback,
                                ? "Unsupported Content-Encoding (%s)"
                                : "Error when decompressing (%s)",
                            back[p].r.contentencoding);
-                  /* Drop the undecoded body and its placeholder so the writer
-                     can't commit the coded bytes as the page. */
+                  /* Drop the undecoded body so the writer can't commit the
+                     coded bytes as the page; url_sav is left untouched. */
                   if (!back[p].r.is_write)
                     deleteaddr(&back[p].r);
-                  UNLINK(back[p].url_sav);
+                  UNLINK(unpacked);
                 }
+                /* A failed decode keeps the previously-mirrored copy: note it,
+                   or the update purge (in old.lst, absent from new.lst) would
+                   delete what we just took care not to overwrite. */
+                if (back[p].r.statuscode == STATUSCODE_INVALID &&
+                    fexist_utf8(back[p].url_sav))
+                  filenote(&opt->state.strc, back[p].url_sav, NULL);
               }
               /* ensure that no remaining temporary file exists */
               unlink(back[p].tmpfile);

src/htscache_selftest.c

@@ -232,11 +232,10 @@ static int disk_fallback_selftest(httrackp *opt) {
   static const char body[] = "BINARY-on-disk-body-0123456789-no-trailing-nul";
   const size_t body_len = sizeof(body) - 1;
 
-  /* X-Save must start with path_html_utf8 so the reader resolves it verbatim
-     (otherwise it re-roots it as a pre-3.40 relative path); then the body we
-     create at fconv(save) is exactly where cache_readex looks for it. */
-  fconcat(save, sizeof(save), StringBuff(opt->path_html_utf8),
-          "example.com/blob.bin");
+  /* a DOS-ified X-Save loses the path_html_utf8 prefix the reader matches on,
+     and gets re-rooted as a pre-3.40 relative path; fconv() only on access */
+  concat(save, sizeof(save), StringBuff(opt->path_html_utf8),
+         "example.com/blob.bin");
 
   /* write only the header (X-In-Cache: 0); the body stays on disk */
   selftest_open_for_write(&cache, opt);
@@ -1253,8 +1252,9 @@ static void corrupt_build_disk(httrackp *opt) {
   memset(corrupt_body_a, 'a', sizeof(corrupt_body_a) - 1);
   remove(reconcile_st_path(opt, "hts-cache/new.zip"));
   remove(reconcile_st_path(opt, "hts-cache/old.zip"));
-  fconcat(save, sizeof(save), StringBuff(opt->path_html_utf8),
-          CORRUPT_ADR "/victim.bin");
+  /* X-Save stored verbatim: see disk_fallback_selftest */
+  concat(save, sizeof(save), StringBuff(opt->path_html_utf8),
+         CORRUPT_ADR "/victim.bin");
   selftest_open_for_write(&cache, opt);
   store_entry(opt, &cache, CORRUPT_ADR, "/canary.html", "canary.html", 200,
               "OK", "text/html", "utf-8", "", "", "", "", corrupt_body_a,

src/httrack.rc

@@ -1,5 +1,5 @@
-// Version resource for httrack.exe, the command line program. See version.rc.
-#define VER_FILE_DESCRIPTION "HTTrack Website Copier (command line)"
-#define VER_ORIGINAL_FILENAME "httrack.exe"
-#define VER_FILETYPE VFT_APP
-#include "version.rc"
+// Version resource for httrack.exe, the command line program. See version.rc.
+#define VER_FILE_DESCRIPTION "HTTrack Website Copier (command line)"
+#define VER_ORIGINAL_FILENAME "httrack.exe"
+#define VER_FILETYPE VFT_APP
+#include "version.rc"

src/libhttrack.rc

@@ -1,5 +1,5 @@
-// Version resource for libhttrack.dll. See version.rc.
-#define VER_FILE_DESCRIPTION "HTTrack Website Copier engine"
-#define VER_ORIGINAL_FILENAME "libhttrack.dll"
-#define VER_FILETYPE VFT_DLL
-#include "version.rc"
+// Version resource for libhttrack.dll. See version.rc.
+#define VER_FILE_DESCRIPTION "HTTrack Website Copier engine"
+#define VER_ORIGINAL_FILENAME "libhttrack.dll"
+#define VER_FILETYPE VFT_DLL
+#include "version.rc"

tests/01_engine-sniff.test

@@ -35,7 +35,8 @@ chk image/avif hex:0000001C6674797061766966 "$yes"
 chk image/avif hex:0000001C6674797068656963 "$no" # heic brand is not avif
 chk image/heic hex:0000001C6674797068656963 "$yes"
 chk image/svg+xml '<svg xmlns="x">' "$yes"
-chk image/svg+xml $'\xef\xbb\xbf  <?xml version="1.0"?>' "$yes" # BOM+ws skip
+# BOM+ws skip; hex, as Windows argv cannot carry the raw BOM through the ANSI codepage
+chk image/svg+xml hex:EFBBBF20203C3F786D6C2076657273696F6E3D22312E30223F3E "$yes"
 
 # audio / video
 chk audio/mpeg 'ID3xxx' "$yes"

tests/51_local-update-codec.test

@@ -0,0 +1,38 @@
+#!/bin/bash
+#
+# A coded re-fetch that fails to decode must not destroy the mirrored file (#557).
+# Pass 1 mirrors from valid gzip; pass 2 (--update) serves an undecodable body for
+# mem.html (in-memory), disk.bin (direct-to-disk) and unsup.html (a coding we have
+# no decoder for). All three must keep their pass-1 content. fresh.html and
+# freshdisk.bin decode on both passes: they are the controls that a good update
+# still lands, in memory and direct-to-disk (the latter renaming the decoded temp
+# over an existing file). Unfixed, the decode target is the mirror itself, so the
+# first three are lost.
+
+set -euo pipefail
+
+: "${top_srcdir:=..}"
+
+# A restrictive umask makes the decoded direct-to-disk file's mode observable:
+# it is committed by renaming a temp, so it needs the chmod filecreate() does.
+umask 077
+
+bash "$top_srcdir/tests/local-crawl.sh" \
+    --rerun-args '--update' \
+    --errors 3 \
+    --log-found 'decompressing.*upcodec/mem\.html' \
+    --log-found 'decompressing.*upcodec/disk\.bin' \
+    --log-found 'Unsupported Content-Encoding.*upcodec/unsup\.html' \
+    --file-matches 'upcodec/mem.html' 'MIRRORED-MEM-V1' \
+    --file-matches 'upcodec/disk.bin' 'MIRRORED-DISK-V1' \
+    --file-min-bytes 'upcodec/disk.bin' 32768 \
+    --file-mode 'upcodec/disk.bin' 644 \
+    --file-matches 'upcodec/unsup.html' 'MIRRORED-UNSUP-V1' \
+    --file-matches 'upcodec/fresh.html' 'FRESH-V2' \
+    --file-not-matches 'upcodec/fresh.html' 'FRESH-V1' \
+    --file-mode 'upcodec/fresh.html' 644 \
+    --file-matches 'upcodec/freshdisk.bin' 'FRESHDISK-V2' \
+    --file-not-matches 'upcodec/freshdisk.bin' 'FRESHDISK-V1' \
+    --file-min-bytes 'upcodec/freshdisk.bin' 32768 \
+    --file-mode 'upcodec/freshdisk.bin' 644 \
+    httrack 'BASEURL/upcodec/index.html'

tests/Makefile.am

@@ -124,6 +124,7 @@ TESTS = \
 	47_local-crange-overflow.test \
 	48_local-crange-memresume.test \
 	49_local-cookiewall.test \
-	50_local-contentcodings.test
+	50_local-contentcodings.test \
+	51_local-update-codec.test
 
 CLEANFILES = check-network_sh.cache

tests/local-crawl.sh

@@ -17,7 +17,7 @@
 #       --errors N --errors-content N --files N --found PATH ... --directory PATH ... \
 #       --log-found REGEX ... --log-not-found REGEX ... \
 #       --file-matches PATH REGEX ... --file-not-matches PATH REGEX ... \
-#       --file-min-bytes PATH N --max-mirror-bytes N \
+#       --file-min-bytes PATH N --file-mode PATH OCTAL --max-mirror-bytes N \
 #       httrack BASEURL/some/path [httrack-args...]
 # --errors counts every "Error:" log line; --errors-content drops transient
 # network failures (codes -2..-6) that flake on busy loopback under -c8.
@@ -26,6 +26,7 @@
 # --file-matches/--file-not-matches grep (ERE) a mirrored file (PATH under the
 # host root), to assert rewritten link/content survived the crawl.
 # --file-min-bytes asserts a mirrored file (PATH) is at least N bytes.
+# --file-mode asserts its octal permissions (e.g. 644); POSIX hosts only.
 # --rerun-args runs a second pass (same server and mirror dir) with the given
 # extra httrack args appended, e.g. an --update run under a cap.
 # --cookie writes a Netscape cookies.txt (scoped to the discovered host:port,
@@ -141,7 +142,7 @@ while test "$pos" -lt "$nargs"; do
         audit+=("${args[$pos]}" "${args[$((pos + 1))]}")
         pos=$((pos + 1))
         ;;
-    --file-matches | --file-not-matches | --file-min-bytes)
+    --file-matches | --file-not-matches | --file-min-bytes | --file-mode)
         audit+=("${args[$pos]}" "${args[$((pos + 1))]}" "${args[$((pos + 2))]}")
         pos=$((pos + 2))
         ;;
@@ -319,9 +320,10 @@ done
 test -n "$hostroot" || die "could not find host root under $out"
 debug "host root: $hostroot"
 
-# No crawl, even a cancelled one, may leave .delayed temporaries (#107, #483).
-info "checking for leftover .delayed files"
-leftovers=$(find "$out" -name '*.delayed' 2>/dev/null | head -5)
+# No crawl, even a cancelled one, may leave engine temporaries: .delayed (#107,
+# #483), or the .z/.u content-coding temps (#557).
+info "checking for leftover engine temporaries"
+leftovers=$(find "$out" \( -name '*.delayed' -o -name '*.z' -o -name '*.u' \) 2>/dev/null | head -5)
 if test -z "$leftovers"; then result "OK"; else
     result "leftover: $leftovers"
     exit 1
@@ -436,6 +438,17 @@ while test "$i" -lt "${#audit[@]}"; do
             exit 1
         fi
         ;;
+    --file-mode)
+        path="${audit[$((i + 1))]}"
+        i=$((i + 2))
+        mode=$(stat -c '%a' "${hostroot}/${path}" 2>/dev/null ||
+            stat -f '%Lp' "${hostroot}/${path}" 2>/dev/null)
+        info "checking ${path} mode ${mode:-none} is ${audit[$i]}"
+        if test "$mode" = "${audit[$i]}"; then result "OK"; else
+            result "wrong mode"
+            exit 1
+        fi
+        ;;
     esac
     i=$((i + 1))
 done

tests/local-server.py

@@ -695,6 +695,80 @@ class Handler(SimpleHTTPRequestHandler):
             extra_headers=[("Content-Encoding", "compress")],
         )
 
+    # --- coded re-fetch that fails to decode (#557) -------------------------
+    # Pass 1 mirrors each file from a valid gzip body; pass 2 (--update) serves
+    # a body that cannot be decoded. The previously-mirrored copy must survive.
+    # fresh.html is the control: its pass-2 body decodes, so it must be updated.
+    UPCODEC_SEEN = {}
+
+    def upcodec_pass(self):
+        """1 on the first body fetch of this path, 2 on the next ones. HEADs
+        don't count, so a stray one can't shift which pass gets the bad body."""
+        if self.command == "HEAD":
+            return 1
+        seen = Handler.UPCODEC_SEEN.get(self.path, 0) + 1
+        Handler.UPCODEC_SEEN[self.path] = seen
+        return seen
+
+    @staticmethod
+    def gzipped(body):
+        return gzip.compress(body)
+
+    @staticmethod
+    def bad_gzip(body):
+        """A gzip stream whose deflate payload is mangled: inflate fails partway
+        through, after some plausible output has already been produced."""
+        raw = bytearray(gzip.compress(body))
+        raw[20:40] = b"\xff" * 20
+        return bytes(raw[:-4])
+
+    def send_coded(self, body, content_type, coding="gzip"):
+        self.send_raw(body, content_type, extra_headers=[("Content-Encoding", coding)])
+
+    def route_upcodec_index(self):
+        self.send_html(
+            '\t<a href="mem.html">mem</a>\n'
+            '\t<a href="disk.bin">disk</a>\n'
+            '\t<a href="unsup.html">unsup</a>\n'
+            '\t<a href="fresh.html">fresh</a>\n'
+            '\t<a href="freshdisk.bin">freshdisk</a>\n'
+        )
+
+    MEM_V1 = b"<html><body><p>MIRRORED-MEM-V1</p></body></html>"
+    DISK_V1 = b"MIRRORED-DISK-V1\n" + b"\x00\x01\x02\xff" * 8192
+    UNSUP_V1 = b"<html><body><p>MIRRORED-UNSUP-V1</p></body></html>"
+
+    def route_upcodec_mem(self):
+        if self.upcodec_pass() == 1:
+            self.send_coded(self.gzipped(self.MEM_V1), "text/html")
+        else:
+            self.send_coded(self.bad_gzip(self.MEM_V1), "text/html")
+
+    def route_upcodec_disk(self):
+        if self.upcodec_pass() == 1:
+            self.send_coded(self.gzipped(self.DISK_V1), "application/octet-stream")
+        else:
+            self.send_coded(self.bad_gzip(self.DISK_V1), "application/octet-stream")
+
+    # Pass 2 switches to a coding we have no decoder for.
+    def route_upcodec_unsup(self):
+        if self.upcodec_pass() == 1:
+            self.send_coded(self.gzipped(self.UNSUP_V1), "text/html")
+        else:
+            self.send_coded(self.UNSUP_V1, "text/html", coding="compress")
+
+    def route_upcodec_fresh(self):
+        pass1 = self.upcodec_pass() == 1
+        body = b"<html><body><p>FRESH-V%d</p></body></html>" % (1 if pass1 else 2)
+        self.send_coded(self.gzipped(body), "text/html")
+
+    # Same, direct-to-disk: the update pass decodes, so the temp is renamed over
+    # an existing mirror file.
+    def route_upcodec_freshdisk(self):
+        pass1 = self.upcodec_pass() == 1
+        body = b"FRESHDISK-V%d\n" % (1 if pass1 else 2) + b"\x03\x02\x01\xfe" * 8192
+        self.send_coded(self.gzipped(body), "application/octet-stream")
+
     # Echo what httrack advertised, so a crawl can assert the header.
     def route_codec_ae(self):
         self.send_raw(
@@ -1392,6 +1466,12 @@ class Handler(SimpleHTTPRequestHandler):
         "/codec/bad.html": route_codec_bad,
         "/codec/bin.dat": route_codec_bin,
         "/codec/ae.html": route_codec_ae,
+        "/upcodec/index.html": route_upcodec_index,
+        "/upcodec/mem.html": route_upcodec_mem,
+        "/upcodec/disk.bin": route_upcodec_disk,
+        "/upcodec/unsup.html": route_upcodec_unsup,
+        "/upcodec/fresh.html": route_upcodec_fresh,
+        "/upcodec/freshdisk.bin": route_upcodec_freshdisk,
         "/types/index.html": route_types_index,
         "/types/control.php": route_types,
         "/types/photo.png": route_types,