c3788110e8d865774fff8197cf5b47350472ba64 chmalee Tue Aug 4 11:26:55 2026 -0700 Fix hubSpace hook error handling and nested hub path handling, refs #37964 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> diff --git src/hg/lib/userdata.c src/hg/lib/userdata.c index 2cb2abf7f61..73e5a1a1639 100644 --- src/hg/lib/userdata.c +++ src/hg/lib/userdata.c @@ -15,30 +15,36 @@ #include "hgConfig.h" #include "dystring.h" #include "cheapcgi.h" #include "customFactory.h" #include "wikiLink.h" #include "userdata.h" #include "jksql.h" #include "hdb.h" #include "hubSpace.h" #include "hubSpaceQuotas.h" #include "errCatch.h" #include "twoBit.h" #include "trackHub.h" #include <limits.h> +// how long a pre-finish hook waits for another upload to the same hub, and how +// often it retries while waiting. A big batch into one hub queues on this lock, +// so hg.conf can raise the wait without a rebuild +#define HUB_LOCK_TIMEOUT_DEFAULT "300" +#define HUB_LOCK_POLL_MS 100 + char *emailForUserName(char *userName) /* Fetch the email for this user from gbMembers hgcentral table */ { struct sqlConnection *sc = hConnectCentral(); struct dyString *query = sqlDyStringCreate("select email from gbMembers where userName = '%s'", userName); char *email = sqlQuickString(sc, dyStringCannibalize(&query)); hDisconnectCentral(&sc); // this should be freeMem'd: return email; } char *getEncodedUserNamePath(char *userName) /* Compute the path for just the userName part of the users upload */ { struct dyString *ret = dyStringNew(0); @@ -374,42 +380,84 @@ { char *firstName = tbf->indexList->name; int size = twoBitSeqSize(tbf, firstName); int end = (size < 1000) ? size : 1000; struct dyString *ds = dyStringCreate("%s:1-%d", firstName, end); result = dyStringCannibalize(&ds); } if (tbf) twoBitClose(&tbf); } errCatchEnd(errCatch); errCatchFree(&errCatch); return result; } -static char *hubPathFromParentDir(char *parentDir, char *userDataDir) -/* Assume parentDir does not have leading '/' or '.', parse out the first dir component - * and add it to the users directory*/ +char *hubRootFromParentDir(char *parentDir) +/* Return the first '/' separated component of parentDir, which is the hub itself. + * The hub.txt and the hubSpace dir row for a hub both live at that level, while + * hubNameFromPath gives the immediately containing directory, which for a nested + * parentDir like 'myHub/hg38' is a subdirectory of the hub */ { char *copy = cloneString(parentDir); char *firstSlash = strchr(copy, '/'); -if (!firstSlash) - { +if (firstSlash) + *firstSlash = 0; return copy; } -*firstSlash = 0; -return catTwoStrings(userDataDir, copy); + +char *hubPathFromParentDir(char *parentDir, char *userDataDir) +/* Return the directory holding this hub's hub.txt, that is the user's directory + * plus the hub component of parentDir */ +{ +char *hubRoot = hubRootFromParentDir(parentDir); +char *hubPath = catTwoStrings(userDataDir, hubRoot); +freeMem(hubRoot); +return hubPath; +} + +static char *hubSubPathFromParentDir(char *parentDir) +/* Return the part of parentDir below the hub component, keeping its trailing '/', + * or "" when the file sits directly in the hub. This is the prefix a bigDataUrl + * needs, since hub.txt is at the hub and the file may be in a subdirectory */ +{ +char *firstSlash = strchr(parentDir, '/'); +if (firstSlash && firstSlash[1] != '\0') + return cloneString(firstSlash + 1); +return cloneString(""); +} + +static char *pathRelativeToHubDir(char *filePath, char *hubDir) +/* Return the part of filePath below hubDir. twoBitPath and bigDataUrl are relative + * to the hub.txt, so a file in a subdirectory needs that subdirectory in its setting. + * Fall back to the file name if filePath is not under hubDir */ +{ +// the '/' test keeps a hub from matching a sibling whose name it is a prefix of +if (startsWith(hubDir, filePath) && filePath[strlen(hubDir)] == '/') + { + char *rel = filePath + strlen(hubDir); + while (*rel == '/') + rel++; + if (*rel != '\0') + return cloneString(rel); + } +// only reachable if the file is not under the hub, which means the two paths +// disagree about symlinks. The setting we fall back to will not resolve +fprintf(stderr, "warning: '%s' is not under hub dir '%s', hub.txt setting " + "will use the file name alone\n", filePath, hubDir); +char *lastSlash = strrchr(filePath, '/'); +return cloneString(lastSlash ? lastSlash + 1 : filePath); } static void upgradeHubTxtForAssembly(char *hubFile, char *db, char *twoBitFileName) /* If hubFile exists but lacks a twoBitPath line, rewrite it to insert an * assembly-hub stanza (twoBitPath + stub organism/scientificName/description/ * defaultPos) immediately after the 'genome' line, and replace that line's * db value with the 2bit's assembly name. Called when a 2bit arrives after * a plain track-hub hub.txt has already been synthesized for this hub. * No-op if hubFile doesn't exist or already has twoBitPath. */ { if (!fileExists(hubFile)) return; // Collect all lines, matching directives against skipLeadingSpaces so that // indented (tab/space) stanzas are handled the same as column-0 ones. @@ -430,244 +478,281 @@ return; } if (genomeIdx < 0 && startsWith("genome ", trimmed)) genomeIdx = i; slAddHead(&lines, slNameNew(line)); i++; } lineFileClose(&lf); slReverse(&lines); if (genomeIdx < 0) { slNameFreeList(&lines); return; } -char *twoBitBase = strrchr(twoBitFileName, '/'); -twoBitBase = twoBitBase ? twoBitBase + 1 : twoBitFileName; +char *hubDir = cloneString(hubFile); +char *lastSlash = strrchr(hubDir, '/'); +if (lastSlash) + *lastSlash = 0; +else + { + freeMem(hubDir); + hubDir = cloneString("."); + } +char *twoBitBase = pathRelativeToHubDir(twoBitFileName, hubDir); char *defaultPos = defaultPosFromTwoBit(twoBitFileName); struct dyString *out = dyStringNew(1024); struct slName *ln; for (ln = lines, i = 0; ln; ln = ln->next, i++) { if (i == genomeIdx) { dyStringPrintf(out, "genome %s\n" "twoBitPath %s\n" "organism %s\n" "scientificName %s\n" "description %s\n" "defaultPos %s\n", db, twoBitBase, db, db, db, defaultPos ? defaultPos : "chr1:1-1000"); } else { dyStringAppend(out, ln->name); dyStringAppendC(out, '\n'); } } // Write to a sibling temp file and rename into place so a partial write // (ENOSPC, SIGKILL, etc.) cannot leave the user's hub.txt truncated. -char *hubDir = cloneString(hubFile); -char *lastSlash = strrchr(hubDir, '/'); -if (lastSlash) - *lastSlash = 0; -else - strcpy(hubDir, "."); char *tmpFile = cloneString(rTempName(hubDir, "hub", ".txt")); FILE *f = mustOpen(tmpFile, "w"); mustWrite(f, out->string, out->stringSize); carefulClose(&f); mustRename(tmpFile, hubFile); freeMem(tmpFile); freeMem(hubDir); freez(&defaultPos); dyStringFree(&out); slNameFreeList(&lines); } +static void refreshHubTextRow(char *userName, char *hubFile, char *hubName) +/* Bring the hubSpace row for a hub.txt back in line with the file on disk. Called + * after rewriting a hub.txt that already has a row, so My Data and the quota do not + * keep reporting the size and checksum from before the rewrite */ +{ +if (!fileExists(hubFile)) + return; +time_t modTime = fileModTime(hubFile); +struct sqlConnection *conn = hConnectCentral(); +struct dyString *q = sqlDyStringCreate( + "update hubSpace set fileSize=%lld, md5sum='%s', lastModified='%s' " + "where userName='%s' and fileName='hub.txt' and parentDir='%s'", + (long long)fileSize(hubFile), md5HexForFile(hubFile), + sqlUnixTimeToDate(&modTime, TRUE), userName, hubName); +sqlUpdate(conn, dyStringCannibalize(&q)); +hDisconnectCentral(&conn); +} + static void setAssemblyHubTypeForDir(char *userName, char *parentDir) /* Flip this user's hub (dir row + direct-child files) to hubType='assemblyHub'. * Does not recurse into nested parentDirs like hubName/tracks; only the * hubtools-then-UI promotion flow can produce those. */ { if (!userName || !parentDir || parentDir[0] == '\0') return; struct sqlConnection *conn = hConnectCentral(); struct dyString *q = sqlDyStringCreate( "update hubSpace set hubType='assemblyHub' " "where userName='%s' and (parentDir='%s' or (fileName='%s' and parentDir=''))", userName, parentDir, parentDir); sqlUpdate(conn, dyStringCannibalize(&q)); hDisconnectCentral(&conn); } int lockHubDir(char *hubDir) /* Acquire an exclusive flock on hubDir/.hub.lock, creating the lock file * if necessary. Returns a file descriptor; pass to unlockHubDir to release. * Serializes hub.txt read-modify-write across parallel pre-finish processes. */ { struct dyString *lockPath = dyStringCreate("%s%s.hub.lock", hubDir, endsWith(hubDir, "/") ? "" : "/"); -int fd = open(dyStringContents(lockPath), O_RDWR | O_CREAT, 0666); +// O_CLOEXEC so the md5sum children we fork under this lock do not inherit it. +// An inherited fd outliving a killed hook would hold the lock with no owner left +// to release it, and every later upload to the hub would time out +int fd = open(dyStringContents(lockPath), O_RDWR | O_CREAT | O_CLOEXEC, 0666); if (fd < 0) errnoAbort("could not open hub lock %s", dyStringContents(lockPath)); -if (flock(fd, LOCK_EX) < 0) +// poll rather than block, so one stuck upload cannot hold up every other upload +// to this hub indefinitely +// clamp so a mistyped hg.conf value cannot turn into a negative wait +int timeoutSeconds = atoi(cfgOptionDefault("hubSpaceLockTimeout", HUB_LOCK_TIMEOUT_DEFAULT)); +if (timeoutSeconds < 1 || timeoutSeconds > 3600) + timeoutSeconds = atoi(HUB_LOCK_TIMEOUT_DEFAULT); +int timeoutMs = timeoutSeconds * 1000; +int waitedMs = 0; +while (flock(fd, LOCK_EX | LOCK_NB) < 0) + { + if (errno != EWOULDBLOCK) errnoAbort("could not acquire hub lock on %s", dyStringContents(lockPath)); + if (waitedMs >= timeoutMs) + errAbort("Timed out waiting for another upload to this hub to finish. " + "Please try again."); + sleep1000(HUB_LOCK_POLL_MS); + waitedMs += HUB_LOCK_POLL_MS; + } dyStringFree(&lockPath); return fd; } void unlockHubDir(int fd) /* Release an exclusive hub lock acquired by lockHubDir. Closing the fd * releases the flock automatically on Linux. */ { if (fd >= 0) close(fd); } -boolean literalHubTxtExistsOnDisk(char *parentDir, char *userDataDir) -/* Return TRUE if path/hub.txt is a real file on disk. Used by pre-finish to - * decide between synthesize-fresh vs upgrade-in-place. */ -{ -if (!parentDir || !parentDir[0]) return FALSE; -char *hubDir = hubPathFromParentDir(parentDir, userDataDir); -struct dyString *hubFileDy = dyStringCreate("%s%shub.txt", - hubDir, endsWith(hubDir, "/") ? "" : "/"); -char *hubFile = dyStringCannibalize(&hubFileDy); -boolean exists = fileExists(hubFile); -freeMem(hubFile); -return exists; -} - void upgradeExistingHubToAssembly(struct hubSpace *rowForFile, char *userDataDir, char *encodedParentDir) /* When a 2bit lands in a hub, add the assembly stanza to hub.txt (if the * backend owns it) and flip every row for this hub to hubType='assemblyHub'. * No-op unless rowForFile is a 2bit. */ { if (!sameOk(rowForFile->fileType, "2bit")) return; char *hubDir = hubPathFromParentDir(rowForFile->parentDir, userDataDir); struct dyString *hubFileDy = dyStringCreate("%s%shub.txt", hubDir, endsWith(hubDir, "/") ? "" : "/"); char *hubFile = dyStringCannibalize(&hubFileDy); upgradeHubTxtForAssembly(hubFile, rowForFile->db, rowForFile->location); -char *hubNameOnly = encodedParentDir ? hubNameFromPath(encodedParentDir) : NULL; +// setAssemblyHubTypeForDir looks up the hub's row at the top level, so give it the +// hub component of parentDir rather than a nested subdirectory +char *hubNameOnly = encodedParentDir ? hubRootFromParentDir(encodedParentDir) : NULL; if (hubNameOnly && hubNameOnly[0]) + { setAssemblyHubTypeForDir(rowForFile->userName, hubNameOnly); + // hub.txt just changed on disk, so the row's size and md5 are out of date + refreshHubTextRow(rowForFile->userName, hubFile, hubNameOnly); + } freeMem(hubFile); } char *writeHubText(char *path, char *userName, char *db, char *twoBitFileName) /* Create a hub.txt file, optionally creating the directory holding it. * If twoBitFileName is non-NULL, write an assembly hub stanza referencing it * (with stub organism / scientificName / description / defaultPos derived from * the 2bit). For convenience, return the file name of the created hub, which * can be freed. */ { +// an empty path would put the hub.txt at the root of the filesystem +if (isEmpty(path)) + errAbort("no directory given for the hub, cannot create hub.txt"); int oldUmask = 00; oldUmask = umask(0); makeDirsOnPath(path); // restore umask umask(oldUmask); // now make the hub.txt with some basic information char *hubFile = NULL; struct dyString *hubFileDy = dyStringCreate("%s%shub.txt", path, endsWith(path, "/") ? "" : "/"); hubFile = dyStringCannibalize(&hubFileDy); if (fileExists(hubFile)) return hubFile; char *hubName = hubNameFromPath(path); FILE *f = mustOpen(hubFile, "w"); fprintf(f, "hub %s\n" "email %s\n" "shortLabel %s\n" "longLabel %s\n" "useOneFile on\n" "\n" "genome %s\n", hubName, emailForUserName(userName), hubName, hubName, db); if (twoBitFileName) { // Assembly hub: write twoBitPath plus stub fields the user can edit later. // The bigDataUrl/twoBitPath is relative to the hub.txt location. - char *twoBitBase = strrchr(twoBitFileName, '/'); - twoBitBase = twoBitBase ? twoBitBase + 1 : twoBitFileName; + char *twoBitBase = pathRelativeToHubDir(twoBitFileName, path); char *defaultPos = defaultPosFromTwoBit(twoBitFileName); fprintf(f, "twoBitPath %s\n" "organism %s\n" "scientificName %s\n" "description %s\n" "defaultPos %s\n", twoBitBase, db, db, db, defaultPos ? defaultPos : "chr1:1-1000"); freez(&defaultPos); } fprintf(f, "\n"); carefulClose(&f); return hubFile; } -static boolean bigDataUrlExistsInHub(char *hubFileName, char *fileName) -/* Check if a bigDataUrl line already references this file in the hub.txt. +static boolean trackExistsInHub(char *hubFileName, char *track, char *bigDataUrl) +/* Check if this track is already in the hub.txt, either by name or by the file it + * points at. Track names have to be unique within a hub, so that is the key that + * matters, but a user's own hub.txt may reference the file under a different name. * Simple line-by-line check - not a full trackDb parser. */ { -if (!hubFileName || !fileName) +if (!hubFileName) return FALSE; struct lineFile *lf = lineFileMayOpen(hubFileName, TRUE); if (!lf) return FALSE; +boolean found = FALSE; char *line; -while (lineFileNext(lf, &line, NULL)) +while (!found && lineFileNext(lf, &line, NULL)) { char *trimmedLine = skipLeadingSpaces(line); - if (startsWith("bigDataUrl ", trimmedLine)) - { - char *url = trimmedLine + 11; // skip "bigDataUrl " - url = skipLeadingSpaces(url); - if (isEmpty(url)) - continue; - // Check if the URL ends with this filename (handles relative paths) - if (endsWith(url, fileName) || sameString(url, fileName)) + if (track && startsWith("track ", trimmedLine)) { - lineFileClose(&lf); - return TRUE; + char *name = trimSpaces(skipLeadingSpaces(trimmedLine + 6)); + found = sameString(name, track); } + else if (bigDataUrl && startsWith("bigDataUrl ", trimmedLine)) + { + char *url = trimSpaces(skipLeadingSpaces(trimmedLine + 11)); + // compare whole paths: a bare 'track.bb' is a different file from + // 'hg38/track.bb', and 'mydata.bb' is not 'data.bb' + if (startsWith("./", url)) + url += 2; + found = sameString(url, bigDataUrl); } } lineFileClose(&lf); -return FALSE; +return found; } static void writeTrackStanza(char *hubFileName, char *track, char *bigDataUrl, char *type, char *label, char *bigFileLocation) { if ( (sameString(type, "bamIndex") || sameString(type, "tabixIndex") || sameString(type, "text")) ) // don't need to make track stanzas for these supporting files return; // Skip if this file is already referenced in hub.txt (e.g., user uploaded their own hub.txt) -if (bigDataUrlExistsInHub(hubFileName, bigDataUrl)) +if (trackExistsInHub(hubFileName, track, bigDataUrl)) return; FILE *f = mustOpen(hubFileName, "a"); // Always add a leading newline to ensure separation from previous content fprintf(f, "\n"); char *trackDbType = type; if (sameString(type, "bigBed")) { // don't errAbort if the file is actually not a bigBed struct errCatch *errCatch = errCatchNew(); if (errCatchStart(errCatch)) { // figure out the type based on the bbiFile header struct bbiFile *bbi = bigBedFileOpen(bigFileLocation); char tdbType[32]; @@ -679,60 +764,65 @@ errCatchFree(&errCatch); // NOTE: if the file was not actually a bigBed (and so bigBedFileOpen errAborted), we // just want to prevent the errAbort, not prevent creating the stanza itself, as that // would be majorly confusing to the user, so just continue on here } fprintf(f, "track %s\n" "bigDataUrl %s\n" "type %s\n" "shortLabel %s\n" "longLabel %s\n" "\n", track, bigDataUrl, trackDbType, label, label); carefulClose(&f); } -static char *writeHubStanzasForFile(struct hubSpace *rowForFile, char *userDataDir, char *parentDir) +static char *writeHubStanzasForFile(struct hubSpace *rowForFile, char *userDataDir) /* Create a hub.txt (if necessary) and add track stanzas for the file described by rowForFile. * If the file is a 2bit, write the assembly-hub genome stanza instead of a track stanza. * Returns the path to the hub.txt */ { char *hubFileName = NULL; char *hubDir = hubPathFromParentDir(rowForFile->parentDir, userDataDir); boolean isAssemblyHub = sameOk(rowForFile->fileType, "2bit"); char *twoBitForHubText = isAssemblyHub ? rowForFile->location : NULL; hubFileName = writeHubText(hubDir, rowForFile->userName, rowForFile->db, twoBitForHubText); if (!isAssemblyHub) { // NOTE: even though rowForFile->fileName was already cgiEncoded by the pre-finish hook, // we still must cgiEncode again to make the bigDataUrl setting work, as apache needs // to look for a literal '%' in a filename if there was a character encoded. For example, // if the filename from tus was &.bb, tus encodes this to "\u0026.bb", which we write to // disk as %5Cu0026.bb, and apache needs to find at: // https://url/hash/userName/%25Cu0026.bb in order to work in hgTracks - writeTrackStanza(hubFileName, rowForFile->fileName, cgiEncodeFull(rowForFile->fileName), rowForFile->fileType, rowForFile->fileName, rowForFile->location); + // The subdirectory part needs no such encoding: isValidParentDir limits those + // components to letters, digits, '.' and '_'. + char *subPath = hubSubPathFromParentDir(rowForFile->parentDir); + struct dyString *bigDataUrl = dyStringCreate("%s%s", subPath, cgiEncodeFull(rowForFile->fileName)); + writeTrackStanza(hubFileName, rowForFile->fileName, dyStringCannibalize(&bigDataUrl), rowForFile->fileType, rowForFile->fileName, rowForFile->location); + freeMem(subPath); } return hubFileName; } -void createNewTempHubForUpload(char *requestId, struct hubSpace *rowForFile, char *userDataDir, char *parentDir) +void createNewTempHubForUpload(char *requestId, struct hubSpace *rowForFile, char *userDataDir) /* Creates a hub.txt for this upload, and updates the hubSpace table for the * hub.txt and any parentDirs we need to create. */ { // first create the hub.txt if necessary and write the stanza for this track -char *hubPath = writeHubStanzasForFile(rowForFile, userDataDir, parentDir); +char *hubPath = writeHubStanzasForFile(rowForFile, userDataDir); // update the mysql table with a record of the hub.txt: struct hubSpace *hubTextRow = NULL; AllocVar(hubTextRow); hubTextRow->userName = rowForFile->userName; hubTextRow->fileName = "hub.txt"; hubTextRow->fileSize = fileSize(hubPath); hubTextRow->fileType = "hub.txt"; hubTextRow->creationTime = NULL; time_t lastModTime = fileModTime(hubPath); hubTextRow->lastModified = sqlUnixTimeToDate(&lastModTime, TRUE); hubTextRow->db = rowForFile->db; hubTextRow->location = hubPath; hubTextRow->md5sum = md5HexForFile(hubPath); hubTextRow->parentDir = hubNameFromPath(hubPath);