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) 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; +}