Fix OIDC refresh NPE and harden display safety

Address three moderate findings from the device-flow review.

adopt() no longer nulls a live in-memory refresh token when a re-read
store entry carries a valid served token but no refresh_token (a
cross-language peer that never received one, or a tampered file).
Nulling it made tryRefresh() call urlEncode(null) and throw an uncaught
NullPointerException that aborted getToken()/signIn() instead of
degrading to a refresh or an interactive sign-in. adopt() now keeps the
current refresh token and tracks what the file actually carried;
tryRefresh() also guards against a null refresh token defensively.

DisplaySafe.isDisplaySafe() now rejects U+2028 LINE SEPARATOR and U+2029
PARAGRAPH SEPARATOR, which are neither ISO control nor Cf format chars
yet break a rendered log line in ECMAScript/GUI/JSON log consumers. One
classifier change closes the gap across every sanitizer that shares it
(sanitizeForDisplay, OidcAuthException.putSanitized, and
Utf16Sink.putAsPrintable).

The httpTokenProvider Javadoc now documents that a failed HTTP flush
preserves the buffered request and its baked token and re-sends it
verbatim on retry rather than re-pulling, so a flush that keeps failing
until the pulled token expires is then rejected (for example a 401)
until the caller discards the buffered rows.

Add DisplaySafeTest coverage for U+2028/U+2029 and an
OidcDeviceAuthPersistenceTest regression that reproduces the refresh NPE
via a peer entry omitting the refresh token (verified both ways).

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

Author: glasstiger

core/src/main/java/io/questdb/client/Sender.java

@@ -2026,8 +2026,16 @@ public LineSenderBuilder httpToken(String token) {
          * refreshed token is presented each time the link is (re)established; an already-established WebSocket
          * is not re-authenticated mid-stream. The two transports differ on a sustained token outage: over HTTP
          * a failed pull is retried on the next row, but over WebSocket a pull that keeps failing past the
-         * reconnect budget terminates the sender for good, like any persistent reconnect failure. A
-         * lazily-signing-in provider can therefore be wired before the interactive sign-in completes over HTTP,
+         * reconnect budget terminates the sender for good, like any persistent reconnect failure.
+         * <br>
+         * Over HTTP the token is pulled once per request and written into the request buffer ahead of the
+         * buffered rows, so a token refresh is picked up on the next new batch after a successful flush. A
+         * failed flush preserves the buffer - token included - for a later retry and re-sends it verbatim
+         * rather than re-pulling the token, so a flush that keeps failing until the already-pulled token
+         * expires is then rejected (for example a {@code 401}); recover by discarding the buffered rows (close
+         * and rebuild the sender) so the next request pulls a fresh token.
+         * <br>
+         * A lazily-signing-in provider can therefore be wired before the interactive sign-in completes over HTTP,
          * where the first pull is deferred to the first row; over WebSocket a token must already be obtainable
          * when {@code build()} runs, since the initial handshake pulls it - otherwise that {@code build()} (or,
          * over HTTP, the first row) fails. Running on the send/flush and reconnect paths, the provider must

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

@@ -1037,7 +1037,15 @@ private boolean adopt(PersistedToken token) {
         }
         accessToken = token.getAccessToken();
         idToken = token.getIdToken();
-        refreshToken = token.getRefreshToken();
+        // keep the current refresh token when the file carries none, mirroring storeTokens(). A file with a
+        // valid served token but no refresh_token - a cross-language peer that never received one, or a
+        // tampered file - must not null a live in-memory refresh token: doing so would make a later
+        // tryRefresh() urlEncode(null) and throw an uncaught NPE (aborting the sign-in) instead of refreshing
+        // with the token we still hold or degrading to an interactive sign-in.
+        String fileRefreshToken = token.getRefreshToken();
+        if (fileRefreshToken != null) {
+            refreshToken = fileRefreshToken;
+        }
         // the file is attacker-writable (and may have been written under a skewed clock), so bound how long
         // the loaded token is trusted exactly as storeTokens() bounds a token from the wire: never past
         // MAX_EXPIRES_IN_SECONDS from now. Clamp the expiry to [0, now + maxLife]: the ceiling stops a tampered
@@ -1054,8 +1062,10 @@ private boolean adopt(PersistedToken token) {
         // half the lifetime); for a legitimate file the two already agree, and the expiry clamp bounds this to
         // [0, maxLife]
         tokenTtlMillis = Math.max(0L, expiresAtMillis - now);
-        // it is already on disk, so a later non-rotating refresh must not rewrite the file
-        lastPersistedRefreshToken = refreshToken;
+        // track what the file actually carried (which is null when it had no refresh_token but we kept a live
+        // one above), so a later non-rotating refresh does not rewrite an unchanged on-disk token, yet a token
+        // we kept that the file did not carry is not mistaken for already-persisted and can be re-saved
+        lastPersistedRefreshToken = fileRefreshToken;
         return true;
     }
 
@@ -1463,6 +1473,11 @@ private void throwIfClosed() {
     }
 
     private boolean tryRefresh() {
+        if (refreshToken == null) {
+            // nothing to present: degrade to the interactive flow rather than urlEncode(null) and throw.
+            // adopt() keeps a live refresh token, so this only fires if a caller reaches here with none.
+            return false;
+        }
         formSink.clear();
         formSink.putAscii("grant_type=").putAscii(GRANT_TYPE_REFRESH_TOKEN_ENCODED);
         appendParam(formSink, "refresh_token", refreshToken);

core/src/main/java/io/questdb/client/std/str/DisplaySafe.java

@@ -42,7 +42,8 @@ private DisplaySafe() {
      * Returns {@code true} when {@code cp} can be shown verbatim, {@code false} when it must be escaped or
      * stripped. A code point is unsafe if it is a control char (C0/C1, DEL), a Unicode format char (bidi
      * embeddings/overrides/isolates, LRM/RLM marks, zero-width joiners, the BOM, supplementary-plane tag
-     * chars) or a surrogate (a lone half, with no displayable meaning).
+     * chars), a Unicode line/paragraph separator (U+2028/U+2029, which break a rendered log line), or a
+     * surrogate (a lone half, with no displayable meaning).
      */
     public static boolean isDisplaySafe(int cp) {
         // Printable ASCII is the overwhelmingly common case and is never a control, format or surrogate char,
@@ -54,7 +55,12 @@ public static boolean isDisplaySafe(int cp) {
             return false;
         }
         final int type = Character.getType(cp);
-        if (type == Character.FORMAT || type == Character.SURROGATE) {
+        // FORMAT covers bidi/zero-width/joiners/BOM/tag chars; SURROGATE a lone half. LINE_SEPARATOR (U+2028)
+        // and PARAGRAPH_SEPARATOR (U+2029) are Unicode line breaks that split a rendered log line in
+        // ECMAScript/GUI/JSON log consumers, yet they are neither C0/C1 (isISOControl) nor FORMAT, so catch
+        // them here rather than let a tampered field forge an apparent extra log line.
+        if (type == Character.FORMAT || type == Character.SURROGATE
+                || type == Character.LINE_SEPARATOR || type == Character.PARAGRAPH_SEPARATOR) {
             return false;
         }
         // The explicit bidi/BOM set is redundant with the FORMAT category on a conformant JDK, but kept as

core/src/test/java/io/questdb/client/test/cutlass/auth/OidcDeviceAuthPersistenceTest.java

@@ -310,6 +310,37 @@ public void testRefreshUnderLockAdoptsPeerTokenAndSkipsNetwork() throws Exceptio
         });
     }
 
+    @Test(timeout = 30_000)
+    public void testRefreshUnderLockKeepsLiveRefreshTokenWhenPeerEntryOmitsIt() throws Exception {
+        // regression: a peer (or cross-language client, or a tampered file) persists a valid-but-expired served
+        // token with NO refresh_token while we hold a live refresh token in memory. The coordinated re-read must
+        // keep our refresh token, not null it - nulling it made tryRefresh() urlEncode(null) and throw an
+        // uncaught NPE that aborted getToken()/signIn() instead of degrading. With the fix REFRESH-1 is kept and
+        // the refresh succeeds.
+        assertMemoryLeak(() -> {
+            MockOidcServer.Handler handler = (method, path, body) -> {
+                if (path.startsWith(DEVICE_PATH)) {
+                    return MockOidcServer.json(200, deviceAuthJson());
+                }
+                // the token endpoint honours the kept refresh token and returns a fresh access token
+                return MockOidcServer.json(200, tokenJson("REFRESHED-ACCESS", null, "REFRESH-1", 3600));
+            };
+            try (MockOidcServer server = new MockOidcServer(handler)) {
+                FakeTokenStore fake = new FakeTokenStore();
+                long now = System.currentTimeMillis();
+                // our own entry, adopted on load: an expired served token carrying REFRESH-1
+                fake.stored = new PersistedToken("OLD-ACCESS", null, "REFRESH-1", now - 60_000, 300_000);
+                // a peer overwrites the file with a valid-but-expired served token and NO refresh_token (the
+                // frozen on-disk format permits omitting it) while we hold the cross-process lock
+                fake.peerInstallsOnLock = new PersistedToken("PEER-ACCESS", null, null, now - 60_000, 300_000);
+                try (OidcDeviceAuth auth = baseBuilder(server).tokenStore(fake).build()) {
+                    Assert.assertEquals("the live refresh token must be kept and used, not nulled into an NPE",
+                            "REFRESHED-ACCESS", auth.getToken());
+                }
+            }
+        });
+    }
+
     @Test(timeout = 30_000)
     public void testRestartRefreshesExpiredTokenSkippingDeviceFlow() throws Exception {
         assertMemoryLeak(() -> {

core/src/test/java/io/questdb/client/test/std/str/DisplaySafeTest.java

@@ -71,6 +71,19 @@ public void testFormatBidiAndBomAreUnsafe() {
         }
     }
 
+    @Test
+    public void testLineAndParagraphSeparatorsAreUnsafe() {
+        // U+2028 LINE SEPARATOR (Zl) and U+2029 PARAGRAPH SEPARATOR (Zp) are Unicode line breaks that split a
+        // rendered log line in ECMAScript/GUI/JSON log consumers, yet are neither ISO control nor Cf format,
+        // so a tampered field could otherwise forge an apparent extra log line
+        int[] unsafe = {0x2028, 0x2029};
+        for (int cp : unsafe) {
+            String hex = "0x" + Integer.toHexString(cp);
+            Assert.assertTrue("separator " + hex + " must be unsafe", DisplaySafe.isUnsafeForDisplay(cp));
+            Assert.assertFalse("separator " + hex + " must be unsafe", DisplaySafe.isDisplaySafe(cp));
+        }
+    }
+
     @Test
     public void testLoneSurrogatesAreUnsafe() {
         // a lone surrogate half has no displayable meaning; the code-point classifier must reject it