Merge remote-tracking branch 'origin/master' into fix/htscore-745

The path the guard admits (HTS_URLMAXSIZE) plus ".txt" is longer than macOS
accepts, so fopen() failed there. The case could not tell a fixed build from an
unfixed one anyway; what is left covers the guard and the rename on both entry
points.

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

src/htscore.c

@@ -1980,9 +1980,10 @@ int httpmirror(char *url1, httrackp * opt) {
           }
 
           // ATTENTION C'EST ICI QU'ON SAUVE LE FICHIER!!
-          // 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)) {
+          // 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)) {
             file_notify(opt, urladr(), urlfil(), savename(), 1, 1, r.notmodified);
             if (filesave(opt, r.adr, (int) r.size, savename(), urladr(), urlfil()) !=
                 0) {
@@ -3828,12 +3829,11 @@ int htsAddLink(htsmoduleStruct * str, char *link) {
         strcpybuff(codebase, heap(ptr)->fil);
       else
         strcpybuff(codebase, heap(heap(ptr)->precedent)->fil);
-      // empty codebase has no last char; codebase-1 would underflow
-      a = codebase[0] != '\0' ? codebase + strlen(codebase) - 1 : codebase;
+      a = codebase + strlen(codebase) - 1;
       while((*a) && (*a != '/') && (a > codebase))
         a--;
       if (*a == '/')
-        *(a + 1) = '\0';        // cut
+        *(a + 1) = '\0';        // couper
     } else {                    // couper http:// éventuel
       if (strfield(codebase, "http://")) {
         char BIGSTK tempo[HTS_URLMAXSIZE * 2];

src/htsselftest.c

@@ -43,7 +43,6 @@ 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"
@@ -2622,68 +2621,6 @@ 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;
 
@@ -6536,8 +6473,6 @@ 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},

src/htsweb.c

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

tests/01_engine-addlink.test

@@ -1,7 +0,0 @@
-#!/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/65_port-siblings.test

@@ -79,29 +79,6 @@ 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/93_local-changes.test

@@ -94,7 +94,10 @@ expect_counts_match() {
     echo "OK"
 }
 lines() { printf '%s\n' "$@" | sort; }
-digest() { cksum <"$1" | tr -d '[:space:]'; }
+# 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; }
 
 # --- pass 1: nothing to compare against --------------------------------------
 httrack "${common[@]}" -O "$out" --purge-old=0 "${base}/changes/index.html" \
@@ -117,7 +120,6 @@ 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"
@@ -142,19 +144,21 @@ 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 changed | sort)"
+    "$(listed_but_reset changed)"
 # 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. reset.bin never completes a transfer (#746), so its
-# previous copy stands.
-expect "unchanged is exactly the seven stable resources" \
+# on disk change length.
+expect "unchanged is exactly the six stable resources" \
     "$(lines "${host}/changes/codedstable.bin" "${host}/changes/flaky.bin" \
-        "${host}/changes/redirtarget.html" "${host}/changes/reset.bin" \
-        "${host}/changes/sized.html" "${host}/changes/stable.bin" \
-        "${host}/changes/stable.html")" \
-    "$(listed unchanged | sort)"
+        "${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)"
 expect_counts_match "pass 2 counts match its lists"
 
 # Every mirrored file appears once and only once, across all four buckets.
@@ -188,15 +192,19 @@ test -f "${out}/${host}/changes/doomed.html" || {
     exit 1
 }
 echo "OK"
-expect "a failed transfer keeps its bytes" "$resetdigest" \
-    "$(digest "${out}/${host}/changes/reset.bin")"
+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"
 
 # --- 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 gone | sort)"
+    "${host}/changes/transient.html" "$(listed_but_reset gone)"
 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

@@ -1,28 +0,0 @@
-#!/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/Makefile.am

@@ -30,7 +30,6 @@ 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 \
@@ -197,7 +196,6 @@ TESTS = \
 	92_local-proxytrack-ndx-fields.test \
 	93_local-changes.test \
 	94_local-single-file.test \
-	95_local-sitemap.test \
-	96_local-refetch-keep.test
+	95_local-sitemap.test
 
 CLEANFILES = check-network_sh.cache

tests/local-server.py

@@ -1000,62 +1000,6 @@ 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(
@@ -2006,11 +1950,6 @@ 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,