Address code-review findings on OIDC device flow

Follow-up fixes from a review of the OIDC device-flow PR.

Security
- Endpoint.parse rejects a URL query (?...), matching its existing
  fragment (#) rejection: pathOnly() strips the query before the
  issuer-path pin, yet postForm sends endpoint.path verbatim, so a
  tampered /settings could route an unvalidated query to the IdP.

Correctness
- tryRefresh falls back to the interactive flow when a refreshed
  token carries a control/non-ASCII char, instead of letting
  validateTokenChars propagate past the fallback.
- JsonLexer.unescape keeps the backslash on an unknown or malformed
  escape rather than dropping it, so non-conformant text survives.
- DirectUtf8Sink.put(byte[]) range-checks and throws instead of an
  assert that is a no-op in client apps run without -ea.
- FileTokenStore warns once about missing POSIX perms via an
  AtomicBoolean compareAndSet, closing a benign double-warn race.

Performance
- AbstractLineHttpSender precomputes the User-Agent header once
  instead of concatenating it on every newRequest.
- OidcDeviceAuth reuses build()'s parsed endpoints in the
  constructor instead of re-parsing the raw strings.

Docs and tests
- DeviceCodePrompt javadoc says "display-safe text", not "plain ASCII".
- oidc-token-persistence.md reflects the shipped implementation.
- Add direct DirectUtf8Sink.put(byte[]) range/bounds tests and a
  short 1-digit 4xx/5xx status rejection test.

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

Author: glasstiger

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

@@ -40,7 +40,7 @@
 public interface DeviceCodePrompt {
 
     /**
-     * Prints the sign-in instructions to {@code System.out} as plain ASCII, without opening a browser.
+     * Prints the sign-in instructions to {@code System.out} as display-safe text, without opening a browser.
      * The default prompt is {@link #openBrowser()}; use this to opt out of the browser launch.
      */
     DeviceCodePrompt SYSTEM_OUT = challenge -> {

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

@@ -56,6 +56,7 @@
 import java.util.Arrays;
 import java.util.Set;
 import java.util.UUID;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
  * The default {@link TokenStore}: one plaintext JSON file per identity under a directory, with the
@@ -139,8 +140,9 @@ public final class FileTokenStore implements TokenStore {
     private static final long REPLACE_RETRY_SLEEP_MILLIS = 20L;
     private static final int SCHEMA_VERSION = 1;
     // set once if the platform cannot enforce owner-only POSIX permissions on the token files (e.g. Windows),
-    // so the at-rest protection falls back to the directory's inherited ACL; warns the user once
-    private static volatile boolean warnedNoPosixPerms;
+    // so the at-rest protection falls back to the directory's inherited ACL; warns the user exactly once
+    // (compareAndSet, so a race between two threads still prints a single warning)
+    private static final AtomicBoolean warnedNoPosixPerms = new AtomicBoolean();
     private final Path directory;
     private final long lockAcquireBudgetMillis;
     private final long lockStaleMillis;
@@ -610,10 +612,9 @@ private static void warnNoPosixPermsOnce() {
         // best-effort, once per JVM: the token store could not enforce 0600/0700, so the persisted refresh
         // token is protected only by the directory's inherited ACL. ASCII-only, and never includes a path or
         // token byte (a path could itself carry terminal-spoofing characters)
-        if (warnedNoPosixPerms) {
+        if (!warnedNoPosixPerms.compareAndSet(false, true)) {
             return;
         }
-        warnedNoPosixPerms = true;
         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.");

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

@@ -199,13 +199,14 @@ public class OidcDeviceAuth implements QuietCloseable {
     // clock skew at half of this so a short-lived token is not treated as expired the instant it is issued
     private long tokenTtlMillis;
 
-    private OidcDeviceAuth(Builder builder, ClientTlsConfiguration tlsConfig) {
+    private OidcDeviceAuth(Builder builder, ClientTlsConfiguration tlsConfig, Endpoint deviceAuthorizationEndpoint, Endpoint tokenEndpoint) {
         String clientId = builder.clientId;
         // pre-encode the invariant form params once here, so the poll loop and silent refresh do not
         // re-run URLEncoder on every request (mirrors the pre-encoded GRANT_TYPE_* constants)
         this.clientIdEncoded = urlEncode(clientId);
-        this.deviceAuthorizationEndpoint = Endpoint.parse(builder.deviceAuthorizationEndpoint);
-        this.tokenEndpoint = Endpoint.parse(builder.tokenEndpoint);
+        // build() already parsed and validated these endpoints; reuse them rather than re-parse the raw strings
+        this.deviceAuthorizationEndpoint = deviceAuthorizationEndpoint;
+        this.tokenEndpoint = tokenEndpoint;
         String scope = builder.scope;
         this.scopeEncoded = urlEncode(scope);
         String audience = builder.audience;
@@ -226,7 +227,7 @@ private OidcDeviceAuth(Builder builder, ClientTlsConfiguration tlsConfig) {
                 audience != null && !audience.isEmpty() ? audience : null,
                 this.groupsInToken
         );
-        // allocate the native lexer last: an Endpoint.parse above can throw on a malformed url, and
+        // allocate the native lexer last: urlEncode and the TokenStoreKey construction above can throw, and
         // the half-built instance is never returned, so close() could not free an earlier alloc
         this.jsonLexer = new JsonLexer(JSON_LEXER_CACHE_SIZE, JSON_LEXER_MAX_VALUE_BYTES);
     }
@@ -1517,7 +1518,17 @@ private boolean tryRefresh() {
                 && isHttpStatusSuccess()
                 && tokenParser.error.length() == 0;
         if (hasRequiredToken) {
-            storeTokens(tokenParser);
+            try {
+                storeTokens(tokenParser);
+            } catch (OidcAuthException e) {
+                // storeTokens -> validateTokenChars rejects a refreshed served token carrying a control or
+                // non-ASCII char (reachable now that JsonLexer decodes an escaped \r/\n in the response into a
+                // real byte). Fall back to the interactive flow like the transport/parse-failure arms above,
+                // rather than let the rejection propagate out of getToken()/signIn() past the runDeviceFlow()
+                // fallback the caller expects. validateTokenChars runs before any state mutation, so the cached
+                // token and refresh token are left intact for that fallback.
+                return false;
+            }
             return true;
         }
         // the refresh token expired or was revoked, or did not return the token we need; fall back to the
@@ -1631,7 +1642,7 @@ public OidcDeviceAuth build() {
                             .put("), otherwise a slow refresh's live cross-process lock could be stolen by a peer mid-refresh");
                 }
             }
-            return new OidcDeviceAuth(this, tls);
+            return new OidcDeviceAuth(this, tls, deviceEndpoint, parsedTokenEndpoint);
         }
 
         public Builder clientId(String clientId) {
@@ -1943,6 +1954,17 @@ static Endpoint parse(String url) {
             if (url.indexOf('#') >= 0) {
                 throw new OidcAuthException().put("invalid url, a fragment (#) is not supported [url=").put(url).put(']');
             }
+            // reject a query (?...) for the same pin-bypass reason as the fragment above: pathOnly() strips it
+            // before the issuer-path check, yet postForm sends endpoint.path - query included - verbatim on the
+            // wire, so a tampered /settings could advertise an endpoint carrying a query the issuer-path pin
+            // never validated (and a lenient server could even normalize a '..' hidden past the '?'). An OIDC
+            // device/token endpoint carries its parameters in the request body (application/x-www-form-urlencoded),
+            // never the url query - RFC 6749 3.2 permits a query component but no real provider uses one here - so
+            // fail closed. The user-facing verification url, which legitimately carries the user code as a query,
+            // is parsed by BrowserLauncher (java.net.URI), not this method, so it is unaffected.
+            if (url.indexOf('?') >= 0) {
+                throw new OidcAuthException().put("invalid url, a query (?) is not supported [url=").put(url).put(']');
+            }
             int schemeEnd = url.indexOf("://");
             if (schemeEnd < 0) {
                 throw new OidcAuthException().put("invalid url, expected a scheme [url=").put(url).put(']');

core/src/main/java/io/questdb/client/cutlass/json/JsonLexer.java

@@ -409,14 +409,17 @@ private CharSequence unescape(CharSequence raw) {
                         unescapeSink.put((char) cp);
                         i += 6;
                     } else {
-                        // malformed unicode escape: drop the backslash, keep the following character
-                        unescapeSink.put(esc);
+                        // malformed unicode escape: keep the backslash and the 'u' verbatim (lenient), so a
+                        // non-conformant server's literal text survives rather than silently losing a byte
+                        unescapeSink.put('\\').put(esc);
                         i += 2;
                     }
                     break;
                 default:
-                    // unknown escape: drop the backslash, keep the escaped character (lenient)
-                    unescapeSink.put(esc);
+                    // unknown escape: keep the backslash and the escaped character verbatim (lenient), so a
+                    // literal backslash in non-conformant input (e.g. a Windows path in an error body) is not
+                    // dropped
+                    unescapeSink.put('\\').put(esc);
                     i += 2;
                     break;
             }

core/src/main/java/io/questdb/client/cutlass/line/http/AbstractLineHttpSender.java

@@ -83,6 +83,7 @@ public abstract class AbstractLineHttpSender implements Sender {
     private final CharSequence questDBVersion;
     private final Rnd rnd;
     private final StringSink sink = new StringSink();
+    private final String userAgent;
     private final String username;
     protected HttpClient.Request request;
     private HttpClient client;
@@ -203,6 +204,9 @@ protected AbstractLineHttpSender(
                     : HttpClientFactory.newPlainTextInstance(clientConfiguration);
         }
         this.questDBVersion = new BuildInformationHolder().getSwVersion();
+        // precompute the User-Agent header value once: newRequest() runs on every flush (and twice per flush
+        // for a token provider), so concatenating it there would allocate a String each time
+        this.userAgent = "QuestDB/java/" + questDBVersion;
         this.request = newRequest();
         this.maxNameLength = maxNameLength;
         this.rnd = rnd;
@@ -760,7 +764,7 @@ private HttpClient.Request newRequest(boolean pullProviderToken) {
         HttpClient.Request r = client.newRequest(currentHost(), currentPort())
                 .POST()
                 .url(path)
-                .header("User-Agent", "QuestDB/java/" + questDBVersion);
+                .header("User-Agent", userAgent);
         if (username != null) {
             r.authBasic(username, password);
         } else if (httpTokenProvider != null) {

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

@@ -110,7 +110,12 @@ public DirectUtf8Sink put(byte b) {
      * that rely on {@link #isAscii()} should not use this overload for ascii-only content.
      */
     public DirectUtf8Sink put(byte[] src, int lo, int hi) {
-        assert lo >= 0 && hi <= src.length && lo <= hi : "put(byte[]) range out of bounds";
+        // a real check, not an assert: this is public API doing an unchecked Unsafe.copyMemory, and client apps
+        // typically run without -ea, so a bad range must fail with a clear exception rather than a native
+        // out-of-bounds read that corrupts memory or crashes the JVM
+        if (lo < 0 || hi > src.length || lo > hi) {
+            throw new IndexOutOfBoundsException("put(byte[]) range out of bounds [lo=" + lo + ", hi=" + hi + ", len=" + src.length + ']');
+        }
         final int len = hi - lo;
         if (len > 0) {
             setAscii(false);

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

@@ -1092,6 +1092,14 @@ public void testEndpointParseRejectsMalformedUrls() {
         assertBuildFails("https://idp/realms/acme#/../other/device", "https://idp/realms/acme/token", "fragment");
         assertBuildFails("https://idp/d", "https://idp/realms/acme#/../other/token", "fragment");
         assertBuildFails("https://idp/d#", "https://idp/t", "fragment");
+        // a query (?...) is rejected for the same pin-bypass reason: pathOnly() strips it before the issuer-path
+        // pin while postForm sends endpoint.path - query included - verbatim, so a "?..." the pin never validated
+        // would still reach the wire. An OIDC device/token endpoint carries its parameters in the request body,
+        // never the url query, so fail closed (the user-facing verification url may carry one, but it is parsed
+        // by BrowserLauncher, not Endpoint.parse)
+        assertBuildFails("https://idp/realms/acme/device?x=/../other", "https://idp/realms/acme/token", "query");
+        assertBuildFails("https://idp/d", "https://idp/realms/acme/token?client_id=evil", "query");
+        assertBuildFails("https://idp/d?a=b", "https://idp/t", "query");
         // a non-ASCII host is rejected: it would not resolve (the HTTP layer sends the host to the OS resolver
         // as raw UTF-8, no IDNA), and equalsIgnoreCase folds several non-ASCII letters onto ASCII (U+0130 -> i,
         // U+212A -> k, ...), so a homoglyph host could otherwise pass the origin pin against a pinned issuer
@@ -3298,6 +3306,48 @@ public void testShortAllDigitStatusIsNotTreatedAsSuccess() throws Exception {
         });
     }
 
+    @Test
+    public void testShortAllDigitStatusNotTreatedAsTransientOrTerminal() throws Exception {
+        // a real HTTP status is exactly 3 digits. A malformed 1-digit "5" must not be read as a transient 5xx
+        // (which would poll on to the device-code deadline), nor a 1-digit "4" as a terminal 4xx, by the leading
+        // digit alone; both fall through to the fast terminal reject rather than an infinite poll.
+        for (String shortStatus : new String[]{"5", "4"}) {
+            assertMemoryLeak(() -> {
+                String tokenBody = tokenJson("SHOULD-NOT-ACCEPT", null, null, 3600);
+                String rawToken = "HTTP/1.1 " + shortStatus + " X\r\n"
+                        + "Content-Type: application/json\r\n"
+                        + "Transfer-Encoding: chunked\r\n\r\n"
+                        + Integer.toHexString(tokenBody.length()) + "\r\n" + tokenBody + "\r\n"
+                        + "0\r\n\r\n";
+                MockOidcServer.Handler handler = (method, path, body) -> {
+                    if (DEVICE_PATH.equals(path)) {
+                        return MockOidcServer.json(200, "{"
+                                + "\"device_code\":\"DEV\","
+                                + "\"user_code\":\"WDJB-MJHT\","
+                                + "\"verification_uri\":\"https://verify.example/device\","
+                                + "\"expires_in\":300,"
+                                + "\"interval\":1"
+                                + "}");
+                    }
+                    return MockOidcServer.raw(rawToken);
+                };
+                try (MockOidcServer server = new MockOidcServer(handler);
+                     OidcDeviceAuth auth = newAuth(server, false, noopPrompt())) {
+                    try {
+                        auth.signIn();
+                        Assert.fail("expected malformed 1-digit status '" + shortStatus + "' to be rejected fast");
+                    } catch (OidcAuthException e) {
+                        String msg = e.getMessage();
+                        Assert.assertTrue(msg, msg.contains("rejected the request") || msg.contains("refusing to keep polling"));
+                        // must NOT have polled to the device-code deadline (that would be a mis-classified transient)
+                        Assert.assertFalse(msg, msg.contains("device code expired"));
+                        Assert.assertFalse("the unaccepted token must not leak: " + msg, msg.contains("SHOULD-NOT-ACCEPT"));
+                    }
+                }
+            });
+        }
+    }
+
     // Forces the cached access/id token to look expired WITHOUT dropping the refresh token, so the next
     // signIn()/getToken() takes the silent-refresh (or interactive re-sign-in) path. Reflection
     // because the field is private and there is no configurable clock skew to lean on anymore; the client is

core/src/test/java/io/questdb/client/test/cutlass/json/JsonLexerTest.java

@@ -752,11 +752,12 @@ public void testStringEscapesExoticAndLenient() throws Exception {
             assertDecodedValue("{\"v\":\"a" + bs + "bb" + bs + "fc\"}",
                     "a" + ((char) 8) + "b" + ((char) 12) + "c");
             // the lexer is deliberately lenient (not RFC 8259-strict) about malformed or unknown escapes:
-            // it drops the backslash and keeps the following text rather than failing the parse. These pin
-            // that behavior and cover the lenient arms that otherwise carry most of the file's coverage:
-            assertDecodedValue("{\"v\":\"a" + bs + "xb\"}", "axb");          // unknown escape -> drop backslash
-            assertDecodedValue("{\"v\":\"a" + bs + "uZZZZb\"}", "auZZZZb");  // non-hex unicode escape -> literal
-            assertDecodedValue("{\"v\":\"ab" + bs + "u12\"}", "abu12");      // too few hex digits -> literal
+            // it keeps the backslash and the following text verbatim rather than failing the parse, so a
+            // literal backslash in non-conformant input is not silently lost. These pin that behavior and
+            // cover the lenient arms that otherwise carry most of the file's coverage:
+            assertDecodedValue("{\"v\":\"a" + bs + "xb\"}", "a" + bs + "xb");         // unknown escape -> kept verbatim
+            assertDecodedValue("{\"v\":\"a" + bs + "uZZZZb\"}", "a" + bs + "uZZZZb"); // non-hex unicode escape -> literal
+            assertDecodedValue("{\"v\":\"ab" + bs + "u12\"}", "ab" + bs + "u12");     // too few hex digits -> literal
             // a lone (unpaired) high surrogate is emitted as-is, not dropped or replaced
             assertDecodedValue("{\"v\":\"x" + bs + "uD83Dy\"}", "x" + ((char) 0xD83D) + "y");
         });

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

@@ -121,6 +121,43 @@ public void testDirectUtf8Sequence() {
         }
     }
 
+    @Test
+    public void testPutByteArrayRange() {
+        try (DirectUtf8Sink sink = new DirectUtf8Sink(4)) {
+            final byte[] src = "abcdefgh".getBytes(StandardCharsets.UTF_8);
+
+            // a partial range [2, 5) copies exactly "cde" and grows the sink past its initial capacity
+            sink.put(src, 2, 5);
+            Assert.assertEquals(3, sink.size());
+            TestUtils.assertEquals("cde".getBytes(StandardCharsets.UTF_8), sink);
+            // the bulk overload sets the ascii hint to false conservatively, even for ascii bytes
+            Assert.assertFalse(sink.isAscii());
+
+            // an empty range [3, 3) is a no-op
+            final int sizeBefore = sink.size();
+            sink.put(src, 3, 3);
+            Assert.assertEquals(sizeBefore, sink.size());
+
+            // a full range [0, len) appends the whole array
+            sink.clear();
+            sink.put(src, 0, src.length);
+            Assert.assertEquals(src.length, sink.size());
+            TestUtils.assertEquals(src, sink);
+        }
+    }
+
+    @Test
+    public void testPutByteArrayRangeRejectsBadBounds() {
+        try (DirectUtf8Sink sink = new DirectUtf8Sink(4)) {
+            final byte[] src = {1, 2, 3};
+            // a bad range must throw rather than run an unchecked native copy (asserts are off in client apps)
+            assertBadRange(sink, src, -1, 2); // lo < 0
+            assertBadRange(sink, src, 0, 4);  // hi > len
+            assertBadRange(sink, src, 2, 1);  // lo > hi
+            Assert.assertEquals("a rejected put must not advance the sink", 0, sink.size());
+        }
+    }
+
     @Test
     public void testPutUtf8Sequence() {
         try (DirectUtf8Sink sink = new DirectUtf8Sink(1)) {
@@ -208,6 +245,15 @@ public void testUtf8Sequence() {
         }
     }
 
+    private static void assertBadRange(DirectUtf8Sink sink, byte[] src, int lo, int hi) {
+        try {
+            sink.put(src, lo, hi);
+            Assert.fail("expected IndexOutOfBoundsException for lo=" + lo + ", hi=" + hi);
+        } catch (IndexOutOfBoundsException expected) {
+            // ok: the public overload range-checks before the native copy
+        }
+    }
+
     private static void assertUtf8Encoding(DirectUtf8Sink sink, String s) {
         sink.clear();
         sink.put(s);