diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ddaf37..4ba1b70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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`. diff --git a/src/main/java/org/metricshub/winrm/light/LightWinRMService.java b/src/main/java/org/metricshub/winrm/light/LightWinRMService.java index 3d8ce84..4e977b7 100644 --- a/src/main/java/org/metricshub/winrm/light/LightWinRMService.java +++ b/src/main/java/org/metricshub/winrm/light/LightWinRMService.java @@ -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; @@ -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) { @@ -564,13 +566,17 @@ public void close() { * @param step the protocol step to run * @param 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 callStreaming(final Callable step) throws TimeoutException, WinRMException { + private T callStreaming(final Callable step) throws TimeoutException, WinRMException { + checkNotClosed(); try { return step.call(); - } catch (final TimeoutException | RuntimeException e) { + } catch (final TimeoutException | WinRMClientException e) { throw e; } catch (final InterruptedException e) { Thread.currentThread().interrupt(); diff --git a/src/main/java/org/metricshub/winrm/light/WsmanClient.java b/src/main/java/org/metricshub/winrm/light/WsmanClient.java index 006be1b..290a313 100644 --- a/src/main/java/org/metricshub/winrm/light/WsmanClient.java +++ b/src/main/java/org/metricshub/winrm/light/WsmanClient.java @@ -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 @@ -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; diff --git a/src/test/java/org/metricshub/winrm/StreamingApiTest.java b/src/test/java/org/metricshub/winrm/StreamingApiTest.java index a3e366b..831a360 100644 --- a/src/test/java/org/metricshub/winrm/StreamingApiTest.java +++ b/src/test/java/org/metricshub/winrm/StreamingApiTest.java @@ -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; @@ -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 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();