proxyurl test: percent-encoded specials and UTF-8

Pin that the host/port split is byte-transparent: a percent-encoded '@' (%40) or
':' (%3A) in the userinfo stays encoded and never mis-splits host or port, a
second raw '@' is handled by the last-'@' host rule, and UTF-8 bytes in the
username and host label pass through intact. Decoding happens later at auth time,
not in the parser. Raw UTF-8 CLI args match the existing 01_engine-idna.test.

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/htscoremain.c

@@ -2253,21 +2253,12 @@ static int hts_main_internal(int argc, char **argv, httrackp * opt) {
               htsmain_free();
               return -1;
             } else {
-              char *a;
+              char BIGSTK pname[HTS_URLMAXSIZE * 2];
 
               na++;
               opt->proxy.active = 1;
-              // Rechercher MAIS en partant de la fin à cause de user:pass@proxy:port
-              a = argv[na] + strlen(argv[na]) - 1;
-              while((a > argv[na]) && (*a != ':') && (*a != '@'))
-                a--;
-              if (*a == ':') {  // un port est présent, <proxy>:port
-                sscanf(a + 1, "%d", &opt->proxy.port);
-                StringCopyN(opt->proxy.name, argv[na], (int) (a - argv[na]));
-              } else {          // <proxy>
-                opt->proxy.port = 8080;
-                StringCopy(opt->proxy.name, argv[na]);
-              }
+              hts_parse_proxy(argv[na], pname, sizeof(pname), &opt->proxy.port);
+              StringCopy(opt->proxy.name, pname);
             }
             break;
           case 'F':            // user-agent field

src/htslib.c

@@ -3866,6 +3866,10 @@ const char *jump_protocol_const(const char *source) {
     source += p;
   else if ((p = strfield(source, "file:")))
     source += p;
+  else if ((p = strfield(source, "socks5h:")))
+    source += p;
+  else if ((p = strfield(source, "socks5:")))
+    source += p;
   // net_path
   if (strncmp(source, "//", 2) == 0)
     source += 2;
@@ -3874,6 +3878,43 @@ const char *jump_protocol_const(const char *source) {
 
 DECLARE_NON_CONST_VERSION(jump_protocol)
 
+// default proxy port for a -P argument, keyed on the scheme
+static int proxy_default_port(const char *arg) {
+  if (strfield(arg, "socks5h:") || strfield(arg, "socks5:"))
+    return 1080;
+  return 8080;
+}
+
+void hts_parse_proxy(const char *arg, char *name, size_t name_size, int *port) {
+  const char *authority = strstr(arg, "://");
+  const char *a;
+  size_t namelen;
+
+  if (name_size == 0)
+    return;
+  // scan back to the port ':' (or userinfo '@'), but never past the authority,
+  // so a scheme's own colon is not read as a port separator; inspect a[-1] from
+  // one-past-end so no pointer below the string is ever formed
+  authority = (authority != NULL) ? authority + 3 : arg;
+  a = arg + strlen(arg);
+  while (a > authority && 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);
+    namelen = (size_t) (a - 1 - arg);
+  } else {
+    *port = proxy_default_port(arg);
+    namelen = strlen(arg);
+  }
+  if (namelen >= name_size) // arg is user-controlled: truncate, don't overflow
+    namelen = name_size - 1;
+  memcpy(name, arg, namelen);
+  name[namelen] = '\0';
+}
+
 // codage base 64 a vers b
 void code64(unsigned char *a, int size_a, unsigned char *b, int crlf) {
   int i1 = 0, i2 = 0, i3 = 0, i4 = 0;

src/htslib.h

@@ -289,6 +289,12 @@ int may_unknown2(httrackp * opt, const char *mime, const char *filename);
 const char *strrchr_limit(const char *s, char c, const char *limit);
 char *jump_protocol(char *source);
 const char *jump_protocol_const(const char *source);
+
+/* Split a -P proxy argument "[scheme://][user:pass@]host[:port]" into the proxy
+   host string (scheme and any user:pass kept, for later stripping and auth),
+   written NUL-terminated into name[name_size] (truncated to fit), and the port
+   in *port. The port defaults by scheme: 1080 for socks5/socks5h, else 8080. */
+void hts_parse_proxy(const char *arg, char *name, size_t name_size, int *port);
 void code64(unsigned char *a, int size_a, unsigned char *b, int crlf);
 
 #define copychar(catbuff,a) concat(catbuff,(a),NULL)

src/htsselftest.c

@@ -1308,6 +1308,22 @@ static int st_identurl(httrackp *opt, int argc, char **argv) {
   return 0;
 }
 
+static int st_proxyurl(httrackp *opt, int argc, char **argv) {
+  char BIGSTK name[HTS_URLMAXSIZE * 2];
+  int port = -1;
+
+  (void) opt;
+  if (argc < 1) {
+    fprintf(stderr, "proxyurl: needs a proxy argument\n");
+    return 1;
+  }
+  hts_parse_proxy(argv[0], name, sizeof(name), &port);
+  // host= is what the connect actually resolves (scheme + user:pass stripped)
+  printf("name=%s port=%d host=%s\n", name, port,
+         jump_identification_const(name));
+  return 0;
+}
+
 /* Regression for the one-byte fil[] overflow: a 2047-byte hostless "?"-URL used
    to abort in strncat_safe_ when the missing leading '/' pushed fil to 2048. */
 static int st_identabs(httrackp *opt, int argc, char **argv) {
@@ -2756,6 +2772,8 @@ static const struct selftest_entry {
     {"resolve", "<link> <adr> <fil>", "resolve a link against an origin",
      st_resolve},
     {"identurl", "<url>", "split an absolute URL into (adr, fil)", st_identurl},
+    {"proxyurl", "<proxy-arg>", "parse a -P proxy URL into host/port",
+     st_proxyurl},
     {"identabs", "", "ident_url_absolute one-byte fil[] overflow self-test",
      st_identabs},
     {"header", "<raw-header-line> ...", "response header-line parsing",

tests/01_engine-proxyurl.test

@@ -0,0 +1,44 @@
+#!/bin/bash
+#
+
+set -euo pipefail
+
+# hts_parse_proxy splits a -P argument "[scheme://][user:pass@]host[:port]" into
+# the proxy host string and port. -#test=proxyurl <arg> prints
+# "name=.. port=.. host=..", where host= is what the connect resolves.
+
+p() {
+    test "$(httrack -O /dev/null -#test=proxyurl "$1")" == "$2" || exit 1
+}
+
+# bare host, with and without an explicit port
+p 'proxy.example.com:8080' 'name=proxy.example.com port=8080 host=proxy.example.com'
+p 'proxy.example.com' 'name=proxy.example.com port=8080 host=proxy.example.com'
+
+# user:pass@ is kept in name (for proxy auth) and stripped from host
+p 'user:pass@proxy:3128' 'name=user:pass@proxy port=3128 host=proxy'
+p 'user:pass@proxy' 'name=user:pass@proxy port=8080 host=proxy'
+
+# a scheme colon must not be read as a port: http://host with no port used to
+# parse to name=http with a garbage port
+p 'http://proxy:8080' 'name=http://proxy port=8080 host=proxy'
+p 'http://proxy' 'name=http://proxy port=8080 host=proxy'
+
+# socks5/socks5h default to 1080 and resolve to the bare host
+p 'socks5://host:1080' 'name=socks5://host port=1080 host=host'
+p 'socks5://host' 'name=socks5://host port=1080 host=host'
+p 'socks5h://host' 'name=socks5h://host port=1080 host=host'
+# explicit port wins over the 1080 default, with and without userinfo
+p 'socks5://host:9050' 'name=socks5://host port=9050 host=host'
+p 'socks5://user:pass@host:9050' 'name=socks5://user:pass@host port=9050 host=host'
+
+# the split is byte-transparent: percent-escapes stay encoded (decoded later at
+# auth time), only structural ASCII ':'/'@' delimit, and the last '@' ends the
+# userinfo. So %40/%3A and UTF-8 bytes never mis-split host or port.
+p 'socks5://user:p%40ss@host:1080' 'name=socks5://user:p%40ss@host port=1080 host=host'
+p 'socks5://us%3Aer:pass@host:1080' 'name=socks5://us%3Aer:pass@host port=1080 host=host'
+p 'socks5://a@b:c@host:1080' 'name=socks5://a@b:c@host port=1080 host=host'
+p 'socks5://naïve:pass@héllo:1080' 'name=socks5://naïve:pass@héllo port=1080 host=héllo'
+
+# a lone ':' must not underflow the backward scan
+p ':' 'name= port=8080 host='

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

@@ -46,6 +46,7 @@ TESTS = \
 	01_engine-header.test \
 	01_engine-idna.test \
 	01_engine-identurl.test \
+	01_engine-proxyurl.test \
 	01_engine-identabs.test \
 	01_engine-escape-room.test \
 	01_engine-inplace-escape.test \
@@ -124,6 +125,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,