2513833e2a3136be51bb8f849dde30c44fa3a5d2
braney
  Thu Sep 17 09:53:14 2026 -0700
cart: the fifth copy of the malformed pair loop, in loadHash

loadHash searched for the '=' across the whole remaining string, so a setting
with a name and no value ran into the setting behind it and renamed it, and the
same pair at the end of the string aborted the CGI.  This is the copy
cartParseOverHash uses, which reads namedSessionDb.contents and the cart table,
so it has the widest reach of the five and the reader has no way to clear a
string that is already stored.

It now steps over the separators of an empty pair and confines the search to one
pair, behind skipMalformedCgiPairs like the three the same ticket fixed.  With
the flag off the parse is unchanged, including the ampersand winning over the
semicolon DAS uses.

Nothing is broken today.  Every pair in the September hgwbeta session dump has an
'=' in it, all 1004 rows, and the cart writer always emits name=value.

Found in the v504 code review, refs #38349.

refs #38340

diff --git src/hg/lib/cart.c src/hg/lib/cart.c
index 02a080753d3..64ecbb3b31d 100644
--- src/hg/lib/cart.c
+++ src/hg/lib/cart.c
@@ -345,42 +345,69 @@
     }
 if (cartVarHoldsFileNamePair(var))
     {
     if (fileNamePairIsAcceptable(val))
         return TRUE;
     logDroppedFileNameVar(var, "a pair of trash or session-data file names");
     return FALSE;
     }
 return TRUE;
 }
 
 static void loadHash(struct hash *hash, char *contents)
 /* Load a hash from a cart-like string. */
 {
 char *namePt, *dataPt, *nextNamePt;
+boolean skipMalformed = cfgOptionBooleanDefault("skipMalformedCgiPairs", FALSE);
 namePt = contents;
 while (namePt != NULL && namePt[0] != 0)
+    {
+    if (skipMalformed)
+	{
+	/* Step over the separators of an empty pair, then confine the search for
+	 * the '=' to this pair.  Without both, a setting with a name and no value
+	 * renames the setting after it, and the same pair at the end of the string
+	 * aborts the CGI.  This string is a saved session or a cart row rather than
+	 * a request, so the reader has no way to clear it.  refs #38340 */
+	namePt += strspn(namePt, "&;");
+	if (namePt[0] == 0)
+	    break;
+	nextNamePt = strchr(namePt, '&');
+	if (nextNamePt == NULL)
+	    nextNamePt = strchr(namePt, ';');	/* Accomodate DAS. */
+	if (nextNamePt != NULL)
+	    *nextNamePt++ = 0;
+	dataPt = strchr(namePt, '=');
+	if (dataPt == NULL)
+	    {
+	    namePt = nextNamePt;
+	    continue;
+	    }
+	*dataPt++ = 0;
+	}
+    else
 	{
 	dataPt = strchr(namePt, '=');
 	if (dataPt == NULL)
 	    errAbort("Mangled input string %s", namePt);
 	*dataPt++ = 0;
 	nextNamePt = strchr(dataPt, '&');
 	if (nextNamePt == NULL)
 	    nextNamePt = strchr(dataPt, ';');	/* Accomodate DAS. */
 	if (nextNamePt != NULL)
 	     *nextNamePt++ = 0;
+	}
     cgiDecode(dataPt,dataPt,strlen(dataPt));
     if (cartValueIsAcceptable(namePt, dataPt))
         hashAdd(hash, namePt, cloneString(dataPt));
     namePt = nextNamePt;
     }
 }
 
 void cartParseOverHashExt(struct cart *cart, char *contents, boolean merge)
 /* Parse cgi-style contents into a hash table.  If merge is FALSE, this will *not*
  * replace existing members of hash that have same name, so we can
  * support multi-select form inputs (same var name can have multiple
  * values which will be in separate hashEl's). If merge is TRUE, we
  * replace existing values with new values */
 {
 if (merge)