304e190d0af4be54569ac20edc26999673c44f8b
braney
  Tue Aug 18 11:05:19 2026 -0700
hgTrackUi, hui: encode trackDb-derived label text consistently, refs #38123

diff --git src/hg/lib/hui.c src/hg/lib/hui.c
index 982ef64ad30..743ed56095a 100644
--- src/hg/lib/hui.c
+++ src/hg/lib/hui.c
@@ -206,41 +206,44 @@
     dyStringAppend(dyLink,suffix);  // Don't encode since this may contain HTML
 
 freeMem(encTerm);
 freeMem(encValue);
 return dyStringCannibalize(&dyLink);
 }
 
 char *pairsAsHtmlTable( struct slPair *pairs, struct trackDb *tdb, boolean showLongLabel,boolean showShortLabel)
 /* Return a string which is an HTML table of the tags for this track. */
 {
 if (pairs == NULL)
     return "";
 
 struct dyString *dyTable = dyStringCreate("<table style='display:inline-table;'>");
 
+// the labels and the metadata pairs all come from trackDb, which a hub controls, escape
 if (showLongLabel)
-    dyStringPrintf(dyTable,"<tr valign='bottom'><td colspan=2 nowrap>%s</td></tr>",tdb->longLabel);
+    dyStringPrintf(dyTable,"<tr valign='bottom'><td colspan=2 nowrap>%s</td></tr>",
+                   htmlEncode(tdb->longLabel));
 if (showShortLabel)
     dyStringPrintf(dyTable,"<tr valign='bottom'><td align='right' nowrap><i>shortLabel:</i></td>"
-			   "<td nowrap>%s</td></tr>",tdb->shortLabel);
+			   "<td nowrap>%s</td></tr>",htmlEncode(tdb->shortLabel));
 
 for(; pairs; pairs = pairs->next)
     {
     if (!sameString(pairs->name, "meta")  && !isEmpty((char *)pairs->val))
         dyStringPrintf(dyTable,"<tr valign='bottom'><td align='right' nowrap><i>%s:</i></td>"
-                           "<td nowrap>%s</td></tr>",pairs->name, (char *)pairs->val);
+                           "<td nowrap>%s</td></tr>",htmlEncode(pairs->name),
+                           htmlEncode((char *)pairs->val));
     }
 dyStringAppend(dyTable,"</table>");
 return dyStringCannibalize(&dyTable);
 }
 
 char *metadataAsHtmlTable(char *db,struct trackDb *tdb,boolean showLongLabel,boolean showShortLabel)
 // If metadata from metaDb exists, return string of html with table definition
 {
 struct slPair *pairs = NULL;
 
 if ((pairs = trackDbMetaPairs(tdb)) != NULL)
     return pairsAsHtmlTable(pairs, tdb, showLongLabel, showShortLabel);
 
 const struct mdbObj *safeObj = metadataForTable(db,tdb,NULL);
 if (safeObj == NULL || safeObj->vars == NULL)
@@ -2959,41 +2962,50 @@
     }
 members = needMem(sizeof(members_t));
 members->setting = cloneString(setting);
 #define MAX_SUBGROUP_MEMBERS 2000
 char *words[MAX_SUBGROUP_MEMBERS+3];    // members preceded by tag and title, one extra to detect
 count = chopLine(members->setting, words);
 if (count == ArraySize(words))
     warn("Subgroup %s exceeds maximum %d members", words[1], MAX_SUBGROUP_MEMBERS); 
 if (count <= 1)
     {
     freeMem(members->setting);
     freeMem(members);
     tdbExtrasMembersSet(parentTdb, groupNameOrTag, &nullMember);
     return NULL;
     }
+// A subGroup label from a track hub is text from a stranger and is printed into the page in
+// a dozen places, so escape it here, once.  Only for hub tracks: our own trackDb puts real
+// HTML entities in these labels on purpose (the ENCODE composites use &nbsp; and &alpha),
+// and escaping those would show the entity text instead of the character.
+boolean escapeLabels = isHubTrack(parentTdb->track);
 members->groupTag   = words[0];
 members->groupTitle = strSwapChar(words[1],'_',' '); // Titles replace '_' with space
+if (escapeLabels)
+    members->groupTitle = htmlEncode(members->groupTitle);
 members->tags       = needMem(count*sizeof(char*));
 members->titles     = needMem(count*sizeof(char*));
 for (ix = 2,members->count=0; ix < count; ix++)
     {
     char *name,*value;
     if (parseAssignment(words[ix], &name, &value))
 	{
 	members->tags[members->count]  = tagEncode(name);
 	members->titles[members->count] = strSwapChar(value,'_',' ');
+	if (escapeLabels)
+	    members->titles[members->count] = htmlEncode(members->titles[members->count]);
 	members->count++;
 	}
     else
         {
         warn("Subgroup \"%s\" is missing a tag=val pair", words[1]);
         }
     }
 tdbExtrasMembersSet(parentTdb, groupNameOrTag, members);
 
 return members;
 }
 
 static int membersSubGroupIx(members_t* members, char *tag)
 // Returns the index of the subgroup within the members struct (or -1)
 {
@@ -4404,34 +4416,35 @@
     jsIncludeFile("ui.dropdownchecklist.js",NULL);
     jsIncludeFile("ddcl.js",NULL);
     }
 
 // TODO: columnCount (Number of filterBoxes per row) should be configurable through tdb setting
 
 for (filterBy = filterBySet;  filterBy != NULL;  filterBy = filterBy->next)
     {
     puts("<TD>");
     char selectStatement[4096];
     char *setting =  getFilterType(cart, tdb, filterBy->column, FILTERBY_DEFAULT);
     if (filterByColumnIsMultiple(cart, tdb, setting))
         safef(selectStatement, sizeof selectStatement, " (select multiple items - %s)", FILTERBY_HELP_LINK);
     else
         selectStatement[0] = 0;
+    // the title is filterLabel.<field> or an autoSql column comment, both hub supplied
     if(count == 1)
-	printf("<B>%s by %s</B>%s",filterTypeTitle,filterBy->title,selectStatement);
+	printf("<B>%s by %s</B>%s",filterTypeTitle,htmlEncode(filterBy->title),selectStatement);
     else
-	printf("<B>%s</B>",filterBy->title);
+	printf("<B>%s</B>",htmlEncode(filterBy->title));
     puts("</TD>");
     }
 puts("</tr><tr>");
 for (filterBy = filterBySet;  filterBy != NULL;  filterBy = filterBy->next)
     {
     puts("<td>");
     char *setting =  getFilterType(cart, tdb, filterBy->column, FILTERBY_DEFAULT);
     if (advancedFilter(cart, tdb, setting))
         {
         char cartSettingString[4096];
         if (isHighlight)
             {
             safef(cartSettingString, sizeof cartSettingString, "%s.%s.%s", prefix,HIGHLIGHT_TYPE_NAME_LOW, filterBy->column);
             printf("<div ><b>Highlight if  ");
             // ADVANCED BUTTON printf("<div class='advanced' style='display:none'><b>Match if  ");
@@ -4478,43 +4491,43 @@
 	    {
 	    safef(varName, sizeof(varName), "%d",valIx);
 	    name = varName;
 	    label = slValue->name;
 	    }
 	else
 	    {
 	    label = (filterBy->valueAndLabel ? slValue->name + strlen(slValue->name)+1
 					     : slValue->name);
 	    name = slValue->name;
 	    }
 	printf("<OPTION");
 	if (filterBy->slChoices != NULL && slNameInList(filterBy->slChoices,name))
 	    printf(" SELECTED");
 	if (filterBy->useIndex || filterBy->valueAndLabel)
-	    printf(" value='%s'",name);
+	    printf(" value='%s'",htmlEncode(name));    // filterValues are hub supplied
 	if (filterBy->styleFollows)
 	    {
 	    char *styler = label + strlen(label)+1;
 	    if (*styler != '\0')
 		{
 		if (*styler == '#') // Legacy: just the color that follows
 		    printf(" style='color: %s;'",styler);
 		else
 		    printf(" style='%s'",styler);
 		}
 	    }
-	printf(">%s</OPTION>\n",label);
+	printf(">%s</OPTION>\n",htmlEncode(label));   // filterValues are hub supplied
 	}
     printf("</SELECT>\n");
     puts("</td>");
     }
 
 puts("</TR></TABLE>");
 }
 
 void filterBySetCfgUi(struct cart *cart, struct trackDb *tdb,
 		  filterBy_t *filterBySet, boolean onOneLine, char *prefix)
 /* Does the filter UI for a list of filterBy structure */
 {
 char selectIdPrefix[4096];
 safef(selectIdPrefix, sizeof(selectIdPrefix), "fbc_%s", prefix);
 // Our checklists use ddcl.js, which doesn't seem to play nicely when elements have id strings that include '.'
@@ -5556,31 +5569,32 @@
                 printf("<TD><span style=\"color:red\">Missing subgroup</span></TD>");
                 }
             else
                 {
                 if (ix >= 0)
                     {
                     char *term = membership->membership[ix];
                     char *title = membership->titles[ix];
                     char *titleRoot=NULL;
                     if (cvTermIsEmpty(col, title))
                         titleRoot = cloneString(" &nbsp;");
                     else
                         titleRoot = labelRoot(title, NULL);
                     // Each sortable column requires hidden goop (in the "abbr" field currently)
                     // which is the actual sort on value
-                    printf("<TD id='%s_%s' abbr='%s' align='left'>", subtrack->track, col, term);
+                    printf("<TD id='%s_%s' abbr='%s' align='left'>", subtrack->track, col,
+                           htmlEncode(term));
                     printf("&nbsp;");
                     char *link = NULL;
                     if (vocabHash)
                         {
                         struct hash *colHash = hashFindVal(vocabHash, col);
                         if (colHash)
 			    link = vocabLink(colHash, term, titleRoot);
                         }
                     printf("%s", link ? link : titleRoot);
                     puts("</TD>");
                     freeMem(titleRoot);
                     }
                 else if (sameString(col, SUBTRACK_COLOR_SUBGROUP))
                     {
                     char *hue = subtrackColorToCompare(subtrack);
@@ -5592,36 +5606,36 @@
                 }
             }
         }
     else  // Non-sortable tables do not have sort by columns but will display a short label
 	{ // (which may be a configurable link)
 	if (settings->colorPatch)
 	    {
 	    printf("<TD BGCOLOR='#%02X%02X%02X'>&nbsp;&nbsp;&nbsp;&nbsp;</TD>",
 			   subtrack->colorR, subtrack->colorG, subtrack->colorB);
 
 	    }
 	printf("<TD>&nbsp;");
 	hierarchy_t *hierarchy = hierarchySettingGet(parentTdb);
 	indentIfNeeded(hierarchy,membership);
 	hierarchyFree(&hierarchy);
-	printf("%s",subtrack->shortLabel);
+	printf("%s",htmlEncode(subtrack->shortLabel));
 	puts("</TD>");
 	}
 
     // The long label column (note that it may have a metadata dropdown)
-    printf("<TD title='select to copy'>&nbsp;%s", subtrack->longLabel);
+    printf("<TD title='select to copy'>&nbsp;%s", htmlEncode(subtrack->longLabel));
     if (trackDbSetting(parentTdb, "wgEncode") && trackDbSetting(subtrack, "accession"))
 	printf(" [GEO:%s]", trackDbSetting(subtrack, "accession"));
     compositeMetadataToggle(db,subtrack,NULL,TRUE,FALSE);
     printf("&nbsp;");
 
     // Embedded cfg dialogs are within the TD that contains the longLabel.
     //  This allows a wide item to be embedded in the table
     if (cType != cfgNone)
 	{
 	// How to make this thing float to the left?  Container is overflow:visible
 	// and contained (made in js) is position:relative; left: -{some pixels}
 	#define CFG_SUBTRACK_DIV "<DIV id='div_cfg_%s' class='subCfg %s' style='display:none; " \
 				 "overflow:visible;'></DIV>"
 	#define MAKE_CFG_SUBTRACK_DIV(table,view) \
 					printf(CFG_SUBTRACK_DIV,(table),(view)?(view):"noView")
@@ -7418,31 +7432,32 @@
             isDefault = (thisLabel == labelList);
         else if (sameString(defaultLabelList->name, "none"))
             isDefault = FALSE;
         else
             isDefault = (slPairFind(defaultLabelList, thisLabel->name) != NULL);
 
         boolean option = cartUsualBoolean(cart, varName, isDefault);
         cgiMakeCheckBox(varName, option);
 
         // find comment for the column listed
         struct asColumn *col = as->columnList;
         unsigned num = ptToInt(thisLabel->val);
         for(; col && num--; col = col->next)
             ;
         assert(col);
-        printf(" %s&nbsp;&nbsp;&nbsp;", col->comment);
+        // the comment comes from the autoSql inside the hub's bigBed, escape
+        printf(" %s&nbsp;&nbsp;&nbsp;", htmlEncode(col->comment));
         }
     }
 }
 
 static char *colorFieldLabel(struct slPair *p)
 /* Derive a display label for one colorFields entry.
  * Uses the explicit ="label" if provided, otherwise strips a leading "colorBy"
  * prefix and replaces underscores with spaces. */
 {
 if (isNotEmpty((char *)p->val))
     return (char *)p->val;
 char *lbl = p->name;
 if (startsWith("colorBy", lbl))
     lbl += strlen("colorBy");
 char *derived = cloneString(lbl);
@@ -10758,31 +10773,32 @@
 {
 char *version = checkDataVersion(database, tdb);
 
 if (version == NULL)
     {
     // try the hgFixed.trackVersion table
     struct trackVersion *trackVersion = getTrackVersion(database, tdb->track);
     // try trackVersion table with parent, for composites/superTracks
     if (trackVersion == NULL && tdb->parent != NULL)
         trackVersion = getTrackVersion(database, tdb->parent->track);
     if (trackVersion != NULL)
         version = trackVersion->version;
     }
 
 if (isNotEmpty(version))
-    printf("<B>Version:</B> %s <BR>\n", version);
+    // dataVersion can come from a track hub, escape
+    printf("<B>Version:</B> %s <BR>\n", htmlEncode(version));
 }
 
 void printRelatedTracks(char *database, struct hash *trackHash, struct trackDb *tdb, struct cart *cart)
 /* Maybe print a "related track" section */
 {
 if (trackHubDatabase(database))
     return;
 char *relatedTrackTable = cfgOptionDefault("db.relatedTrack","relatedTrack");
 struct sqlConnection *conn = hAllocConn(database);
 if (!sqlTableExists(conn, relatedTrackTable))
     {
     hFreeConn(&conn);
     return;
     }