Drop the historical narration from hts_lastcharptr's contract

The declaration is the API surface, so it keeps the double-evaluation
warning a caller cannot derive from the signature; what the macro
replaced is git's job. Its grep exclusion in the test goes with it,
since that comment was the only htssafe.h line the scan matched.

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

.github/workflows/windows-build.yml

@@ -211,7 +211,7 @@ jobs:
           pass=0 fail=0 skip=0 failed="" skipped="" deadline=0
           for t in 00_runnable.test 01_engine-*.test 01_zlib-*.test \
                    *_local-*.test 13_crawl_proxy_https.test 58_watchdog.test \
-                   60_crawl-log-salvage.test; do
+                   60_crawl-log-salvage.test 116_engine-rtrim.test; do
             elapsed=$((SECONDS - started))
             if [ "$elapsed" -ge "$suite_deadline" ]; then
               echo "::error::suite deadline: ${elapsed}s elapsed, stopping before $t"

src/htsalias.c

@@ -501,13 +501,11 @@ int optinclude_file(const char *name, int *argc, char **argv, char *x_argvblk,
       /* read line */
       linput(fp, line, 250);
       hts_lowcase(line);
+      /* trim first: a blank line is skipped, not parsed as an option */
+      hts_rtrim(line, HTS_REALSPACES);
       if (strnotempty(line)) {
         /* no comment line: # // ; */
         if (strchr("#/;", line[0]) == NULL) {
-          /* right trim */
-          a = line + strlen(line) - 1;
-          while(is_realspace(*a))
-            *(a--) = '\0';
           /* jump "set " and spaces */
           a = line;
           while(is_realspace(*a))
@@ -577,7 +575,6 @@ int optinclude_file(const char *name, int *argc, char **argv, char *x_argvblk,
             }
           }
         }
-
       }
     }
     fclose(fp);

src/htsbauth.c

@@ -491,7 +491,7 @@ char *bauth_prefix(char *prefix, const char *adr, const char *fil) {
   if (a)
     *a = '\0';
   if (strchr(prefix, '/')) {
-    a = prefix + strlen(prefix) - 1;
+    a = hts_lastcharptr(prefix);
     while(*a != '/')
       a--;
     *(a + 1) = '\0';

src/htscore.c

@@ -3774,8 +3774,7 @@ int htsAddLink(htsmoduleStruct * str, char *link) {
         strcpybuff(codebase, heap(ptr)->fil);
       else
         strcpybuff(codebase, heap(heap(ptr)->precedent)->fil);
-      // empty codebase has no last char; codebase-1 would underflow
-      a = codebase[0] != '\0' ? codebase + strlen(codebase) - 1 : codebase;
+      a = hts_lastcharptr(codebase);
       while((*a) && (*a != '/') && (a > codebase))
         a--;
       if (*a == '/')

src/htslib.c

@@ -1322,7 +1322,7 @@ void treathead(t_cookie * cookie, const char *adr, const char *fil, htsblk * ret
       p++;                      // sauter espaces
     if ((int) strlen(rcvd + p) < 250) { // pas trop long?
       char tmp[256];
-      char *a = NULL, *b = NULL;
+      char *a = NULL;
 
       strcpybuff(tmp, rcvd + p);
       a = strstr(tmp, "filename=");
@@ -1335,15 +1335,9 @@ void treathead(t_cookie * cookie, const char *adr, const char *fil, htsblk * ret
 
           while((c = strchr(a, '/')))   /* skip all / (see RFC2616) */
             a = c + 1;
-          b = a + strlen(a) - 1;
-          while(is_space(*b))
-            b--;
-          b++;
-          if (b) {
-            *b = '\0';
-            if ((int) strlen(a) < 200) {        // pas trop long?
-              strcpybuff(retour->cdispo, a);
-            }
+          hts_rtrim(a, HTS_SPACES);
+          if ((int) strlen(a) < 200) { // pas trop long?
+            strcpybuff(retour->cdispo, a);
           }
         }
       }
@@ -3315,8 +3309,9 @@ int ishtml(httrackp * opt, const char *fil) {
   }
 
   /* Search for known ext */
-  for(a = fil_noquery + strlen(fil_noquery) - 1;
-      *a != '.' && *a != '/' && a > fil_noquery; a--) ;
+  for (a = hts_lastcharptr(fil_noquery);
+       *a != '.' && *a != '/' && a > fil_noquery; a--)
+    ;
   if (*a == '.') {              // a une extension
     char BIGSTK fil_noquery[HTS_URLMAXSIZE * 2];
     char *b;
@@ -4253,9 +4248,8 @@ HTSEXT_API hts_boolean get_httptype_sized(httrackp *opt, char *s, size_t ssize,
     return 1;
   } else {
     /* Check html -> text/html */
-    const char *a = fil + strlen(fil) - 1;
+    const char *a = hts_lastcharptr(fil);
 
-    /* a < fil when fil is empty: bound before dereferencing */
     while ((a > fil) && (*a != '.') && (*a != '/'))
       a--;
     if (a >= fil && *a == '.' && strlen(a) < 32) {

src/htsname.c

@@ -823,7 +823,7 @@ int url_savename(lien_adrfilsave *const afs,
   // Change the extension? e.g. php3 saved as html, cgi as html or gif/xbm
   // depending on the resolved type.
   if (ext_chg && !opt->no_type_change) {
-    char *a = fil + strlen(fil) - 1;
+    char *a = hts_lastcharptr(fil);
 
     if ((opt->debug > 1) && (opt->log != NULL)) {
       if (ext_chg == 1)
@@ -858,7 +858,7 @@ int url_savename(lien_adrfilsave *const afs,
   }
   // Rechercher premier / et dernier .
   {
-    const char *a = fil + strlen(fil) - 1;
+    const char *a = hts_lastcharptr(fil);
 
     // passer structures
     start_pos = fil;
@@ -1203,29 +1203,29 @@ int url_savename(lien_adrfilsave *const afs,
     switch (opt->savename_type % 100) {
     case 4:
     case 5:{                   // séparer par types
-        const char *a = fil + strlen(fil) - 1;
+      const char *a = hts_lastcharptr(fil);
 
-        // passer structures
-        while((a > fil) && (*a != '/') && (*a != '\\'))
+      // passer structures
+      while ((a > fil) && (*a != '/') && (*a != '\\'))
+        a--;
+      if ((*a == '/') || (*a == '\\'))
+        a++;
+
+      // html?
+      if ((ext_chg != 0) ? (ishtml_ext(ext) == 1) : (ishtml(opt, fil) == 1)) {
+        if (opt->savename_type % 100 == 5)
+          strcatbuff(afs->save, "html/");
+      } else {
+        const char *a = hts_lastcharptr(fil);
+
+        while ((a > fil) && (*a != '/') && (*a != '.'))
           a--;
-        if ((*a == '/') || (*a == '\\'))
-          a++;
-
-        // html?
-        if ((ext_chg != 0) ? (ishtml_ext(ext) == 1) : (ishtml(opt, fil) == 1)) {
-          if (opt->savename_type % 100 == 5)
-            strcatbuff(afs->save, "html/");
-        } else {
-          const char *a = fil + strlen(fil) - 1;
-
-          while((a > fil) && (*a != '/') && (*a != '.'))
-            a--;
-          if (*a != '.')
-            strcatbuff(afs->save, "other");
-          else
-            strcatbuff(afs->save, a + 1);
-          strcatbuff(afs->save, "/");
-        }
+        if (*a != '.')
+          strcatbuff(afs->save, "other");
+        else
+          strcatbuff(afs->save, a + 1);
+        strcatbuff(afs->save, "/");
+      }
         /*strcatbuff(save,a); */
         /* add name */
         ADD_STANDARD_NAME(0);
@@ -1258,7 +1258,7 @@ int url_savename(lien_adrfilsave *const afs,
         }
         afs->save[i + j] = '\0';
         // ajouter extension
-        a = fil + strlen(fil) - 1;
+        a = hts_lastcharptr(fil);
         while((a > fil) && (*a != '/') && (*a != '.'))
           a--;
         if (*a == '.') {
@@ -1303,7 +1303,7 @@ int url_savename(lien_adrfilsave *const afs,
   // cela évite les /chez/toto et les /chez/toto/index.html incompatibles
   if (opt->savename_type != -1 &&
       opt->savename_delayed != HTS_SAVENAME_DELAYED_HARD) {
-    char *a = afs->save + strlen(afs->save) - 1;
+    char *a = hts_lastcharptr(afs->save);
 
     while((a > afs->save) && (*a != '.') && (*a != '/'))
       a--;
@@ -1420,8 +1420,10 @@ int url_savename(lien_adrfilsave *const afs,
   if (opt->savename_83 > 0) {
     char *a, *last;
 
-    for(last = afs->save + strlen(afs->save) - 1;
-        last != afs->save && *last != '/' && *last != '\\' && *last != '.'; last--) ;
+    for (last = hts_lastcharptr(afs->save);
+         last != afs->save && *last != '/' && *last != '\\' && *last != '.';
+         last--)
+      ;
     if (*last != '.') {
       last = NULL;
     }
@@ -1676,8 +1678,8 @@ int url_savename(lien_adrfilsave *const afs,
 #endif
         } else {                // utilisé par un AUTRE, changer de nom
           char BIGSTK tempo[HTS_URLMAXSIZE * 2];
-          char *a = afs->save + strlen(afs->save) - 1;
-          char *b;
+          char *a = hts_lastcharptr(afs->save);
+          size_t stem;
           int n = 2;
           char collisionSeparator =
               ((opt->savename_83 != HTS_SAVENAME_83_ISO9660) ? '-' : '_');
@@ -1699,18 +1701,16 @@ int url_savename(lien_adrfilsave *const afs,
             strcatbuff(tempo, afs->save);
 
           // tester la présence d'un -xx (ex: index-2.html -> index-3.html)
-          b = tempo + strlen(tempo) - 1;
-          while(isdigit((unsigned char) *b))
-            b--;
-          if (*b == collisionSeparator) {
-            sscanf(b + 1, "%d", &n);
-            *b = '\0';          // couper
+          stem = hts_rtrimlen(tempo, "0123456789");
+          if (stem != 0 && tempo[stem - 1] == collisionSeparator) {
+            sscanf(tempo + stem, "%d", &n);
+            tempo[stem - 1] = '\0'; // couper
             n++;                // plus un
           }
           // en plus il faut gérer le 8-3 .. pas facile le client
           if (opt->savename_83) {
             int max;
-            char *a = tempo + strlen(tempo) - 1;
+            char *a = hts_lastcharptr(tempo);
 
             while((a > tempo) && (*a != '/'))
               a--;

src/htsparse.c

@@ -2057,7 +2057,7 @@ int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
                   }
                   q = strchr(a, '?');   // ne pas traiter après '?'
                   if (!q)
-                    q = a + strlen(a) - 1;
+                    q = hts_lastcharptr(a);
                   while((p = strstr(a, "//")) && (!done)) {     // remplacer // par /
                     if (p > q) {    // après le ? (toto.cgi?param=1//2.3)
                       done = 1; // stopper
@@ -2208,7 +2208,7 @@ int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
                 // OUI!!
 #if HTS_TILDE_SLASH
                 if (hts_lastchar(lien) != '/') {
-                  char *a = lien + strlen(lien) - 1;
+                  char *a = hts_lastcharptr(lien);
 
                   // éviter aussi index~1.html
                   while(a > lien && (*a != '~') && (*a != '/')
@@ -2254,7 +2254,7 @@ int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
                 // vérifier que l'on ne doit pas ajouter de .class
                 if (!error) {
                   if (add_class) {
-                    char *a = lien + strlen(lien) - 1;
+                    char *a = hts_lastcharptr(lien);
 
                     while((a > lien) && (*a != '/') && (*a != '.'))
                       a--;
@@ -2309,7 +2309,7 @@ int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
                         {
                           char *a;
 
-                          a = lien + strlen(lien) - 1;
+                          a = hts_lastcharptr(lien);
                           while((*a) && (*a != '/') && (a > lien))
                             a--;
                           if (*a == '/') {
@@ -2320,8 +2320,8 @@ int htsparse(htsmoduleStruct * str, htsmoduleStructExtended * stre) {
                     }
 
                     if (!error) {       // pas d'erreur?
-                      if (p_type == 2) {        // code ET PAS codebase      
-                        char *a = lien + strlen(lien) - 1;
+                      if (p_type == 2) { // code ET PAS codebase
+                        char *a = hts_lastcharptr(lien);
                         char *start_of_filename = jump_identification(lien);
 
                         if (start_of_filename != NULL

src/htssafe.h

@@ -569,6 +569,38 @@ static HTS_INLINE HTS_UNUSED hts_boolean hts_choplastchar(char *s) {
   return HTS_FALSE;
 }
 
+/* htslib.h's is_space() and is_realspace() as hts_rtrim() sets; -#test=rtrim
+   keeps them in sync. */
+#define HTS_SPACES " \"\n\r\t\f\v'"
+#define HTS_REALSPACES " \n\r\t\f\v"
+
+/* Length of s once its trailing bytes from set are dropped; 0 if they all are.
+   Counts down from the end, so it stops at s rather than below the buffer. */
+static HTS_INLINE HTS_UNUSED size_t hts_rtrimlen(const char *s,
+                                                 const char *set) {
+  size_t len = strlen(s);
+
+  while (len != 0 && strchr(set, s[len - 1]) != NULL)
+    len--;
+  return len;
+}
+
+/* Drop the trailing bytes of s that occur in set. */
+static HTS_INLINE HTS_UNUSED void hts_rtrim(char *s, const char *set) {
+  s[hts_rtrimlen(s, set)] = '\0';
+}
+
+/* Offset of the last character of s, or 0 when s is empty. */
+static HTS_INLINE HTS_UNUSED size_t hts_lastcharoffset(const char *s) {
+  const size_t len = strlen(s);
+
+  return len != 0 ? len - 1 : 0;
+}
+
+/* Address of the last character of S, or of its terminating NUL when S is
+   empty. S is evaluated twice, so pass an lvalue. */
+#define hts_lastcharptr(S) ((S) + hts_lastcharoffset(S))
+
 /* Thin aliases over the libc allocator/memcpy (historical "t" suffix); no
    added bounds checking. freet() also NULLs the freed pointer and tolerates
    NULL. memcpybuff() despite the name is a raw memcpy: the caller owns the

src/htsselftest.c

@@ -6710,9 +6710,10 @@ static int st_changes_race(httrackp *opt, int argc, char **argv) {
   return err;
 }
 
-/* The x[strlen(x) - 1] class (#770). The string starts mid-arena so the byte
-   it must not touch is a real neighbour; poisoned with '#', not 0, or a stray
-   NUL terminator would read as untouched. */
+/* The x[strlen(x) - 1] class (#770) and its pointer spelling x + strlen(x) - 1
+   (#781). The string starts mid-arena so the byte it must not touch is a real
+   neighbour; poisoned with '#', not 0, or a stray NUL terminator would read as
+   untouched. */
 static int st_lastchar(httrackp *opt, int argc, char **argv) {
   enum { off = 8 };
 
@@ -6779,11 +6780,52 @@ static int st_lastchar(httrackp *opt, int argc, char **argv) {
   CHECK(s[0] == '\0');
   CHECK(arena[guard] == '#');
 
+  /* the pointer spelling (#781): on an empty string the address must be the
+     terminating NUL, never the byte before it */
+  REPOISON("");
+  CHECK(hts_lastcharoffset(s) == 0);
+  CHECK(hts_lastcharptr(s) == s);
+  CHECK(*hts_lastcharptr(s) == '\0');
+  *hts_lastcharptr(s) = 'Z'; /* a write through it must stay inside s */
+  CHECK(arena[guard] == '#');
+  CHECK(s[0] == 'Z');
+
+  /* the neighbour must not be mistaken for the string's own last byte */
+  REPOISON("");
+  arena[guard] = '/';
+  CHECK(hts_lastcharptr(s) == s);
+  CHECK(*hts_lastcharptr(s) != '/');
+  CHECK(arena[guard] == '/');
+
+  /* the walk-back loops the sites use must stop at once on an empty string */
+  REPOISON("");
+  {
+    const char *p = hts_lastcharptr(s);
+    int steps = 0;
+
+    while (p > s && *p != '/')
+      p--, steps++;
+    CHECK(steps == 0);
+    CHECK(p == s);
+  }
+
+  REPOISON("ab/");
+  CHECK(hts_lastcharoffset(s) == 2);
+  CHECK(hts_lastcharptr(s) == s + 2);
+  CHECK(*hts_lastcharptr(s) == '/');
+  REPOISON("/");
+  CHECK(hts_lastcharptr(s) == s);
+  CHECK(*hts_lastcharptr(s) == '/');
+  CHECK(arena[guard] == '#');
+
   /* control: the canary must be able to fail, or the checks above prove
      nothing. Clobber it exactly as the unguarded idiom would. */
   REPOISON("");
   s[-1] = '\0';
   CHECK(arena[guard] != '#');
+  REPOISON("");
+  *(s + strlen(s) - 1) = 'X';
+  CHECK(arena[guard] != '#');
 
 #undef REPOISON
 #undef CHECK
@@ -6792,6 +6834,91 @@ static int st_lastchar(httrackp *opt, int argc, char **argv) {
   return err;
 }
 
+/* hts_rtrim() and the sets it is called with. The string starts mid-arena, and
+   the byte below it is poisoned with '#' rather than 0, or the stray NUL the
+   old loop wrote there would read as untouched. */
+static int st_rtrim(httrackp *opt, int argc, char **argv) {
+  enum { off = 8 };
+
+  char arena[24];
+  char *const s = &arena[off];
+  const int guard = off - 1;
+  int err = 0;
+  int c;
+
+  (void) opt;
+  (void) argc;
+  (void) argv;
+
+#define REPOISON(str)                                                          \
+  do {                                                                         \
+    memset(arena, '#', sizeof(arena));                                         \
+    strlcpybuff(s, (str), sizeof(arena) - off);                                \
+  } while (0)
+#define CHECK(cond)                                                            \
+  do {                                                                         \
+    if (!(cond)) {                                                             \
+      printf("  FAIL line %d: %s\n", __LINE__, #cond);                         \
+      err = 1;                                                                 \
+    }                                                                          \
+  } while (0)
+
+  /* nothing but spaces: the case that ran the old loop off the front */
+  REPOISON("   ");
+  hts_rtrim(s, HTS_REALSPACES);
+  CHECK(s[0] == '\0');
+  CHECK(arena[guard] == '#');
+
+  /* a space sitting below the string must not be eaten as if it were part of
+     it, which is exactly what the old loop did */
+  REPOISON("   ");
+  arena[guard] = ' ';
+  hts_rtrim(s, HTS_REALSPACES);
+  CHECK(s[0] == '\0');
+  CHECK(arena[guard] == ' ');
+
+  REPOISON("");
+  hts_rtrim(s, HTS_REALSPACES);
+  CHECK(s[0] == '\0');
+  CHECK(arena[guard] == '#');
+
+  REPOISON("a b \t\r\n");
+  hts_rtrim(s, HTS_REALSPACES);
+  CHECK(strcmp(s, "a b") == 0);
+  CHECK(arena[guard] == '#');
+
+  REPOISON("ab");
+  hts_rtrim(s, HTS_REALSPACES);
+  CHECK(strcmp(s, "ab") == 0);
+
+  /* quotes count as space for is_space() but not for is_realspace() */
+  REPOISON("v\" ");
+  hts_rtrim(s, HTS_REALSPACES);
+  CHECK(strcmp(s, "v\"") == 0);
+  REPOISON("v\" ");
+  hts_rtrim(s, HTS_SPACES);
+  CHECK(strcmp(s, "v") == 0);
+
+  /* the sets must stay the macros they stand for */
+  for (c = 1; c < 256; c++) {
+    const char b = (char) c;
+
+    CHECK((strchr(HTS_SPACES, b) != NULL) == (is_space(b) != 0));
+    CHECK((strchr(HTS_REALSPACES, b) != NULL) == (is_realspace(b) != 0));
+  }
+
+  /* control: the canary must be able to fail */
+  REPOISON("   ");
+  s[-1] = '\0';
+  CHECK(arena[guard] != '#');
+
+#undef REPOISON
+#undef CHECK
+
+  printf("rtrim self-test: %s\n", err ? "FAIL" : "OK");
+  return err;
+}
+
 /* ------------------------------------------------------------ */
 /* Registry: name -> handler, with a usage hint and a one-line description. */
 /* ------------------------------------------------------------ */
@@ -6847,8 +6974,10 @@ static const struct selftest_entry {
     {"strsafe", "[overflow|overflow-buff|overflow-src [str]]",
      "bounded string-op self-test", st_strsafe},
     {"copyopt", "", "copy_htsopt option-copy self-test", st_copyopt},
-    {"lastchar", "", "last-char helpers never index before the buffer (#770)",
+    {"lastchar", "",
+     "last-char helpers never index before the buffer (#770, #781)",
      st_lastchar},
+    {"rtrim", "", "hts_rtrim never walks below the buffer", st_rtrim},
     {"changes", "", "--changes bucket accounting and JSON escaping (#714)",
      st_changes},
     {"changes-race", "<dir>", "--changes under a late transfer thread (#714)",

src/htstools.c

@@ -324,8 +324,7 @@ int lienrelatif(char *s, size_t ssize, const char *link, const char *curr_fil) {
   curr = _curr;
   strlcpybuff(curr, curr_fil, sizeof(_curr));
   if ((a = strchr(curr, '?')) == NULL) { // cut at the ? (query parameters)
-    // an empty path has no last character: curr-1 would read before the buffer
-    a = curr[0] != '\0' ? curr + strlen(curr) - 1 : curr;
+    a = hts_lastcharptr(curr);
   }
   while((*a != '/') && (a > curr))
     a--;                        // chercher dernier / du chemin courant

src/proxy/store.c

@@ -903,10 +903,11 @@ int PT_LoadCache__New(PT_Index index_, const char *filename) {
       coucal hashtable = index->hash;
 
       /* Compute base path for this index - the filename MUST be absolute! */
-      for(slashes = 2, abpath = filename + (int) strlen(filename) - 1;
-          abpath > filename && ((*abpath != '/' && *abpath != '\\')
-                                || --slashes > 0);
-          abpath--) ;
+      for (slashes = 2, abpath = hts_lastcharptr(filename);
+           abpath > filename &&
+           ((*abpath != '/' && *abpath != '\\') || --slashes > 0);
+           abpath--)
+        ;
       index->path[0] = '\0';
       if (slashes == 0 && *abpath != 0) {
         int i;
@@ -1499,10 +1500,11 @@ static int PT_LoadCache__Old(PT_Index index_, const char *filename) {
 
       /* -------------------- COPY OF THE __New() CODE -------------------- */
       /* Compute base path for this index - the filename MUST be absolute! */
-      for(slashes = 2, abpath = filename + (int) strlen(filename) - 1;
-          abpath > filename && ((*abpath != '/' && *abpath != '\\')
-                                || --slashes > 0);
-          abpath--) ;
+      for (slashes = 2, abpath = hts_lastcharptr(filename);
+           abpath > filename &&
+           ((*abpath != '/' && *abpath != '\\') || --slashes > 0);
+           abpath--)
+        ;
       index->path[0] = '\0';
       if (slashes == 0 && *abpath != 0) {
         int i;

tests/01_engine-lastchar.test

@@ -1,8 +1,8 @@
 #!/bin/bash
 #
-# Runs the lastchar self-test (#770), then keeps the idiom it replaced from
-# coming back: x[strlen(x) - 1] indexes one byte before the buffer when x is
-# empty, and the guard is never where the index is.
+# Runs the lastchar self-test (#770, #781), then keeps the idiom it replaced
+# from coming back: x[strlen(x) - 1] and x + strlen(x) - 1 both land one byte
+# before the buffer when x is empty, and the guard is never where they are.
 
 set -eu
 
@@ -33,3 +33,36 @@ test -z "$hits" || {
     echo "$hits" >&2
     exit 1
 }
+
+# Same for the pointer spelling. The three survivors sit in commented-out
+# blocks in htsname.c; the rest are this class being named, plus the self-test's
+# own deliberate underflow.
+hits=$(command grep -rnE '\+ *\(?(int|size_t)?\)? *strlen *\([^)]*\) *- *1' \
+    "$top_srcdir/src" --include='*.c' --include='*.h' --exclude-dir=coucal |
+    grep -v 'htsname\.c:.*char\* a=\(fil+strlen(fil)\|save+strlen(save)\)-1;' |
+    grep -v 'htsselftest\.c:.*pointer spelling x + strlen(x) - 1' |
+    grep -v 'htsselftest\.c:.*\*(s + strlen(s) - 1) =' || true)
+
+test -z "$hits" || {
+    echo "x + strlen(x) - 1 reintroduced; use hts_lastcharptr:" >&2
+    echo "$hits" >&2
+    exit 1
+}
+
+# url_savename() has nine of these sites and reaches all of them with an empty
+# "fil", which "?x=1" produces. Names are asserted loosely, since the point is
+# the run: on unfixed code the sanitized CI leg aborts inside htsname.c.
+for args in "?x=1 text/plain" "?x=1 text/html" "?x=1 text/html type=4"; do
+    # shellcheck disable=SC2086
+    name=$(httrack -#test=savename $args) || {
+        echo "httrack -#test=savename $args exited non-zero: $name" >&2
+        exit 1
+    }
+    case "$name" in
+    "savename: "?*) ;;
+    *)
+        echo "-#test=savename $args produced no name: $name" >&2
+        exit 1
+        ;;
+    esac
+done

tests/116_engine-rtrim.test

@@ -0,0 +1,107 @@
+#!/bin/bash
+#
+# Backward scans that start at s + strlen(s) - 1 and decrement with no lower
+# bound: the config-file right trim and the collision-rename digit scan both
+# walked off the front of a stack buffer. Neither ASan nor _FORTIFY_SOURCE sees
+# the self-test's overrun (it lands in the same frame), hence the poisoned byte.
+
+set -euo pipefail
+
+: "${top_srcdir:=..}"
+
+# Resolve httrack before any cd: PATH may carry a build-relative entry.
+bin=$(command -v httrack) || {
+    echo "FAIL: httrack not found on PATH" >&2
+    exit 1
+}
+case "$bin" in
+/*) ;;
+*) bin="$(cd "$(dirname "$bin")" && pwd)/$(basename "$bin")" ;;
+esac
+
+tmp=$(mktemp -d "${TMPDIR:-/tmp}/httrack_rtrim.XXXXXX") || exit 1
+trap 'set +e; rm -rf "$tmp"' EXIT
+trap 'rm -rf "$tmp"' HUP INT QUIT PIPE TERM
+
+fail() {
+    echo "FAIL: $1" >&2
+    exit 1
+}
+
+# --- 1. the helper itself ---------------------------------------------------
+out=$("$bin" -#test=rtrim) || fail "httrack -#test=rtrim exited non-zero: $out"
+test "$out" = "rtrim self-test: OK" || fail "expected 'rtrim self-test: OK', got: $out"
+
+# --- 2. collision rename over an all-digit name -----------------------------
+# Two pages with the same all-digit name save to 2024.html, so the second goes
+# through the -<n> rename and the digit scan has nothing but digits to walk
+# over. Driven twice, because the two routes differ in who picks the URLs.
+site="$tmp/site"
+mkdir -p "$site/a" "$site/b"
+printf '%s\n' '<html><body><a href="a/2024.html">a</a> <a href="b/2024.html">b</a></body></html>' \
+    >"$site/index.html"
+echo '<html><body>A</body></html>' >"$site/a/2024.html"
+echo '<html><body>B</body></html>' >"$site/b/2024.html"
+
+# -g flattens the tree, and pins depth to 0: the operator supplies both URLs.
+out1="$tmp/out1"
+mkdir -p "$out1"
+(cd "$out1" && "$bin" -g "file://$site/a/2024.html" "file://$site/b/2024.html" \
+    --quiet -n >.log 2>&1) || fail "collision crawl exited non-zero: $(tail -3 "$out1/.log")"
+
+test -f "$out1/2024.html" || fail "2024.html not mirrored"
+test -f "$out1/2024-2.html" || fail "colliding name was not renamed to 2024-2.html"
+
+# -N "%n.%t" collides the same way with depth unrestricted, so one starting URL
+# is enough and the crawled page picks both colliding names itself.
+out3="$tmp/out3"
+mkdir -p "$out3"
+(cd "$out3" && "$bin" "file://$site/index.html" -N "%n.%t" --quiet -n >.log 2>&1) ||
+    fail "userdef-savename crawl exited non-zero: $(tail -3 "$out3/.log")"
+test -f "$out3/2024.html" || fail "2024.html not mirrored under -N '%n.%t'"
+test -f "$out3/2024-2.html" ||
+    fail "colliding name was not renamed under -N '%n.%t' (links followed, not argv)"
+
+# The rename must keep counting, not restart: a third collision is -3.
+mkdir -p "$site/c"
+echo '<html><body>C</body></html>' >"$site/c/2024.html"
+out2="$tmp/out2"
+mkdir -p "$out2"
+(cd "$out2" && "$bin" -g "file://$site/a/2024.html" "file://$site/b/2024.html" \
+    "file://$site/c/2024.html" --quiet -n >.log 2>&1) ||
+    fail "three-way collision crawl exited non-zero: $(tail -3 "$out2/.log")"
+test -f "$out2/2024-3.html" || fail "third colliding name was not renamed to 2024-3.html"
+
+# --- 3. a blank config line is skipped, not parsed ---------------------------
+# HTS_HTTRACKRC is ".httrackrc" on POSIX and "httrackrc" on Windows.
+rcdir="$tmp/rc"
+mkdir -p "$rcdir"
+echo '<html><body>hi</body></html>' >"$rcdir/index.html"
+printf '   \nzzz-not-an-option\n' >"$rcdir/.httrackrc"
+cp "$rcdir/.httrackrc" "$rcdir/httrackrc"
+
+rc=0
+log=$( (cd "$rcdir" && "$bin" "file://$rcdir/index.html" --quiet -n 2>&1)) || rc=$?
+test "$rc" -eq 0 || fail "rc-file crawl exited $rc: $log"
+log=${log//$'\r'/} # the empty-option check below must not miss a CRLF line
+
+# The line after the blank one must still be read, or this proves nothing.
+case "$log" in
+*"Unknown option: zzz-not-an-option"*) ;;
+*) fail "config file was not parsed past the blank line: $log" ;;
+esac
+# The blank line itself must not become an empty option.
+case "$log" in
+*"Unknown option: "$'\n'*) fail "blank config line was parsed as an empty option: $log" ;;
+esac
+
+# --- 4. the call sites stay bounded -----------------------------------------
+# Sections 2 and 3 only go red under a sanitizer: on a plain build the byte
+# below the buffer is usually not a digit, so a reverted scan stops anyway and
+# produces the same names. Pin the two sites at the source instead.
+command grep -q 'hts_rtrim(line, HTS_REALSPACES)' "$top_srcdir/src/htsalias.c" ||
+    fail "htsalias.c right trim no longer goes through hts_rtrim"
+command grep -q 'hts_rtrimlen(tempo, "0123456789")' "$top_srcdir/src/htsname.c" ||
+    fail "htsname.c collision-suffix scan no longer goes through hts_rtrimlen"
+
+exit 0

tests/Makefile.am

@@ -226,6 +226,7 @@ TESTS = \
 	102_local-ftp-refetch.test \
 	103_teardown-status.test \
 	104_engine-warc-longurl.test \
-	105_suite-timeout.test
+	105_suite-timeout.test \
+	116_engine-rtrim.test
 
 CLEANFILES = check-network_sh.cache