Never let the log salvage's cleanup fail the test

dump_crawl_logs runs on the timeout path, where the caller is already
handling a failure: rm -rf can fail on Windows while the killed httrack
still holds a file, and under set -e that would abort the branch whose
whole job is to salvage the evidence.

Renumber the test to 60: #611 adds a 59 too.

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

.github/workflows/windows-build.yml

@@ -158,13 +158,21 @@ jobs:
 
           pass=0 fail=0 skip=0 failed="" skipped=""
           for t in 00_runnable.test 01_engine-*.test 01_zlib-*.test \
-                   *_local-*.test 13_crawl_proxy_https.test 58_watchdog.test; do
+                   *_local-*.test 13_crawl_proxy_https.test 58_watchdog.test \
+                   60_crawl-log-salvage.test; do
             rc=0
             run_with_timeout 600 bash "$t" >"$t.log" 2>&1 || rc=$?
             case "$rc" in
               0) pass=$((pass + 1)); echo "PASS $t" ;;
               77) skip=$((skip + 1)) skipped="$skipped $t"; echo "SKIP $t" ;;
-              124) fail=$((fail + 1)) failed="$failed $t"; echo "FAIL $t (timed out, tree killed)" ;;
+              124)
+                fail=$((fail + 1)) failed="$failed $t"
+                echo "FAIL $t (timed out, tree killed)"
+                # Re-running a wedge traced would just hang again: salvage the
+                # killed crawl's own logs into the artifact instead.
+                dump_crawl_logs >>"$t.log"
+                tail -n 25 "$t.log" | sed 's/^/      /'
+                ;;
               *)
                 fail=$((fail + 1)) failed="$failed $t"
                 echo "FAIL $t (exit $rc)"

man/httrack.1

@@ -3,7 +3,7 @@
 .\"
 .\" This file is generated by man/makeman.sh; do not edit by hand.
 .\" SPDX-License-Identifier: GPL-3.0-or-later
-.TH httrack 1 "14 July 2026" "httrack website copier"
+.TH httrack 1 "16 July 2026" "httrack website copier"
 .SH NAME
 httrack \- offline browser : copy websites to a local directory
 .SH SYNOPSIS
@@ -165,7 +165,7 @@ pause transfer if N bytes reached, and wait until lock file is deleted (\-\-max\
 .IP \-cN
 number of multiple connections (*c8) (\-\-sockets[=N])
 .IP \-TN
-timeout, number of seconds after a non\-responding link is shutdown (\-\-timeout[=N])
+timeout, number of seconds after a non\-responding link is shutdown; also bounds host name resolution (\-\-timeout[=N])
 .IP \-RN
 number of retries, in case of timeout or non\-fatal errors (*R1) (\-\-retries[=N])
 .IP \-JN

src/htsback.c

@@ -2816,6 +2816,9 @@ void back_wait(struct_back * sback, httrackp * opt, cache_back * cache,
                 if (SSL_set_fd(back[i].r.ssl_con, (int) back[i].r.soc) == 1) {
                   SSL_set_connect_state(back[i].r.ssl_con);
                   back[i].status = STATUS_SSL_WAIT_HANDSHAKE;   /* handshake wait */
+                  // the handshake gets its own timeout window, as connect does
+                  if (back[i].timeout > 0)
+                    back[i].timeout_refresh = time_local();
                 } else
                   back[i].r.statuscode = STATUSCODE_SSL_HANDSHAKE;
               } else
@@ -2875,6 +2878,11 @@ void back_wait(struct_back * sback, httrackp * opt, cache_back * cache,
       }
 #if HTS_USEOPENSSL
       else if (back[i].status == STATUS_SSL_WAIT_HANDSHAKE) {       // wait for SSL handshake
+        // a peer that never speaks TLS must be reaped by --timeout too (#607)
+        if (!gestion_timeout)
+          if (back[i].timeout > 0)
+            gestion_timeout = 1;
+
         /* SSL mode */
         if (back[i].r.ssl) {
           int conn_code;
@@ -4265,6 +4273,8 @@ void back_wait(struct_back * sback, httrackp * opt, cache_back * cache,
                 strcpybuff(back[i].r.msg, "Connect Time Out");
               else if (back[i].status == STATUS_WAIT_DNS)
                 strcpybuff(back[i].r.msg, "DNS Time Out");
+              else if (back[i].status == STATUS_SSL_WAIT_HANDSHAKE)
+                strcpybuff(back[i].r.msg, "SSL/TLS Handshake Time Out");
               else
                 strcpybuff(back[i].r.msg, "Receive Time Out");
               back[i].status = STATUS_READY;    // terminé

src/htsdns_selftest.c

@@ -63,9 +63,14 @@ typedef struct mock_host {
   int gai_err; /* non-zero: getaddrinfo returns this */
   int naddr;
   mock_addr addr[6];
-  int calls; /* times the backend resolved this host */
+  int calls;   /* times the backend resolved this host */
+  int slow_ms; /* non-zero: block this long, as a black-hole resolver would */
 } mock_host;
 
+/* Long enough to outlast the 1s --timeout the bounded resolve is checked
+   against, short enough to keep the self-test quick. */
+#define MOCK_SLOW_MS 3000
+
 static mock_host mock_hosts[] = {
     {"v4only.test", 0, 1, {{AF_INET, {1, 2, 3, 4}}}, 0},
     {"v6only.test", 0, 1, {{AF_INET6, {0x20, 0x01, 0x0d, 0xb8, [15] = 1}}}, 0},
@@ -95,8 +100,15 @@ static mock_host mock_hosts[] = {
       {AF_INET, {10, 0, 0, 6}}},
      0},
     {"nodns.test", EAI_NONAME, 0, {{0}}, 0},
+    /* resolves, but only well after --timeout: the #606 wedge */
+    {"slow.test", 0, 1, {{AF_INET, {127, 0, 0, 9}}}, 0, MOCK_SLOW_MS},
 };
 
+/* Serializes mock_host bookkeeping: a timed-out resolve is abandoned, so its
+   worker is still inside the backend while the test reads the counters. */
+static htsmutex mock_lock = HTSMUTEX_INIT;
+static int mock_finished = 0; /* backend calls that have returned */
+
 static mock_host *mock_find(const char *name) {
   for (size_t i = 0; i < sizeof(mock_hosts) / sizeof(mock_hosts[0]); i++) {
     if (strcmp(mock_hosts[i].name, name) == 0)
@@ -106,8 +118,34 @@ static mock_host *mock_find(const char *name) {
 }
 
 static void mock_reset_calls(void) {
+  hts_mutexlock(&mock_lock);
   for (size_t i = 0; i < sizeof(mock_hosts) / sizeof(mock_hosts[0]); i++)
     mock_hosts[i].calls = 0;
+  mock_finished = 0;
+  hts_mutexrelease(&mock_lock);
+}
+
+static int mock_read_calls(const char *name) {
+  int calls;
+
+  hts_mutexlock(&mock_lock);
+  calls = mock_find(name)->calls;
+  hts_mutexrelease(&mock_lock);
+  return calls;
+}
+
+/* Wait for n backend calls to return, ordering their writes against ours. */
+static void mock_wait_finished(int n) {
+  for (;;) {
+    hts_boolean done;
+
+    hts_mutexlock(&mock_lock);
+    done = (mock_finished >= n) ? HTS_TRUE : HTS_FALSE;
+    hts_mutexrelease(&mock_lock);
+    if (done)
+      break;
+    Sleep(10);
+  }
 }
 
 /* Build one addrinfo node owning its sockaddr (freed by mock_freeaddrinfo). */
@@ -133,10 +171,10 @@ static struct addrinfo *mock_mkai(const mock_addr *a) {
   return ai;
 }
 
-static int HTS_RESOLVER_CALL mock_getaddrinfo(const char *node,
-                                              const char *service,
-                                              const struct addrinfo *hints,
-                                              struct addrinfo **res) {
+static int HTS_RESOLVER_CALL mock_getaddrinfo_(const char *node,
+                                               const char *service,
+                                               const struct addrinfo *hints,
+                                               struct addrinfo **res) {
   mock_host *const h = mock_find(node);
   const int want = (hints != NULL) ? hints->ai_family : PF_UNSPEC;
   struct addrinfo *head = NULL, *tail = NULL;
@@ -145,7 +183,11 @@ static int HTS_RESOLVER_CALL mock_getaddrinfo(const char *node,
   *res = NULL;
   if (h == NULL)
     return EAI_NONAME;
+  hts_mutexlock(&mock_lock);
   h->calls++; /* a real backend hit; a cached host skips this */
+  hts_mutexrelease(&mock_lock);
+  if (h->slow_ms != 0)
+    Sleep(h->slow_ms);
   if (h->gai_err != 0)
     return h->gai_err;
   for (int i = 0; i < h->naddr; i++) {
@@ -165,6 +207,18 @@ static int HTS_RESOLVER_CALL mock_getaddrinfo(const char *node,
   return 0;
 }
 
+static int HTS_RESOLVER_CALL mock_getaddrinfo(const char *node,
+                                              const char *service,
+                                              const struct addrinfo *hints,
+                                              struct addrinfo **res) {
+  const int ret = mock_getaddrinfo_(node, service, hints, res);
+
+  hts_mutexlock(&mock_lock);
+  mock_finished++;
+  hts_mutexrelease(&mock_lock);
+  return ret;
+}
+
 static void HTS_RESOLVER_CALL mock_freeaddrinfo(struct addrinfo *res) {
   while (res != NULL) {
     struct addrinfo *const next = res->ai_next;
@@ -370,6 +424,91 @@ int dns_selftests(httrackp *opt) {
   return failures;
 }
 
+/* Probes how long acquiring opt->state.lock takes while a resolve is in
+   flight. hts_has_stopped() takes that same lock and mutates nothing, and the
+   API promises it stays callable from another thread during a mirror. */
+typedef struct lock_probe {
+  httrackp *opt;
+  htsmutex lock;
+  hts_boolean done;
+  TStamp blocked_ms;
+} lock_probe;
+
+static void lock_probe_thread(void *arg) {
+  lock_probe *const p = (lock_probe *) arg;
+  TStamp start;
+
+  Sleep(MOCK_SLOW_MS / 10); /* let the resolve get under way first */
+  start = mtime_local();
+  (void) hts_has_stopped(p->opt);
+  hts_mutexlock(&p->lock);
+  p->blocked_ms = mtime_local() - start;
+  p->done = HTS_TRUE;
+  hts_mutexrelease(&p->lock);
+}
+
+int dns_timeout_selftests(httrackp *opt) {
+  SOCaddr addrs[HTS_MAXADDRNUM];
+  const char *err = NULL;
+  lock_probe probe;
+  TStamp start, elapsed;
+  int count;
+
+  failures = 0;
+  hts_dns_set_resolver_backend(&mock_backend);
+  IPV6_resolver = 0;
+  mock_reset_calls();
+  opt->timeout = 1; /* the bound under test */
+
+  memset(&probe, 0, sizeof(probe));
+  probe.opt = opt;
+  probe.lock = HTSMUTEX_INIT;
+  hts_mutexinit(&probe.lock);
+  CHECK(hts_newthread(lock_probe_thread, &probe) == 0);
+
+  start = mtime_local();
+  count = hts_dns_resolve_all(opt, "slow.test", addrs, HTS_MAXADDRNUM, &err);
+  elapsed = mtime_local() - start;
+
+  /* the resolve returns on opt->timeout, not when the resolver deigns to
+     answer: this is what lets --max-time and --timeout fire (#606). The bound
+     is derived from opt->timeout, never from the mock's sleep, or a resolve
+     that ignored opt->timeout would still pass under the mock. */
+  CHECK(elapsed < (TStamp) opt->timeout * 1000 + 500);
+  CHECK(count == 0); /* a timeout is reported as "does not resolve" */
+
+  /* state.lock is not held across the resolve; a concurrent stop query, which
+     the mirror API promises stays live, is not blocked behind it */
+  for (;;) {
+    hts_boolean done;
+    TStamp blocked;
+
+    hts_mutexlock(&probe.lock);
+    done = probe.done;
+    blocked = probe.blocked_ms;
+    hts_mutexrelease(&probe.lock);
+    if (done) {
+      CHECK(blocked < 500);
+      break;
+    }
+    Sleep(20);
+  }
+  hts_mutexfree(&probe.lock);
+
+  /* a timeout is not an answer, so it must not be negative-cached: the host is
+     resolved again rather than written off for the rest of the crawl */
+  CHECK(mock_read_calls("slow.test") == 1);
+  count = hts_dns_resolve_all(opt, "slow.test", addrs, HTS_MAXADDRNUM, &err);
+  CHECK(count == 0);
+  CHECK(mock_read_calls("slow.test") == 2); /* re-resolved, not cached */
+
+  /* Both resolves were abandoned mid-backend; wait for their workers to leave
+     it before returning. The backend stays installed: an abandoned worker
+     still reads it (to free its addrinfo) after the last call returns. */
+  mock_wait_finished(2);
+  return failures;
+}
+
 #else
 
 int dns_selftests(httrackp *opt) {

src/htsdns_selftest.h

@@ -46,6 +46,12 @@ typedef struct httrackp httrackp;
    Returns the number of failed checks (0 == success). */
 int dns_selftests(httrackp *opt);
 
+/* Drive a deliberately slow (mock) resolver, asserting that a resolve is
+   bounded by opt->timeout, does not hold opt->state.lock while it runs, and
+   does not cache a timeout as an answer (#606). Takes a few seconds.
+   Returns the number of failed checks (0 == success). */
+int dns_timeout_selftests(httrackp *opt);
+
 #endif
 
 #endif

src/htshelp.c

@@ -496,8 +496,8 @@ void help(const char *app, int more) {
   infomsg("");
   infomsg("Flow control:");
   infomsg("  cN number of multiple connections (*c8)");
-  infomsg
-    ("  TN timeout, number of seconds after a non-responding link is shutdown");
+  infomsg("  TN timeout, number of seconds after a non-responding link is"
+          " shutdown; also bounds host name resolution");
   infomsg
     ("  RN number of retries, in case of timeout or non-fatal errors (*R1)");
   infomsg

src/htslib.c

@@ -65,6 +65,7 @@ Please visit our Website: http://www.httrack.com
 #endif /* _WIN32 */
 #include <stdarg.h>
 
+#include <ctype.h>
 #include <string.h>
 #include <time.h>
 #include <stdarg.h>
@@ -3747,6 +3748,20 @@ static int proxy_default_port(const char *arg) {
   return hts_proxy_is_socks(arg) ? 1080 : 8080;
 }
 
+// port "a" of -P argument "arg": digits fitting TCP's 1..65535, else the scheme
+// default. Not sscanf("%d"): past INT_MAX it wraps to a garbage port (#602)
+static int parse_proxy_port(const char *a, const char *arg) {
+  char *end;
+  long p;
+
+  if (!isdigit((unsigned char) *a)) // strtol would eat a sign or leading space
+    return proxy_default_port(arg);
+  p = strtol(a, &end, 10);
+  if (*end != '\0' || p < 1 || p > 65535) // ERANGE lands out of range too
+    return proxy_default_port(arg);
+  return (int) p;
+}
+
 void hts_parse_proxy(const char *arg, char *name, size_t name_size, int *port) {
   const char *authority = strstr(arg, "://");
   const char *a;
@@ -3762,10 +3777,7 @@ void hts_parse_proxy(const char *arg, char *name, size_t name_size, int *port) {
   while (a > authority && a[-1] != ':' && a[-1] != '@' && a[-1] != ']')
     a--;
   if (a > authority && a[-1] == ':') {
-    int p = -1;
-
-    sscanf(a, "%d", &p);
-    *port = (p > 0) ? p : proxy_default_port(arg);
+    *port = parse_proxy_port(a, arg);
     namelen = (size_t) (a - 1 - arg);
   } else {
     *port = proxy_default_port(arg);
@@ -5084,50 +5096,164 @@ HTSEXT_API int check_hostname_dns(const char *const hostname) {
   return hts_dns_resolve_nocache(hostname, &buffer) != NULL;
 }
 
-// Needs locking
-// Internal DNS cache. Fill out[0..count-1] with up to max addresses for _iadr,
-// resolving (and caching the full list) on a miss. Returns the count.
-static int hts_dns_resolve_list_(httrackp *opt, const char *_iadr,
-                                 SOCaddr *const out, const int max,
-                                 const char **error) {
-  char BIGSTK iadr[HTS_URLMAXSIZE * 2];
-  coucal cache = hts_cache(opt); // le cache dns
+/* A resolve in flight. Refcounted: a timed-out resolve is abandoned, not
+   cancelled, so the last of caller/worker to leave frees the job. */
+typedef struct dns_resolve_job {
+  htsmutex lock;
+  int refcount;
+  hts_boolean done;
+  char *hostname;
+  SOCaddr addr[HTS_MAXADDRNUM];
   int count;
+  const char *error;
+} dns_resolve_job;
+
+/* Copy the first min(count, max) addresses of src into dest. */
+static void dns_copy_addrs(SOCaddr *dest, SOCaddr *src, int count, int max) {
+  int i;
+
+  for (i = 0; i < count && i < max; i++)
+    SOCaddr_copy_SOCaddr(dest[i], src[i]);
+}
+
+static void dns_job_release(dns_resolve_job *job) {
+  hts_boolean last;
+
+  hts_mutexlock(&job->lock);
+  last = (--job->refcount == 0) ? HTS_TRUE : HTS_FALSE;
+  hts_mutexrelease(&job->lock);
+  if (last) {
+    hts_mutexfree(&job->lock);
+    freet(job->hostname);
+    freet(job);
+  }
+}
+
+/* Outlives a timed-out resolve, so it writes only the job: never opt (freed
+   before the thread wait at exit) nor the DNS cache. */
+static void dns_resolve_thread(void *arg) {
+  dns_resolve_job *const job = (dns_resolve_job *) arg;
+  SOCaddr resolved[HTS_MAXADDRNUM];
+  const char *error = NULL;
+  const int count = hts_dns_resolve_nocache_list(job->hostname, resolved,
+                                                 HTS_MAXADDRNUM, &error);
+
+  hts_mutexlock(&job->lock);
+  dns_copy_addrs(job->addr, resolved, count, HTS_MAXADDRNUM);
+  job->count = count;
+  job->error = error;
+  job->done = HTS_TRUE; /* published last: gates the caller's read of addr[] */
+  hts_mutexrelease(&job->lock);
+  dns_job_release(job);
+}
+
+/* Resolve hostname on a worker thread, giving up after timeout seconds.
+   Returns the address count, or -1 on timeout -- distinct from 0 ("does not
+   resolve"), which is a real answer and gets negative-cached. */
+static int hts_dns_resolve_nocache_list_bounded(const char *hostname,
+                                                SOCaddr *const out,
+                                                const int max,
+                                                const int timeout,
+                                                const char **error) {
+  dns_resolve_job *job;
+  TStamp deadline;
+  int count = -1;
+  int poll_ms = 1;
+
+  if (timeout <= 0) /* no bound asked for (--timeout 0) */
+    return hts_dns_resolve_nocache_list(hostname, out, max, error);
+
+  job = calloct(1, sizeof(*job));
+  assertf(job != NULL);
+  hts_mutexinit(&job->lock);
+  job->hostname = strdupt(hostname);
+  job->refcount = 2; /* this caller + the worker */
+  if (hts_newthread(dns_resolve_thread, job) != 0) {
+    job->refcount = 1; /* no worker: fall back to resolving inline */
+    dns_job_release(job);
+    return hts_dns_resolve_nocache_list(hostname, out, max, error);
+  }
+
+  deadline = mtime_local() + (TStamp) timeout * 1000;
+  for (;;) {
+    hts_boolean done;
+
+    hts_mutexlock(&job->lock);
+    done = job->done;
+    if (done) {
+      count = job->count;
+      dns_copy_addrs(out, job->addr, count, max);
+      if (error != NULL)
+        *error = job->error;
+    }
+    hts_mutexrelease(&job->lock);
+    if (done || mtime_local() >= deadline)
+      break;
+    Sleep(poll_ms);
+    if (poll_ms < 50) /* short first polls keep a fast resolve fast */
+      poll_ms *= 2;
+  }
+  dns_job_release(job);
+  return count;
+}
+
+int hts_dns_resolve_all(httrackp *opt, const char *iadr, SOCaddr *out, int max,
+                        const char **error) {
+  char BIGSTK host[HTS_URLMAXSIZE * 2];
+  SOCaddr resolved[HTS_MAXADDRNUM];
+  coucal cache;
+  int count, i;
 
   assertf(opt != NULL);
-  assertf(_iadr != NULL);
   assertf(out != NULL);
+  if (!strnotempty(iadr) || max <= 0) {
+    return 0;
+  }
 
-  strcpybuff(iadr, jump_identification_const(_iadr));
-  // couper éventuel :
+  /* cache key and resolver input: identification and any ":port" stripped */
+  strcpybuff(host, jump_identification_const(iadr));
   {
     char *a;
 
-    if ((a = jump_toport(iadr)))
+    if ((a = jump_toport(host)))
       *a = '\0';
   }
 
-  /* get IP from the dns cache */
-  count = hts_ghbn_all(cache, iadr, out, max);
+  hts_mutexlock(&opt->state.lock);
+  cache = hts_cache(opt);
+#if HTS_INET6 != 0
+  hts_resolver_check_env(); /* settle the backend before a worker reads it */
+#endif
+  count = hts_ghbn_all(cache, host, out, max);
+  hts_mutexrelease(&opt->state.lock);
   if (count >= 0) { // cache hit (0 == negative-cached)
     return count;
-  } else { // non présent dans le cache dns, tester
-    SOCaddr resolved[HTS_MAXADDRNUM];
-    t_dnscache *record;
-    int i;
+  }
 
 #if DEBUGDNS
-    printf("resolving (not cached) %s\n", iadr);
+  printf("resolving (not cached) %s\n", host);
 #endif
 
-    count = hts_dns_resolve_nocache_list(iadr, resolved, HTS_MAXADDRNUM, error);
+  /* Resolve with no lock held: getaddrinfo can block for a long time, and
+     state.lock also gates the stop request (#606). */
+  count = hts_dns_resolve_nocache_list_bounded(host, resolved, HTS_MAXADDRNUM,
+                                               opt->timeout, error);
 
 #if HTS_WIDE_DEBUG
-    DEBUG_W("gethostbyname done\n");
+  DEBUG_W("gethostbyname done\n");
 #endif
 
-    /* attempt to store new entry (coucal owns it and dups the host key) */
-    record = malloct(sizeof(t_dnscache));
+  if (count < 0) { /* timed out: no answer to cache, and none to report */
+    if (error != NULL)
+      *error = "host name resolution timed out";
+    return 0;
+  }
+
+  hts_mutexlock(&opt->state.lock);
+  { /* store the full list (coucal owns the record and dups the host key; a
+       concurrent resolve of the same host replaces, and frees, this one) */
+    t_dnscache *const record = malloct(sizeof(t_dnscache));
+
     if (record != NULL) {
       memset(record, 0, sizeof(*record));
       record->host_count = count;
@@ -5137,28 +5263,13 @@ static int hts_dns_resolve_list_(httrackp *opt, const char *_iadr,
         memcpy(record->host_addr[i], &SOCaddr_sockaddr(resolved[i]),
                record->host_length[i]);
       }
-      coucal_add_pvoid(cache, iadr, record);
+      coucal_add_pvoid(cache, host, record);
     }
-
-    /* copy result to caller (cache store may have failed; result still valid)
-     */
-    for (i = 0; i < count && i < max; i++) {
-      SOCaddr_copy_SOCaddr(out[i], resolved[i]);
-    }
-    return count;
-  } // retour hp du cache
-}
-
-int hts_dns_resolve_all(httrackp *opt, const char *iadr, SOCaddr *out, int max,
-                        const char **error) {
-  int count;
-
-  if (!strnotempty(iadr) || max <= 0) {
-    return 0;
   }
-  hts_mutexlock(&opt->state.lock);
-  count = hts_dns_resolve_list_(opt, iadr, out, max, error);
   hts_mutexrelease(&opt->state.lock);
+
+  /* copy result to caller (cache store may have failed; result still valid) */
+  dns_copy_addrs(out, resolved, count, max);
   return count;
 }
 

src/htslib.h

@@ -222,7 +222,8 @@ LLint http_xfread1(htsblk * r, int bufl);
 /* Cached resolver: fill out[0..count-1] with up to max addresses for iadr (in
    resolver order), returning the count (0 = does not resolve, negative-cached).
    Resolves once per host; later calls read the DNS cache. Must hold no lock
-   (brackets opt->state.lock itself). */
+   (brackets opt->state.lock itself, never across the resolve). A miss resolves
+   on a worker thread bounded by opt->timeout; a timeout reports 0, uncached. */
 int hts_dns_resolve_all(httrackp *opt, const char *iadr, SOCaddr *out, int max,
                         const char **error);
 HTS_INLINE SOCaddr *hts_dns_resolve2(httrackp *opt, const char *iadr,

src/htsselftest.c

@@ -2109,6 +2109,15 @@ static int st_dns(httrackp *opt, int argc, char **argv) {
   return err;
 }
 
+static int st_dnstimeout(httrackp *opt, int argc, char **argv) {
+  const int err = dns_timeout_selftests(opt);
+
+  (void) argc;
+  (void) argv;
+  printf("dns-timeout-selftest: %s\n", err ? "FAIL" : "OK");
+  return err;
+}
+
 static int st_cookies(httrackp *opt, int argc, char **argv) {
   static t_cookie cookie;
   char hdr[1024];
@@ -3133,6 +3142,8 @@ static const struct selftest_entry {
     {"cache-corrupt", "<dir>", "cache read-side corruption self-test",
      st_cache_corrupt},
     {"dns", "", "DNS resolver/cache self-test", st_dns},
+    {"dnstimeout", "", "a slow DNS resolve is bounded and holds no lock",
+     st_dnstimeout},
     {"cookies", "", "cookie request-header self-test", st_cookies},
     {"useragent", "", "default User-Agent self-test", st_useragent},
     {"makeindex", "[dir]", "hts_finish_makeindex footer/refresh self-test",

tests/01_engine-dnstimeout.test

@@ -0,0 +1,15 @@
+#!/bin/bash
+#
+
+set -euo pipefail
+
+# A resolve against a deliberately slow mock getaddrinfo (no network) must
+# return on --timeout, must not hold opt->state.lock meanwhile, and must not
+# cache the timeout as an answer (#606).
+# 'run' is an ignored placeholder argument.
+out=$(httrack -#test=dnstimeout run)
+
+test "$out" = "dns-timeout-selftest: OK" || {
+    echo "expected 'dns-timeout-selftest: OK', got: $out" >&2
+    exit 1
+}

tests/01_engine-proxyurl.test

@@ -71,3 +71,23 @@ p 'http://a]b:c@host' 'name=http://a]b:c@host port=8080 host=host kind=http'
 
 # a lone ':' must not underflow the backward scan
 p ':' 'name= port=8080 host= kind=http'
+
+# a port is *DIGIT in 1..65535, else the scheme default: sscanf("%d") used to
+# wrap past INT_MAX and hand back a garbage port (#602)
+p 'http://host:65535' 'name=http://host port=65535 host=host kind=http'
+p 'http://host:1' 'name=http://host port=1 host=host kind=http'
+p 'http://host:65536' 'name=http://host port=8080 host=host kind=http'
+p 'http://host:2147483648' 'name=http://host port=8080 host=host kind=http'
+p 'http://host:4294967296' 'name=http://host port=8080 host=host kind=http'
+p 'http://host:99999999999999' 'name=http://host port=8080 host=host kind=http'
+# discriminating: this one wrapped to a plausible 80, so range-checking the
+# sscanf result instead of rejecting the overflow still passes everything above
+p 'http://host:4294967376' 'name=http://host port=8080 host=host kind=http'
+p 'http://host:999999999999999999999999999999' 'name=http://host port=8080 host=host kind=http'
+p 'http://host:0' 'name=http://host port=8080 host=host kind=http'
+p 'http://host:-1' 'name=http://host port=8080 host=host kind=http'
+p 'http://host:80x' 'name=http://host port=8080 host=host kind=http'
+p 'http://host: 80' 'name=http://host port=8080 host=host kind=http'
+p 'http://host:' 'name=http://host port=8080 host=host kind=http'
+# the fallback is the scheme's own default, not a hardcoded 8080
+p 'socks5://host:99999999999999' 'name=socks5://host port=1080 host=host kind=socks'

tests/59_local-tls-stall.test

@@ -0,0 +1,95 @@
+#!/bin/bash
+#
+# Issue #607: a peer that accepts the TCP connect but never speaks TLS leaves the
+# slot stuck in the handshake. The per-slot --timeout must reap it; before the
+# fix only --max-time did, so these crawls ran until the kill guard.
+
+set -euo pipefail
+
+: "${top_srcdir:=..}"
+
+# shellcheck source=tests/testlib.sh
+. "$top_srcdir/tests/testlib.sh"
+
+if test "${HTTPS_SUPPORT:-}" == "no"; then
+    echo "no https support compiled, skipping"
+    exit 77
+fi
+python=$(find_python) || {
+    echo "python3 missing, skipping"
+    exit 77
+}
+
+server=$(nativepath "$top_srcdir/tests/tls-stall-server.py")
+tmpdir=$(mktemp -d)
+serverpid=
+
+cleanup() {
+    stop_server "$serverpid"
+    rm -rf "$tmpdir"
+    return 0
+}
+trap cleanup EXIT
+
+# start_stall_server <tag> <mode-args...>: sets $port from the announced one.
+start_stall_server() {
+    local tag="$1"
+    shift
+    "$python" "$server" "$@" >"$tmpdir/$tag.out" 2>"$tmpdir/$tag.err" &
+    serverpid=$!
+    port=
+    for _ in $(seq 1 50); do
+        line=$(head -n1 "$tmpdir/$tag.out" 2>/dev/null || true)
+        if test "${line%% *}" == "PORT"; then
+            port="${line#PORT }"
+            return 0
+        fi
+        kill -0 "$serverpid" 2>/dev/null || {
+            echo "server exited early: $(cat "$tmpdir/$tag.err")"
+            exit 1
+        }
+        sleep 0.1
+    done
+    echo "could not discover server port"
+    exit 1
+}
+
+# no -E/--max-time on purpose: --timeout is the only thing that can end these.
+# The kill guard stands in for the hang, so a wall time near it means no reap.
+crawl_wall() {
+    local out="$tmpdir/$1"
+    shift
+    local start
+    start=$(date +%s)
+    run_with_timeout 60 httrack -O "$out" -c1 --robots=0 --retries=0 --quiet -Z \
+        "$@" >>"$tmpdir/log" 2>&1 || true
+    echo $(($(date +%s) - start))
+}
+
+# 1. handshake stalled from the first byte: reaped at --timeout, not before.
+start_stall_server direct direct
+wall=$(crawl_wall crawl1 "https://127.0.0.1:$port/" --timeout=5)
+if test "$wall" -ge 30 || test "$wall" -lt 3; then
+    echo "FAIL: stalled handshake reaped after ${wall}s, expected about 5s" >&2
+    cat "$tmpdir/log" >&2
+    exit 1
+fi
+grep -q 'Handshake Time Out' "$tmpdir/crawl1/hts-log.txt" || {
+    echo "FAIL: crawl ended in ${wall}s but not on a handshake timeout" >&2
+    cat "$tmpdir/crawl1/hts-log.txt" >&2
+    exit 1
+}
+echo "OK: stalled TLS handshake reaped by --timeout after ${wall}s"
+
+# 2. the handshake window is its own, not what the connect left over: a proxy
+# that takes 4s to answer CONNECT must still leave the full --timeout=5 for the
+# handshake (~9s total). Sharing the connect's clock would reap at ~5s.
+stop_server "$serverpid"
+start_stall_server proxy proxy 4
+wall=$(crawl_wall crawl2 "https://127.0.0.1:443/" -P "127.0.0.1:$port" --timeout=5)
+if test "$wall" -ge 20 || test "$wall" -lt 7; then
+    echo "FAIL: handshake after a slow connect reaped at ${wall}s, expected about 9s" >&2
+    cat "$tmpdir/log" >&2
+    exit 1
+fi
+echo "OK: handshake keeps its own timeout window after a slow connect (${wall}s)"

tests/60_crawl-log-salvage.test

@@ -0,0 +1,46 @@
+#!/bin/bash
+#
+# Unit-tests dump_crawl_logs (testlib.sh): the killed test's hts-log.txt must
+# reach the uploaded logs, or a Windows wedge past --max-time stays
+# undiagnosable (#605).
+
+set -euo pipefail
+
+testdir=$(cd "$(dirname "$0")" && pwd)
+# shellcheck source=tests/testlib.sh
+. "${testdir}/testlib.sh"
+
+TMPDIR=$(mktemp -d "${TMPDIR:-/tmp}/httrack_salv.XXXXXX")
+export TMPDIR
+trap 'rm -rf "$TMPDIR"' EXIT
+fail() {
+    echo "FAIL: $*" >&2
+    exit 1
+}
+
+verdict="More than 120 seconds passed.. giving up"
+
+# Vacuity control: with nothing left behind the assertions below cannot pass on
+# ambient output.
+test -z "$(dump_crawl_logs)" || fail "dumped something with no crawl tmpdir"
+
+# A killed local-crawl.sh leaves exactly this shape.
+d="${TMPDIR}/httrack_local.probe"
+mkdir -p "$d/crawl"
+echo "$verdict" >"$d/crawl/hts-log.txt"
+echo "httrack stdout marker" >"$d/log"
+
+# Append to an unterminated line, as a killed test's own log always ends.
+art="${TMPDIR}/artifact.log"
+printf '[running httrack ...] ..\t' >"$art"
+dump_crawl_logs >>"$art"
+
+grep -qF "$verdict" "$art" || fail "hts-log.txt verdict not salvaged"
+grep -qF "httrack stdout marker" "$art" || fail "httrack stdout not salvaged"
+grep -q '^--- .*hts-log.txt' "$art" || fail "dump glued its header onto the unterminated line"
+
+# Cleared, so the next timing-out test cannot re-report this crawl as its own.
+test ! -d "$d" || fail "salvaged tmpdir left behind"
+test -z "$(dump_crawl_logs)" || fail "salvaged crawl reported twice"
+
+echo "crawl log salvage OK"

tests/Makefile.am

@@ -3,7 +3,7 @@
 # silently drop it from the dist tarball and break "make distcheck".
 EXTRA_DIST = $(TESTS) crawl-test.sh run-all-tests.sh check-network.sh \
 	proxy-https-server.py socks5-server.py proxy-connect-server.py \
-	proxytestlib.py \
+	proxytestlib.py tls-stall-server.py \
 	local-crawl.sh local-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 \
@@ -35,6 +35,7 @@ TESTS = \
 	01_engine-copyopt.test \
 	01_engine-crange.test \
 	01_engine-dns.test \
+	01_engine-dnstimeout.test \
 	01_engine-doitlog.test \
 	01_engine-entities.test \
 	01_engine-filelist.test \
@@ -136,6 +137,8 @@ TESTS = \
 	55_local-chunked.test \
 	56_local-proxy-noleak.test \
 	57_local-proxy-connect.test \
-	58_watchdog.test
+	58_watchdog.test \
+	59_local-tls-stall.test \
+	60_crawl-log-salvage.test
 
 CLEANFILES = check-network_sh.cache

tests/proxytestlib.py

@@ -18,6 +18,15 @@ PROXY_LOG = "proxy.log"
 ORIGIN_LOG = "origin-headers.log"
 
 
+def bind_ephemeral():
+    """Listening socket on a free loopback port, and that port."""
+    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)
+    return srv, srv.getsockname()[1]
+
+
 def pipe(src, dst):
     """Relay bytes one way until EOF, then tear both ends down."""
     try:
@@ -113,11 +122,7 @@ def handle_client(conn, logdir, mode, default_port):
 
 
 def start_proxy(logdir, mode, default_port):
-    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)
-    port = srv.getsockname()[1]
+    srv, port = bind_ephemeral()
 
     def accept_loop():
         while True:

tests/socks5-server.py

@@ -19,7 +19,7 @@ import threading
 # python3 -P (PYTHONSAFEPATH) drops the script's own directory from sys.path
 sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
 
-from proxytestlib import pipe  # noqa: E402
+from proxytestlib import bind_ephemeral, pipe  # noqa: E402
 
 # The one name the proxy answers for; a .invalid TLD never resolves (RFC 6761),
 # so a locally-resolving client could not reach us -- success proves remote DNS.
@@ -180,11 +180,7 @@ def handle_socks(conn, logdir, mode):
 
 
 def start_socks(logdir, mode):
-    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)
-    port = srv.getsockname()[1]
+    srv, port = bind_ephemeral()
 
     def serve():
         while True:

tests/testlib.sh

@@ -44,6 +44,25 @@ stop_server() {
     return 0
 }
 
+# Dump and clear the crawl logs a hard-killed test leaves in TMPDIR (its cleanup
+# trap never ran): hts-log.txt alone records "More than N seconds passed.. giving
+# up", so a wedge past --max-time is undiagnosable without it (#605).
+dump_crawl_logs() {
+    local d f
+    for d in "${TMPDIR:-/tmp}"/httrack_local.*; do
+        test -d "$d" || continue
+        for f in "$d/crawl/hts-log.txt" "$d/log" "$d/log.2"; do
+            test -f "$f" || continue
+            # Leading newline: the killed test's last line has no terminator.
+            printf '\n--- %s (last 200 lines)\n' "$f"
+            tail -n 200 "$f"
+        done
+        # so a later test's dump cannot re-report this one; never fatal, the
+        # caller is already handling a failure and Windows may still hold a file
+        rm -rf "$d" || true
+    done
+}
+
 # Kill a backgrounded job and its whole descendant tree. POSIX: the caller must
 # have put the job in its own process group (run_with_timeout does) so we signal
 # the group; a bare kill would orphan the grandchildren. Windows: the tree is

tests/tls-stall-server.py

@@ -0,0 +1,43 @@
+#!/usr/bin/env python3
+"""Peers that accept a connection and never speak TLS (#607).
+
+Modes: "direct" stalls the handshake straight away; "proxy <secs>" answers a
+CONNECT after <secs> before stalling, so the handshake starts on a clock the
+connect has already eaten into. Prints "PORT <n>" once listening.
+Usage: tls-stall-server.py [direct | proxy <secs>]
+"""
+import os
+import sys
+import threading
+import time
+
+# python3 -P (PYTHONSAFEPATH) drops the script's own directory from sys.path
+sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
+
+from proxytestlib import bind_ephemeral  # noqa: E402
+
+held = []  # keep every socket open: a close would fail the handshake outright
+
+
+def stall(conn, delay):
+    if delay is not None:
+        rfile = conn.makefile("rb")
+        while rfile.readline() not in (b"\r\n", b"\n", b""):
+            pass
+        time.sleep(delay)
+        conn.sendall(b"HTTP/1.0 200 Connection established\r\n\r\n")
+    held.append(conn)
+
+
+def main():
+    mode = sys.argv[1] if len(sys.argv) > 1 else "direct"
+    delay = float(sys.argv[2]) if mode == "proxy" else None
+    srv, port = bind_ephemeral()
+    sys.stdout.reconfigure(newline="\n")  # Windows would emit \r\n
+    print("PORT %d" % port, flush=True)
+    while True:
+        conn, _ = srv.accept()
+        threading.Thread(target=stall, args=(conn, delay), daemon=True).start()
+
+
+main()