Reject non-ASCII OIDC hosts and fix review nits

A review flagged several minor issues; this commit addresses the
actionable ones.

Endpoint.parse now rejects a non-ASCII host. String.equalsIgnoreCase
folds several non-ASCII letters onto ASCII (U+0130 -> i, U+212A -> k,
...), so a homoglyph host advertised by a tampered /settings could
otherwise pass the origin pin against a pinned issuer; a non-ASCII host
would not resolve anyway (the HTTP layer sends it to the OS resolver as
raw UTF-8, no IDNA). Endpoint.parse also lower-cases the URL scheme
before matching so a case-insensitive HTTPS/Http (RFC 3986) builds,
matching the browser launcher; toLowerCase folds only ASCII, so a
homoglyph scheme stays rejected. Renamed sameOrigin to isSameOrigin per
the is/has convention.

FileTokenStore.save now retries the atomic rename on a transient Windows
AccessDeniedException (a sharing violation from a concurrent reader), so
a routine read/write overlap does not needlessly degrade best-effort
persistence. POSIX rename over an open file is unaffected.

Smaller nits: DirectUtf8Sink.put(byte[],int,int) asserts its range;
AbstractLineHttpSender.close() nulls jsonErrorParser after freeing it;
Utf16Sink.putAsPrintable skips a redundant charAt on the BMP fast path;
and a duplicated comment in TokenStoreKey is removed.

Adds OidcDeviceAuthTest cases: a non-ASCII host and an uppercase scheme
(both proven to fail against the pre-fix code), and the issuer-pin
accept path for host casing and an implicit vs explicit 443 port.

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

@@ -39,6 +39,7 @@
 import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.AccessDeniedException;
 import java.nio.file.AtomicMoveNotSupportedException;
 import java.nio.file.DirectoryStream;
 import java.nio.file.FileAlreadyExistsException;
@@ -130,6 +131,12 @@ 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;
+    // 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
+    // small - persistence is best-effort and the in-memory token is valid regardless.
+    private static final int REPLACE_MAX_ATTEMPTS = 5;
+    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
@@ -286,12 +293,7 @@ public void save(TokenStoreKey key, PersistedToken token) {
             boolean moved = false;
             try {
                 writeAndFlush(tmp, content);
-                try {
-                    Files.move(tmp, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING);
-                } catch (AtomicMoveNotSupportedException e) {
-                    // a rare filesystem without atomic rename; a plain replace still beats a partial write
-                    Files.move(tmp, target, StandardCopyOption.REPLACE_EXISTING);
-                }
+                replaceTarget(tmp, target);
                 moved = true;
             } finally {
                 if (!moved) {
@@ -536,6 +538,31 @@ private static void releaseLock(Path lock, String nonce) {
         }
     }
 
+    private static void replaceTarget(Path tmp, Path target) throws IOException {
+        // atomically rename tmp over target. On Windows a concurrent reader in any process holding target open
+        // can make the rename fail transiently with AccessDeniedException (a sharing violation); retry a few
+        // times on a short backoff before giving up, so a routine read/write overlap does not needlessly degrade
+        // persistence (best-effort - the in-memory token is still valid). POSIX rename over an open file never
+        // hits this. AtomicMoveNotSupported (a rare filesystem) falls back to a plain replace, which still beats
+        // leaving a partial write.
+        AccessDeniedException lastDenied = null;
+        for (int attempt = 0; attempt < REPLACE_MAX_ATTEMPTS; attempt++) {
+            if (attempt > 0) {
+                Os.sleep(REPLACE_RETRY_SLEEP_MILLIS);
+            }
+            try {
+                Files.move(tmp, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING);
+                return;
+            } catch (AtomicMoveNotSupportedException e) {
+                Files.move(tmp, target, StandardCopyOption.REPLACE_EXISTING);
+                return;
+            } catch (AccessDeniedException e) {
+                lastDenied = e;
+            }
+        }
+        throw lastDenied;
+    }
+
     private static void restrictToOwner(Path directory) {
         // best-effort: the at-rest protection of the plaintext token files is exactly these owner-only
         // directory permissions, so re-tighten a pre-existing directory another tool/umask left loose rather

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

@@ -360,10 +360,10 @@ public static OidcDeviceAuth fromQuestDB(String questdbUrl, DiscoveryOptions opt
         // still applies to every endpoint, enforced by validateEndpointOrigins in build().
         if (resolvedIssuer != null) {
             Endpoint pin = Endpoint.parse(resolvedIssuer);
-            if (tokenEndpointFromSettings && !sameOrigin(Endpoint.parse(tokenEndpoint), pin)) {
+            if (tokenEndpointFromSettings && !isSameOrigin(Endpoint.parse(tokenEndpoint), pin)) {
                 throw endpointOriginNotPinned("token endpoint", tokenEndpoint, originOf(pin));
             }
-            if (deviceEndpointFromSettings && !sameOrigin(Endpoint.parse(deviceAuthorizationEndpoint), pin)) {
+            if (deviceEndpointFromSettings && !isSameOrigin(Endpoint.parse(deviceAuthorizationEndpoint), pin)) {
                 throw endpointOriginNotPinned("device authorization endpoint", deviceAuthorizationEndpoint, originOf(pin));
             }
         }
@@ -810,6 +810,14 @@ private static boolean isLoopbackHost(String host) {
         return host != null && (host.equalsIgnoreCase("localhost") || (host.startsWith("127.") && isDottedIpv4(host)));
     }
 
+    private static boolean isSameOrigin(Endpoint a, Endpoint b) {
+        // scheme (via isTls), host and port - the security origin; path is deliberately not compared, the
+        // token and device endpoints legitimately differ in path on one authorization server. Endpoint.parse
+        // rejects a non-ASCII host, so this equalsIgnoreCase host compare only ever folds ASCII case - no
+        // non-ASCII homoglyph can fold onto a pinned issuer host here.
+        return a.isTls == b.isTls && a.port == b.port && a.host.equalsIgnoreCase(b.host);
+    }
+
     private static String originOf(Endpoint endpoint) {
         return (endpoint.isTls ? "https://" : "http://") + endpoint.host + ':' + endpoint.port;
     }
@@ -917,12 +925,6 @@ private static void requireSecureTransport(boolean isTls, String label, String u
         }
     }
 
-    private static boolean sameOrigin(Endpoint a, Endpoint b) {
-        // scheme (via isTls), host and port - the security origin; path is deliberately not compared, the
-        // token and device endpoints legitimately differ in path on one authorization server
-        return a.isTls == b.isTls && a.port == b.port && a.host.equalsIgnoreCase(b.host);
-    }
-
     private static String sanitizeForDisplay(String value) {
         if (value == null) {
             return null;
@@ -984,20 +986,20 @@ private static void validateEndpointOrigins(Endpoint tokenEndpoint, Endpoint dev
         // the issuer origin must then be configured without an issuer. fromQuestDB pins differently: it
         // origin-pins only the /settings-advertised endpoints itself (a discovered endpoint is trusted), so
         // it passes no issuer here and relies on this method only for the co-location check.
-        if (!sameOrigin(tokenEndpoint, deviceAuthorizationEndpoint)) {
+        if (!isSameOrigin(tokenEndpoint, deviceAuthorizationEndpoint)) {
             throw new OidcAuthException()
                     .put("the OIDC token and device authorization endpoints are on different origins (")
                     .put(originOf(tokenEndpoint)).put(" vs ").put(originOf(deviceAuthorizationEndpoint))
                     .put("); refusing to send credentials. This indicates a misconfigured or tampered OIDC configuration");
         }
         if (issuer != null) {
-            if (!sameOrigin(tokenEndpoint, issuer)) {
+            if (!isSameOrigin(tokenEndpoint, issuer)) {
                 throw new OidcAuthException()
                         .put("the OIDC token endpoint origin (").put(originOf(tokenEndpoint))
                         .put(") does not match the issuer origin (").put(originOf(issuer))
                         .put("); refusing to send credentials to an endpoint outside the trusted issuer");
             }
-            if (!sameOrigin(deviceAuthorizationEndpoint, issuer)) {
+            if (!isSameOrigin(deviceAuthorizationEndpoint, issuer)) {
                 throw new OidcAuthException()
                         .put("the OIDC device authorization endpoint origin (").put(originOf(deviceAuthorizationEndpoint))
                         .put(") does not match the issuer origin (").put(originOf(issuer))
@@ -1946,7 +1948,10 @@ static Endpoint parse(String url) {
                 throw new OidcAuthException().put("invalid url, expected a scheme [url=").put(url).put(']');
             }
             boolean isTls;
-            String scheme = url.substring(0, schemeEnd);
+            // lower-case the scheme before matching: RFC 3986 schemes are case-insensitive, so HTTPS/Http are
+            // valid. toLowerCase(Locale.ROOT) folds only ASCII case, so a homoglyph scheme (a long-s for the s,
+            // say) does NOT fold onto http/https and still falls through to the reject below.
+            String scheme = url.substring(0, schemeEnd).toLowerCase(Locale.ROOT);
             if ("https".equals(scheme)) {
                 isTls = true;
             } else if ("http".equals(scheme)) {
@@ -2006,6 +2011,17 @@ static Endpoint parse(String url) {
             if (host.isEmpty()) {
                 throw new OidcAuthException().put("invalid url, the host is empty [url=").put(url).put(']');
             }
+            // reject a non-ASCII host. The HTTP layer hands the host to the OS resolver as raw UTF-8 with no
+            // IDNA, so a non-ASCII name would not resolve anyway; and a non-ASCII code point makes the origin-pin
+            // host compare (isSameOrigin -> String.equalsIgnoreCase) unsafe, because equalsIgnoreCase folds
+            // several non-ASCII letters (U+0130, U+0131, U+017F, U+212A, ...) onto ASCII - so a homoglyph host
+            // advertised by a tampered /settings could otherwise pass the pin against the issuer. LDH ASCII
+            // hosts, punycode (xn--...) and dotted IPv4 are all ASCII and unaffected.
+            for (int i = 0, n = host.length(); i < n; i++) {
+                if (host.charAt(i) > 0x7f) {
+                    throw new OidcAuthException().put("invalid url, the host contains a non-ASCII character [url=").put(url).put(']');
+                }
+            }
             return new Endpoint(host, port, path, isTls);
         }
     }

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

@@ -70,8 +70,6 @@ public TokenStoreKey(
             String audience,
             boolean groupsInToken
     ) {
-        // the identity fields are required; reject a null up front with a clear error rather than letting it
-        // surface later as a raw NullPointerException deep inside a TokenStore's serialize/fingerprint path
         // the identity fields are required; reject a null up front with a clear error rather than letting it
         // surface later as a raw NullPointerException deep inside a TokenStore's serialize/fingerprint path
         if (clientId == null || tokenEndpoint == null || deviceAuthorizationEndpoint == null || scope == null) {

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

@@ -475,7 +475,7 @@ public void close() {
                 flush0(true);
             }
         } finally {
-            Misc.free(jsonErrorParser);
+            jsonErrorParser = Misc.free(jsonErrorParser);
             closed = true;
             client = Misc.free(client);
         }

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

@@ -110,6 +110,7 @@ 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";
         final int len = hi - lo;
         if (len > 0) {
             setAscii(false);

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

@@ -52,8 +52,11 @@ default void putAsPrintable(CharSequence nonPrintable) {
             final int cp = Character.codePointAt(nonPrintable, i);
             final int count = Character.charCount(cp);
             if (DisplaySafe.isDisplaySafe(cp)) {
-                for (int j = 0; j < count; j++) {
-                    put(nonPrintable.charAt(i + j));
+                if (count == 1) {
+                    put((char) cp); // BMP: cp already is the char, so skip the redundant charAt re-read
+                } else {
+                    put(nonPrintable.charAt(i));
+                    put(nonPrintable.charAt(i + 1));
                 }
             } else {
                 DisplaySafe.putUnicodeEscape(this, cp);

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

@@ -213,6 +213,23 @@ public void testAudienceSentOnRefresh() throws Exception {
         });
     }
 
+    @Test(timeout = 30_000)
+    public void testBuilderIssuerPinAcceptsHostCasingAndImplicitPort() throws Exception {
+        assertMemoryLeak(() -> {
+            // the origin pin (isSameOrigin) folds host case (ASCII) and treats an implicit https port as 443, so
+            // an endpoint differing from the issuer only in host case or an explicit :443 is still same-origin
+            try (OidcDeviceAuth ignored = OidcDeviceAuth.builder()
+                    .clientId("c")
+                    .deviceAuthorizationEndpoint("https://IDP.Example:443/as/device")
+                    .tokenEndpoint("https://idp.example/as/token")
+                    .issuer("https://Idp.Example")
+                    .build()
+            ) {
+                // accepted: host-case and implicit-vs-explicit 443 differences do not defeat the origin pin
+            }
+        });
+    }
+
     @Test(timeout = 30_000)
     public void testBuilderIssuerPinAcceptsMatchingOrigin() throws Exception {
         assertMemoryLeak(() -> {
@@ -1024,6 +1041,23 @@ public void testEndpointParseRejectsDisplayUnsafeUrl() {
         }
     }
 
+    @Test(timeout = 30_000)
+    public void testEndpointParseAcceptsUppercaseScheme() throws Exception {
+        assertMemoryLeak(() -> {
+            // RFC 3986 schemes are case-insensitive, so HTTPS/Http must build - matching BrowserLauncher's
+            // case-insensitive scheme allowlist. (Endpoint.parse lower-cases only ASCII, so a homoglyph scheme
+            // is still rejected as "expected http or https".)
+            try (OidcDeviceAuth ignored = OidcDeviceAuth.builder()
+                    .clientId("c")
+                    .deviceAuthorizationEndpoint("HTTPS://idp.example/device")
+                    .tokenEndpoint("Https://idp.example/token")
+                    .build()
+            ) {
+                // accepted: build() did not reject the mixed-case https scheme
+            }
+        });
+    }
+
     @Test(timeout = 30_000)
     public void testEndpointParseRejectsMalformedUrls() {
         // Endpoint.parse rejects malformed endpoint URLs at build time
@@ -1058,6 +1092,11 @@ 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 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
+        assertBuildFails("https://\u0130dp/d", "https://idp/t", "non-ASCII"); // U+0130, folds to i
+        assertBuildFails("https://idp/d", "https://\u212Aelvin/t", "non-ASCII"); // U+212A Kelvin, folds to k
     }
 
     @Test(timeout = 30_000)