3eedbb7636648978b0371f2c54378e75fd1ac78c
braney
  Fri Sep 11 12:20:14 2026 -0700
Skip a CGI or cookie pair with no =value in three more parsers, refs #38340

The loop that #38335 fixed in the query string parsers is copied in three more
places, and each one still looks for the '=' across the whole rest of the string
instead of inside the pair it is reading.  A pair with no value therefore runs
into the pair after it and takes its value, and the same pair at the end of the
string has no '=' left to find and aborts.

lib/cheapcgi.c parseCookies         one bad cookie aborts every CGI for that
browser, on every request, until the
reader clears the cookie by hand
hg/hgSession/backup.c               a session backup silently leaves out a
custom track
hg/utils/refreshNamedSessionCustomTracks
one bad session aborts the child, the
parent exits non-zero, and every session
after it goes unscanned, so the trash
cleaner removes their custom track files

All three are behind the hg.conf flag skipMalformedCgiPairs, off by default, and
registered as a release gate in hgConfCatalog.  The kent libraries do not read
hg.conf, so hgConfig.c hands the setting to cheapcgi the way cfgSetLogCgiVars
already hands it cgiSetMaxLogLen.  The query string parsers do not read the
flag; they were fixed unconditionally under #38335.

refreshNamedSessionCustomTracks rebuilds the session contents as it walks, so it
copies a malformed pair through untouched rather than stepping over it.  A
session carrying one comes back byte for byte the same.

Adds lib/tests/cgiCookieTest and hg/hgSession/tests/backupParseTest.  Both read
every case with the flag off and on, so they pin the old behavior as well as the
new one.  The nightly tool has no seam for a unit test; its loop sits inside a
function that runs its own query.

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

diff --git src/hg/utils/refreshNamedSessionCustomTracks/refreshNamedSessionCustomTracks.c src/hg/utils/refreshNamedSessionCustomTracks/refreshNamedSessionCustomTracks.c
index 92e13e2dbc0..21a931f057e 100644
--- src/hg/utils/refreshNamedSessionCustomTracks/refreshNamedSessionCustomTracks.c
+++ src/hg/utils/refreshNamedSessionCustomTracks/refreshNamedSessionCustomTracks.c
@@ -224,40 +224,77 @@
 sqlSafef(query, sizeof(query),
 	  "select contents from %s "
 	  "where userName='%s' and sessionName = '%s'", savedSessionTable, userName, sessionName);
 char *contents = sqlQuickString(conn, query);
 if (!contents)
     return;
 
 int contentLength = strlen(contents);
 struct dyString *newContents = dyStringNew(contentLength+1);
 struct dyString *oneSetting = dyStringNew(contentLength / 4);
 char *contentsToChop = cloneString(contents);
 char *namePt = contentsToChop;
 
 
 verbose(3, "Scanning %s %s\n", userName, sessionName);
+boolean skipMalformed = cfgOptionBooleanDefault("skipMalformedCgiPairs", FALSE);
+
 while (isNotEmpty(namePt))
     {
-    char *dataPt = strchr(namePt, '=');
+    char *dataPt;
     char *nextNamePt;
+    if (skipMalformed)
+	{
+	/* Confine the search for the '=' to this pair.  Otherwise a pair with
+	 * no value renames the pair after it, so a ctfile_ setting behind one
+	 * is never seen and its custom track file is left to expire.  A pair
+	 * with no value at the end of the contents aborts this child, the
+	 * parent exits non-zero, and every session after it in the table goes
+	 * unscanned.  refs #38340 */
+	size_t sepCount = strspn(namePt, "&");
+	if (sepCount > 0)
+	    {
+	    /* Copy the separators of an empty pair through untouched.  This
+	     * loop rebuilds newContents as it walks, so anything it steps over
+	     * silently would be dropped from the session under -hardcore. */
+	    dyStringAppendN(newContents, namePt, sepCount);
+	    namePt += sepCount;
+	    if (namePt[0] == 0)
+		break;
+	    }
+	nextNamePt = strchr(namePt, '&');
+	if (nextNamePt != NULL)
+	    *nextNamePt++ = 0;
+	dataPt = strchr(namePt, '=');
+	if (dataPt == NULL)
+	    {
+	    dyStringPrintf(newContents, "%s%s", namePt, (nextNamePt ? "&" : ""));
+	    namePt = nextNamePt;
+	    continue;
+	    }
+	*dataPt++ = 0;
+	}
+    else
+	{
+	dataPt = strchr(namePt, '=');
 	if (dataPt == NULL)
 	    errAbort("ERROR: Mangled session content string %s", namePt);
 	*dataPt++ = 0;
 	nextNamePt = strchr(dataPt, '&');
 	if (nextNamePt != NULL)
 	    *nextNamePt++ = 0;
+	}
     dyStringClear(oneSetting);
     dyStringPrintf(oneSetting, "%s=%s%s",
 		   namePt, dataPt, (nextNamePt ? "&" : ""));
     if (startsWith(CT_FILE_VAR_PREFIX, namePt))
 	{
 	cgiDecode(dataPt, dataPt, strlen(dataPt));
 	boolean thisGotLiveCT = FALSE, thisGotExpiredCT = FALSE;
 	verbose(3, "Found variable %s = %s\n", namePt, dataPt);
 	/* If the file does not exist, omit this setting from newContents so 
 	 * it doesn't get copied from session to session.  If it does exist,
 	 * leave it up to customFactoryTestExistence to parse the file for 
 	 * possible customTrash table references, some of which may exist 
 	 * and some not. */
 	if (! fileExists(dataPt))
 	    {