c2a6ef817930149ad6f55818fa0196d99c47e02b
braney
  Fri Sep 11 10:40:11 2026 -0700
cheapcgi: skip a CGI pair with no =value instead of aborting, refs #38335

Both query string parsers looked for the '=' across the whole rest of the
string rather than inside the current pair.  A pair with no '=' in it
therefore ran into the pair after it and took its value.  "g-catV2&db=hg38"
was stored as one variable named "g-catV2&db", so db was lost with no
warning, and that corrupt name was copied on into the cart.  The same pair
at the end of the string had no '=' left to find and aborted the whole
request, which is what the "Mangled CGI input string g-catV2" entries in the
hgw1 logs were.

Both parsers now find the end of the pair first, keeping the existing
separator precedence ('&', then ';' for DAS), and skip a pair with no '='.
A mixed "a=1;b=2&c=3" still parses the way it did.

Adds lib/tests/cgiParseTest, which runs 18 query strings through both
parsers.  It covers the empty pair of #38185 as well.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

diff --git src/lib/cheapcgi.c src/lib/cheapcgi.c
index eb8f48a7e38..b502e3c32cf 100644
--- src/lib/cheapcgi.c
+++ src/lib/cheapcgi.c
@@ -987,62 +987,77 @@
  * 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, "&;");
 }
 
+static char *endCurrentPair(char *pair)
+/* Zero-terminate the var=val pair that starts at pair, and return the start of
+ * whatever follows it, or NULL if it was the last one.
+ *
+ * The parsers below used to look for the separator only after the '=', which
+ * made them read across the end of a pair that has no '=' in it at all.  A
+ * query string of "g-catV2&db=hg38" was stored as one variable named
+ * "g-catV2&db", so db was lost with no warning, and the same pair at the end of
+ * the string had no '=' left to find and aborted the whole request.  Finding
+ * the end of the pair first confines both parsers to one pair at a time.
+ * refs #38335 */
+{
+char *end = strchr(pair, '&');
+if (end == NULL)
+    end = strchr(pair, ';');	/* Accomodate DAS. */
+if (end == NULL)
+    return NULL;
+*end = 0;
+return end+1;
+}
+
 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 = skipEmptyPairs(*pInput);
+char *var, *val;
+for (;;)
+    {
+    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
-    {
-    *pInput = end+1;
-    *end = 0;
+    *pInput = endCurrentPair(var);
+    val = strchr(var, '=');
+    if (val != NULL)
+        break;
+    /* A pair with no '=' in it names nothing.  Skip it rather than throwing
+     * away the rest of the request over it.  refs #38335 */
     }
+*val++ = 0;
 *retVar = var;
 *retVal = val;
-cgiDecode(val,val,end-val);
+cgiDecode(val,val,strlen(val));
 return TRUE;
 }
 
 void cgiSetMaxLogLen(int l)
 /* set the size of variable values that are dumped to stderr. Default is 0, which means no logging */
 {
 logCgiVarMaxLen = l;
 }
 
 void cgiParseInputAbort(char *input, struct hash **retHash,
         struct cgiVar **retList)
 /* Parse cgi-style input into a hash table and list.  This will alter
  * the input data.  The hash table will contain references back
  * into input, so please don't free input until you're done with
  * the hash. Prints message aborts if there's an error.
@@ -1051,41 +1066,40 @@
 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 = skipEmptyPairs(namePt)) != NULL && namePt[0] != 0)
     {
+    nextNamePt = endCurrentPair(namePt);
     dataPt = strchr(namePt, '=');
     if (dataPt == NULL)
 	{
-	errAbort("Mangled CGI input string %s", namePt);
+	/* A pair with no '=' in it names nothing.  Skip it rather than
+	 * aborting and throwing away the rest of the request.  refs #38335 */
+	namePt = nextNamePt;
+	continue;
 	}
     *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
 
     cgiDecode(namePt,namePt,strlen(namePt));	/* for unusual ct names */
     cgiDecode(dataPt,dataPt,strlen(dataPt));
     AllocVar(el);
     el->val = dataPt;
     slAddHead(&list, el);
     hashAddSaveName(hash, namePt, el, &el->name);
     namePt = nextNamePt;
 
     }
 
 if (logMsg)