03583f9df6c2c04a7e1072b021bbdd73d5072926
braney
  Mon Aug 31 15:36:58 2026 -0700
cheapcgi: skip an empty pair in a query string, refs #38185

cgiParseInputAbort and cgiParseNext end a value at the first separator after
it, so an empty pair left the next variable named "&name".  Nothing looks
that name up, so the variable was lost with no warning.  This is what broke
the link in #38145: the URL had "&&" in front of hgS_doLoadUrl, so the saved
session never loaded and the reporter saw a browser without the tracks he
expected.  The same empty pair at the end of a query string has no '=' after
it, and aborted the CGI instead, so the behavior differed by position.

Both parsers now skip the separators of an empty pair and carry on.
cgiEncode escapes everything but alphanumerics, '.' and '_', so no encoded
name can begin with a separator and none is ever eaten.

Checked against the unpatched library over every string of "& ; = a b % +"
up to length five, 19608 inputs per parser.  Nothing that parsed before now
aborts, no variable is ever lost, and every difference is the intended one.
484 inputs to cgiParseInputAbort and 62 to cgiParseNext used to abort and
now parse.

diff --git src/lib/cheapcgi.c src/lib/cheapcgi.c
index 15b31173d28..f586199c8eb 100644
--- src/lib/cheapcgi.c
+++ src/lib/cheapcgi.c
@@ -929,40 +929,58 @@
 }
 
 void cgiDictionaryFreeList(struct cgiDictionary **pList)
 /* Free up a whole list of cgiDictionaries */
 {
 struct cgiDictionary *el, *next;
 
 for (el = *pList; el != NULL; el = next)
     {
     next = el->next;
     cgiDictionaryFree(&el);
     }
 *pList = NULL;
 }
 
+static char *skipEmptyPairs(char *s)
+/* Return the start of the next variable name in a var=val&var=val... string,
+ * skipping the separators of any empty pairs, as in the "&&" of "a=1&&b=2".
+ * Returns a pointer to the terminating zero when nothing is left.
+ *
+ * Without this the parsers below, which split on the first separator after a
+ * value, read the leftover one as part of the next name: "a=1&&b=2" stores "b"
+ * under "&b".  Nothing looks that name up, so the variable is silently lost.
+ * The same empty pair at the end of the string has no '=' after it and instead
+ * aborts the CGI.  cgiEncode escapes everything but alphanumerics, '.' and '_',
+ * so no encoded name can begin with a separator and none is ever eaten here.
+ * refs #38185 */
+{
+if (s == NULL)
+    return NULL;
+return s + strspn(s, "&;");
+}
+
 boolean cgiParseNext(char **pInput, char **retVar, char **retVal)
 /* Parse out next var/val in a var=val&var=val... cgi formatted string 
  * This will insert zeroes and other things into string. 
  * Usage:
  *     char *pt = cgiStringStart;
  *     char *var, *val
  *     while (cgiParseNext(&pt, &var, &val))
  *          printf("%s\t%s\n", var, val); */
 {
-char *var = *pInput;
+char *var = skipEmptyPairs(*pInput);
 if (var == NULL || var[0] == 0)
     return FALSE;
 char *val = strchr(var, '=');
 if (val == NULL)
     errAbort("Mangled CGI input string %s", var);
 *val++ = 0;
 char *end = strchr(val, '&');
 if (end == NULL)
     end = strchr(val, ';');  // For DAS
 if (end == NULL)
     {
     end = val + strlen(val);
     *pInput = NULL;
     }
 else
@@ -991,31 +1009,31 @@
  * To clean up - slFreeList, hashFree, and only then free input. */
 {
 char *namePt, *dataPt, *nextNamePt;
 struct hash *hash = *retHash;
 struct cgiVar *list = *retList, *el;
 
 if (!hash)
   hash = newHash(6);
 slReverse(&list);
 
 struct dyString *logMsg = NULL;
 if (logCgiVarMaxLen > 0)
     logMsg = dyStringNew(1024);	
 
 namePt = input;
-while (namePt != NULL && namePt[0] != 0)
+while ((namePt = skipEmptyPairs(namePt)) != NULL && namePt[0] != 0)
     {
     dataPt = strchr(namePt, '=');
     if (dataPt == NULL)
 	{
 	errAbort("Mangled CGI input string %s", namePt);
 	}
     *dataPt++ = 0;
     nextNamePt = strchr(dataPt, '&');
     if (nextNamePt == NULL)
 	nextNamePt = strchr(dataPt, ';');	/* Accomodate DAS. */
     if (nextNamePt != NULL)
          *nextNamePt++ = 0;
 
     if (logMsg && dataPt && strlen(dataPt) < logCgiVarMaxLen)
         dyStringPrintf(logMsg, "%s=%s ", namePt, dataPt); // if dataPt is empty string, still print it, could be important