11d5b26d9798079ec5adedc103216ce818e2da5e max Thu Sep 10 05:19:27 2026 -0700 Widen the session data directory hash from 8 to 10 hex characters sessionDirFromNames() named a session's durable data directory with 8 hex characters of md5(sessionName). 32 bits was fine while a directory only had to be unique among one user's sessions, but every anonymous "Share a link" session belongs to the single reserved user "l", which makes it a birthday problem across all of them: two unrelated sessions land in the same directory more likely than not at around 77,000 anonymous sessions, and at 500,000 we would expect about 29 such pairs. Two sessions sharing a directory means cleaning up one takes the other's custom track files with it. sessionDirHashLen is now 10 (40 bits), which moves the even-odds point past a million sessions. The two fan-out levels snapshotSessionDir() added for user "l" do not help here, since they are a prefix of the same hash: they spread the entries over 65536 directories but leave the number of distinct leaf names unchanged. Both directory layouts change name as a result, so snapshotCleaner would have walked past anything written earlier and orphaned its files. Both dir-naming functions grew a hashLen argument, sessionDirHashLenLegacy records the old value, and snapshotCleanAnon() now tries the old spelling of both layouts as well as the new one. Existing sessions keep working either way: the cart stores the absolute path of each durable file, so nothing looks a session's directory up by name except the cleaner. refs #10138 diff --git src/hg/lib/snapshotSession.c src/hg/lib/snapshotSession.c index 007a24897be..d7cec8668dd 100644 --- src/hg/lib/snapshotSession.c +++ src/hg/lib/snapshotSession.c @@ -47,56 +47,70 @@ boolean snapshotHasRequired(struct snapshotType *type, struct cart *cart) /* Return FALSE when type declares a requiredVar that is missing/empty in cart. */ { if (type == NULL || isEmpty(type->requiredVar)) return TRUE; return isNotEmpty(cartOptionalString(cart, type->requiredVar)); } boolean snapshotIsSnapshotName(char *sessionName) /* Return TRUE if sessionName is a snapshot name (starts with the "__" prefix). */ { return sessionName != NULL && startsWith(snapshotNamePrefix, sessionName); } -static char *snapshotSessionDir(char *sessionDataDir, char *encUserName, char *encSessionName) +static char *snapshotSessionDirHashLen(char *sessionDataDir, char *encUserName, + char *encSessionName, int hashLen) /* Alloc and return the durable data directory for one snapshot, or NULL if sessionDataDir is empty. * Like sessionData's sessionDirFromNames but with two extra hash levels drawn from the session name, * so a single high-volume user - the anonymous "l", which owns most snapshots - never accumulates * millions of entries in one directory. Layout: - * sessionDataDir / <2hex md5(user)> / / <2hex sess> / <2hex sess> / <8hex md5(session)> + * sessionDataDir / <2hex md5(user)> / / <2hex sess> / <2hex sess> / + * The two fan-out levels are a prefix of the same hash, so they spread the entries out without + * adding uniqueness: it is hashLen alone that decides how likely two snapshots are to land in the + * same directory, which is why it is sessionDirHashLen and not 8. hashLen is passed in only so + * that the cleaner can also name a directory written before it was widened. * Snapshots use their own layout (not sessionDirFromNames), so this never affects normal sessions. */ { if (isEmpty(sessionDataDir)) return NULL; if (sessionDataDir[0] != '/') errAbort("config setting sessionDataDir must be an absolute path (starting with '/')"); +if (hashLen < 4 || hashLen > 32) + errAbort("snapshotSessionDirHashLen: hashLen must be in [4,32], got %d", hashLen); char *userHash = md5HexForString(encUserName); char *sessHash = md5HexForString(encSessionName); char fan1[3], fan2[3]; safencpy(fan1, sizeof fan1, sessHash, 2); /* first 2 hex of the session hash */ safencpy(fan2, sizeof fan2, sessHash + 2, 2); /* next 2 hex -> 65536 buckets total */ userHash[2] = '\0'; -sessHash[8] = '\0'; +sessHash[hashLen] = '\0'; struct dyString *dy = dyStringCreate("%s/%s/%s/%s/%s/%s", sessionDataDir, userHash, encUserName, fan1, fan2, sessHash); freeMem(userHash); freeMem(sessHash); return dyStringCannibalize(&dy); } +static char *snapshotSessionDir(char *sessionDataDir, char *encUserName, char *encSessionName) +/* Alloc and return the durable data directory for one snapshot, or NULL if sessionDataDir is + * empty. */ +{ +return snapshotSessionDirHashLen(sessionDataDir, encUserName, encSessionName, sessionDirHashLen); +} + char *snapshotNewName(struct sqlConnection *conn, char *encUserName) /* See snapshotSession.h. */ { char query[512]; int tries; for (tries = 0; tries < 100; tries++) { char *tok = makeRandomKey(snapshotTokenBits); char *name = catTwoStrings(snapshotNamePrefix, tok); freeMem(tok); sqlSafef(query, sizeof query, "select count(*) from %s where userName='%s' and sessionName='%s'", namedSessionTable, encUserName, name); if (sqlQuickNum(conn, query) == 0) return name; @@ -259,33 +273,42 @@ struct slName *toClean = NULL; struct sqlResult *sr = sqlGetResult(conn, query); char **row; while ((row = sqlNextRow(sr)) != NULL) slNameAddHead(&toClean, row[0]); sqlFreeResult(&sr); int n = 0; struct slName *s; for (s = toClean; s != NULL; s = s->next) { if (!dryRun) { /* Remove the durable files first, then the row, so a crash never orphans the DB pointer. * Minimal snapshots live under the fanned-out snapshotSessionDir; a full anonymous share - * (e.g. the top-right "Share a link" when logged out) uses sessionData's flat layout - remove - * whichever exists (removeDirTree ignores a missing path). */ - char *snapDir = snapshotSessionDir(sessionDataDir, snapshotAnonUser, s->name); - char *flatDir = sessionDirFromNames(sessionDataDir, snapshotAnonUser, s->name); - removeDirTree(snapDir); - removeDirTree(flatDir); - freez(&snapDir); - freez(&flatDir); + * (e.g. the top-right "Share a link" when logged out) uses sessionData's flat layout. Both + * layouts changed name when sessionDirHashLen was widened from 8, so try the old spelling + * too - otherwise a snapshot saved before the change would leave its files behind forever. + * Remove whichever exist (removeDirTree ignores a missing path). */ + char *dirs[4]; + dirs[0] = snapshotSessionDir(sessionDataDir, snapshotAnonUser, s->name); + dirs[1] = sessionDirFromNames(sessionDataDir, snapshotAnonUser, s->name); + dirs[2] = snapshotSessionDirHashLen(sessionDataDir, snapshotAnonUser, s->name, + sessionDirHashLenLegacy); + dirs[3] = sessionDirFromNamesHashLen(sessionDataDir, snapshotAnonUser, s->name, + sessionDirHashLenLegacy); + int i; + for (i = 0; i < ArraySize(dirs); i++) + { + removeDirTree(dirs[i]); + freez(&dirs[i]); + } sqlSafef(query, sizeof query, "DELETE FROM %s WHERE userName='%s' AND sessionName='%s'", namedSessionTable, snapshotAnonUser, s->name); sqlUpdate(conn, query); } n++; } slFreeList(&toClean); return n; }