Cover the SOCKS5 origin port in the self-test, and trim comments

Reverting htsproxy.c alone to the wrap-then-check version passed the whole
suite, so nothing pinned that half of the fix and a later refactor could have
dropped it silently. The scripted SOCKS5 harness now asserts a bad origin port
is refused with no byte sent.

65616 would not have tested anything: it fits an int, so the old range check
already caught it. The case the old check could not see is 4294967376, which
overflowed the sscanf("%d") into a plausible 80 and passed; the trailing-junk
and signed spellings sscanf also accepted are covered alongside it. Against
master's htsproxy.c the new assertion fails.

Also note at the SOCKS5 call site that an empty "host:" stays refused there
while the direct path accepts it, so the asymmetry reads as intended rather
than as an oversight, and halve two comments.

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

lang.indexes

@@ -23,7 +23,7 @@ pt:7
 ro:25
 ru:8
 sk:20
-si:24
+sl:24
 sv:17
 tr:10
 uk:22

lang/Finnish.txt

@@ -930,7 +930,6 @@ Server terminated
 Palvelin lopetettu
 A fatal error has occurred during this mirror
 Tällä peilillä tapahtui vakava virhe
-
 Proxy type:
 Välityspalvelimen tyyppi:
 Proxy protocol. HTTP: standard proxy. HTTP (CONNECT tunnel): sends every request through a CONNECT tunnel, for CONNECT-only proxies like Tor's HTTPTunnelPort. SOCKS5: default port 1080.

lang/Italiano.txt

@@ -374,7 +374,7 @@ Create error logging and report files
 Crea file di log per segnalare errori e informazioni
 Generate DOS 8-3 filenames ONLY
 Genera solo nomi di file in formato 8.3
-Generate ISO 9660 filenames ONLY for CDROM medias
+Generate ISO9660 filenames ONLY for CDROM medias
 Genera nomi di file in formato ISO9660 per i CDROM
 Do not create HTML error pages
 Non generare pagine d'errore HTML

lang/Portugues-Brasil.txt

@@ -918,7 +918,7 @@ normal\nextended\ndebug
 normal\nextendido\ncorrigir
 Download web site(s)\nDownload web site(s) + questions\nGet individual files\nDownload all sites in pages (multiple mirror)\nTest links in pages (bookmark test)\n* Continue interrupted download\n* Update existing download
 Copiar site(s) da Web\nCopiar site(s) interativos da Web (perguntas)\nReceber arquivos específicos\nCopiar todas as páginas do site (alternação múltipla)\nTestar links nas páginas (testar indicador)\n* Retomar download interrompido\n* Atualizar download existente
-Relative URL / Absolute URL (default)\nAbsolute URL / Absolute URL\nAbsolute URI / Absolute URL\nOriginal URL / Original URL
+Relative URI / Absolute URL (default)\nAbsolute URL / Absolute URL\nAbsolute URI / Absolute URL\nOriginal URL / Original URL
 URI Relativa / URL Absoluta (padrão)\nURL Absoluta / URL Absoluta\nURI Absoluta / URL Absoluta\nURL Original / URL Original
 Open Source offline browser
 Abrir origem offline no navegador

lang/Slovenian.txt

@@ -3,7 +3,7 @@ Slovenian
 LANGUAGE_FILE
 Slovenian
 LANGUAGE_ISO
-si
+sl
 LANGUAGE_AUTHOR
 Jadran Rudec,iur.\r\njrudec@email.si \r\n
 LANGUAGE_CHARSET

src/htsdns_selftest.c

@@ -402,6 +402,50 @@ int dns_selftests(httrackp *opt) {
       deletesoc(s);
   }
 
+  /* A URL port outside 1..65535 must refuse the link, not fold into range and
+     connect elsewhere (#614). *addr_count discriminates: 0 only if refused
+     before the resolve, still 2 for one merely truncated or defaulted. */
+  {
+    /* an empty "dual.test:" means the default port (WHATWG, curl): keep it */
+    static const char *const good[] = {"dual.test:1",    "dual.test:80",
+                                       "dual.test:8080", "dual.test:65535",
+                                       "dual.test:080",  "dual.test:"};
+    /* 65616 and 4294967376 are load-bearing: both wrap to a plausible 80 */
+    static const char *const bad[] = {
+        "dual.test:0",          "dual.test:65536",      "dual.test:65616",
+        "dual.test:99999",      "dual.test:2147483648", "dual.test:4294967296",
+        "dual.test:4294967376", "dual.test:-1",         "dual.test:-23437",
+        "dual.test:80x",        "dual.test:+80",        "dual.test: 80",
+        "dual.test:0x50"};
+    size_t k;
+
+    for (k = 0; k < sizeof(good) / sizeof(good[0]); k++) {
+      htsblk r;
+      int count = -1;
+      T_SOC s;
+
+      hts_init_htsblk(&r);
+      s = newhttp_addr(opt, good[k], &r, -1, 0, 0, &count);
+      CHECK(count == 2); /* accepted: reached the resolve */
+      if (s != INVALID_SOCKET)
+        deletesoc(s);
+    }
+
+    for (k = 0; k < sizeof(bad) / sizeof(bad[0]); k++) {
+      htsblk r;
+      int count = -1;
+      T_SOC s;
+
+      hts_init_htsblk(&r);
+      s = newhttp_addr(opt, bad[k], &r, -1, 0, 0, &count);
+      CHECK(s == INVALID_SOCKET);
+      CHECK(count == 0); /* refused before resolving, not a failed connect */
+      CHECK(strstr(r.msg, "Invalid port") != NULL);
+      if (s != INVALID_SOCKET)
+        deletesoc(s);
+    }
+  }
+
   /* Connect-fallback decision (consumer of the multi-address list): when a
      stuck connect should abandon the current address for the next one. */
   {

src/htslib.c

@@ -2167,15 +2167,18 @@ T_SOC newhttp_addr(httrackp *opt, const char *_iadr, htsblk *retour, int port,
 #endif
 
       if (a != NULL) {
-        int i = -1;
-
-        iadr2[0] = '\0';
-        sscanf(a + 1, "%d", &i);
-        if (i != -1) {
-          port = (unsigned short int) i;
+        // folding a nonsense port into 0..65535 crawls one neither the link nor
+        // a port filter named; an empty "host:" just means the default (#614)
+        if (a[1] != '\0' && !hts_parse_url_port(a + 1, &port)) {
+          if (retour != NULL) {
+            snprintf(retour->msg, sizeof(retour->msg), "Invalid port: %s",
+                     a + 1);
+          }
+          return INVALID_SOCKET;
         }
 
-        // adresse véritable (sans :xx)
+        iadr2[0] = '\0';
+        // the address itself, without the ":port"
         strncatbuff(iadr2, iadr, (int) (a - iadr));
         resolve_host = iadr2;
       }
@@ -3748,18 +3751,27 @@ 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) {
+hts_boolean hts_parse_url_port(const char *a, int *port) {
   char *end;
   long p;
 
   if (!isdigit((unsigned char) *a)) // strtol would eat a sign or leading space
-    return proxy_default_port(arg);
+    return HTS_FALSE;
   p = strtol(a, &end, 10);
   if (*end != '\0' || p < 1 || p > 65535) // ERANGE lands out of range too
+    return HTS_FALSE;
+  *port = (int) p;
+  return HTS_TRUE;
+}
+
+// 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) {
+  int port;
+
+  if (!hts_parse_url_port(a, &port))
     return proxy_default_port(arg);
-  return (int) p;
+  return port;
 }
 
 void hts_parse_proxy(const char *arg, char *name, size_t name_size, int *port) {

src/htslib.h

@@ -288,6 +288,12 @@ 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);
 
+/* Parse a URL's port text "a" (after the ':', up to the end of the string):
+   TRUE and *port set for a bare decimal in 1..65535, else FALSE and *port left
+   alone. Not sscanf("%d"), which range-checks nothing and wraps past INT_MAX.
+ */
+hts_boolean hts_parse_url_port(const char *a, int *port);
+
 /* 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

src/htsproxy.c

@@ -491,14 +491,11 @@ static int socks5_handshake_stream(httrackp *opt, socks5_stream *st,
     if ((unsigned char) host[i] < ' ')
       return socks5_fail(msg, msgsize, "SOCKS5: invalid origin host");
   }
-  if (portsep != NULL) {
-    int p = -1;
-
-    sscanf(portsep + 1, "%d", &p);
-    if (p <= 0 || p > 65535)
-      return socks5_fail(msg, msgsize, "SOCKS5: invalid origin port");
-    port = p;
-  }
+  // the old range check ran after sscanf("%d") had wrapped a huge value into a
+  // plausible port (#614). An empty "host:" stays refused here, unlike the
+  // direct path, as it was before #614.
+  if (portsep != NULL && !hts_parse_url_port(portsep + 1, &port))
+    return socks5_fail(msg, msgsize, "SOCKS5: invalid origin port");
   if (link_has_authorization(proxy_name)) {
     if (!socks5_credentials(opt, proxy_name, user, sizeof(user), &userlen, pass,
                             sizeof(pass), &passlen, msg, msgsize))

src/htsselftest.c

@@ -1453,6 +1453,25 @@ static int st_socks5(httrackp *opt, int argc, char **argv) {
   assertf(socks5_handshake_scripted(opt, "origin.test:8443", proxy, &io) == 1);
   assertf(memcmp(io.sent + io.sent_len - 2, "\x20\xfb", 2) == 0);
 
+  /* a bad origin port is refused before any byte goes out (#614). 4294967376 is
+     the case the old range check could not see: it overflowed the sscanf("%d")
+     into a plausible 80 and passed. 65616 would not prove anything here, since
+     it fits an int and the old check already caught it. */
+  {
+    static const char *const bad[] = {"origin.test:4294967376",
+                                      "origin.test:80x", "origin.test:+80",
+                                      "origin.test: 80", "origin.test:8.0"};
+    size_t k;
+
+    for (k = 0; k < sizeof(bad) / sizeof(bad[0]); k++) {
+      len = socks5_reply(script, 0x01, v4, sizeof(v4));
+      io.reply = script;
+      io.reply_len = len;
+      assertf(socks5_handshake_scripted(opt, bad[k], proxy, &io) == 0);
+      assertf(io.sent_len == 0);
+    }
+  }
+
   /* credentials: split on the first colon of the escaped userinfo, so %3a stays
      inside the username and a colon in the password is not a delimiter */
   {

src/webhttrack

@@ -33,6 +33,19 @@ function log {
     return 0
 }
 
+# Map a POSIX locale ("zh_TW.UTF-8@euro") to its lang.indexes number, English (1) if unknown
+function lang_index {
+    local locale=$1 indexes=$2 tag n t
+    tag=$(echo "${locale}" | cut -f1 -d'.' | cut -f1 -d'@' | tr '[:upper:]' '[:lower:]' | tr -cd 'a-z_')
+    # a few languages carry a region-specific entry (zh_tw, pt_br); else use the bare language
+    for t in "${tag}" "${tag%%_*}"; do
+        n=$(grep -E "^${t}:" "${indexes}" | cut -f2 -d':')
+        test -n "${n}" && break
+    done
+    test -n "${n}" || n=1
+    echo "${n}"
+}
+
 function launch_browser {
     log "Launching $1"
     browser=$1
@@ -59,13 +72,11 @@ test -f "${DISTPATH}/lang.indexes" || ! log "Could not find ${DISTPATH}/lang.ind
 test -d "${DISTPATH}/lang" || ! log "Could not find ${DISTPATH}/lang" || exit 1
 test -d "${DISTPATH}/html" || ! log "Could not find ${DISTPATH}/html" || exit 1
 
-# Locale
-HTSLANG="${LC_MESSAGES}"
-! test -n "${HTSLANG}" && HTSLANG="${LC_ALL}"
+# Locale: POSIX precedence, LC_ALL overrides LC_MESSAGES overrides LANG
+HTSLANG="${LC_ALL}"
+! test -n "${HTSLANG}" && HTSLANG="${LC_MESSAGES}"
 ! test -n "${HTSLANG}" && HTSLANG="${LANG}"
-HTSLANG="$(echo "$LANG" | cut -f1 -d'.' | cut -f1 -d'_')"
-LANGN=$(grep -E "^${HTSLANG}:" "${DISTPATH}/lang.indexes" | cut -f2 -d':')
-! test -n "${LANGN}" && LANGN=1
+LANGN=$(lang_index "${HTSLANG}" "${DISTPATH}/lang.indexes")
 
 # Find the browser
 # note: not all systems have sensible-browser or www-browser alternative

tests/61_webhttrack-locale.test

@@ -0,0 +1,91 @@
+#!/bin/bash
+#
+# webhttrack must map the caller's locale to the right lang.indexes number.
+# Guards: LC_ALL/LC_MESSAGES were ignored, and zh_tw/pt_br were unreachable.
+
+set -euo pipefail
+
+testdir=$(cd "$(dirname "$0")" && pwd)
+distdir=$(cd "${testdir}/.." && pwd)
+script="${distdir}/src/webhttrack"
+
+fail() {
+    echo "FAIL: $*" >&2
+    exit 1
+}
+
+test -r "${script}" || fail "no ${script}"
+test -r "${distdir}/lang.indexes" || fail "no ${distdir}/lang.indexes"
+
+# Lift the locale code out of the launcher, so this tests the shipped logic itself.
+funcs=$(sed -n '/^function lang_index/,/^}/p' "${script}")
+# End on the next section, not a blank line: a blank line grown inside the block
+# would truncate the lift and silently stop testing everything below it.
+grep -q '^# Find the browser' "${script}" || fail "locale block terminator moved in ${script}"
+block=$(sed -n '/^# Locale/,/^# Find the browser/p' "${script}" | sed '$d')
+echo "${block}" | grep -q 'LANGN=.*lang_index' || fail "could not lift the whole locale block from ${script}"
+
+# Run the lifted block against whatever locale vars the caller exported.
+run_block() {
+    # shellcheck disable=SC2034 # read by the eval'd locale block
+    DISTPATH="${distdir}"
+    # the launcher runs under neither -e ("! test -n x && y" returns 1) nor -u
+    set +eu
+    eval "${funcs}"
+    eval "${block}"
+    # shellcheck disable=SC2153 # LANGN is set by the eval'd block, not a LANG typo
+    echo "${LANGN}"
+}
+
+# Resolve LANG/LC_ALL/LC_MESSAGES (empty = unset) to the language number.
+lang_of() {
+    (
+        unset LANG LC_ALL LC_MESSAGES
+        # only the value matters here; hide bash's setlocale gripe when the locale is not installed
+        # shellcheck disable=SC2030 # confining the locale to this subshell is the point
+        {
+            if test -n "$1"; then export LANG="$1"; fi
+            if test -n "$2"; then export LC_ALL="$2"; fi
+            if test -n "$3"; then export LC_MESSAGES="$3"; fi
+        } 2>/dev/null
+        run_block
+    )
+}
+
+check() {
+    local want=$1 got desc
+    shift
+    desc="LANG='$1' LC_ALL='$2' LC_MESSAGES='$3'"
+    got=$(lang_of "$1" "$2" "$3")
+    test "${got}" = "${want}" || fail "${desc}: want ${want}, got ${got}"
+    echo "ok: ${desc} -> ${got}"
+}
+
+#      want  LANG            LC_ALL          LC_MESSAGES
+check 2 "fr_FR.UTF-8" "" ""
+check 14 "zh_TW.UTF-8" "" ""                      # region-qualified: Chinese-BIG5, not Simplified (13)
+check 12 "pt_BR.UTF-8" "" ""                      # region-qualified: Portugues-Brasil, not Portugues (7)
+check 13 "zh_CN.UTF-8" "" ""                      # unlisted region falls back to the bare language
+check 24 "sl_SI.UTF-8" "" ""                      # Slovenian is ISO 639-1 "sl"
+check 4 "" "de_DE.UTF-8" ""                       # LC_ALL alone
+check 4 "" "" "de_DE.UTF-8"                       # LC_MESSAGES alone
+check 4 "fr_FR.UTF-8" "de_DE.UTF-8" ""            # LC_ALL wins over LANG
+check 4 "fr_FR.UTF-8" "" "de_DE.UTF-8"            # LC_MESSAGES wins over LANG
+check 2 "de_DE.UTF-8" "fr_FR.UTF-8" "de_DE.UTF-8" # LC_ALL wins over LC_MESSAGES
+check 2 "fr_FR@euro" "" ""                        # @modifier without a charset
+check 1 "C" "" ""                                 # C, unknown and unset all mean English
+check 1 "xx_YY.UTF-8" "" ""
+check 1 "" "" ""
+
+# Exported but empty is unset (POSIX), so it must not mask LANG. lang_of cannot
+# express this: it exports nothing for an empty argument.
+got=$(
+    unset LANG LC_ALL LC_MESSAGES
+    # shellcheck disable=SC2031 # confining the locale to this subshell is the point
+    { export LANG="fr_FR.UTF-8" LC_ALL="" LC_MESSAGES=""; } 2>/dev/null
+    run_block
+)
+test "${got}" = 2 || fail "empty LC_ALL/LC_MESSAGES must not mask LANG: want 2, got ${got}"
+echo "ok: LANG='fr_FR.UTF-8' with LC_ALL='' exported -> ${got}"
+
+echo "PASS"

tests/62_lang-integrity.test

@@ -0,0 +1,90 @@
+#!/bin/bash
+#
+# lang/*.txt are line pairs: an English msgid, then its translation. A stray
+# blank or a drifted msgid silently unhooks every translation that follows,
+# rather than failing, so assert the pairing and the join against English.txt.
+# Pairs are physical lines here; a msgid ending in \ continues onto the next one
+# for the engine (linput_cpp) but not for us. None do today.
+
+set -euo pipefail
+
+langdir="${top_srcdir:-..}/lang"
+def="${top_srcdir:-..}/lang.def"
+eng="$langdir/English.txt"
+for f in "$eng" "$def"; do
+    [ -f "$f" ] || {
+        echo "cannot find $f"
+        exit 1
+    }
+done
+
+tmp=$(mktemp -d)
+trap 'rm -rf "$tmp"' EXIT
+keys="$tmp/english.keys"
+
+# Byte-wise: each file is in its own declared legacy charset, lang.def is CRLF.
+LC_ALL=C awk 'NR%2==1 { sub(/\r$/, ""); print }' "$eng" >"$keys"
+
+# An empty parse would make every check below pass vacuously.
+nkeys=$(wc -l <"$keys")
+if [ "$nkeys" -lt 400 ]; then
+    echo "only $nkeys msgids parsed from $eng; the parse is broken"
+    exit 1
+fi
+
+nfiles=0
+fail=0
+for f in "$langdir"/*.txt; do
+    nfiles=$((nfiles + 1))
+    LC_ALL=C awk -v keys="$keys" -v name="${f##*/}" '
+        BEGIN { while ((getline k < keys) > 0) known[k] = 1 }
+        { sub(/\r$/, "") }
+        NR % 2 == 1 {
+            if ($0 == "")
+                printf "%s:%d: blank line where an English msgid belongs\n", name, NR
+            else if (!($0 in known))
+                printf "%s:%d: msgid absent from English.txt: %s\n", name, NR, $0
+            else
+                next
+            bad++
+        }
+        END {
+            if (NR % 2) {
+                printf "%s: odd line count (%d): msgid/translation pairing is broken\n", name, NR
+                bad++
+            }
+            exit(bad > 0)
+        }
+    ' "$f" || fail=1
+done
+
+if [ "$nfiles" -lt 25 ]; then
+    echo "only $nfiles language files found in $langdir"
+    exit 1
+fi
+
+# lang.def maps each LANG_* macro to the English string the GUI compiles in,
+# which is the lookup key into every lang/*.txt: no msgid, no translations.
+LC_ALL=C awk -v keys="$keys" '
+    BEGIN { while ((getline k < keys) > 0) known[k] = 1 }
+    { sub(/\r$/, "") }
+    NR % 2 == 1 { macro = $0; next }
+    macro ~ /^(LANG_|LISTDEF_)/ {
+        n++
+        if (!($0 in known)) {
+            printf "lang.def:%d: %s maps to \"%s\", which is not a msgid in English.txt\n", NR, macro, $0
+            bad++
+        }
+    }
+    END {
+        if (n < 400) {
+            printf "lang.def: only %d LANG_ entries parsed; the parse is broken\n", n
+            bad++
+        }
+        exit(bad > 0)
+    }
+' "$def" || fail=1
+
+[ "$fail" -eq 0 ] || exit 1
+
+echo "$nfiles language files consistent with $nkeys English msgids"

tests/Makefile.am

@@ -139,6 +139,8 @@ TESTS = \
 	57_local-proxy-connect.test \
 	58_watchdog.test \
 	59_local-tls-stall.test \
-	60_crawl-log-salvage.test
+	60_crawl-log-salvage.test \
+	61_webhttrack-locale.test \
+	62_lang-integrity.test
 
 CLEANFILES = check-network_sh.cache