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
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,15 @@ Consequences:
parentheses and modifiers removed, empty catch blocks named and commented, and two loops
restructured. The only signature change is the removal of the unused `target` parameter from the
`CipherGen` constructor (an internal NTLM helper).

### Fixed

- **Streaming terminals now report protocol failures as `WinRMClientException`** (#188).
`WqlRequest.stream()`, `CommandRequest.start()`, `RemoteFile.openStream()`/`openReader()`,
`RemoteDirectoryListing.stream()` and the closing of a `RemoteProcess` let raw
`IllegalStateException`s escape for an unexpected HTTP status (e.g. a 503 from a proxy), an
authentication fallback that exhausted every scheme, or a malformed response. They are now
wrapped like the blocking terminals' failures, with the raw exception in the cause chain; the
typed exceptions (`WinRMFaultException`, `WinRMAuthenticationException`, `WinRMTimeoutException`)
pass through unchanged, and genuine caller errors (invalid options, closed client, input after
the end of stdin) stay `IllegalArgumentException` / `IllegalStateException`.
14 changes: 10 additions & 4 deletions src/main/java/org/metricshub/winrm/light/LightWinRMService.java
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,8 @@ private Chunk adapt(final WsmanClient.RemoteCommand.Chunk chunk) {

@Override
public void send(final byte[] data, final boolean end) throws TimeoutException, WindowsRemoteException {
// A caller bug (input after the end mark), reported as such before any protocol step.
remoteCommand.checkStdinOpen();
callStreaming(() -> {
remoteCommand.send(data, end);
return null;
Expand All @@ -540,7 +542,7 @@ public int exitCode() {
public void close() {
try {
remoteCommand.close();
} catch (final RuntimeException e) {
} catch (final WinRMClientException e) {
// Typed protocol failures (e.g. a fault answering the terminate Signal) pass through.
throw e;
} catch (final InterruptedException e) {
Expand All @@ -564,13 +566,17 @@ public void close() {
* @param step the protocol step to run
* @param <T> the step's result type
* @return the step's result
* @throws IllegalStateException when this executor was closed: a handle outliving its client is a
* caller bug, reported as such rather than as a protocol failure
* @throws TimeoutException when the step exceeds the inactivity timeout
* @throws WinRMException when the step fails with a checked failure
* @throws WinRMException when the step fails, with the raw failure as its cause — the typed
* {@link WinRMClientException}s pass through unchanged
*/
private static <T> T callStreaming(final Callable<T> step) throws TimeoutException, WinRMException {
private <T> T callStreaming(final Callable<T> step) throws TimeoutException, WinRMException {
checkNotClosed();
try {
return step.call();
} catch (final TimeoutException | RuntimeException e) {
} catch (final TimeoutException | WinRMClientException e) {
Comment thread
bertysentry marked this conversation as resolved.
throw e;
} catch (final InterruptedException e) {
Thread.currentThread().interrupt();
Expand Down
22 changes: 16 additions & 6 deletions src/main/java/org/metricshub/winrm/light/WsmanClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,21 @@ Chunk pollChunk(final long askMs, final long maxWaitMs) throws Exception {
}
}

/**
* Reject input once the command's standard input is closed: a caller bug, reported before any
* protocol step.
*
* @throws IllegalStateException when the command has completed or its input was ended
*/
void checkStdinOpen() {
if (finished || exitCode != null) {
throw new IllegalStateException("The command has completed: its standard input is closed.");
}
if (stdinEnded) {
throw new IllegalStateException("The command's standard input has already been closed.");
}
}

/**
* Feed standard input to the running command: one or more WSMan Send requests carrying the
* bytes as base64 {@code stdin} streams, the last one flagged {@code End} when {@code end} is
Expand All @@ -755,12 +770,7 @@ Chunk pollChunk(final long askMs, final long maxWaitMs) throws Exception {
* @param end whether this is the last input the command will get
*/
void send(final byte[] data, final boolean end) throws Exception {
if (finished || exitCode != null) {
throw new IllegalStateException("The command has completed: its standard input is closed.");
}
if (stdinEnded) {
throw new IllegalStateException("The command's standard input has already been closed.");
}
checkStdinOpen();
if (data.length == 0 && !end) {
// Nothing to say and no EOF to announce: an empty Send would be a pure round trip.
return;
Expand Down
62 changes: 62 additions & 0 deletions src/test/java/org/metricshub/winrm/StreamingApiTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.metricshub.winrm.exceptions.WinRMClientException;
import org.metricshub.winrm.exceptions.WinRMTimeoutException;
import org.metricshub.winrm.exceptions.WqlSyntaxException;
import org.metricshub.winrm.light.FakeWsmanServer;
Expand Down Expand Up @@ -300,6 +301,67 @@ private static String stderrChunk(final String text) {
return stream("stderr", COMMAND_ID, text.getBytes(StandardCharsets.UTF_8));
}

private static Throwable rootCause(final Throwable e) {
Throwable cause = e;
while (cause.getCause() != null) {
cause = cause.getCause();
}
return cause;
}

@Test
void streamOutlivingItsClientFailsAsAClosedClient() throws Exception {
server.enqueue(200, envelope(enumeratePage("uuid:CTX-1", service("Spooler", "Running"))));

final WinRMClient client = builder().build();
final Iterator<WqlRow> iterator = client.wql("SELECT Name FROM Win32_Service").stream().iterator();
assertEquals("Spooler", iterator.next().string("Name"));
client.close();

// The next row needs a Pull on a closed client: a caller bug, not a protocol failure.
final IllegalStateException e = assertThrows(IllegalStateException.class, iterator::next);
assertEquals("This instance has been closed and a new one must be created.", e.getMessage());
}

@Test
void streamReportsAnUnexpectedHttpStatusAsAClientException() {
// A 503 from a proxy, or a non-WinRM service on the port: a protocol failure, not a caller bug.
server.enqueue(503, fault("999", "Service unavailable"));

try (WinRMClient client = builder().build()) {
final WinRMClientException e = assertThrows(
WinRMClientException.class,
() -> client.wql("SELECT Name FROM Win32_Service").stream()
);
assertTrue(e.getMessage().contains("HTTP 503"), e.getMessage());
assertTrue(rootCause(e) instanceof IllegalStateException);
}
}

@Test
void startReportsAnUnexpectedHttpStatusAsAClientException() {
server.enqueue(503, fault("999", "Service unavailable"));

try (WinRMClient client = builder().build()) {
final WinRMClientException e = assertThrows(WinRMClientException.class, () -> client.command("dir").start());
assertTrue(e.getMessage().contains("HTTP 503"), e.getMessage());
assertTrue(rootCause(e) instanceof IllegalStateException);
}
}

@Test
void closeReportsAnUnexpectedHttpStatusAsAClientException() throws Exception {
enqueueCommandStartup();
// The terminate Signal of an early close is answered with a 503.
server.enqueue(503, fault("999", "Service unavailable"));

try (WinRMClient client = builder().build()) {
final RemoteProcess process = client.command("dir").start();
final WinRMClientException e = assertThrows(WinRMClientException.class, process::close);
assertTrue(e.getMessage().contains("HTTP 503"), e.getMessage());
}
}

@Test
void startStreamsOutputWhileTheCommandIsStillRunning() throws Exception {
enqueueCommandStartup();
Expand Down
Loading