From 6a6395c18adba0163658fd3d6f435ebe069d3a03 Mon Sep 17 00:00:00 2001 From: Aayush Atharva Date: Sun, 20 Sep 2026 18:35:12 +0000 Subject: [PATCH] Keep credentials out of logs and exception messages --- .../main/java/org/asynchttpclient/Realm.java | 6 +++--- .../netty/NettyResponseFuture.java | 2 +- .../netty/channel/NettyConnectListener.java | 2 +- .../netty/handler/intercept/Interceptors.java | 4 ++-- .../ProxyUnauthorized407Interceptor.java | 8 ++++++-- .../intercept/Unauthorized401Interceptor.java | 8 ++++++-- .../netty/request/NettyRequestSender.java | 17 ++++++++++++----- .../netty/timeout/ReadTimeoutTimerTask.java | 2 +- .../asynchttpclient/spnego/SpnegoEngine.java | 2 +- .../util/AuthenticatorUtils.java | 2 +- .../org/asynchttpclient/util/ProxyUtils.java | 3 ++- .../java/org/asynchttpclient/RealmTest.java | 18 ++++++++++++++++++ .../netty/NettyResponseFutureTest.java | 14 ++++++++++++++ .../netty/timeout/TimeoutTimerTaskTest.java | 8 ++++++-- .../java/org/asynchttpclient/uri/UriTest.java | 14 ++++++++++++++ 15 files changed, 88 insertions(+), 22 deletions(-) diff --git a/client/src/main/java/org/asynchttpclient/Realm.java b/client/src/main/java/org/asynchttpclient/Realm.java index 765feb0388..e7ef410a03 100644 --- a/client/src/main/java/org/asynchttpclient/Realm.java +++ b/client/src/main/java/org/asynchttpclient/Realm.java @@ -271,17 +271,17 @@ public String getBasicAuthHeader() { public String toString() { return "Realm{" + "principal='" + principal + '\'' + - ", password='" + password + '\'' + + ", password=" + (password == null ? "null" : "") + ", scheme=" + scheme + ", realmName='" + realmName + '\'' + ", nonce='" + nonce + '\'' + ", algorithm='" + algorithm + '\'' + - ", response='" + response + '\'' + + ", response=" + (response == null ? "null" : "") + ", opaque='" + opaque + '\'' + ", qop='" + qop + '\'' + ", nc='" + nc + '\'' + ", cnonce='" + cnonce + '\'' + - ", uri=" + uri + + ", uri=" + (uri == null ? null : uri.toUrlWithoutUserInfo()) + ", usePreemptiveAuth=" + usePreemptiveAuth + ", charset=" + charset + ", ntlmHost='" + ntlmHost + '\'' + diff --git a/client/src/main/java/org/asynchttpclient/netty/NettyResponseFuture.java b/client/src/main/java/org/asynchttpclient/netty/NettyResponseFuture.java index b1f1e1532d..f18e051f47 100755 --- a/client/src/main/java/org/asynchttpclient/netty/NettyResponseFuture.java +++ b/client/src/main/java/org/asynchttpclient/netty/NettyResponseFuture.java @@ -818,7 +818,7 @@ public String toString() { // NettyRequest deliberately keeps Object.toString() so request headers are not rendered here. ",\n\tnettyRequest=" + nettyRequest + // ",\n\tfuture=" + future + // - ",\n\turi=" + getUri() + // + ",\n\turi=" + getUri().toUrlWithoutUserInfo() + // ",\n\tkeepAlive=" + keepAlive + // ",\n\tredirectCount=" + redirectCount + // ",\n\ttimeoutsHolder=" + TIMEOUTS_HOLDER_FIELD.get(this) + // diff --git a/client/src/main/java/org/asynchttpclient/netty/channel/NettyConnectListener.java b/client/src/main/java/org/asynchttpclient/netty/channel/NettyConnectListener.java index 7849dac022..b725f9d978 100755 --- a/client/src/main/java/org/asynchttpclient/netty/channel/NettyConnectListener.java +++ b/client/src/main/java/org/asynchttpclient/netty/channel/NettyConnectListener.java @@ -233,7 +233,7 @@ protected void onSuccess(Channel value) { boolean http2Negotiated = ApplicationProtocolNames.HTTP_2.equals(alpnProtocol); if (http2Negotiated && uri.isWebSocket()) { LOGGER.warn("Server negotiated HTTP/2 for WebSocket request to {}; WebSocket over HTTP/2 " - + "(RFC 8441) is not supported — continuing on HTTP/1.1", uri); + + "(RFC 8441) is not supported, continuing on HTTP/1.1", uri.toUrlWithoutUserInfo()); } if (http2Negotiated && !uri.isWebSocket()) { try { diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java index 5b1e5b954d..58a8f095d7 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Interceptors.java @@ -236,8 +236,8 @@ private boolean processAuthenticationInfo(Channel channel, NettyResponseFuture futu } final Request nextRequest = nextRequestBuilder.build(); - LOGGER.debug("Sending proxy authentication to {}", request.getUri()); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("Sending proxy authentication to {}", request.getUri().toUrlWithoutUserInfo()); + } if (channel instanceof Http2StreamChannel) { channel.close(); requestSender.sendNextRequest(nextRequest, future); @@ -320,7 +322,9 @@ public boolean exitAfterHandling407(Channel channel, NettyResponseFuture futu } final Request nextRequest = nextRequestBuilder.build(); - LOGGER.debug("Sending proxy authentication to {}", request.getUri()); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("Sending proxy authentication to {}", request.getUri().toUrlWithoutUserInfo()); + } if (channel instanceof Http2StreamChannel) { // HTTP/2 stream channels are single-use — close the stream and send the auth retry. channel.close(); diff --git a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Unauthorized401Interceptor.java b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Unauthorized401Interceptor.java index 237755687f..2d2b33d686 100644 --- a/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Unauthorized401Interceptor.java +++ b/client/src/main/java/org/asynchttpclient/netty/handler/intercept/Unauthorized401Interceptor.java @@ -135,7 +135,9 @@ public boolean exitAfterHandling401(Channel channel, NettyResponseFuture futu HttpHeaders requestHeaders = new DefaultHttpHeaders().add(request.getHeaders()); final Request nextRequest = future.getCurrentRequest().toBuilder().setHeaders(requestHeaders).build(); - LOGGER.debug("Sending authentication to {}", request.getUri()); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("Sending authentication to {}", request.getUri().toUrlWithoutUserInfo()); + } if (channel instanceof Http2StreamChannel) { channel.close(); requestSender.sendNextRequest(nextRequest, future); @@ -280,7 +282,9 @@ public boolean exitAfterHandling401(Channel channel, NettyResponseFuture futu final Request nextRequest = future.getCurrentRequest().toBuilder().setHeaders(requestHeaders).build(); - LOGGER.debug("Sending authentication to {}", request.getUri()); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("Sending authentication to {}", request.getUri().toUrlWithoutUserInfo()); + } if (channel instanceof Http2StreamChannel) { // HTTP/2 stream channels are single-use — close the stream and send the auth retry. channel.close(); diff --git a/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java b/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java index 7c5fe6ff3e..23ab04d221 100755 --- a/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java +++ b/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java @@ -1497,7 +1497,8 @@ private Channel pollPooledChannel(NettyResponseFuture future, Request request Channel h2Channel = channelManager.pollHttp2Connection(override); if (h2Channel != null) { if (LOGGER.isDebugEnabled()) { - LOGGER.debug("Using HTTP/2 multiplexed Channel '{}' for '{}' to '{}'", h2Channel, request.getMethod(), uri); + LOGGER.debug("Using HTTP/2 multiplexed Channel '{}' for '{}' to '{}'", h2Channel, request.getMethod(), + uri.toUrlWithoutUserInfo()); } return h2Channel; } @@ -1505,7 +1506,8 @@ private Channel pollPooledChannel(NettyResponseFuture future, Request request Channel channel = channelManager.poll( PrincipalScopedPartitionKey.scope(override, pooledIdentity(future, request))); if (channel != null && LOGGER.isDebugEnabled()) { - LOGGER.debug("Using pooled Channel '{}' for '{}' to '{}'", channel, request.getMethod(), uri); + LOGGER.debug("Using pooled Channel '{}' for '{}' to '{}'", channel, request.getMethod(), + uri.toUrlWithoutUserInfo()); } return channel; } @@ -1526,7 +1528,8 @@ private Channel pollPooledChannel(NettyResponseFuture future, Request request Channel h2Channel = channelManager.pollHttp2Connection(partitionKey); if (h2Channel != null) { if (LOGGER.isDebugEnabled()) { - LOGGER.debug("Using HTTP/2 multiplexed Channel '{}' for '{}' to '{}'", h2Channel, request.getMethod(), uri); + LOGGER.debug("Using HTTP/2 multiplexed Channel '{}' for '{}' to '{}'", h2Channel, request.getMethod(), + uri.toUrlWithoutUserInfo()); } return h2Channel; } @@ -1542,7 +1545,8 @@ private Channel pollPooledChannel(NettyResponseFuture future, Request request PrincipalScopedPartitionKey.scope(partitionKey, pooledIdentity(future, request))); if (channel != null && LOGGER.isDebugEnabled()) { - LOGGER.debug("Using pooled Channel '{}' for '{}' to '{}'", channel, request.getMethod(), uri); + LOGGER.debug("Using pooled Channel '{}' for '{}' to '{}'", channel, request.getMethod(), + uri.toUrlWithoutUserInfo()); } return channel; } @@ -1554,7 +1558,10 @@ public void replayRequest(final NettyResponseFuture future, FilterContext fc, future.setChannelState(ChannelState.NEW); future.touch(); - LOGGER.debug("\n\nReplaying request '{}' to '{}'\n for Future {}\n", newRequest.getMethod(), newRequest.getUri(), future); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("\n\nReplaying request '{}' to '{}'\n for Future {}\n", newRequest.getMethod(), + newRequest.getUri().toUrlWithoutUserInfo(), future); + } try { future.getAsyncHandler().onRetry(); } catch (Exception e) { diff --git a/client/src/main/java/org/asynchttpclient/netty/timeout/ReadTimeoutTimerTask.java b/client/src/main/java/org/asynchttpclient/netty/timeout/ReadTimeoutTimerTask.java index f9646e56f4..1ee82da2a9 100755 --- a/client/src/main/java/org/asynchttpclient/netty/timeout/ReadTimeoutTimerTask.java +++ b/client/src/main/java/org/asynchttpclient/netty/timeout/ReadTimeoutTimerTask.java @@ -92,7 +92,7 @@ void warnIfIndefinitelySuspended() { && indefiniteSuspensionWarningLogged.compareAndSet(false, true)) { LOGGER.warn("Response body reads for {} remain suspended while the request timeout is disabled; " + "the exchange retains its transport resources until it is resumed or canceled", - nettyResponseFuture.getUri()); + nettyResponseFuture.getUri().toUrlWithoutUserInfo()); } } } diff --git a/client/src/main/java/org/asynchttpclient/spnego/SpnegoEngine.java b/client/src/main/java/org/asynchttpclient/spnego/SpnegoEngine.java index d67d923bb7..5481c6df41 100644 --- a/client/src/main/java/org/asynchttpclient/spnego/SpnegoEngine.java +++ b/client/src/main/java/org/asynchttpclient/spnego/SpnegoEngine.java @@ -222,7 +222,7 @@ public String generateToken(String host) throws SpnegoEngineException { gssContext.dispose(); String tokenstr = Base64.getEncoder().encodeToString(token); - log.debug("Sending response '{}' back to the server", tokenstr); + log.debug("Sending a {}-byte SPNEGO token back to the server", tokenstr.length()); return tokenstr; } catch (GSSException gsse) { diff --git a/client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java b/client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java index e18c6d80f5..2a49e4345a 100644 --- a/client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java +++ b/client/src/main/java/org/asynchttpclient/util/AuthenticatorUtils.java @@ -685,7 +685,7 @@ private static void appendQuotedStringContent(StringBuilder builder, @Nullable S // SCRAM reauthentication — handled via interceptor break; default: - throw new IllegalStateException("Invalid Authentication " + realm); + throw new IllegalStateException("Invalid Authentication scheme " + realm.getScheme()); } } diff --git a/client/src/main/java/org/asynchttpclient/util/ProxyUtils.java b/client/src/main/java/org/asynchttpclient/util/ProxyUtils.java index a7bf5b7e20..120c926ca7 100644 --- a/client/src/main/java/org/asynchttpclient/util/ProxyUtils.java +++ b/client/src/main/java/org/asynchttpclient/util/ProxyUtils.java @@ -174,7 +174,8 @@ private static ProxyServerSelector createProxyServerSelector(final ProxySelector } return null; } catch (URISyntaxException e) { - logger.warn(uri + " couldn't be turned into a java.net.URI", e); + logger.warn("{} couldn't be turned into a java.net.URI: {}", uri.toUrlWithoutUserInfo(), + e.getReason()); return null; } }; diff --git a/client/src/test/java/org/asynchttpclient/RealmTest.java b/client/src/test/java/org/asynchttpclient/RealmTest.java index e1ac3c187c..c564b31d2d 100644 --- a/client/src/test/java/org/asynchttpclient/RealmTest.java +++ b/client/src/test/java/org/asynchttpclient/RealmTest.java @@ -353,4 +353,22 @@ private String getMd5(String what) throws Exception { byte[] hash = md.digest(); return StringUtils.toHexString(hash); } + /** + * Realm reaches logs through exception messages and through anything that renders a future, so its + * toString is a credential sink rather than a debugging convenience. + */ + @Test + public void toStringRedactsTheSecretsAndTheUserInfo() { + Realm realm = digestAuthRealm("user", "hunter2") + .setUri(Uri.create("https://user:hunter2@example.com/protected")) + .setResponse("d41d8cd98f00b204e9800998ecf8427e") + .build(); + + String rendered = realm.toString(); + assertFalse(rendered.contains("hunter2"), rendered); + assertFalse(rendered.contains("d41d8cd98f00b204e9800998ecf8427e"), rendered); + assertTrue(rendered.contains("principal='user'"), rendered); + assertTrue(rendered.contains("https://example.com/protected"), rendered); + } + } diff --git a/client/src/test/java/org/asynchttpclient/netty/NettyResponseFutureTest.java b/client/src/test/java/org/asynchttpclient/netty/NettyResponseFutureTest.java index 6efd572531..5f4f5b4965 100644 --- a/client/src/test/java/org/asynchttpclient/netty/NettyResponseFutureTest.java +++ b/client/src/test/java/org/asynchttpclient/netty/NettyResponseFutureTest.java @@ -193,4 +193,18 @@ private static void assertTerminalDeactivatesResponseBodyControl(Consumer future = new NettyResponseFuture<>(request, mock(AsyncHandler.class), null, 3, + ChannelPoolPartitioning.PerHostChannelPoolPartitioning.INSTANCE, null, null); + + String rendered = future.toString(); + assertFalse(rendered.contains("user:pw"), rendered); + assertTrue(rendered.contains("https://example.com:8443/secret?token=abc"), rendered); + } + } diff --git a/client/src/test/java/org/asynchttpclient/netty/timeout/TimeoutTimerTaskTest.java b/client/src/test/java/org/asynchttpclient/netty/timeout/TimeoutTimerTaskTest.java index 85c363d274..7d8de299b9 100644 --- a/client/src/test/java/org/asynchttpclient/netty/timeout/TimeoutTimerTaskTest.java +++ b/client/src/test/java/org/asynchttpclient/netty/timeout/TimeoutTimerTaskTest.java @@ -34,6 +34,7 @@ import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -120,7 +121,7 @@ public Object onCompleted(org.asynchttpclient.Response response) { @Test public void indefiniteSuspensionLogsOneWarning() { - Request request = new RequestBuilder().setUrl("http://example.com").build(); + Request request = new RequestBuilder().setUrl("http://user:pw@example.com/path").build(); NettyResponseFuture future = new NettyResponseFuture<>(request, new AsyncCompletionHandler() { @Override public Object onCompleted(org.asynchttpclient.Response response) { @@ -146,7 +147,10 @@ public Object onCompleted(org.asynchttpclient.Response response) { .filter(event -> event.getLevel() == Level.WARN) .collect(java.util.stream.Collectors.toList()); assertEquals(1, warnings.size()); - assertTrue(warnings.get(0).getFormattedMessage().contains("request timeout is disabled")); + String message = warnings.get(0).getFormattedMessage(); + assertTrue(message.contains("request timeout is disabled")); + assertFalse(message.contains("user:pw"), message); + assertTrue(message.contains("http://example.com/path"), message); } finally { logger.detachAppender(appender); appender.stop(); diff --git a/client/src/test/java/org/asynchttpclient/uri/UriTest.java b/client/src/test/java/org/asynchttpclient/uri/UriTest.java index a9b15ebbb1..f3fe653c0f 100644 --- a/client/src/test/java/org/asynchttpclient/uri/UriTest.java +++ b/client/src/test/java/org/asynchttpclient/uri/UriTest.java @@ -22,6 +22,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -396,4 +397,17 @@ public void testToRelativeUrlMatchesLegacyPathConcat() { "toRelativeUrl() must equal the legacy :path concatenation for " + url); } } + @RepeatedIfExceptionsTest(repeats = 5) + public void testToUrlWithoutUserInfoDropsOnlyTheUserInfo() { + Uri uri = Uri.create("https://user:pw@example.com:8443/secret/path?token=abc"); + assertEquals("https://example.com:8443/secret/path?token=abc", uri.toUrlWithoutUserInfo()); + assertTrue(uri.toUrl().contains("user:pw")); + } + + @RepeatedIfExceptionsTest(repeats = 5) + public void testToUrlWithoutUserInfoReturnsTheMemoisedUrlWhenThereIsNone() { + Uri uri = Uri.create("https://example.com/path?q=1"); + assertSame(uri.toUrl(), uri.toUrlWithoutUserInfo()); + } + }