cec5ead054791f2a6601f308a56d280c08bd8ef7 braney Fri Sep 4 13:32:36 2026 -0700 quickLift: do not take the lift path on half a pair of settings A hub can set quickLiftDb without setting quickLiftUrl, and nothing filters hub trackDb settings. The alignment loaders gated on quickLiftDb alone, so such a stanza took the lift path with no chain file and hgTracks died in endsWith(NULL, ".bb") from bigChainGetLinkFile, by way of quickLiftLoadChains. Verified: SIGSEGV in strlen from common.c:1653, page truncated mid-HTML. This is reachable on a production browser now that bigChain and bigMaf are liftable, because those carry their own bigDataUrl and so need no trustTrackDb. quickLiftIsLifted requires both halves, and every gate now uses it, which also settles the two different predicates that were in use for the same question. quickLiftLoadChains returns an empty list for a NULL file as well, so the older bed and genePred callers are covered whatever their gate does. quickLiftSql now checks that a row has at least as many columns as the loader is going to read. The native psl loader has always checked this, and the quickLift path replacing it did not, so a table of the wrong type walked off the end of the row; the psl caller now states the 21 columns it needs. quickLiftMafs held a maf component name in a fixed buffer through safecpy, which aborts rather than truncates, so a long name from a hub took hgTracks down. It clones instead. htcBigPslAliInWindow used a trackDb pointer its lookup can leave NULL, which its sibling htcBigPslAli already checked for. And aliTrackParam formats a URL parameter into a fixed buffer with safef, which aborts on a long one. refs #38249 diff --git src/hg/lib/quickLift.c src/hg/lib/quickLift.c index 1b58fb45b8f..0d01f7ee357 100644 --- src/hg/lib/quickLift.c +++ src/hg/lib/quickLift.c @@ -297,30 +297,36 @@ hDisconnectCentral(&conn); if (geneId) ret = atoi(geneId); return ret; } #define QUICKLIFT_RANGE_PAD 100000 static struct chain *quickLiftLoadChains(char *quickLiftFile, char *chrom, int start, int end) /* Load the chains from quickLiftFile that overlap a padded window around the * destination range. */ { +// A track can name the assembly it came from without naming a chain file, since nothing +// stops a hub from setting one of the pair and not the other. With no chains there is +// nothing to lift, and every caller copes with an empty answer. +if (quickLiftFile == NULL) + return NULL; + // need to add some padding to these coordinates int padStart = start - QUICKLIFT_RANGE_PAD; if (padStart < 0) padStart = 0; char *linkFileName = bigChainGetLinkFile(quickLiftFile); return chainLoadIdRangeHub(NULL, quickLiftFile, linkFileName, chrom, padStart, end + QUICKLIFT_RANGE_PAD, -1); } static void quickLiftChainQueryRange(struct chain *chain, int *retQStart, int *retQEnd) /* Return the query-side ("other" species) coordinate range spanned by the * aligned blocks of chain, corrected for query strand. chain->blockList must * not be NULL. */ { @@ -454,30 +460,37 @@ for(chain = chainList; chain; chain = chain->next) { if (chain->blockList == NULL) continue; int qStart, qEnd; quickLiftChainQueryRange(chain, &qStart, &qEnd); // now grab the items if (query == NULL) sr = hRangeQuery(conn, table, chain->qName, qStart, qEnd, extraWhere, &rowOffset); else sr = sqlGetResult(conn, query); + // numFields is what the loader will read, so it is also the least the row can have. + // The native loaders check this; without it a table of the wrong type walks off the + // end of the row. + if ((numFields > 0) && (sqlCountColumns(sr) < numFields + rowOffset)) + errAbort("table %s in %s has %d columns, need at least %d", + table, sqlGetDatabase(conn), sqlCountColumns(sr), numFields + rowOffset); + while ((row = sqlNextRow(sr)) != NULL) { item = loader(row + rowOffset, numFields); slAddHead(&itemList, item); } // now squirrel the swapped chains we used to use to make the retrieved items back to us chainSwap(chain); liftOverAddChainHash(chainHash, chain); } return itemList; } struct genePred *quickLiftGenePreds(struct sqlConnection *conn, char *quickLiftFile, char *table, char *chrom, int start, int end, char *extraWhere, struct hash *chainHash) @@ -730,39 +743,42 @@ for (maf = mafList; maf != NULL; maf = nextMaf) { nextMaf = maf->next; maf->next = NULL; // The first row of a MAF is its reference, and a reference row is always forward. struct mafComp *ref = maf->components; if ((ref == NULL) || (ref->strand != '+') || (ref->size <= 0)) { mafAliFree(&maf); continue; } // the chains are keyed on the sequence name in the other assembly - char srcBuf[1024]; - safecpy(srcBuf, sizeof srcBuf, ref->src); + // mafSplitSrcGetChrom writes into what it is given, so it needs a copy, and the copy + // has to be allocated: a maf component name comes from a hub and safecpy into a + // fixed buffer would abort on a long one rather than truncate. + char *srcBuf = cloneString(ref->src); char *srcChrom = mafSplitSrcGetChrom(srcBuf, sourceDb); int refStart = ref->start; int refEnd = refStart + ref->size; struct chain *chain = liftOverChainForRange(chainHash, srcChrom, refStart, refEnd); if (chain == NULL) { + freeMem(srcBuf); mafAliFree(&maf); continue; } struct cBlock *cb; for (cb = chain->blockList; cb != NULL; cb = cb->next) { int runStart = max(cb->tStart, refStart); int runEnd = min(cb->tEnd, refEnd); if (runStart >= runEnd) continue; struct mafAli *sub = mafSubset(maf, ref->src, runStart, runEnd); if (sub == NULL) continue; @@ -773,36 +789,47 @@ // The lift turns the block over, so turn every row over with it. A chain // keeps its query side reverse complemented, so the forward start of the run // comes from the far end of it. mafFlipStrand(sub); destStart = chain->qSize - (cb->qStart + (runEnd - cb->tStart)); } struct mafComp *subRef = sub->components; freeMem(subRef->src); subRef->src = cloneString(refSrc); subRef->srcSize = refSrcSize; subRef->strand = '+'; subRef->start = destStart; slAddHead(&outList, sub); } + freeMem(srcBuf); mafAliFree(&maf); } slReverse(&outList); return outList; } +boolean quickLiftIsLifted(struct trackDb *tdb) +// TRUE when this track's data comes from another assembly and there is enough to lift it. +// Both halves have to be there: the chain file that does the lifting and the assembly the +// data came from. A hub can set either one on its own, and half the pair is no use. +{ +return (tdb != NULL) && + (trackDbSetting(tdb, "quickLiftUrl") != NULL) && + (trackDbSetting(tdb, "quickLiftDb") != NULL); +} + boolean quickLiftIsOwnChainTrack(struct trackDb *tdb) // TRUE when this is the chain track quickLift builds to show the lift itself. That stanza // carries quickLiftUrl and quickLiftDb like any lifted track, but its data is already in // reference coordinates and must not be lifted a second time. The giveaway is that its // bigDataUrl IS the quickLift chain file. { char *quickLiftFile = trackDbSetting(tdb, "quickLiftUrl"); if (quickLiftFile == NULL) return FALSE; if (startsWithNoCase("bigQuickLiftChain", tdb->type)) return TRUE; char *bigDataUrl = trackDbSetting(tdb, "bigDataUrl"); return (bigDataUrl != NULL) && sameString(bigDataUrl, quickLiftFile);