diff --git a/CHANGELOG.md b/CHANGELOG.md index 5631deb..52006a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -158,6 +158,13 @@ Consequences: ### Fixed +- **A streaming read resumed after a long pause no longer times out spuriously** (#198). When a + streaming consumer (e.g. a `RemoteProcess` read slowly) paused longer than the inactivity + timeout and the host dropped the idle connection meanwhile, the reconnection inherited the + expired deadline of the previous request: its TCP connect and TLS handshake got a 1 ms budget + and failed with a timeout (*No response from the WinRM service*). Each + reconnection now gets a full inactivity timeout of its own. + - **WQL array properties keep all their elements** (#189). A WMI array (`IPAddress`, `DefaultIPGateway`, `Capabilities`, ...) used to yield only its last element, without any error. Its elements are now joined with `|` (`"192.0.2.10|fe80::1"`); the new diff --git a/src/main/java/org/metricshub/winrm/light/HttpTransport.java b/src/main/java/org/metricshub/winrm/light/HttpTransport.java index a178c55..102a882 100644 --- a/src/main/java/org/metricshub/winrm/light/HttpTransport.java +++ b/src/main/java/org/metricshub/winrm/light/HttpTransport.java @@ -136,9 +136,9 @@ void pollTimeout(final int budgetMillis) { * A server that enforces the WSMan OperationTimeout by answering with the op-timeout fault * reaches the caller through that fault instead; both surface as the same timeout. *
- * The bound is absolute per request leg (armed at the start of each {@link #post}): one whole - * response must arrive within the inactivity timeout — a peer trickling bytes must not restart - * the clock with every byte and hold a streaming fetch forever. + * The bound is absolute per request leg (armed at the start of each {@link #post} and + * {@link #connect}): one whole response must arrive within the inactivity timeout — a peer + * trickling bytes must not restart the clock with every byte and hold a streaming fetch forever. * * @param inactivityTimeoutMillis the longest tolerated silence in milliseconds */ @@ -281,10 +281,18 @@ 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. + *
+ * In streaming mode this starts a request leg of its own, with a fresh inactivity deadline: + * a reconnection after a long pause must not inherit the expired deadline of the previous leg. * * @throws IOException when the connection cannot be established */ void connect() throws IOException { + if (deadlinePerLeg) { + // Streaming mode: this whole leg — a reconnect included — must complete within the + // inactivity timeout, however many reads it takes (see inactivityTimeout(int)). + deadlineEpochMillis = Utils.getCurrentTimeMillis() + readTimeoutMillis; + } ensureConnected(); } @@ -344,12 +352,7 @@ private void ensureConnected() throws IOException { Response post(final String path, final byte[] body, final String contentType, final String authorization) throws IOException { - if (deadlinePerLeg) { - // Streaming mode: this whole leg — a reconnect included — must complete within the - // inactivity timeout, however many reads it takes (see inactivityTimeout(int)). - deadlineEpochMillis = Utils.getCurrentTimeMillis() + readTimeoutMillis; - } - ensureConnected(); + connect(); try { if (deadlineEpochMillis != 0) { // Several HTTP legs can run under one poll deadline (reconnect, authentication diff --git a/src/test/java/org/metricshub/winrm/light/HttpTransportDeadlineTest.java b/src/test/java/org/metricshub/winrm/light/HttpTransportDeadlineTest.java index 96590ea..7ba05de 100644 --- a/src/test/java/org/metricshub/winrm/light/HttpTransportDeadlineTest.java +++ b/src/test/java/org/metricshub/winrm/light/HttpTransportDeadlineTest.java @@ -20,6 +20,7 @@ * ╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱ */ +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -31,6 +32,7 @@ import java.net.Socket; import java.net.SocketTimeoutException; import java.nio.charset.StandardCharsets; +import javax.net.ssl.SSLContext; import org.junit.jupiter.api.Test; /** @@ -38,6 +40,9 @@ * span several HTTP round trips (a reconnect plus a re-authentication exchange), and every leg * must be capped by what is LEFT of the poll's budget — a peer answering each leg just fast enough * must not be able to stretch the poll to several multiples of the requested wait. + *
+ * Also covers the streaming {@link HttpTransport#inactivityTimeout(int)} deadline, armed afresh + * for every request leg, an explicit reconnection included. */ class HttpTransportDeadlineTest { @@ -118,6 +123,57 @@ void aTricklingPeerCannotStretchAStreamingRoundTrip() throws Exception { } } + @Test + void aStreamingReconnectGetsAFreshDeadline() throws Exception { + try (ServerSocket server = new ServerSocket(0)) { + // Every connection: take the ClientHello, stay silent 200 ms, then hang up. + final Thread handler = new Thread( + () -> { + try { + while (true) { + try (Socket socket = server.accept()) { + socket.getInputStream().read(new byte[4096]); + Thread.sleep(200); + } + } + } catch (final IOException | InterruptedException ignored) { + // server socket closed: test over + } + }, + "hang-up-tls-server" + ); + handler.setDaemon(true); + handler.start(); + + final HttpTransport transport = new HttpTransport( + "127.0.0.1", + server.getLocalPort(), + 60_000, + SSLContext.getDefault().getSocketFactory(), + false + ); + try { + // A first streaming leg arms its deadline, then fails (the server hangs up)... + transport.inactivityTimeout(1_000); + assertThrows(IOException.class, () -> transport.post("/wsman", new byte[0], null, null)); + // ...and the consumer pauses past that leg's deadline before reconnecting explicitly. + Thread.sleep(1_100); + // The reconnection is a leg of its own: its handshake must get a fresh 1 s budget and + // see the hang-up, not time out on the expired deadline's 1 ms floor. + final long start = System.nanoTime(); + final IOException e = assertThrows(IOException.class, transport::connect); + final long elapsedMillis = (System.nanoTime() - start) / 1_000_000; + assertFalse(e instanceof SocketTimeoutException, "the reconnection inherited an expired deadline: " + e); + assertTrue( + elapsedMillis >= 150, + "the handshake must wait for the server's hang-up; took " + elapsedMillis + " ms" + ); + } finally { + transport.close(); + } + } + } + /** * One connection: read the request head, then trickle the response one byte every 300 ms. * {@code SO_TIMEOUT} applies per read, so without an absolute bound every byte would reset the