Merge remote-tracking branch 'origin/master' into fix-908-bash-shell-validate

A shell can be in POSIX mode because it was invoked as sh or because the
environment forces it, and only the second probe can say which. Nothing held
that down, so a version deciding from the environment alone, without
re-probing, passed every case in the suite while telling the user to clear a
variable that would not have helped. Test 151 now runs a bash symlinked as sh
with POSIXLY_CORRECT=1 set as well, and requires the message to name the path.

The comment records why that branch cannot simply read POSIXLY_CORRECT:
autoconf runs "set -o posix" on configure's own shell, so it is set there no
matter what the user's environment holds.

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

configure.ac

@@ -38,6 +38,68 @@ VERSION_INFO="3:7:0"
 AM_MAINTAINER_MODE
 AC_USE_SYSTEM_EXTENSIONS
 
+# A real bash, for "make deb" and the test harness. Not searched into BASH: bash presets
+# that to its own path, and macOS /bin/sh is a bash, so the macro never searches (#895).
+# BASH_SHELL isn't preset the way BASH is, so AC_ARG_VAR needs no guard. Kept ahead of the
+# compiler probes so a bad override dies before them.
+AC_ARG_VAR([BASH_SHELL], [path to a real (non-POSIX-mode) bash])
+# AC_PATH_PROGS drops a relative override and searches instead, which loses the user's intent.
+# Nothing quotes $(BASH_SHELL) in the Makefiles, and quoting could not save it anyway: make
+# splits on whitespace, expands '$' and treats '#' as a comment before any shell sees it.
+case $BASH_SHELL in
+*[[[:space:]]]* | *'#'* | *'$'* | *'`'* | *'\'* | *'"'* | *"'"* | *';'* | *'&'* | *'|'* | \
+*'<'* | *'>'* | *'('* | *')'* | *'*'* | *'?'* | *'@<:@'* | *'@:>@'* | *'{'* | *'}'*)
+  AC_MSG_ERROR([BASH_SHELL must not contain shell or make metacharacters, got: $BASH_SHELL]) ;;
+'' | [[\\/]]* | ?:[[\\/]]*) ;;
+*) AC_MSG_ERROR([BASH_SHELL must be an absolute path, got: $BASH_SHELL]) ;;
+esac
+hts_bash_override=$BASH_SHELL
+AC_PATH_PROGS([BASH_SHELL], [bash], [/bin/bash])
+
+# An absolute override is taken verbatim, so BASH_SHELL=/bin/sh would put #895 back and only
+# surface at "make check" or "make deb" (#908). What we found ourselves is only a warning:
+# a box with no bash still builds, it just cannot run those two.
+AC_MSG_CHECKING([whether $BASH_SHELL is a bash outside POSIX mode])
+hts_bash_why=
+hts_bash_env=no
+if test ! -x "$BASH_SHELL"; then
+  hts_bash_why="not an executable file"
+elif test -z "$("$BASH_SHELL" -c 'echo "${BASH_VERSINFO[[0]]}"' 2>/dev/null)"; then
+  # Not BASH_VERSION: that is an ordinary variable, so any shell echoes back a spoofed one.
+  hts_bash_why="not a bash: it reports no BASH_VERSINFO"
+else
+  # sh-mode bash reports a version too, so only SHELLOPTS tells the two apart.
+  case $("$BASH_SHELL" -c 'echo ":$SHELLOPTS:"' 2>/dev/null) in
+  *:posix:*)
+    hts_bash_why="a bash in POSIX sh-mode"
+    # POSIXLY_CORRECT and an exported SHELLOPTS do that to every bash on the box, so no path
+    # can pass and blaming this one would send the user hunting for another. Reading them
+    # here would not do: configure puts its own shell in posix mode, which sets both.
+    case $(env -u POSIXLY_CORRECT -u SHELLOPTS "$BASH_SHELL" -c 'echo ":$SHELLOPTS:"' 2>/dev/null) in
+    '' | *:posix:*) ;; # no "env -u", or posix whatever the environment: blame the path
+    *) hts_bash_env=yes ;;
+    esac
+    ;;
+  esac
+fi
+if test -z "$hts_bash_why"; then
+  AC_MSG_RESULT([yes])
+else
+  AC_MSG_RESULT([no])
+  hts_bash_msg="POSIXLY_CORRECT or SHELLOPTS forces every bash into POSIX sh-mode, $BASH_SHELL included. Clear them for configure and for make, which hands them to make check and make deb: env -u POSIXLY_CORRECT -u SHELLOPTS ..."
+  if test "$hts_bash_env" = yes; then
+    if test -n "$hts_bash_override"; then
+      AC_MSG_ERROR([$hts_bash_msg])
+    fi
+    AC_MSG_WARN([$hts_bash_msg])
+  else
+    if test -n "$hts_bash_override"; then
+      AC_MSG_ERROR([BASH_SHELL=$BASH_SHELL is $hts_bash_why])
+    fi
+    AC_MSG_WARN([no usable bash found: $BASH_SHELL is $hts_bash_why. "make check" and "make deb" need one; pass BASH_SHELL=/path/to/bash])
+  fi
+fi
+
 AC_PROG_CC
 AM_PROG_CC_C_O
 m4_warn([obsolete],
@@ -55,12 +117,6 @@ LT_INIT
 AC_PROG_LN_S
 LT_INIT
 
-# A real bash, for "make deb" and the test harness. Not searched into BASH: bash presets
-# that to its own path, and macOS /bin/sh is a bash, so the macro never searches (#895).
-# BASH_SHELL isn't preset the way BASH is, so AC_ARG_VAR needs no guard.
-AC_ARG_VAR([BASH_SHELL], [path to a real (non-POSIX-mode) bash])
-AC_PATH_PROGS([BASH_SHELL], [bash], [/bin/bash])
-
 # Export LD_LIBRARY_PATH name or equivalent.
 AC_SUBST(SHLIBPATH_VAR,$shlibpath_var)
 

src/htsback.c

@@ -3493,16 +3493,15 @@ void back_wait(struct_back * sback, httrackp * opt, cache_back * cache,
 
           // réception de données depuis socket ou fichier
           if (back[i].status) {
-            if (back[i].status == STATUS_WAIT_HEADERS)
-              retour_fread = http_xfread1(&(back[i].r), HTS_XFREAD_LINE_BLOCK);
+            if (back[i].status == STATUS_WAIT_HEADERS)  // recevoir par bloc de lignes
+              retour_fread = http_xfread1(&(back[i].r), 0);
             else if (back[i].status == STATUS_CHUNK_WAIT || back[i].status == STATUS_CHUNK_CR) {        // recevoir longueur chunk en hexa caractère par caractère
               // backuper pour lire dans le buffer chunk
               htsblk r;
               /* Block mode bounds the trailer section, which declares no length
                  of its own, by HTS_LINE_BLOCK_SIZE. */
-              const int chunk_read_mode = back_in_chunk_trailers(&back[i])
-                                              ? HTS_XFREAD_LINE_BLOCK
-                                              : HTS_XFREAD_LINE;
+              const int chunk_read_mode =
+                  back_in_chunk_trailers(&back[i]) ? 0 : -1;
 
               memcpy(&r, &(back[i].r), sizeof(htsblk));
               back[i].r.is_write = 0;   // mémoire
@@ -3512,7 +3511,7 @@ void back_wait(struct_back * sback, httrackp * opt, cache_back * cache,
               back[i].r.out = NULL;
               back[i].r.is_file = 0;
               //
-              // one line, or the whole trailer block
+              // ligne par ligne
               retour_fread = http_xfread1(&(back[i].r), chunk_read_mode);
               // modifier et restaurer
               back[i].chunk_adr = back[i].r.adr;        // adresse

src/htslib.c

@@ -1834,8 +1834,12 @@ int check_writeinput_t(T_SOC soc, int timeout) {
     return 0;
 }
 
-// Read one block: bufl is a byte count, or one of the HTS_XFREAD_* line modes.
-// Note: the +1 in the mallocs is the trailing NUL appended to the data.
+// idem, sauf qu'ici on peut choisir la taille max de données à recevoir
+// SI bufl==0 alors le buffer est censé être de 8kos, et on recoit par bloc de lignes
+// en éliminant les cr (ex: header), arrêt si double-lf
+// SI bufl==-1 alors le buffer est censé être de 8kos, et on recoit ligne par ligne
+// en éliminant les cr (ex: header), arrêt si double-lf
+// Note: les +1 dans les malloc sont dûs à l'octet nul rajouté en fin de fichier
 LLint http_xfread1(htsblk * r, int bufl) {
   int nl = -1;
 
@@ -1948,14 +1952,14 @@ LLint http_xfread1(htsblk * r, int bufl) {
 
     }                           // stockage disque ou mémoire
 
-  } else if (bufl == HTS_XFREAD_RESERVE) {
+  } else if (bufl == -2) {      // force reserve
     if (r->adr == NULL) {
       r->adr = (char *) malloct(HTS_LINE_BLOCK_SIZE);
       r->size = 0;
       return 0;
     }
     return -1;
-  } else { // line modes: byte by byte, CR dropped
+  } else {                      // réception d'un en-tête octet par octet
     int count = 256;
     int tot_nl = 0;
     int lf_detected = 0;
@@ -1973,8 +1977,10 @@ LLint http_xfread1(htsblk * r, int bufl) {
           // lecture
           nl = hts_read(r, r->adr + r->size, 1);
           if (nl > 0) {
-            // exit on a blank line (LF seen twice, or LF as the first byte),
-            // or on the first LF in HTS_XFREAD_LINE mode
+            // exit if:
+            // lf detected AND already detected before
+            // or
+            // lf detected AND first character read
             if (*(r->adr + r->size) == 10) {
               if (lf_detected || (at_beginning) || (bufl < 0))
                 count = -1;
@@ -2039,7 +2045,7 @@ htsblk http_test(httrackp * opt, const char *adr, const char *fil, char *loc) {
 
     // tant qu'on a des données, et qu'on ne recoit pas deux LF, et que le timeout n'arrie pas
     do {
-      if (http_xfread1(&retour, HTS_XFREAD_LINE_BLOCK) < 0)
+      if (http_xfread1(&retour, 0) < 0)
         e = 1;
       else {
         if (retour.adr != NULL) {

src/htslib.h

@@ -252,11 +252,6 @@ void treatfirstline(htsblk * retour, const char *rcvd);
    blank-line-terminated block it reads: a header section or a chunk trailer
    section. Overrunning it fails the transfer. */
 #define HTS_LINE_BLOCK_SIZE 8192
-/* http_xfread1() read modes: a positive bufl reads at most that many raw bytes,
-   these three read CR-stripped lines into the HTS_LINE_BLOCK_SIZE buffer. */
-#define HTS_XFREAD_LINE_BLOCK 0 /* lines up to a blank one (header/trailer) */
-#define HTS_XFREAD_LINE (-1)    /* one line, stopping at the first LF */
-#define HTS_XFREAD_RESERVE (-2) /* allocate only; no caller (#923) */
 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).

tests/151_bash-shell-validate.test

@@ -0,0 +1,141 @@
+#!/bin/bash
+#
+# An absolute BASH_SHELL is taken verbatim by AC_PATH_PROGS and a relative one is dropped
+# for the search result, so configure has to check what it ends up with: BASH_SHELL=/bin/sh
+# otherwise puts #895 back and only shows up at "make check" or "make deb" (#908).
+
+set -euo pipefail
+
+sh=${BASH_SHELL:-}
+test -n "$sh" || {
+    echo "BASH_SHELL is empty; tests/Makefile.am must export the configured value" >&2
+    exit 1
+}
+
+configure="${abs_top_srcdir:-}/configure"
+test -r "$configure" || {
+    echo "no configure script at $configure; tests/Makefile.am must export abs_top_srcdir" >&2
+    exit 1
+}
+
+tmp=$(mktemp -d)
+trap 'set +e; rm -rf "$tmp"' EXIT
+trap 'set +e; rm -rf "$tmp"; exit 1' HUP INT TERM
+
+# Symlink farm, not the real srcdir: an in-tree config.status makes autoconf refuse it.
+mkdir "$tmp/src"
+for f in "$abs_top_srcdir"/*; do
+    case "${f##*/}" in
+    config.status | config.log | config.h | stamp-h1 | Makefile) continue ;;
+    esac
+    ln -s "$f" "$tmp/src/"
+done
+
+mkdir "$tmp/notexec" "$tmp/with space" "$tmp/fakebin"
+: >"$tmp/notexec/bash"
+chmod 644 "$tmp/notexec/bash"
+ln -s "$sh" "$tmp/mybash"
+ln -s "$sh" "$tmp/with space/bash"
+ln -s "$sh" "$tmp/sh" # bash invoked as "sh" enters POSIX mode
+# A "bash" the PATH search will find first, and that answers -c like any other shell.
+printf '#!/bin/sh\nexec /bin/sh "$@"\n' >"$tmp/fakebin/bash"
+chmod 755 "$tmp/fakebin/bash"
+
+n=0
+status=0
+log=
+rundir=
+run() { # run <label> <env argument>...
+    local label=$1
+    shift
+    n=$((n + 1))
+    rundir="$tmp/run$n"
+    mkdir "$rundir"
+    status=0
+    (cd "$rundir" && env "$@" bash "$tmp/src/configure" --disable-https) \
+        >"$rundir/log" 2>&1 || status=$?
+    log=$(cat "$rundir/log")
+    echo "run $n ($label): exit $status"
+}
+
+reject() { # reject <label> <expected message> <env argument>...
+    local label=$1 want=$2
+    shift 2
+    run "$label" "$@"
+    test "$status" -ne 0 || {
+        echo "configure accepted $label" >&2
+        exit 1
+    }
+    grep -q "$want" <<<"$log" || {
+        echo "$label rejected without '$want':" >&2
+        tail -5 <<<"$log" >&2
+        exit 1
+    }
+}
+
+# accept <label> <expected $(BASH_SHELL), "" for any> <expected message, "" for none> <env argument>...
+accept() {
+    local label=$1 path=$2 want=$3
+    shift 3
+    run "$label" "$@"
+    test "$status" -eq 0 || {
+        echo "configure rejected $label (exit $status):" >&2
+        tail -10 <<<"$log" >&2
+        exit 1
+    }
+    got=$(sed -n 's/^BASH_SHELL = //p' "$rundir/Makefile")
+    test -n "$got" || {
+        echo "$label configured, but the Makefile carries no BASH_SHELL" >&2
+        exit 1
+    }
+    if test -n "$path" && test "$got" != "$path"; then
+        echo "$label reached the Makefile as $got" >&2
+        exit 1
+    fi
+    if test -n "$want"; then
+        grep -q "$want" <<<"$log" || {
+            echo "$label configured without '$want':" >&2
+            tail -5 <<<"$log" >&2
+            exit 1
+        }
+    fi
+}
+
+reject relative 'BASH_SHELL must be an absolute path' BASH_SHELL=relbash
+reject missing 'is not an executable file' "BASH_SHELL=$tmp/missing/bash"
+reject non-executable 'is not an executable file' "BASH_SHELL=$tmp/notexec/bash"
+reject sh-mode 'is a bash in POSIX sh-mode' "BASH_SHELL=$tmp/sh"
+# The #908 case: dash on Linux, bash in sh-mode on macOS, unusable either way. The spoofed
+# BASH_VERSION is what an ordinary shell echoes straight back, so it cannot be the probe.
+reject /bin/sh 'BASH_SHELL=/bin/sh is ' BASH_SHELL=/bin/sh BASH_VERSION=9.9
+
+# make splits the value on whitespace, cuts it at a '#' and expands a '$', and no Makefile
+# quotes $(BASH_SHELL).
+meta='must not contain shell or make metacharacters'
+reject space "$meta" "BASH_SHELL=$tmp/with space/bash"
+reject semicolon "$meta" 'BASH_SHELL=/opt/a;b/bash'
+# shellcheck disable=SC2016 # the '$' has to reach configure unexpanded
+reject dollar "$meta" 'BASH_SHELL=/opt/a$b/bash'
+reject hash "$meta" 'BASH_SHELL=/opt/a#b/bash'
+
+# An environment in POSIX mode leaves no path that could pass, so the message must blame it
+# and not the shell.
+reject posix-env 'POSIXLY_CORRECT or SHELLOPTS' "BASH_SHELL=$sh" POSIXLY_CORRECT=1
+
+# Both at once: clearing the variable would still leave a bash invoked as sh, so the second
+# probe has to decide which one to blame instead of reading the environment.
+reject sh-mode-in-posix-env "BASH_SHELL=$tmp/sh is a bash in POSIX sh-mode" \
+    "BASH_SHELL=$tmp/sh" POSIXLY_CORRECT=1
+if grep -q 'POSIXLY_CORRECT or SHELLOPTS' <<<"$log"; then
+    echo "a bash invoked as sh was blamed on the environment:" >&2
+    tail -5 <<<"$log" >&2
+    exit 1
+fi
+
+accept override "$tmp/mybash" '' "BASH_SHELL=$tmp/mybash"
+accept empty '' '' BASH_SHELL=
+# No override: an unusable bash is a warning, so a box that has none still builds.
+accept searched "$tmp/fakebin/bash" 'no usable bash found' -u BASH_SHELL "PATH=$tmp/fakebin:$PATH"
+accept searched-posix-env '' 'POSIXLY_CORRECT or SHELLOPTS' -u BASH_SHELL POSIXLY_CORRECT=1
+
+echo "configure validated $n BASH_SHELL values"

tests/tests-list.mk

@@ -221,4 +221,5 @@ TESTS += 146_bash-shell.test
 TESTS += 147_local-proxytrack-webdav-overflow.test
 TESTS += 149_local-chunked-trailer.test
 TESTS += 148_engine-spoolname.test
+TESTS += 151_bash-shell-validate.test
 TESTS += 150_engine-strsprintf.test