Harden OIDC token flow, store lock, name escaping

A batch of moderate review findings across the OIDC device flow, the
token store, the ILP-over-HTTP flush path, and the QWP senders, plus the
test-quality issues they surfaced.

OidcDeviceAuth:
- getToken()'s peer-wait budget was one httpTimeoutMillis, but a lock
  holder doing a silent refresh can legitimately hold for up to
  LOCK_HOLD_HTTP_TIMEOUT_MULTIPLE times that, so a concurrent caller
  threw on a refresh that was going to succeed. The peer now waits the
  holder's own worst-case hold.
- getToken() no longer forecloses a silent refresh when the served-kind
  token is null but a refresh token exists (the partial grant a
  groups-in-token sign-in with no id_token leaves behind): it attempts
  the refresh instead of throwing "no token has been obtained yet".
- close()'s javadoc claimed it returns after at most one HTTP request
  timeout; corrected to describe the in-flight refresh's real worst
  case, an OS-bounded connect stall.

FileTokenStore:
- inLock() now serializes same-identity critical sections within one JVM
  with a process-wide lock, so two OidcDeviceAuth instances for one
  identity cannot both run the read-refresh-write when the cross-process
  file lock degrades and double-POST the same rotating refresh token,
  which a reuse-detecting IdP revokes the whole family for.
- warnNoPosixPermsOnce/warnPersistence log via SLF4J instead of
  System.err, so a host application can filter and redirect them.

ILP over HTTP:
- The response-body reads inherited the raw request_timeout instead of
  the per-flush budget (base plus the throughput extension), so a
  tuned-low request_timeout with request_min_throughput could abort a
  large, still-progressing chunked error body and turn it into a retry
  of a non-retryable status. The body reads now use the per-flush budget.

QWP:
- A rejected table or column name was spliced raw into the error message
  (QwpWebSocketSender, QwpUdpSender, QwpTableBuffer); it now routes
  through putAsPrintable, matching the ILP name/error render, so a
  BOM/bidi/control char in a hostile name cannot reorder or forge the
  displayed text.

Tests:
- A swallowed Assert.fail on a mock-server thread, a mutual-exclusion
  test that passed if a contender died silently (now with a barrier and
  a run counter), a vacuous enum valueOf cross-check, and a sleep-gated
  negative assertion that could pass before its waiter thread started are
  all fixed to assert on the main thread and to prove the thread is
  genuinely blocked.
- Added coverage for a unicode escape at the end of a JSON value, the
  TokenStore.inLock default, putRawMessage's token stamp, and the
  null-served-kind refresh; each new production fix's test is proven to
  fail when the fix is reverted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: glasstiger

core/src/main/java/io/questdb/client/cutlass/auth/FileTokenStore.java

@@ -34,6 +34,9 @@
 import io.questdb.client.std.str.DirectUtf8Sink;
 import io.questdb.client.std.str.StringSink;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import java.io.IOException;
 import java.lang.management.ManagementFactory;
 import java.nio.ByteBuffer;
@@ -57,7 +60,9 @@
 import java.util.EnumSet;
 import java.util.Set;
 import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReentrantLock;
 
 /**
  * The default {@link TokenStore}: one plaintext JSON file per identity under a directory, with the
@@ -131,6 +136,7 @@ public final class FileTokenStore implements TokenStore {
     private static final int JSON_LEXER_CACHE_SIZE = 1024;
     private static final int JSON_LEXER_MAX_VALUE_BYTES = 1 << 20;
     private static final long LOCK_POLL_SLICE_MILLIS = 50L;
+    private static final Logger LOG = LoggerFactory.getLogger(FileTokenStore.class);
     // reject a token file larger than this; a real entry is a few KB even with a group-laden id token, so
     // anything past this is corrupt or hostile and is not read into memory
     private static final long MAX_FILE_BYTES = 1 << 20;
@@ -144,6 +150,14 @@ public final class FileTokenStore implements TokenStore {
     // attacker-writable directory as the token file, and a real owner stamp (pid@host + millis + UUID) is a
     // few hundred bytes, so anything past this cap is corrupt or hostile and is not read into memory
     private static final int MAX_LOCK_FILE_BYTES = 1 << 12;
+    // Serializes same-identity critical sections WITHIN this JVM, keyed on the identity fingerprint. Two
+    // OidcDeviceAuth instances for one identity in a single process (e.g. an ILP Sender and a QwpQueryClient)
+    // have separate instance locks, so only this shared lock stops them running the read-refresh-write
+    // concurrently and double-POSTing the same parent refresh token - which a reuse-detecting IdP revokes the
+    // whole token family for. The cross-process file lock's lock-free degrade must not license an intra-process
+    // race, so this in-process lock is taken first and is never subject to that degrade. Bounded by identity
+    // count (a handful), so it never grows unbounded.
+    private static final ConcurrentHashMap<String, ReentrantLock> PROCESS_LOCKS = new ConcurrentHashMap<>();
     // Windows can fail the atomic token-file rename with a transient AccessDeniedException (a sharing violation)
     // when a concurrent reader in any process holds the target open; retry the rename this many times on a short
     // backoff before giving up, so a routine read/write overlap does not needlessly degrade persistence. Kept
@@ -255,27 +269,39 @@ public void clear(TokenStoreKey key) {
 
     @Override
     public boolean inLock(TokenStoreKey key, CriticalSection action) {
-        Path lock = null;
-        // the unique owner nonce stamped into the lock when we acquired it, or null if we did not (or could
-        // not) acquire one and are running lock-free; releaseLock deletes the lock only when it still carries
-        // this nonce, so we never delete a lock a peer has since stolen
-        String nonce = null;
+        // First serialize other threads of THIS JVM sharing the same identity: the cross-process file lock below
+        // degrades to lock-free after lockAcquireBudgetMillis, which is a fine cross-process fallback but must
+        // not let two threads of one process run the critical section at once (they would double-POST the same
+        // rotating refresh token and get the whole family revoked on a reuse-detecting IdP). This lock is not
+        // subject to the file lock's degrade. ReentrantLock is safe even though inLock's contract forbids
+        // nesting - a mistaken re-entry cannot self-deadlock.
+        final ReentrantLock processLock = PROCESS_LOCKS.computeIfAbsent(key.hash(), k -> new ReentrantLock());
+        processLock.lock();
         try {
-            ensureDirectory();
-            lock = lockFile(key);
-            nonce = acquireLock(lock);
-        } catch (IOException e) {
-            // could not prepare the lock directory or file; run without the lock. Layer-1 atomic
-            // replacement still keeps every reader consistent - only a rotating-refresh-token race across
-            // processes is left unguarded for this one refresh.
-            nonce = null;
-        }
-        try {
-            return action.run();
-        } finally {
-            if (nonce != null) {
-                releaseLock(lock, nonce);
+            Path lock = null;
+            // the unique owner nonce stamped into the lock when we acquired it, or null if we did not (or could
+            // not) acquire one and are running lock-free; releaseLock deletes the lock only when it still carries
+            // this nonce, so we never delete a lock a peer has since stolen
+            String nonce = null;
+            try {
+                ensureDirectory();
+                lock = lockFile(key);
+                nonce = acquireLock(lock);
+            } catch (IOException e) {
+                // could not prepare the lock directory or file; run without the cross-process lock. Layer-1
+                // atomic replacement still keeps every reader consistent - only a rotating-refresh-token race
+                // across processes is left unguarded for this one refresh.
+                nonce = null;
+            }
+            try {
+                return action.run();
+            } finally {
+                if (nonce != null) {
+                    releaseLock(lock, nonce);
+                }
             }
+        } finally {
+            processLock.unlock();
         }
     }
 
@@ -638,9 +664,9 @@ private static void warnNoPosixPermsOnce() {
         if (!warnedNoPosixPerms.compareAndSet(false, true)) {
             return;
         }
-        System.err.println("questdb client: the OIDC token store could not enforce owner-only (0600/0700) "
-                + "permissions on this filesystem; the persisted refresh token is protected only by the "
-                + "directory's default ACL. Back the store with an OS keychain for at-rest encryption.");
+        LOG.warn("the OIDC token store could not enforce owner-only (0600/0700) permissions on this "
+                + "filesystem; the persisted refresh token is protected only by the directory's default ACL. "
+                + "Back the store with an OS keychain for at-rest encryption.");
     }
 
     private static void writeAndFlush(Path file, byte[] content) throws IOException {

core/src/main/java/io/questdb/client/cutlass/auth/OidcDeviceAuth.java

@@ -45,6 +45,9 @@
 import io.questdb.client.std.str.DirectUtf8Sequence;
 import io.questdb.client.std.str.StringSink;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import java.io.UnsupportedEncodingException;
 import java.net.URLEncoder;
 import java.util.Locale;
@@ -145,6 +148,7 @@ public class OidcDeviceAuth implements QuietCloseable {
     // connect instead). build() requires the FileTokenStore staleness window to exceed this multiple as a floor;
     // the default window adds ample headroom for a typical connection stall on top of it (see build())
     private static final int LOCK_HOLD_HTTP_TIMEOUT_MULTIPLE = 4;
+    private static final Logger LOG = LoggerFactory.getLogger(OidcDeviceAuth.class);
     // upper bound on the device code lifetime (the device authorization response's expires_in), so a
     // hostile or buggy provider cannot make the client poll for an absurd duration; matches the Python client
     private static final int MAX_DEVICE_CODE_TTL_SECONDS = 1800;
@@ -436,8 +440,12 @@ public void clearCache() {
      * {@link OidcAuthException} instead of polling until the device code expires. The signal is observed
      * between polls (within ~100ms while waiting out a poll interval); a poll request already in flight
      * is not interrupted, so {@code close()} acquires the lock - and returns - only once that request
-     * finishes or times out, i.e. after at most one HTTP request timeout
-     * (see {@link Builder#httpTimeoutMillis(int)}), not the full device-code lifetime. The exception is a
+     * finishes or times out, not the full device-code lifetime. That bound is the in-flight operation's own
+     * worst case, which is NOT a single HTTP request timeout: a silent refresh under the lock runs a send, an
+     * await and a body parse (each bounded by {@link Builder#httpTimeoutMillis(int)}), and its connection phase -
+     * DNS, TCP connect, TLS handshake - is bounded by the OS, not by that timeout, so a black-holed token
+     * endpoint can hold the lock, and this {@code close()}, for the OS connect timeout (commonly ~2 minutes on
+     * Linux) rather than a single httpTimeoutMillis. The exception is a
      * {@link DeviceCodePrompt} that blocks in {@code promptUser} - for example the default
      * {@link DeviceCodePrompt#openBrowser()} prompt while it hands the verification URL to the OS browser,
      * which is not bounded by the HTTP timeout: the flow holds the lock across that one-off prompt, so a
@@ -509,13 +517,17 @@ public String getToken() {
             throwIfClosed();
             maybeLoadFromStore();
             final String cachedToken = groupsInToken ? idToken : accessToken;
+            if (cachedToken != null && System.currentTimeMillis() < expiresAtMillis - effectiveSkewMillis()) {
+                return cachedToken;
+            }
+            // The served-kind token is absent or expired. Try a silent refresh whenever a refresh token is
+            // available - including the case where a prior grant returned only the OTHER kind, leaving the served
+            // kind null: a refresh may yield the served kind and avoid forcing an interactive sign-in. selectToken()
+            // reports a clear error if the refresh still did not produce the kind the server expects.
+            if (refreshToken != null && tryRefreshCoordinated()) {
+                return selectToken();
+            }
             if (cachedToken != null) {
-                if (System.currentTimeMillis() < expiresAtMillis - effectiveSkewMillis()) {
-                    return cachedToken;
-                }
-                if (refreshToken != null && tryRefreshCoordinated()) {
-                    return selectToken();
-                }
                 throw new OidcAuthException("the cached token expired and could not be refreshed without an interactive sign-in; call signIn() to sign in again");
             }
             throw new OidcAuthException("no token has been obtained yet; call signIn() to sign in before using getToken()");
@@ -1062,9 +1074,15 @@ private void acquireForGetToken() {
         // wait behind such a refresh - so poll for the lock in short slices rather than fail every concurrent
         // caller sharing this instance on each token refresh (the old unconditional tryLock() did exactly that).
         // Polling, not one blocking acquire, lets us still fail fast the moment an interactive sign-in - or
-        // close() - begins while we wait. Bound the total wait by httpTimeoutMillis so a stuck or pathologically
-        // slow holder degrades to a retryable failure instead of stalling the flush path without bound.
-        final long deadline = System.currentTimeMillis() + httpTimeoutMillis;
+        // close() - begins while we wait. Bound the total wait so a stuck or pathologically slow holder degrades
+        // to a retryable failure instead of stalling the flush path without bound - but size the bound to the
+        // holder's OWN worst-case hold, not a single httpTimeoutMillis. A legitimate silent refresh under the
+        // lock runs a send, an await and a body parse, each bounded by httpTimeoutMillis
+        // (LOCK_HOLD_HTTP_TIMEOUT_MULTIPLE x in total, the same figure the FileTokenStore lock-stale floor is
+        // derived from), so a peer that waited only one httpTimeoutMillis would fail every concurrent caller
+        // behind a refresh that is going to succeed.
+        final long deadline = System.currentTimeMillis()
+                + (long) LOCK_HOLD_HTTP_TIMEOUT_MULTIPLE * httpTimeoutMillis;
         while (true) {
             throwIfClosed();
             if (interactiveSignInInProgress) {
@@ -1625,8 +1643,8 @@ private void warnPersistence(String operation, Throwable cause) {
         // could itself hold terminal-spoofing characters - sanitize the detail before printing, as every other
         // untrusted display string is sanitized (sanitizeForDisplay is null-safe).
         String detail = sanitizeForDisplay(cause.getMessage());
-        System.err.println("questdb client: OIDC token store " + operation
-                + " failed; continuing without persistence" + (detail != null ? " [" + detail + ']' : ""));
+        LOG.warn("OIDC token store {} failed; continuing without persistence{}",
+                operation, detail != null ? " [" + detail + ']' : "");
     }
 
     /**