998149024f8bde04e8b76638043cfeac3d158731
max
  Tue Sep 15 05:47:27 2026 -0700
Queue the cart cookie and the content policy instead of printing them, refs #38353

cartWriteHeaderAndCont() guards on cgiDidContentType(), which any cgiPrintContentType()
anywhere sets.  An early warn() during cartNew -- "Unable to load session file" reaches the
early warning handler, which calls htmlStart -- prints the header before there is a cart,
and from then on the Set-Cookie and Content-Security-Policy lines were silently skipped.
Nothing changed on the wire, since before the guard they landed in the page body as text
and were equally inert, but skipping them silently is not the behaviour to keep.

cartWriteCookie() and cspWriteResponseHeader() now hand their lines to cgiAddHttpHeader(),
so whichever call prints the content type prints them too and the order of the calls no
longer matters.  cartAndCookieWithHtml() queues the policy before the early handlers are
pushed, so even a page written by that early warn carries one.  The cookie cannot be queued
that early -- there is no cart yet -- and is still lost on that path; the comment says so.

cgiAddHttpHeader() now does what its own comment already promised and ignores a header
added after the block was closed, rather than growing a list nothing will ever print.

getCspPolicyString() is declared in htmshell.h so hCommon.c can queue the value on its own.

diff --git src/hg/lib/cart.c src/hg/lib/cart.c
index f631631b6a1..02a080753d3 100644
--- src/hg/lib/cart.c
+++ src/hg/lib/cart.c
@@ -2857,82 +2857,90 @@
 
 }
 
 void cartResetInDb(char *cookieName)
 /* Clear cart in database. */
 {
 char *hguid = getCookieId(cookieName);
 char *hgsid = getSessionId();
 struct sqlConnection *conn = cartDefaultConnector();
 clearDbContents(conn, userDbTable(), hguid);
 clearDbContents(conn, sessionDbTable(), hgsid);
 cartDefaultDisconnector(&conn);
 }
 
 void cartWriteCookie(struct cart *cart, char *cookieName)
-/* Write out HTTP Set-Cookie statement for cart. */
+/* Queue the HTTP Set-Cookie statement(s) for the cart.  cgiPrintContentType() writes them,
+ * so this has to run before that does but does not have to be the thing that writes them:
+ * a caller that has already closed the header block loses the cookie rather than printing
+ * a Set-Cookie line into the page body, where it never did anything anyway. */
 {
 char *domain = cfgVal("central.domain");
 if (sameWord("HTTPHOST", domain))
     {
     // IE9 does not accept portnames in cookie domains
     char *hostWithPort = hHttpHost();
     struct netParsedUrl npu;
     netParseUrl(hostWithPort, &npu);
     if (strchr(npu.host, '.') != NULL)	// Domains without a . don't seem to be kept
 	domain = cloneString(npu.host);
     else
         domain = NULL;
     }
 
 char userIdKey[256];
 cartDbSecureId(userIdKey, sizeof userIdKey, cart->userInfo);
 // Some users reported blank cookie values. Do we see that here?
 if (sameString(userIdKey,"")) // make sure we do not write any blank cookies.
     {
     // Be sure we do not lose this message.
     // Because the error happens so early we cannot trust that the warn and error handlers
     // are setup correctly and working.
     verbose(1, "unexpected error in cartWriteCookie: userId string is empty.");
     dumpStack( "unexpected error in cartWriteCookie: userId string is empty.");
     warn(      "unexpected error in cartWriteCookie: userId string is empty.");
     }
 else
     {
+    char cookie[1024];
     if (!isEmpty(domain))
-	printf("Set-Cookie: %s=%s; path=/; domain=%s; expires=%s\r\n",
+	safef(cookie, sizeof cookie, "%s=%s; path=/; domain=%s; expires=%s",
 		cookieName, userIdKey, domain, cookieDate());
     else
-	printf("Set-Cookie: %s=%s; path=/; expires=%s\r\n",
+	safef(cookie, sizeof cookie, "%s=%s; path=/; expires=%s",
 		cookieName, userIdKey, cookieDate());
+    cgiAddHttpHeader("Set-Cookie", cookie);
     }
 if (geoMirrorEnabled())
     {
     // This occurs after the user has manually choosen to go back to the original site; we store redirect value into a cookie so we 
     // can use it in subsequent hgGateway requests before loading the user's cart
     char *redirect = cgiOptionalString("redirect");
     if (redirect)
         {
-        printf("Set-Cookie: redirect=%s; path=/; domain=%s; expires=%s\r\n", redirect, cgiServerName(), cookieDate());
+        char cookie[1024];
+        safef(cookie, sizeof cookie, "redirect=%s; path=/; domain=%s; expires=%s",
+                redirect, cgiServerName(), cookieDate());
+        cgiAddHttpHeader("Set-Cookie", cookie);
         }
     }
 /* Validate login cookies if login is enabled */
 if (loginSystemEnabled())
     {
     struct slName *newCookies = loginValidateCookies(cart), *sl;
     for (sl = newCookies;  sl != NULL;  sl = sl->next)
-        printf("Set-Cookie: %s\r\n", sl->name);
+        cgiAddHttpHeader("Set-Cookie", sl->name);
     }
 }
 
 static void cartJsonStart()
 /* Write the necessary headers for Apache */
 {
 cgiPrintContentType("application/json");
 }
 
 static void cartJsonEnd(struct jsonWrite *jw)
 /* Write the final string which may have nothing in it */
 {
 if (jw)
     puts(jw->dy->string);
 }
@@ -3035,49 +3043,59 @@
     exit(0);
     }
 
 // activate optional debuging output for CGIs
 verboseCgi(cgiUsualString("verbose", NULL));
 cartExclude(cart, "verbose");
 
 return cart;
 }
 
 void cartWriteHeaderAndCont(struct cart* cart, char *cookieName, char *contType)
 /* write http headers including cookie and content type line.
  * contType defaults to text/html when NULL.
  * cookieName defaults to hUserCookie() when NULL */
 {
-/* cgiPrintContentType() writes the header only once per process, so the flows that reach here
- * twice (e.g. hgc emitting it early via cartAndCookieWithHtml, then webStart asking again) do
- * not need to check first.  Return early anyway, so we do not write a second cookie either. */
+/* Nothing can be added to a header block that has already been closed, so there is nothing
+ * useful left to do.  The flows that reach here twice - hgc emitting the header early via
+ * cartAndCookieWithHtml and then webStart asking again - are the common case; the other one
+ * is an early warn() during cartNew, which writes its own header before there is a cart to
+ * take a cookie from.  cartAndCookieWithHtml queues the content policy ahead of that warn
+ * for exactly that reason; the cookie cannot be queued that early and is simply lost. */
 if (cgiDidContentType())
     return;
 if (!cookieName)
     cookieName = hUserCookie();
 
+/* These two queue header lines and cgiPrintContentType writes them, so their order here is
+ * a matter of taste rather than of the wire format. */
 cspWriteResponseHeader();
 cartWriteCookie(cart, cookieName);
 cgiPrintContentType(contType);
 }
 
 struct cart *cartAndCookieWithHtml(char *cookieName, char **exclude,
                                    struct hash *oldVars, boolean doContentType)
 /* Load cart from cookie and session cgi variable.  Write cookie
  * and optionally content-type part HTTP preamble to web page.  Don't
  * write any HTML though. */
 {
+/* Queue the content policy before anything can write a header.  An early warn during
+ * cartForSession() below prints the Content-Type line itself, and after that no header
+ * line can be added, so a policy queued only at cartWriteHeaderAndCont() time would be
+ * missing from exactly the pages that report a problem. */
+cspWriteResponseHeader();
 // Note: early abort works fine but early warn does not
 htmlPushEarlyHandlers();
 struct cart *cart = cartForSession(cookieName, exclude, oldVars);
 popWarnHandler();
 popAbortHandler();
 
 if (doContentType)
     cartWriteHeaderAndCont(cart, cookieName, NULL);
 
 return cart;
 }
 
 struct cart *cartAndCookie(char *cookieName, char **exclude,
                            struct hash *oldVars)
 /* Load cart from cookie and session cgi variable.  Write cookie and