c62c5810fffbf329f4a2555d72de0b56fb4752a4
max
  Thu Sep 17 06:37:59 2026 -0700
pyLib: align the python bottleneck code with hg/lib/botDelay.c, refs #38369

#Preview2 week - bugs introduced now will need a build patch to fix
hgGeneGraph is the only CGI using this library. Its bottleneck handling had
drifted a long way from the C implementation, so bring it back in line:

- check the hguid cookie against the userDb table before using it as the
bottleneck key, and fall back to the hgsid and then the address, as
getBotCheckString() in botDelay.c does. Honours newBotDelay.
- port recordHguidIpAndMaybeForceCaptcha() and its hguidIpTracking.* settings,
so the same hg.conf keys configure the C and the python side.
- run the bottleneck before deciding whether to serve the request, matching the
order in earlyBotCheck(). Skip the whole thing for bottleneck.except.
- when the tracking trips, redirect to hgTracks, which runs the same check and
can put up the Cloudflare challenge itself.

New helpers, all ports of their C namesakes: sqlUpdate(), sqlQuickNum(),
cfgOptionBooleanDefault(), cfgOptionEnvDefault(), userDbTable(),
sessionDbTable(), cartDbParseId(), cartDbHasSessionKey(), isValidHguid(),
isValidHgsidForEarlyBotCheck(), botException(), cgiWasSpoofed() and
hConnectCentralNoCache(). pymysql does not autocommit, so sqlUpdate() commits.

Also in passing:
- sqlTableExists() was missing its return and so was always false.
- makeRandomKey() used float division and produced a 32 character key with
base64 padding where the C version produces 28 characters.
- cartDbLoadFromId() called cgi.escape(), gone from python since 3.8.
- cfgOptionBoolean() read the config dict without the parse guard cfgOption has.
- a two statement string literal in hgBotDelay() discarded half its message.

diff --git src/hg/pyLib/hgLib3.py src/hg/pyLib/hgLib3.py
index 8c84468a815..2e2d0546453 100644
--- src/hg/pyLib/hgLib3.py
+++ src/hg/pyLib/hgLib3.py
@@ -1,27 +1,27 @@
 # Library functions for genome browser CGI scripts written in Python 3
 
 # Because this library is loaded for every CGI execution, only a
 # fairly minimal set of functions is implemented here, e.g. hg.conf parsing,
 # bottleneck, cart loading, mysql queries.
 
 # The cart is currently read-only. More work is needed to allow writing a cart.
 
 # General rules for CGI in Python:
 # - never insert values into SQL queries. Write %s in the query and provide the
 #   arguments to sqlQuery as a list.  
-# - never print incoming HTTP argument as raw text. Run it through cgi.escape to 
+# - never print incoming HTTP argument as raw text. Run it through html.escape to 
 #   destroy javascript code in them.
 
 try:
     import pymysql.cursors
 except:
     print("Installation error - could not load pymysql for Python. Please tell your system administrator to run " \
         "one of these commands as root: 'pip install pymysql'. Sometimes, pip is called 'pip3'.")
     exit(0)
 
 # Imports from the Python 3 standard library
 # This is a CGI, not a WSGI - minimize global imports. Each library import can take up to 20msecs.
 import os, cgi, sys, logging, time
 
 from os.path import join, isfile, normpath, abspath, dirname, isdir, splitext
 from collections import namedtuple
@@ -38,30 +38,35 @@
 # debug level: a number. the higher, the more debug info is printed
 # to see most debug messages, set to 1
 # another way to change this variable is by setting the URL variable "debug" to 1
 verboseLevel = 0
 
 cgiArgs = None
 
 # like in the kent tree, we keep track of whether we have already output the content-type line
 contentLineDone = False
 
 # show the bot delay warning message before other printing is done?
 doWarnBot = False
 # current bot delay in milliseconds
 botDelayMsecs = 0
 
+# set when one hguid cookie has been seen from too many IP addresses, see
+# recordHguidIpAndMaybeForceCaptcha(). This is the python side of the "captcha" CGI variable
+# that lib/botDelay.c sets for lib/cart.c:forceUserIdOrCaptcha()
+forceCaptcha = False
+
 # two global variables: the first is the botDelay limit after which the page is slowed down and a warning is shown
 # the second is the limit after which the page is not shown anymore
 botDelayWarn = 1500
 botDelayBlock = 3000
 
 jksqlTrace = False
 
 forceUnicode = False
 
 def warn(format, *args):
     print (format % args)
 
 def errAbort(msg, status=None, headers = None):
     " show msg and abort. Like errAbort.c "
     printContentType(status=status, headers=headers)
@@ -114,50 +119,74 @@
         jksqlTrace = True
     
     return hgConf
 
 def cfgOption(name, default=None):
     " return hg.conf option or default "
     global hgConf
 
     if not hgConf:
         parseHgConf()
 
     return hgConf.get(name, default)
 
 def cfgOptionBoolean(name, default=False):
     " return True if option is set to 1, on or true, or default if not set "
-    val = hgConf.get(name, default) in [True, "on", "1", "true"]
+    val = cfgOption(name, default) in [True, "on", "1", "true"]
+    return val
+
+def cfgOptionBooleanDefault(name, default=False):
+    """ like hg/lib/hgConfig.c:cfgOptionBooleanDefault: return the option as a boolean,
+    or 'default' when the option is not set at all. Unlike cfgOptionBoolean(), a default
+    of True stays True only as long as hg.conf does not say otherwise. """
+    val = cfgOption(name)
+    if val is None:
+        return default
+    return val in [True, "on", "1", "true"]
+
+def cfgOptionEnvDefault(envName, name, default=None):
+    " like hg/lib/hgConfig.c:cfgOptionEnvDefault: environment wins over hg.conf "
+    val = os.environ.get(envName)
+    if val is not None:
         return val
+    return cfgOption(name, default)
+
+def userDbTable():
+    " port of lib/cartDb.c:userDbTable "
+    return cfgOptionEnvDefault("HGDB_USERDBTABLE", "userDbName", "userDb")
+
+def sessionDbTable():
+    " port of lib/cartDb.c:sessionDbTable "
+    return cfgOptionEnvDefault("HGDB_SESSIONDBTABLE", "sessionDbName", "sessionDb")
 
 def sqlConnect(db, host=None, user=None, passwd=None):
     """ connect to sql server specified in hg.conf with given db. Like jksql.c. """
     cfg = parseHgConf()
     if host==None:
         host, user, passwd = cfg["db.host"], cfg["db.user"], cfg["db.password"]
     conn = pymysql.connect(host=host, user=user, passwd=passwd, db=db, charset="utf8")
 
     # we will need this info later
     conn.failoverConn = None
     conn.db = db
     conn.host = host
     return conn
 
 def sqlTableExists(conn, table):
     " return True if table exists. Like jksql.c "
     query = "SHOW TABLES LIKE %s"
-    sqlQueryExists(conn, query, table)
+    return sqlQueryExists(conn, query, (table,))
 
 def sqlQueryExists(conn, query, args=None):
     " return true if query returns a result. Like jksql.c. No caching for now, unlike hdb.c. "
     cursor = conn.cursor()
     rows = cursor.execute(query, args)
     row = cursor.fetchone()
 
     res = (row!=None)
     cursor.close()
     return res
 
 def _sqlConnectFailover(conn):
     " connect the failover connection of a connection "
     cfg = parseHgConf()
     if "slow-db.host" not in cfg:
@@ -238,30 +267,59 @@
 
     if jksqlTrace:
         timeDiff = _timeDeltaSeconds(datetime.now(), startTime)
         sys.stderr.write("SQL_FETCH 0 %s %s %.3f\n" % (conn.host, conn.db, timeDiff))
 
     colNames = [desc[0] for desc in cursor.description]
     Rec = namedtuple("MysqlRow", colNames)
 
     if forceUnicode:
         data = byteToUnicode(data)
 
     recs = [Rec(*row) for row in data]
 
     return recs
 
+def sqlUpdate(conn, query, args=None):
+    """ Run a query that changes the database and commit it. Like jksql.c:sqlUpdate.
+    pymysql opens its connections with autocommit switched off, so without the commit
+    here the write is rolled back when the CGI exits and nothing is ever stored.
+    """
+    cursor = conn.cursor()
+
+    if jksqlTrace:
+        sys.stderr.write("SQL_UPDATE 0 %s %s %s %s\n" % (conn.host, conn.db, query, args))
+
+    cursor.execute(query, args)
+    conn.commit()
+    cursor.close()
+
+def sqlQuickNum(conn, query, args=None):
+    " return the first field of the first row as an int, 0 if there is no row. Like jksql.c "
+    cursor = conn.cursor()
+
+    if jksqlTrace:
+        sys.stderr.write("SQL_QUERY 0 %s %s %s %s\n" % (conn.host, conn.db, query, args))
+
+    cursor.execute(query, args)
+    row = cursor.fetchone()
+    cursor.close()
+
+    if row is None or row[0] is None:
+        return 0
+    return int(row[0])
+
 def htmlPageEnd(oldJquery=False):
     " close html body/page "
     print("</body>")
     print("</html>")
 
 def printMenuBar(oldJquery=False):
     baseDir = "../"
     " print the menubar. Mostly copied from src/hg/hgMenuBar.c "
 
     print ("<noscript><div class='noscript'><div class='noscript-inner'><p><b>JavaScript is disabled in your web browser</b></p>")
     print ("<p>You must have JavaScript enabled in your web browser to use the Genome Browser</p></div></div></noscript>\n")
 
     menuPath = "../htdocs/inc/globalNavBar.inc"
     navBarStr = open(menuPath, "r").read()
     print (navBarStr)
@@ -364,31 +422,33 @@
     if ret!=0 and mustRun:
         errAbort("Could not run command %s" % cmd)
     return ret
 
 def printContentType(contType="text/html", status=None, fname=None, headers=None):
     """
     print the HTTP Content-type header line with an optional file name for downloads.
     Also optionally prints the bot delay note. The argument 'status' must be an int.
     """
     global contentLineDone
     if not contentLineDone:
         contentLineDone = True
         print("Content-type: %s; charset=utf-8" % contType)
 
         if status:
-            if status==400:
+            if status==302:
+                print("Status: 302 Found")
+            elif status==400:
                 print("Status: 400 Bad Request")
             elif status==429:
                 print("Status: 429 Too Many Requests")
             else:
                 raise Exception("Unknown status code, please update hgLib.py")
 
         if fname is not None:
             print("Content-Disposition: attachment; filename=%s" % fname)
 
         if headers:
             for key, val in headers.items():
                 print("%s: %s" % (key, val))
 
         print()  # this newline is essential, it means: end of header lines
 
@@ -445,81 +505,266 @@
 
     return None
 
 def getCookieUser():
     " port of lib/botDelay.c:getCookieUser: get hguid cookie value  "
     user = None
     centralCookie = cfgOption("central.cookie", default="hguid")
 
     if centralCookie:
         user = findCookieData(centralCookie)
 
     return user
 
 def showCookieError():
     " output error message if cookie not found "
-    print("Content-type: text/html\n\n")
+    printContentType()
     print("<html><body>")
     print("Sorry, the gene interactions viewer requires that you visit the genome browser first once, to defend against bots. ")
     print("<a href='hgTracks'>Click here</a> to visit the genome browser, then come back to this page.")
     print("</body></html>")
     sys.exit(0)
 
+def cartDbParseId(cartId):
+    """ split a cart identifier of the form 12345_sessionKey into (12345, "sessionKey").
+    Port of lib/cartDb.c:cartDbParseId. Unlike the C version, which runs the id through
+    sqlUnsignedLong() and aborts on anything that is not a number, a value we cannot parse
+    returns (None, None) here: this is called on a cookie an attacker picked, so a bad
+    value has to be an ordinary negative answer and not an error page.
+    """
+    if not cartId:
+        return None, None
+
+    idStr, _, sessionKey = cartId.partition("_")
+    # userDb.id is a bigint unsigned, so it never has more than 20 digits. The length check
+    # is not cosmetic: python 3.11 and later refuse to convert a very long digit string and
+    # would raise instead of returning an answer.
+    if not idStr.isdigit() or len(idStr) > 20:
+        return None, None
+
+    if sessionKey=="":
+        sessionKey = None
+
+    return int(idStr), sessionKey
+
+# cache for cartDbHasSessionKey(), which is a static in the C code
+userDbHasSessionKey = None
+
+def cartDbHasSessionKey(conn, table):
+    " return True if the table has a sessionKey column. Port of lib/cartDb.c:cartDbHasSessionKey "
+    global userDbHasSessionKey
+
+    if userDbHasSessionKey is None:
+        query = "SHOW COLUMNS FROM "+table+" LIKE 'sessionKey'"
+        userDbHasSessionKey = sqlQueryExists(conn, query)
+
+    return userDbHasSessionKey
+
+# cache for isValidHguid(), so the cookie is not looked up twice per request
+validHguidCache = {}
+
+def isValidHguid(cookieUserId):
+    """ Check that the hguid cookie really names a row in the userDb table, port of
+    lib/botDelay.c:isValidHguid.
+
+    This is the check that the python code was missing. Anyone can send any string as a
+    cookie, so without it the bottleneck server counts requests against a name the caller
+    invented, and a fresh name gets a fresh allowance.
+    """
+    if not cookieUserId:
+        return False
+
+    if cookieUserId in validHguidCache:
+        return validHguidCache[cookieUserId]
+
+    userId, sessionKey = cartDbParseId(cookieUserId)
+    if userId is None:
+        validHguidCache[cookieUserId] = False
+        return False
+
+    table = userDbTable()
+    conn = hConnectCentralNoCache()
+    try:
+        if sessionKey is None:
+            # old mirrors have no sessionKey column and their cookies are a bare number.
+            # Where the column does exist, a cookie without a key is not acceptable: the ids
+            # are sequential, so it could simply be guessed.
+            if cartDbHasSessionKey(conn, table):
+                isValid = False
+            else:
+                query = "SELECT id FROM "+table+" WHERE id=%(id)s"
+                isValid = sqlQueryExists(conn, query, {"id":userId})
+        else:
+            query = "SELECT id FROM "+table+" WHERE id=%(id)s AND sessionKey=%(sessionKey)s"
+            isValid = sqlQueryExists(conn, query, {"id":userId, "sessionKey":sessionKey})
+    finally:
+        conn.close()
+
+    validHguidCache[cookieUserId] = isValid
+    return isValid
+
+def isValidHgsidForEarlyBotCheck(rawHgsid):
+    """ port of lib/botDelay.c:isValidHgsidForEarlyBotCheck. We only check the shape of the
+    string here, not the database - that happens later when the cart is loaded.
+    """
+    import re
+    # just in case it is egregiously large, only the first part is needed to decide
+    hgsid = rawHgsid[:49]
+    # \Z, not $: python's $ also matches just before a trailing newline, the POSIX regex the
+    # C code uses does not, and we want the same answer on both sides
+    return re.match(r"^[0-9][0-9]*_[a-zA-Z0-9]{28}\Z", hgsid) is not None
+
+def botException():
+    " return True if the client address is on the bottleneck.except list. Port of lib/botDelay.c "
+    exceptIps = cfgOption("bottleneck.except")
+    if not exceptIps:
+        return False
+
+    remoteAddr = os.environ.get("REMOTE_ADDR")
+    if not remoteAddr:
+        return False
+
+    return remoteAddr in exceptIps.split()
+
+def recordHguidIpAndMaybeForceCaptcha():
+    """ port of lib/botDelay.c:recordHguidIpAndMaybeForceCaptcha.
+
+    When hguidIpTracking is switched on in hg.conf, note this request's (hguid, address)
+    pair in the hgcentral tracking table. If one hguid has been seen from more than
+    hguidIpTracking.maxIps different addresses within hguidIpTracking.windowSeconds, set
+    the module-level forceCaptcha flag. The C code sets a "captcha" CGI variable at this
+    point instead, which lib/cart.c:forceUserIdOrCaptcha() then acts on.
+
+    This is what catches a cookie that has been copied around a botnet: the bottleneck
+    server can only count requests, and traffic spread thinly over hundreds of thousands
+    of addresses never trips a per-address or per-cookie limit.
+    """
+    global forceCaptcha
+
+    if not cfgOptionBooleanDefault("hguidIpTracking.enabled", False):
+        return
+
+    cookieUserId = getCookieUser()
+    clientIp = os.environ.get("REMOTE_ADDR")
+    if not cookieUserId or not clientIp:
+        return
+
+    if not isValidHguid(cookieUserId):
+        return
+
+    userId, _ = cartDbParseId(cookieUserId)
+
+    maxIps = int(cfgOption("hguidIpTracking.maxIps", "10"))
+    windowSeconds = int(cfgOption("hguidIpTracking.windowSeconds", "600"))
+    table = cfgOption("hguidIpTracking.table", "hguidIpAccess")
+
+    conn = hConnectCentralNoCache()
+    try:
+        query = "INSERT INTO "+table+" (userId, ip, lastSeen) VALUES (%(userId)s, %(ip)s, NOW()) " \
+                "ON DUPLICATE KEY UPDATE lastSeen=NOW()"
+        sqlUpdate(conn, query, {"userId":userId, "ip":clientIp})
+
+        query = "SELECT COUNT(DISTINCT ip) FROM "+table+" WHERE userId=%(userId)s " \
+                "AND lastSeen > NOW() - INTERVAL %(windowSeconds)s SECOND"
+        distinctIps = sqlQuickNum(conn, query, {"userId":userId, "windowSeconds":windowSeconds})
+    finally:
+        conn.close()
+
+    if distinctIps > maxIps:
+        forceCaptcha = True
+
+def sendToBrowserForCaptcha():
+    """ This hguid is in use from too many addresses at once. Send the caller to hgTracks,
+    which is a C CGI and runs the same check, so it will put up the Cloudflare challenge
+    itself. We cannot show the challenge here: checking the token that comes back needs the
+    Cloudflare secret from hg.conf, and there is no reason to hand that to a python CGI when
+    the C code a redirect away already does it.
+    """
+    sys.stderr.write("hgLib.py captchaRedirect\n")
+    printContentType(status=302, headers={"Location" : "hgTracks"})
+    print("<html><body>")
+    print("Please visit the <a href='hgTracks'>genome browser</a> first, then come back to this page.")
+    print("</body></html>")
+    sys.exit(0)
+
 def getBotCheckString(ip, fraction):
-    " port of lib/botDelay.c:getBotCheckString: compose user.ip fraction for bot check  "
-    userId = getCookieUser()
+    """ port of lib/botDelay.c:getBotCheckString: compose the string that the bottleneck
+    server counts against, "<key> <fraction>".
 
-    if not userId:
-        showCookieError()
+    Like the C code, prefer a validated hguid cookie, then an hgsid that at least has the
+    right shape, and only fall back to the address when there is neither.
+    """
+    if not cfgOptionBooleanDefault("newBotDelay", True):
+        # the old system, only relevant on mirrors: bottleneck on cookie or address
+        cookieUserId = getCookieUser()
+        if cookieUserId:
+            return "%s.%s %f" % (cookieUserId, ip, fraction)
+        return "%s %f" % (ip, fraction)
+
+    cookieUserId = getCookieUser()
+    if isValidHguid(cookieUserId):
+        return "uid%s %f" % (cookieUserId, fraction)
+
+    # this happens on sites that use the Cloudflare challenge only for a caller that has
+    # never been to the browser, or one that made its cookie up
+    hgsid = cgiString("hgsid")
+    if hgsid and isValidHgsidForEarlyBotCheck(hgsid):
+        return "sid%s %f" % (hgsid, fraction)
 
-    botCheckString = "uid%s %f" % (userId, fraction)
+    if hgsid:
+        # we were given an invalid hgsid - penalize this source in case of abuse.
+        # As in the C code this only changes the string sent to the bottleneck server,
+        # not the delay thresholds the caller applies.
+        fraction *= 5
 
-    return botCheckString
+    return "%s %f" % (ip, fraction)
 
 def hgBotDelay(fraction=1.0, useBytes=None, botCheckString=None):
     """
     Implement bottleneck delay, get bottleneck server from hg.conf.
     This behaves similar to the function src/hg/lib/botDelay.c:hgBotDelay
-    It does not use the hgsid, currently it always uses the IP address.
-    Using the hgsid makes little sense. It is more lenient than the C version.
 
     If useBytes is set, use only the first x bytes of the IP address. This helps
     block bots that all use similar IP addresses, at the risk of blocking
     entire institutes.
     """
     global hgConf
     global doWarnBot
     global botDelayMsecs
 
     ip = os.environ.get("REMOTE_ADDR")
     if not ip: # skip if not called from Apache
         return
+
+    if botException(): # our own QA scripts and anyone else on the exception list
+        return
+
     if useBytes is not None and ip.count(".")==3: # do not do this for ip6 addresses
         ip = ".".join(ip.split(".")[:useBytes])
 
     host = cfgOption("bottleneck.host")
     port = cfgOption("bottleneck.port")
 
-    if not host or not port or not ip:
+    if not host or not port:
         return
 
     warnMsg = None
     if botCheckString is None:
         botCheckString = getBotCheckString(ip, fraction)
     else:
-        warnMsg = "Too many parallel requests for this CGI program. Please wait for a while and try this page again. If the problem persists, "
-        "please email us at genome@soe.ucsc.edu."
+        warnMsg = "Too many parallel requests for this CGI program. Please wait for a while and " \
+            "try this page again. If the problem persists, please email us at genome@soe.ucsc.edu."
 
     millis = botDelayTime(host, port, botCheckString)
     debug(1, "Bottleneck delay: %d msecs" % millis)
     botDelayMsecs = millis
 
     if millis > (botDelayBlock/fraction):
         # retry-after time factor 10 is based on the example in the bottleneck help message
         sys.stderr.write("hgLib.py hogExit\n")
         printContentType(status=429, headers={"Retry-after" : str(millis / 200)})
         print("<html><head></head><body>")
         if warnMsg:
             print(warnMsg)
             print(millis)
             print(botDelayBlock)
             print(fraction)
@@ -685,35 +930,47 @@
         fh = gzip.open(fname)
     else:
         fh = open(fname)
 
     lines = fh.read().splitlines()
     return lines
 
 def cgiString(name, default=None):
     " get named cgi variable as a string, like lib/cheapcgi.c "
     val = cgiArgs.getfirst(name, default=default)
     return val
 
 def cgiGetAll():
     return cgiArgs
 
+def cgiWasSpoofed():
+    """ True when this is being run from a command line rather than by the web server, like
+    lib/cheapcgi.c:cgiWasSpoofed. Apache always sets REQUEST_METHOD for a CGI, so this cannot
+    be reached over HTTP. QA and developers run these from a shell, where there is no browser
+    to go and fetch a cookie with.
+    """
+    return "REQUEST_METHOD" not in os.environ
+
 def makeRandomKey(numBits=128+33):
     " copied line-by-line from kent/src/lib/htmlshell.c:makeRandomKey "
     import base64
-    numBytes = (numBits + 7) / 8  # round up to nearest whole byte.
-    numBytes = int(((numBytes+2)/3)*3) # round up to the nearest multiple of 3 to avoid equals-char padding in base64 output
+    # the C code does these two in integer arithmetic. Python's "/" is float division, which
+    # rounded 21 bytes up to 23 and so produced a 32-character key ending in the "=" padding
+    # this is meant to avoid - where C produces 28 characters, which is what the rest of the
+    # code, including the hgsid format check, expects.
+    numBytes = (numBits + 7) // 8  # round up to nearest whole byte.
+    numBytes = ((numBytes+2)//3)*3 # round up to the nearest multiple of 3 to avoid equals-char padding in base64 output
     f = open("/dev/urandom", "rb") # open random system device for read-only access.
     binaryString = f.read(numBytes)
     f.close()
     return base64.b64encode(binaryString, b"Aa").decode("latin1")  # replace + and / with characters that are URL-friendly.
 
 
 # ============ Nonce and CSP functions =============
 
 nonce = None;
 
 def getNonce():
     " make nonce one-use-per-page "
     global nonce
     if nonce:
         return nonce
@@ -1003,121 +1260,149 @@
     jsInlineF("document.getElementById('%s').on%s = function(event) {if (!event) {event=window.event}; %s};\n", idText, eventName, jsText)
 
 def jsOnEventByIdF(eventName, idText, format, *args):
     " Add js mapping for inline event with printf formatting "
     checkValidEvent(eventName)
     jsInlineF("document.getElementById('%s').on%s = function(event) {if (!event) {event=window.event}; ", idText, eventName)
     jsInlineF(format, *args)
     jsInlineF("};\n")
 
 #============ END of javascript inline-separation routines ===============
 
 def cartDbLoadFromId(conn, table, cartId, oldCart):
     " Like src/hg/lib/cart.c, opens cart table and parses cart contents given a cartId of the format 123123_csctac "
     if cartId==None:
         return {}
-    cartFields = cartId.split("_")
-    if len(cartFields)!=2:
-        errAbort("Could not parse identifier %s for cart table %s" % (cgi.escape(cartId), table))
-    idStr, secureId = cartFields
+
+    idStr, secureId = cartDbParseId(cartId)
+    if idStr is None or secureId is None:
+        # an identifier we cannot parse is simply not a valid one. It used to abort here,
+        # which handed anyone sending a malformed cookie an error page of their choosing.
+        return None
 
     query = "SELECT contents FROM "+table+" WHERE id=%(id)s and sessionKey=%(sessionKey)s"
     rows = sqlQuery(conn, query, {"id":idStr, "sessionKey":secureId})
     if len(rows)==0:
         # invalid cart ID
         return None
 
     cartList = urllib.parse.parse_qs(rows[0][0])
 
     # by default, python returns a dict with key -> list of vals. We need only the first one
     for key, vals in cartList.items():
         oldCart[key] =vals[0]
     return oldCart
 
 centralConn = None
 
 def hConnectCentral():
     " similar to src/hg/lib/hdb.c:hConnectCentral. We use a much simpler cache, because we usually read all rows into memory. "
     global centralConn
     if centralConn:
         return centralConn
 
+    centralConn = hConnectCentralNoCache()
+    return centralConn
+
+def hConnectCentralNoCache():
+    """ similar to src/hg/lib/hdb.c:hConnectCentralNoCache: a new connection that the caller
+    closes again. The bot checks run before the bottleneck delay, and a delay can be a sleep of
+    many seconds, so they must not leave a connection sitting on the MariaDB server. Opening one
+    costs less than a millisecond.
+    """
     centralDb = cfgOption("central.db")
     if centralDb is None:
         errAbort("Could not find central.db in hg.conf. Installation error.")
 
     centralUser = cfgOption("central.user")
     if centralUser is None:
         errAbort("Could not find central.user in hg.conf. Installation error.")
 
     centralPwd = cfgOption("central.password")
     if centralPwd is None:
         errAbort("Could not find central.password in hg.conf. Installation error.")
 
     centralHost = cfgOption("central.host")
     if centralHost is None:
         errAbort("Could not find central.host in hg.conf. Installation error.")
 
-    conn = sqlConnect(centralDb, host=centralHost, user=centralUser, passwd=centralPwd)
-
-    centralConn = conn
-    return conn
+    return sqlConnect(centralDb, host=centralHost, user=centralUser, passwd=centralPwd)
 
 def cartNew(conn, table):
     " create a new cart and return ID "
     sessionKey = makeRandomKey()
     sqlQuery(conn, "INSERT %s VALUES(0,'',0,now(),now(),0,%s" % (table, sessionKey));
     sqlLastId = conn.insert_id()
     newId = "%d_%s" % (sqlLastId, sessionKey)
     return newId
 
 def cartAndCookieSimple():
     """ Make the cart from the user cookie (user settings) and the hgsid CGI parameter (session settings, e.g. a browser tab).
         This is somewhat similar to cartAndCookie from hg/lib/cart.c
         Two important differences: this cart does not add all CGI arguments automatically.
         It also does not run cart.c:cartJustify, so track priorities are not applied.
         Also, if there is no hgsid parameter or no cookie, we do not create a new cart.
     """
     # no cgiApoptosis yet - maybe needed in the future. see cart.c / cartNew
 
     hguid = getCookieUser()
     hgsid = cgiString("hgsid")
 
     conn = hConnectCentral()
 
     cart = {}
-    userInfo = cartDbLoadFromId(conn, "userDb", hguid, cart)
+    userInfo = cartDbLoadFromId(conn, userDbTable(), hguid, cart)
     if userInfo is None:
         # invalid cookie hguid
         showCookieError()
 
-    sessionInfo = cartDbLoadFromId(conn, "sessionDb", hgsid, cart)
+    sessionInfo = cartDbLoadFromId(conn, sessionDbTable(), hgsid, cart)
     if sessionInfo is None:
         # tolerate invalid hgsid
         sessionInfo = {}
     return cart
 
 def cartString(cart, default=None):
     " Get a string from the cart. For better readability for programmers used to the C code. "
     return cart.get(cart, default)
 
 def cgiSetup(bottleneckFraction=1.0, useBytes=None, botCheckString=None):
     """ do the usual browser CGI setup: parse the hg.conf file, parse the CGI
     variables, get the cart, do bottleneck delay. Returns the cart.
 
     This is not part of the C code (though it maybe should).
     """
     parseHgConf()
     global cgiArgs
     cgiArgs = cgi.FieldStorage() # Python has built-in cgiSpoof support: sys.argv[1] is the query string if run from the command line
 
     if cgiString("debug"):
         global verboseLevel
         verboseLevel = int(cgiString("debug"))
 
+    # our own QA machines and anyone else on bottleneck.except skip all of this, as they do
+    # in lib/botDelay.c:earlyBotCheck and lib/cart.c:forceUserIdOrCaptcha
+    if not botException():
+        # the order here follows lib/botDelay.c:earlyBotCheck - note the cookie/address pair,
+        # then go through the bottleneck, and only then decide whether to serve the request.
+        # Deciding first, as this used to, meant a caller without a cookie never reached the
+        # bottleneck server at all and so was never counted or slowed down.
+        recordHguidIpAndMaybeForceCaptcha()
+
         hgBotDelay(fraction=bottleneckFraction, useBytes=useBytes, botCheckString=botCheckString)
 
+        if forceCaptcha:
+            sendToBrowserForCaptcha()
+
+        # A caller with no usable cookie has never been to the genome browser. The C CGIs
+        # answer that with the Cloudflare challenge (lib/cart.c:forceUserIdOrCaptcha); we
+        # cannot show it here, so keep the older affordance and send them to the browser to
+        # pick up a cookie. Doing it here rather than inside getBotCheckString(), where it
+        # used to sit, is the point: the bottleneck server has now seen the request.
+        if not cgiWasSpoofed() and not isValidHguid(getCookieUser()):
+            showCookieError()
+
     cart = cartAndCookieSimple()
     return cart
 
 #if __file__=="__main__":
     #pass