Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 12 additions & 9 deletions src/main/java/org/metricshub/winrm/light/HttpTransport.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
* <p>
* 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
*/
Expand Down Expand Up @@ -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.
* <p>
* 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();
}

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -31,13 +32,17 @@
import java.net.Socket;
import java.net.SocketTimeoutException;
import java.nio.charset.StandardCharsets;
import javax.net.ssl.SSLContext;
import org.junit.jupiter.api.Test;

/**
* Unit test of the {@link HttpTransport#pollTimeout(int)} deadline: one deadline-bounded poll may
* 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.
* <p>
* Also covers the streaming {@link HttpTransport#inactivityTimeout(int)} deadline, armed afresh
* for every request leg, an explicit reconnection included.
*/
class HttpTransportDeadlineTest {

Expand Down Expand Up @@ -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
Expand Down
Loading