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/tests/cgiParseTest.c src/lib/tests/cgiParseTest.c
new file mode 100644
index 00000000000..cc08c0fc612
--- /dev/null
+++ src/lib/tests/cgiParseTest.c
@@ -0,0 +1,95 @@
+/* cgiParseTest - check how the two cgi query string parsers treat a malformed pair. */
+
+/* Copyright (C) 2026 The Regents of the University of California
+ * See kent/LICENSE or http://genome.ucsc.edu/license/ for licensing information. */
+
+#include "common.h"
+#include "cheapcgi.h"
+#include "errCatch.h"
+#include "hash.h"
+
+static char *cases[] = {
+/* the ordinary shape, and the same thing with semicolons, which DAS clients send */
+"db=hg38&position=chr1:1-1000",
+"a=1;b=2;c=3",
+/* mixed separators: the ampersand still ends the pair, so the semicolon stays in the value */
+"a=1;b=2&c=3",
+/* an empty pair, at the front, in the middle and at the end. The middle one used to
+ * name the next variable "&position", so nothing looked it up, and the last one used
+ * to abort the whole request. refs #38185 */
+"&db=hg38&position=chr1:1-1000",
+"db=hg38&&position=chr1:1-1000",
+"db=hg38&position=chr1:1-1000&&",
+"&&;&db=hg38",
+/* a pair with a name but no =value, in the same three places. The trailing one used to
+ * abort, and the other two used to run into the pair after them and take its value,
+ * losing that variable without a word. refs #38335 */
+"g-catV2&db=hg38",
+"db=hg38&i&position=chr1:1-1000",
+"db=hg38&g-catV2",
+"g-catV2",
+/* an empty value is a value, and is kept */
+"a=1&b=&c=3",
+"=v",
+"a=1&=&b=2",
+/* nothing at all */
+"",
+"&",
+/* an equals sign in a value belongs to the value */
+"hgt.customText=track name=one",
+/* the escapes a value can carry, including an ampersand that is not a separator */
+"position=chr1%3A1-1000&name=a+b&other=x%26y",
+};
+
+static void showParseNext(char *in)
+/* Print what cgiParseNext makes of in. */
+{
+printf("next: ");
+struct errCatch *errCatch = errCatchNew();
+if (errCatchStart(errCatch))
+ {
+ char *s = cloneString(in);
+ char *pt = s, *var, *val;
+ while (cgiParseNext(&pt, &var, &val))
+ printf("[%s=%s]", var, val);
+ }
+errCatchEnd(errCatch);
+if (errCatch->gotError)
+ printf("aborted: %s", trimSpaces(errCatch->message->string));
+errCatchFree(&errCatch);
+printf("\n");
+}
+
+static void showParseInput(char *in)
+/* Print what cgiParseInputAbort makes of in. */
+{
+printf("hash: ");
+struct errCatch *errCatch = errCatchNew();
+if (errCatchStart(errCatch))
+ {
+ char *s = cloneString(in);
+ struct hash *hash = NULL;
+ struct cgiVar *list = NULL, *var;
+ cgiParseInputAbort(s, &hash, &list);
+ for (var = list; var != NULL; var = var->next)
+ printf("[%s=%s]", var->name, var->val);
+ }
+errCatchEnd(errCatch);
+if (errCatch->gotError)
+ printf("aborted: %s", trimSpaces(errCatch->message->string));
+errCatchFree(&errCatch);
+printf("\n");
+}
+
+int main(int argc, char *argv[])
+{
+int i;
+for (i = 0; i < ArraySize(cases); ++i)
+ {
+ printf("in : %s\n", cases[i]);
+ showParseNext(cases[i]);
+ showParseInput(cases[i]);
+ printf("\n");
+ }
+return 0;
+}