diff --git a/src/main/java/org/metricshub/winrm/WinRMClient.java b/src/main/java/org/metricshub/winrm/WinRMClient.java index 3d2e88f..623cac0 100644 --- a/src/main/java/org/metricshub/winrm/WinRMClient.java +++ b/src/main/java/org/metricshub/winrm/WinRMClient.java @@ -287,6 +287,8 @@ public static final class Builder { private int consoleCodePage; private SSLContext sslContext; private Duration timeout = DEFAULT_TIMEOUT; + private int retries; + private Duration retryDelay = Duration.ZERO; private Builder(final String hostname) { Utils.checkNonBlank(hostname, "hostname"); @@ -441,6 +443,54 @@ public Builder timeout(final Duration timeout) { return this; } + /** + * Retry transient connection failures. Default: no retry — any failure is reported + * immediately. + *

+ * The policy is deliberately narrow, to preserve at-most-once execution for + * non-idempotent commands: a WSMan round trip is retried only when it failed to establish + * and authenticate the connection — TCP connect, DNS resolution, the TLS handshake, or a + * transport error during the authentication handshake — because there the request provably + * never reached the server. A request that was actually sent (a command that may have + * started, a query that may be executing) is never retried; neither are credential + * rejections ({@link org.metricshub.winrm.exceptions.WinRMAuthenticationException}) or + * WSMan faults ({@link org.metricshub.winrm.exceptions.WinRMFaultException}). + *

+ * The policy is connection-scoped: it applies to every operation on the client, including + * reconnections in the middle of one (WinRM connections are re-established transparently + * when the server drops an idle one). Retries stay inside each operation's wall-clock + * deadline — when the timeout elapses mid-pause, the operation fails with + * {@link org.metricshub.winrm.exceptions.WinRMTimeoutException} exactly as it would without + * a retry policy. For the streaming terminals (whose timeout is an inactivity timeout with + * no overall deadline), each connection attempt is bounded by that timeout, so a retried + * reconnection can extend the tolerated silence accordingly. + * + *

{@code
+		 * WinRMClient client = WinRMClient.builder("server01")
+		 * 	.credentials("ACME\\admin", password)
+		 * 	.retries(2, Duration.ofSeconds(5)) // up to 2 retries, pausing 5 s before each
+		 * 	.build();
+		 * }
+ * + * @param retries how many times a failed connection attempt is retried (0 disables retrying) + * @param delay the pause before each retry ({@link Duration#ZERO} retries immediately) + * @return this builder + * @throws IllegalArgumentException when {@code retries} is negative, or {@code delay} is + * null or negative + */ + public Builder retries(final int retries, final Duration delay) { + if (retries < 0) { + throw new IllegalArgumentException("retries must not be negative."); + } + Utils.checkNonNull(delay, "delay"); + if (delay.isNegative()) { + throw new IllegalArgumentException("delay must not be negative."); + } + this.retries = retries; + this.retryDelay = delay; + return this; + } + /** * Set the console code page of the remote command shell. Default: 65001 (UTF-8), which makes * command output UTF-8 whatever the remote locale — the right choice for reading output and @@ -499,7 +549,9 @@ public WinRMClient build() { authentications, sslContext, trustAllCertificates, - consoleCodePage + consoleCodePage, + retries, + toMillis(retryDelay) ); return new WinRMClient(executor, endpoint.getHostname(), endpoint.getNamespace(), timeout); } catch (final WinRMException e) { diff --git a/src/main/java/org/metricshub/winrm/light/HttpTransport.java b/src/main/java/org/metricshub/winrm/light/HttpTransport.java index 91339c0..a178c55 100644 --- a/src/main/java/org/metricshub/winrm/light/HttpTransport.java +++ b/src/main/java/org/metricshub/winrm/light/HttpTransport.java @@ -277,6 +277,29 @@ private boolean isStalePeerClosed() { } } + /** + * Establish (or validate) the connection now instead of lazily on the next {@link #post}. Lets + * the caller separate "could not reach the endpoint" — where nothing has been sent and a retry + * is provably safe — from a failure of a request that may already be executing. + * + * @throws IOException when the connection cannot be established + */ + void connect() throws IOException { + ensureConnected(); + } + + /** + * How many milliseconds remain of the active deadline-bounded poll ({@link #pollTimeout(int)}), + * or {@link Long#MAX_VALUE} when no poll deadline is active. Lets a caller decide whether a + * retry pause still fits inside the poll's hard bound. + */ + long remainingPollBudgetMillis() { + if (deadlineEpochMillis == 0 || deadlinePerLeg) { + return Long.MAX_VALUE; + } + return deadlineEpochMillis - Utils.getCurrentTimeMillis(); + } + private void ensureConnected() throws IOException { if (isConnected()) { return; diff --git a/src/main/java/org/metricshub/winrm/light/LightWinRMService.java b/src/main/java/org/metricshub/winrm/light/LightWinRMService.java index 329d873..71b7558 100644 --- a/src/main/java/org/metricshub/winrm/light/LightWinRMService.java +++ b/src/main/java/org/metricshub/winrm/light/LightWinRMService.java @@ -142,9 +142,64 @@ public static LightWinRMService createInstance( final SSLContext sslContext, final boolean trustAllCertificates, final int consoleCodePage + ) throws WinRMException { + return createInstance( + winRMEndpoint, + timeout, + ticketCache, + authentications, + sslContext, + trustAllCertificates, + consoleCodePage, + 0, + 0L + ); + } + + /** + * Create a light WinRM executor with an opt-in retry policy for transient connection failures. + * A round trip is retried only when it failed to establish and authenticate the connection — + * TCP connect, DNS resolution, TLS handshake, or a transport error during the authentication + * handshake — i.e. when its request provably never reached the server; a request that may have + * started executing is never retried, preserving at-most-once execution semantics. + * + * @param winRMEndpoint endpoint with credentials (mandatory) + * @param timeout timeout in milliseconds (must be > 0) + * @param ticketCache Kerberos ticket cache path (used by the Kerberos scheme; {@code null} logs + * in with the password) + * @param authentications requested authentication schemes, tried in order (NTLM and/or Kerberos); + * {@code null}/empty means NTLM only + * @param sslContext the {@link SSLContext} providing the HTTPS socket factory (hostname + * verification stays on); {@code null} uses the default configuration + * @param trustAllCertificates when {@code true} (and no {@code sslContext} is given), trust every + * server certificate and skip hostname verification — insecure, testing only + * @param consoleCodePage the console code page of the command shell; 0 keeps the default 65001, + * which makes command output UTF-8 whatever the remote locale + * @param connectRetries how many times one round trip may re-attempt to connect and authenticate + * (must be >= 0); 0 keeps the historical fail-fast behavior + * @param retryDelay the pause in milliseconds before each retry (must be >= 0) + * @return a new {@code LightWinRMService} + * @throws WinRMException on invalid arguments or an unsupported authentication request + */ + public static LightWinRMService createInstance( + final WinRMEndpoint winRMEndpoint, + final long timeout, + final java.nio.file.Path ticketCache, + final List authentications, + final SSLContext sslContext, + final boolean trustAllCertificates, + final int consoleCodePage, + final int connectRetries, + final long retryDelay ) throws WinRMException { Utils.checkNonNull(winRMEndpoint, "winRMEndpoint"); Utils.checkArgumentNotZeroOrNegative(timeout, "timeout"); + if (connectRetries < 0) { + throw new IllegalArgumentException("connectRetries must not be negative."); + } + if (retryDelay < 0) { + throw new IllegalArgumentException("retryDelay must not be negative."); + } // HTTPS wraps the transport in TLS and exchanges plaintext SOAP; HTTP uses NTLM message sealing. // TLS validates by default (platform trust store + hostname verification); see LightTls. A @@ -179,7 +234,9 @@ public static LightWinRMService createInstance( verifyHostname, authScheme, winRMEndpoint.getRawUsername(), - consoleCodePage + consoleCodePage, + connectRetries, + retryDelay ); return new LightWinRMService(winRMEndpoint, client); } diff --git a/src/main/java/org/metricshub/winrm/light/WsmanClient.java b/src/main/java/org/metricshub/winrm/light/WsmanClient.java index bd85c4f..0c66ff0 100644 --- a/src/main/java/org/metricshub/winrm/light/WsmanClient.java +++ b/src/main/java/org/metricshub/winrm/light/WsmanClient.java @@ -22,6 +22,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.net.SocketTimeoutException; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; @@ -84,6 +85,13 @@ final class WsmanClient implements AutoCloseable { private final AuthScheme auth; private final HttpTransport transport; + // Opt-in retry policy for transient connection failures (issue #158): how many times one round + // trip may re-attempt to establish and authenticate the connection, and the pause before each + // attempt. Applies ONLY to failures where the round trip's request provably never reached the + // server (see send()); 0 retries — the default — keeps the historical fail-fast behavior. + private final int connectRetries; + private final long retryDelayMs; + private String pendingAuthorization; private String shellId; @@ -150,10 +158,14 @@ private static void checkNotCancelled() throws InterruptedException { final boolean verifyHostname, final AuthScheme auth, final String rawUsername, - final int consoleCodePage + final int consoleCodePage, + final int connectRetries, + final long retryDelayMs ) { this.timeoutMs = timeoutMs; this.consoleCodePage = consoleCodePage; + this.connectRetries = connectRetries; + this.retryDelayMs = retryDelayMs; // A non-null socket factory selects HTTPS: TLS wraps the transport and the SOAP travels plaintext. this.url = (sslSocketFactory != null ? "https" : "http") + "://" + host + ":" + port + "/wsman"; this.rawUsername = rawUsername; @@ -1055,9 +1067,44 @@ private Decoded send(final String soap) throws Exception { final byte[] body = soap.getBytes(StandardCharsets.UTF_8); // Stays null while the loop retries the next authentication scheme on a fresh connection. Decoded decoded = null; + int retriesLeft = connectRetries; while (decoded == null) { if (!auth.isAuthenticated()) { - pendingAuthorization = auth.authenticate(transport); + try { + // Connect explicitly (rather than letting post() do it lazily) so every failure to + // REACH the endpoint — TCP connect, DNS resolution, TLS handshake — surfaces here, + // alongside the authentication round trips: the one phase where this round trip's + // request provably never reached the server, which is the only situation where a + // retry cannot duplicate a side effect (issue #158). + transport.connect(); + pendingAuthorization = auth.authenticate(transport); + } catch (final Exception e) { + // Transient connection failure: apply the opt-in retry policy — but only to a + // transport I/O failure (possibly wrapped, e.g. by an ordered authentication + // fallback), and unless the client is closing (best-effort cleanup must not + // linger), the retries are exhausted, or a deadline-bounded poll cannot absorb + // the pause within its hard bound. A cancelled worker (its caller was already + // told the operation timed out) stops in the sleep. + if (closed + || + retriesLeft <= 0 + || + !isConnectionFailure(e) + || + transport.remainingPollBudgetMillis() <= retryDelayMs) { + throw e; + } + retriesLeft--; + auth.reset(); + Thread.sleep(retryDelayMs); + // close() cannot interrupt a worker sleeping here: recheck after the pause, or + // the retry would revive the hard-closed transport and send the (possibly + // non-idempotent) operation after the client was closed. + if (closed) { + throw e; + } + continue; + } } // The handshake's Authorization accompanies the first real request; later requests on the // already-authenticated connection carry no Authorization header. @@ -1100,6 +1147,22 @@ private Decoded send(final String soap) throws Exception { return decoded; } + /** + * Whether the failure is a transport I/O failure — directly, or through its cause chain: an + * ordered authentication fallback reports "all schemes failed" with the last scheme's failure + * as the cause, which must still be recognized when that failure was a transient transport + * error. Anything without an {@link IOException} in the chain (a protocol violation, a + * rejected credential, an interrupt) is not a connection failure and must not be retried. + */ + private static boolean isConnectionFailure(final Exception exception) { + for (Throwable cause = exception; cause != null; cause = cause.getCause()) { + if (cause instanceof IOException) { + return true; + } + } + return false; + } + // --- XML helpers -------------------------------------------------------- static Document parse(final byte[] xml) throws Exception { diff --git a/src/main/java/org/metricshub/winrm/service/WinRMExecutorFactory.java b/src/main/java/org/metricshub/winrm/service/WinRMExecutorFactory.java index b25682c..50e0032 100644 --- a/src/main/java/org/metricshub/winrm/service/WinRMExecutorFactory.java +++ b/src/main/java/org/metricshub/winrm/service/WinRMExecutorFactory.java @@ -116,4 +116,49 @@ public static WindowsRemoteExecutor createInstance( consoleCodePage ); } + + /** + * Create a {@link WindowsRemoteExecutor} with an opt-in retry policy for transient connection + * failures. A round trip is retried only when it failed to establish and authenticate the + * connection — i.e. when its request provably never reached the server — so at-most-once + * execution semantics are preserved. + * + * @param winRMEndpoint endpoint with credentials (mandatory) + * @param timeout timeout in milliseconds (must be > 0) + * @param ticketCache Kerberos ticket cache path (may be {@code null}) + * @param authentications requested authentication schemes (may be {@code null}) + * @param sslContext the {@link SSLContext} providing the HTTPS socket factory (hostname + * verification stays on); {@code null} uses the default configuration + * @param trustAllCertificates when {@code true} (and no {@code sslContext} is given), trust every + * server certificate and skip hostname verification — insecure, testing only + * @param consoleCodePage the console code page of the command shell; 0 keeps the default 65001 + * @param connectRetries how many times one round trip may re-attempt to connect and authenticate + * (must be >= 0); 0 keeps the historical fail-fast behavior + * @param retryDelay the pause in milliseconds before each retry (must be >= 0) + * @return an executor backed by {@link LightWinRMService} + * @throws WinRMException for any problem creating the executor + */ + public static WindowsRemoteExecutor createInstance( + final WinRMEndpoint winRMEndpoint, + final long timeout, + final Path ticketCache, + final List authentications, + final SSLContext sslContext, + final boolean trustAllCertificates, + final int consoleCodePage, + final int connectRetries, + final long retryDelay + ) throws WinRMException { + return LightWinRMService.createInstance( + winRMEndpoint, + timeout, + ticketCache, + authentications, + sslContext, + trustAllCertificates, + consoleCodePage, + connectRetries, + retryDelay + ); + } } diff --git a/src/site/markdown/timeouts-and-errors.md b/src/site/markdown/timeouts-and-errors.md index 0fe7b4e..cdb810b 100644 --- a/src/site/markdown/timeouts-and-errors.md +++ b/src/site/markdown/timeouts-and-errors.md @@ -45,6 +45,48 @@ long — but as soon as the server stays silent for a whole timeout, the operati For commands, `RemoteProcess.waitFor(Duration)` provides an overall deadline on top when one is needed. +## Retrying transient connection failures + +By default there is **no retry**: a transient network failure — a dropped connection, a DNS +hiccup, a momentarily unreachable host — fails the operation immediately. The opt-in +`retries(...)` builder setting rides out such failures, which long-running monitoring and +automation workloads usually want: + +```java +WinRMClient client = WinRMClient.builder("server.example.com") + .credentials("DOMAIN\\Administrator", password) + .retries(2, Duration.ofSeconds(5)) // up to 2 retries, pausing 5 s before each + .build(); +``` + +The policy is deliberately narrow, to preserve **at-most-once execution** for non-idempotent +commands: a WSMan round trip is retried only when it failed to **establish and authenticate the +connection** — TCP connect, DNS resolution, the TLS handshake, or a transport error during the +authentication handshake — because there the request provably never reached the server. WQL +queries get no broader surface even though they are idempotent: re-issuing a WS-Enumeration +`Pull` whose response was lost could skip or duplicate rows. + +The policy is connection-scoped: it applies to every round trip of every operation on the +client, including reconnections in the middle of one (WinRM connections are re-established +transparently when the server drops an idle one). What is **never** retried: + +* a request that reached the wire — a command that may have started, a query that may be + executing; +* credential rejections + ([`WinRMAuthenticationException`](apidocs/org/metricshub/winrm/exceptions/WinRMAuthenticationException.html)); +* WSMan faults + ([`WinRMFaultException`](apidocs/org/metricshub/winrm/exceptions/WinRMFaultException.html)). + +Retries stay inside each operation's **wall-clock deadline**: when the timeout elapses mid-pause, +the operation fails with +[`WinRMTimeoutException`](apidocs/org/metricshub/winrm/exceptions/WinRMTimeoutException.html) +exactly as it would without a retry policy. For the streaming terminals (whose timeout is an +inactivity timeout with no overall deadline), each connection attempt is bounded by that timeout, +so a retried reconnection can extend the tolerated silence accordingly. + +Users migrating from winrm4j: `retries(1, Duration.ofSeconds(5))` is the closest equivalent of +its default `failureRetryPolicy` (one retry, 5-second pause). + ## The exception surface The fluent API is **unchecked**: every failure is a diff --git a/src/test/java/org/metricshub/winrm/WinRMClientBuilderTest.java b/src/test/java/org/metricshub/winrm/WinRMClientBuilderTest.java index 2b0dd66..487a6f6 100644 --- a/src/test/java/org/metricshub/winrm/WinRMClientBuilderTest.java +++ b/src/test/java/org/metricshub/winrm/WinRMClientBuilderTest.java @@ -85,6 +85,20 @@ void timeoutMustBeAtLeastOneMillisecond() { assertThrows(IllegalArgumentException.class, () -> validBuilder().timeout(Duration.ofNanos(1))); } + @Test + void retriesMustBeNonNegativeWithANonNegativeDelay() { + assertThrows(IllegalArgumentException.class, () -> validBuilder().retries(-1, Duration.ofSeconds(5))); + assertThrows(IllegalArgumentException.class, () -> validBuilder().retries(1, null)); + assertThrows(IllegalArgumentException.class, () -> validBuilder().retries(1, Duration.ofSeconds(-1))); + // 0 retries (the default) and a zero delay are both valid: build() must accept them. + try (WinRMClient client = validBuilder().retries(0, Duration.ZERO).build()) { + assertEquals("host", client.hostname()); + } + try (WinRMClient client = validBuilder().retries(2, Duration.ofSeconds(5)).build()) { + assertEquals("host", client.hostname()); + } + } + @Test void authenticationMustNotBeEmpty() { assertThrows(IllegalArgumentException.class, () -> validBuilder().authentication()); diff --git a/src/test/java/org/metricshub/winrm/light/FakeWsmanServer.java b/src/test/java/org/metricshub/winrm/light/FakeWsmanServer.java index 56c99b4..ae2bfd9 100644 --- a/src/test/java/org/metricshub/winrm/light/FakeWsmanServer.java +++ b/src/test/java/org/metricshub/winrm/light/FakeWsmanServer.java @@ -56,6 +56,9 @@ public final class FakeWsmanServer implements AutoCloseable { /** One scripted HTTP response: status code and the plaintext SOAP body to encrypt and serve. */ static final class Scripted { + /** Sentinel status: close the connection after reading the request, without responding. */ + static final int DROP = -1; + final int status; final String soapBody; final long delayMillis; @@ -106,6 +109,7 @@ static final class Scripted { private final Deque script = new ArrayDeque<>(); private final List decryptedRequests = new CopyOnWriteArrayList<>(); + private final java.util.concurrent.atomic.AtomicInteger connectionsToDrop = new java.util.concurrent.atomic.AtomicInteger(); private volatile boolean closed; private volatile boolean chunkedResponses; @@ -164,6 +168,33 @@ public FakeWsmanServer enqueueDelayed(final int status, final String soapBody, f return this; } + /** + * Queue a scripted connection drop: after reading (and decrypting) the request, the connection + * is closed without any response — simulating a transient network failure on a request that + * DID reach the server. + * + * @return this server, for chaining + */ + public FakeWsmanServer enqueueDrop() { + synchronized (script) { + script.addLast(new Scripted(Scripted.DROP, null)); + } + return this; + } + + /** + * Drop the next {@code count} incoming TCP connections as soon as they are accepted, before + * reading anything — simulating a transient failure while the connection is being established + * and authenticated, where the client's request provably never reached the server. + * + * @param count how many connections to drop + * @return this server, for chaining + */ + public FakeWsmanServer dropNextConnections(final int count) { + connectionsToDrop.set(count); + return this; + } + /** * Serve the scripted bodies with {@code Transfer-Encoding: chunked} — several chunks, a chunk * extension, and trailer fields after the terminating chunk — instead of {@code Content-Length}, @@ -270,6 +301,9 @@ private void handleConnection(final Socket socket) { // NTLM state is bound to the TCP connection, exactly like a real WinRM host. WinRMSession serverSession = null; try (socket) { + if (connectionsToDrop.getAndUpdate(n -> n > 0 ? n - 1 : 0) > 0) { + return; // scripted connection drop: close without reading anything + } socket.setTcpNoDelay(true); final BufferedInputStream in = new BufferedInputStream(socket.getInputStream()); final OutputStream out = socket.getOutputStream(); @@ -308,14 +342,22 @@ private void handleConnection(final Socket socket) { } // fall through: the request that carried the Type 3 also carries the first sealed body } - serveScripted(out, serverSession, request.body); + if (!serveScripted(out, serverSession, request.body)) { + return; // scripted mid-exchange drop: close without responding + } } } catch (final IOException | RuntimeException e) { // connection torn down (client close, test shutdown) — nothing to do } } - private void serveScripted(final OutputStream out, final WinRMSession session, final byte[] sealedBody) + /** + * Serve the next scripted response for one decrypted request. + * + * @return {@code true} to keep the connection alive, {@code false} when the script asked for a + * connection drop instead of a response + */ + private boolean serveScripted(final OutputStream out, final WinRMSession session, final byte[] sealedBody) throws IOException { final byte[] plaintext = NtlmCrypto.decrypt(session, sealedBody); decryptedRequests.add(new String(plaintext, StandardCharsets.UTF_8)); @@ -324,6 +366,9 @@ private void serveScripted(final OutputStream out, final WinRMSession session, f synchronized (script) { next = script.pollFirst(); } + if (next != null && next.status == Scripted.DROP) { + return false; + } if (next == null) { // Loud, decryptable failure so an over-consuming test fails on an assertion, not a hang. next = new Scripted( @@ -338,11 +383,12 @@ private void serveScripted(final OutputStream out, final WinRMSession session, f Thread.sleep(next.delayMillis); } catch (final InterruptedException e) { Thread.currentThread().interrupt(); - return; // test shutdown + return false; // test shutdown } } final byte[] sealed = NtlmCrypto.encryptAndSign(session, next.soapBody.getBytes(StandardCharsets.UTF_8)); respond(out, next.status, null, NtlmCrypto.ENCRYPTED_CONTENT_TYPE, sealed); + return true; } // --- NTLM server side ------------------------------------------------------- diff --git a/src/test/java/org/metricshub/winrm/light/WsmanRetryTest.java b/src/test/java/org/metricshub/winrm/light/WsmanRetryTest.java new file mode 100644 index 0000000..372892b --- /dev/null +++ b/src/test/java/org/metricshub/winrm/light/WsmanRetryTest.java @@ -0,0 +1,320 @@ +package org.metricshub.winrm.light; + +/*- + * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ + * WinRM Java Client + * ჻჻჻჻჻჻ + * Copyright 2023 - 2026 MetricsHub + * ჻჻჻჻჻჻ + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * ╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱ + */ + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.metricshub.winrm.light.FakeWsmanResponses.enqueueEnumeration; +import static org.metricshub.winrm.light.FakeWsmanResponses.enqueueShellCreation; +import static org.metricshub.winrm.light.FakeWsmanResponses.instance; + +import java.net.ServerSocket; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeoutException; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.metricshub.winrm.WinRMClient; +import org.metricshub.winrm.WinRMHttpProtocolEnum; +import org.metricshub.winrm.exceptions.WinRMException; +import org.metricshub.winrm.service.WinRMEndpoint; +import org.metricshub.winrm.service.client.auth.AuthenticationEnum; + +/** + * The opt-in retry policy for transient connection failures (issue #158): a round trip is retried + * only while establishing and authenticating the connection — where its request provably never + * reached the server — and never once the request may have executed. Exercised end to end against + * {@link FakeWsmanServer}, in-process, no Windows host required. + */ +class WsmanRetryTest { + + private static final String DOMAIN = "FAKE"; + private static final String USER = "user"; + private static final String PASSWORD = "s3cret-Passw0rd"; + private static final long TIMEOUT = 30_000L; + + private FakeWsmanServer server; + + @BeforeEach + void startServer() throws Exception { + server = new FakeWsmanServer(DOMAIN, USER, PASSWORD); + } + + @AfterEach + void stopServer() { + server.close(); + } + + private LightWinRMService service(final int port, final int retries, final long retryDelayMillis) + throws Exception { + return service(port, retries, retryDelayMillis, List.of(AuthenticationEnum.NTLM)); + } + + private LightWinRMService service( + final int port, + final int retries, + final long retryDelayMillis, + final List authentications + ) throws Exception { + final WinRMEndpoint endpoint = new WinRMEndpoint( + WinRMHttpProtocolEnum.HTTP, + "127.0.0.1", + port, + DOMAIN + "\\" + USER, + PASSWORD.toCharArray(), + null + ); + return LightWinRMService.createInstance( + endpoint, + TIMEOUT, + null, + authentications, + null, + false, + 0, + retries, + retryDelayMillis + ); + } + + /** A local port with no listener: connecting to it is refused immediately. */ + private static int refusedPort() throws Exception { + try (ServerSocket socket = new ServerSocket(0)) { + return socket.getLocalPort(); + } + } + + // --- retryable: the request never reached the server ----------------------- + + @Test + void retriesAConnectionDroppedDuringTheHandshake() throws Exception { + // The first TCP connection is dropped before anything is read: the authentication handshake + // fails, the operation's request was never sent — the retry succeeds on a fresh connection. + server.dropNextConnections(1); + enqueueEnumeration(server, instance("Win32_Service", "Name", "Spooler")); + + try (LightWinRMService service = service(server.port(), 1, 50L)) { + final List> rows = service.executeWql("SELECT Name FROM Win32_Service", TIMEOUT); + assertEquals(1, rows.size()); + assertEquals("Spooler", rows.get(0).get("Name")); + // The operation itself went out exactly once: at-most-once execution held. + assertEquals(1, server.decryptedRequests().size()); + } + } + + @Test + void failsImmediatelyWithoutARetryPolicy() throws Exception { + server.dropNextConnections(1); + enqueueEnumeration(server, instance("Win32_Service", "Name", "Spooler")); + + try (LightWinRMService service = service(server.port(), 0, 0L)) { + assertThrows(WinRMException.class, () -> service.executeWql("SELECT Name FROM Win32_Service", TIMEOUT)); + } + } + + @Test + void exhaustsRetriesAgainstAnUnreachableEndpoint() throws Exception { + // Nothing listens on the port: every attempt is refused. The configured pauses prove the + // retries actually ran before the failure was reported. + final long start = System.currentTimeMillis(); + try (LightWinRMService service = service(refusedPort(), 2, 200L)) { + assertThrows(WinRMException.class, () -> service.executeWql("SELECT Name FROM Win32_Service", TIMEOUT)); + } + assertTrue(System.currentTimeMillis() - start >= 400L, "expected two 200 ms retry pauses"); + } + + @Test + void retryPolicyPlumbsThroughTheFluentBuilder() throws Exception { + server.dropNextConnections(1); + enqueueEnumeration(server, instance("Win32_Service", "Name", "Spooler")); + + try ( + WinRMClient client = WinRMClient + .builder("127.0.0.1") + .port(server.port()) + .credentials(DOMAIN + "\\" + USER, PASSWORD.toCharArray()) + .retries(1, Duration.ofMillis(50)) + .build()) { + assertEquals("Spooler", client.wql("SELECT Name FROM Win32_Service").execute().rows().get(0).string("Name")); + } + } + + @Test + void retriesATransportFailureWrappedByAnOrderedAuthenticationFallback() throws Exception { + // With an ordered fallback list the handshake failure reaches the retry policy wrapped in + // the fallback's "all requested authentication schemes failed": its transport cause must + // still be recognized as a retryable connection failure. + server.dropNextConnections(2); + enqueueEnumeration(server, instance("Win32_Service", "Name", "Spooler")); + + try ( + LightWinRMService service = service( + server.port(), + 1, + 50L, + List.of(AuthenticationEnum.NTLM, AuthenticationEnum.NTLM) + )) { + final List> rows = service.executeWql("SELECT Name FROM Win32_Service", TIMEOUT); + assertEquals(1, rows.size()); + assertEquals(1, server.decryptedRequests().size()); + } + } + + // --- never retried: the request may have executed -------------------------- + + @Test + void doesNotRetryARequestThatReachedTheServer() throws Exception { + // The shell is created, then the connection is dropped AFTER the Command request was read: + // the command may have started, so the failure must surface without any second Command. + enqueueShellCreation(server); + server.enqueueDrop(); + + try (LightWinRMService service = service(server.port(), 3, 10L)) { + assertThrows( + WinRMException.class, + () -> service.executeCommand("echo risky", null, StandardCharsets.UTF_8, TIMEOUT) + ); + final long commandRequests = server + .decryptedRequests() + .stream() + .filter(request -> request.contains(":CommandLine>")) + .count(); + assertEquals(1, commandRequests, "a command that may have started must never be re-sent"); + } + } + + @Test + void doesNotRetryACredentialRejection() throws Exception { + final WinRMEndpoint endpoint = new WinRMEndpoint( + WinRMHttpProtocolEnum.HTTP, + "127.0.0.1", + server.port(), + DOMAIN + "\\" + USER, + "wrong-password".toCharArray(), + null + ); + final long start = System.currentTimeMillis(); + try ( + LightWinRMService service = LightWinRMService.createInstance( + endpoint, + TIMEOUT, + null, + List.of(AuthenticationEnum.NTLM), + null, + false, + 0, + 3, + 60_000L + )) { + final WinRMException e = assertThrows( + WinRMException.class, + () -> service.executeWql("SELECT Name FROM Win32_Service", TIMEOUT) + ); + assertTrue(e.getMessage().contains("Authentication error"), e.getMessage()); + } + // With a 60 s pause configured, a fast failure proves the rejection was not retried. + assertTrue(System.currentTimeMillis() - start < 30_000L, "a credential rejection must not be retried"); + } + + @Test + void closeDuringARetryPauseAbortsInsteadOfRevivingTheConnection() throws Exception { + // close() cannot interrupt a worker sleeping between retries: the pause must recheck the + // closed flag afterward, or the retry would reconnect the hard-closed transport and send + // the (possibly non-idempotent) operation after the client was closed. + server.dropNextConnections(1); + enqueueEnumeration(server, instance("Win32_Service", "Name", "Spooler")); + + try (LightWinRMService service = service(server.port(), 3, 800L)) { + final Thread closer = new Thread(() -> { + try { + Thread.sleep(300L); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + return; + } + service.close(); + }); + closer.start(); + assertThrows(WinRMException.class, () -> service.executeWql("SELECT Name FROM Win32_Service", TIMEOUT)); + closer.join(); + assertEquals(0, server.decryptedRequests().size(), "no request may be sent after close()"); + } + } + + // --- the wall-clock deadline still governs --------------------------------- + + @Test + void retryPausesStayInsideTheWallClockDeadline() throws Exception { + // Retries against an unreachable endpoint would run for ~25 s; the 1.5 s wall-clock deadline + // must cut them short and report the documented timeout instead. + try (LightWinRMService service = service(refusedPort(), 50, 500L)) { + final long start = System.currentTimeMillis(); + assertThrows(TimeoutException.class, () -> service.executeWql("SELECT Name FROM Win32_Service", 1_500L)); + assertTrue(System.currentTimeMillis() - start < 20_000L, "the deadline must cut the retry loop short"); + } + } + + // --- factory validation ----------------------------------------------------- + + @Test + void rejectsNegativeRetrySettings() { + final WinRMEndpoint endpoint = new WinRMEndpoint( + WinRMHttpProtocolEnum.HTTP, + "127.0.0.1", + 5985, + DOMAIN + "\\" + USER, + PASSWORD.toCharArray(), + null + ); + assertThrows( + IllegalArgumentException.class, + () -> LightWinRMService.createInstance( + endpoint, + TIMEOUT, + null, + List.of(AuthenticationEnum.NTLM), + null, + false, + 0, + -1, + 0L + ) + ); + assertThrows( + IllegalArgumentException.class, + () -> LightWinRMService.createInstance( + endpoint, + TIMEOUT, + null, + List.of(AuthenticationEnum.NTLM), + null, + false, + 0, + 1, + -1L + ) + ); + } +}