From 99f8e22aa1d03432a2f3e768812f2f99877b96f8 Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Wed, 30 Sep 2026 00:41:51 +0200 Subject: [PATCH 1/2] Keep every element of WQL array properties, joined with a separator WS-Management returns a WMI array property as sibling elements sharing one name; each one used to overwrite the previous, so only the last element survived (IPAddress, IPSubnet, DefaultIPGateway, Capabilities...). The elements are now joined with "|" by default; the new WinRMClient.Builder.arraySeparator(String) changes the separator. The WqlRow Javadoc now matches the code: a WMI NULL is an empty string and null means "no such property". Documented in wql.md, the builder options table and the changelog. Closes #189 Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 6 ++++ .../org/metricshub/winrm/WinRMClient.java | 16 +++++++++ .../java/org/metricshub/winrm/WqlRow.java | 6 ++-- .../winrm/light/LightWinRMService.java | 9 +++++ .../metricshub/winrm/light/WsmanClient.java | 20 +++++++---- src/site/markdown/index.md | 1 + src/site/markdown/wql.md | 6 +++- .../winrm/light/BasicAuthCloseRaceTest.java | 1 + .../winrm/light/WsmanClientParityTest.java | 33 +++++++++++++++++-- 9 files changed, 87 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ba1b70..5631deb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -158,6 +158,12 @@ Consequences: ### Fixed +- **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 + `WinRMClient.Builder.arraySeparator(String)` changes the separator. The `WqlRow` Javadoc now + says what the code does: a WMI `NULL` is an empty string, `null` means "no such property". + - **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 diff --git a/src/main/java/org/metricshub/winrm/WinRMClient.java b/src/main/java/org/metricshub/winrm/WinRMClient.java index f33df20..e49daed 100644 --- a/src/main/java/org/metricshub/winrm/WinRMClient.java +++ b/src/main/java/org/metricshub/winrm/WinRMClient.java @@ -330,6 +330,7 @@ public static final class Builder { private boolean trustAllCertificates; private int consoleCodePage; private boolean loadUserProfile; + private String arraySeparator = LightWinRMService.DEFAULT_ARRAY_SEPARATOR; private SSLContext sslContext; private Duration timeout = DEFAULT_TIMEOUT; private int retries; @@ -609,6 +610,20 @@ public Builder loadUserProfile() { return this; } + /** + * Set the string that joins the elements of a WMI array property in a WQL row, such as + * {@code IPAddress} in {@code Win32_NetworkAdapterConfiguration}. Default: {@code |} + * ({@code "192.0.2.10|fe80::1"}). + * + * @param arraySeparator the separator (may be empty, not {@code null}) + * @return this builder + */ + public Builder arraySeparator(final String arraySeparator) { + Utils.checkNonNull(arraySeparator, "arraySeparator"); + this.arraySeparator = arraySeparator; + return this; + } + /** * Build the client. This does not connect yet: the connection is established and * authenticated by the first operation. @@ -656,6 +671,7 @@ public WinRMClient build() { trustAllCertificates, consoleCodePage, loadUserProfile, + arraySeparator, retries, toMillis(retryDelay) ); diff --git a/src/main/java/org/metricshub/winrm/WqlRow.java b/src/main/java/org/metricshub/winrm/WqlRow.java index db24afb..5c432f5 100644 --- a/src/main/java/org/metricshub/winrm/WqlRow.java +++ b/src/main/java/org/metricshub/winrm/WqlRow.java @@ -46,7 +46,8 @@ public final class WqlRow { * back to a case-insensitive match — WMI property names are case-insensitive. * * @param property the property name - * @return the property value, or {@code null} when the property is absent or null + * @return the property value, or {@code null} when the row has no such property (a WMI + * {@code NULL} is an empty string) */ public Object get(final String property) { Utils.checkNonNull(property, "property"); @@ -65,7 +66,8 @@ public Object get(final String property) { * Get the value of a property as a string. Same lookup semantics as {@link #get(String)}. * * @param property the property name - * @return the property value as a string, or {@code null} when the property is absent or null + * @return the property value as a string, or {@code null} when the row has no such property + * (a WMI {@code NULL} is an empty string) */ public String string(final String property) { final Object value = get(property); diff --git a/src/main/java/org/metricshub/winrm/light/LightWinRMService.java b/src/main/java/org/metricshub/winrm/light/LightWinRMService.java index 4e977b7..416bbd8 100644 --- a/src/main/java/org/metricshub/winrm/light/LightWinRMService.java +++ b/src/main/java/org/metricshub/winrm/light/LightWinRMService.java @@ -59,6 +59,9 @@ */ public final class LightWinRMService implements WindowsRemoteExecutor { + /** The default string joining the elements of a WMI array property in a WQL row. */ + public static final String DEFAULT_ARRAY_SEPARATOR = "|"; + private final WinRMEndpoint winRMEndpoint; private final WsmanClient client; private final AtomicBoolean closed = new AtomicBoolean(false); @@ -203,6 +206,7 @@ public static LightWinRMService createInstance( trustAllCertificates, consoleCodePage, false, + DEFAULT_ARRAY_SEPARATOR, connectRetries, retryDelay ); @@ -229,6 +233,8 @@ public static LightWinRMService createInstance( * which makes command output UTF-8 whatever the remote locale * @param loadUserProfile whether the command shell loads the user profile (registry hive, * per-user environment variables); {@code false} is the historical behavior + * @param arraySeparator the string joining the elements of a WMI array property in a WQL row; + * see {@link #DEFAULT_ARRAY_SEPARATOR} * @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) @@ -245,10 +251,12 @@ public static LightWinRMService createInstance( final boolean trustAllCertificates, final int consoleCodePage, final boolean loadUserProfile, + final String arraySeparator, final int connectRetries, final long retryDelay ) throws WinRMException { Utils.checkNonNull(winRMEndpoint, "winRMEndpoint"); + Utils.checkNonNull(arraySeparator, "arraySeparator"); Utils.checkArgumentNotZeroOrNegative(timeout, "timeout"); if (connectRetries < 0) { throw new IllegalArgumentException("connectRetries must not be negative."); @@ -298,6 +306,7 @@ public static LightWinRMService createInstance( winRMEndpoint.getRawUsername(), consoleCodePage, loadUserProfile, + arraySeparator, connectRetries, retryDelay ); diff --git a/src/main/java/org/metricshub/winrm/light/WsmanClient.java b/src/main/java/org/metricshub/winrm/light/WsmanClient.java index 290a313..abf8b42 100644 --- a/src/main/java/org/metricshub/winrm/light/WsmanClient.java +++ b/src/main/java/org/metricshub/winrm/light/WsmanClient.java @@ -91,6 +91,7 @@ final class WsmanClient implements AutoCloseable { private final long timeoutMs; private final int consoleCodePage; private final boolean loadUserProfile; + private final String arraySeparator; private final String url; private final String rawUsername; private final AuthScheme auth; @@ -189,12 +190,14 @@ private static void checkNotCancelled() throws InterruptedException { final String rawUsername, final int consoleCodePage, final boolean loadUserProfile, + final String arraySeparator, final int connectRetries, final long retryDelayMs ) { this.timeoutMs = timeoutMs; this.consoleCodePage = consoleCodePage; this.loadUserProfile = loadUserProfile; + this.arraySeparator = arraySeparator; this.connectRetries = connectRetries; this.retryDelayMs = retryDelayMs; // A non-null socket factory selects HTTPS: TLS wraps the transport and the SOAP travels plaintext. @@ -371,7 +374,7 @@ private WqlEnumeration( private void ingest(final Document doc) { page = new ArrayList<>(); cursor = 0; - collectItems(doc, page); + collectItems(doc, page, arraySeparator); endOfSequence = hasEnumerationElement(doc, "EndOfSequence"); // Pull only while the server hands back a context (matching the CXF backend). context = endOfSequence ? null : textNS(doc, WS_ENUMERATION_NS, "EnumerationContext"); @@ -1304,15 +1307,19 @@ private static String textNS(final Document doc, final String namespace, final S return nodes.getLength() > 0 ? nodes.item(0).getTextContent() : null; } - static void collectItems(final Document doc, final List> rows) { + static void collectItems(final Document doc, final List> rows, final String arraySeparator) { // The Items wrapper comes in the WS-Enumeration namespace (EnumerateResponse) or the WSMan // namespace (PullResponse) depending on the operation; accept both, like the CXF backend, and // nothing else — a WMI property or class named "Items" must not be mistaken for the wrapper. - collectRows(doc.getElementsByTagNameNS(WS_ENUMERATION_NS, "Items"), rows); - collectRows(doc.getElementsByTagNameNS(WSMAN_NS, "Items"), rows); + collectRows(doc.getElementsByTagNameNS(WS_ENUMERATION_NS, "Items"), rows, arraySeparator); + collectRows(doc.getElementsByTagNameNS(WSMAN_NS, "Items"), rows, arraySeparator); } - private static void collectRows(final NodeList items, final List> rows) { + private static void collectRows( + final NodeList items, + final List> rows, + final String arraySeparator + ) { for (int i = 0; i < items.getLength(); i++) { final NodeList instances = items.item(i).getChildNodes(); for (int j = 0; j < instances.getLength(); j++) { @@ -1325,7 +1332,8 @@ private static void collectRows(final NodeList items, final List a + arraySeparator + b); } } if (!row.isEmpty()) { diff --git a/src/site/markdown/index.md b/src/site/markdown/index.md index 32f4004..7a2b761 100644 --- a/src/site/markdown/index.md +++ b/src/site/markdown/index.md @@ -152,6 +152,7 @@ Besides the host name passed to `builder(...)`, only `credentials(...)` is manda | `namespace(String)` | `ROOT\CIMV2` | [Choosing a namespace](wql.html#choosing-a-namespace) | | `loadUserProfile()` | not loaded | [Loading the user profile](commands.html#loading-the-user-profile) | | `consoleCodePage(int)` | 65001 (UTF-8) | [Input encoding](commands.html#input-encoding) | +| `arraySeparator(String)` | `\|` | [Reading the result](wql.html#reading-the-result) | ## Where to go next diff --git a/src/site/markdown/wql.md b/src/site/markdown/wql.md index 562b1e8..efc9d73 100644 --- a/src/site/markdown/wql.md +++ b/src/site/markdown/wql.md @@ -109,7 +109,7 @@ Each [`WqlRow`](apidocs/org/metricshub/winrm/WqlRow.html) exposes the instance p | Method | Returns | Description | | --- | --- | --- | | `string(String)` | `String` | The property value, or `null` when the row has no such property. | -| `get(String)` | `Object` | The same value (currently always a `String`). | +| `get(String)` | `Object` | The same value (always a `String`). | | `asMap()` | `Map` | All properties, in server order (unmodifiable). | Property lookup is **case-insensitive**, matching WMI semantics: `row.string("name")` and @@ -118,6 +118,10 @@ Property lookup is **case-insensitive**, matching WMI semantics: `row.string("na Values are the text WinRM sends, unconverted: parse numbers, booleans and dates yourself. A WMI `NULL` comes back as an empty string. +A WMI **array** property (`IPAddress` in `Win32_NetworkAdapterConfiguration`, `Capabilities` in +`Win32_DiskDrive`, ...) comes back as a single string, its elements joined with `|`: +`"192.0.2.10|fe80::1"`. The builder's `arraySeparator(String)` changes the separator. + ### Column order and case These rules apply to `WqlResult.columns()`; each row's `asMap()` keeps the server's order, and diff --git a/src/test/java/org/metricshub/winrm/light/BasicAuthCloseRaceTest.java b/src/test/java/org/metricshub/winrm/light/BasicAuthCloseRaceTest.java index ab6055b..2cb8216 100644 --- a/src/test/java/org/metricshub/winrm/light/BasicAuthCloseRaceTest.java +++ b/src/test/java/org/metricshub/winrm/light/BasicAuthCloseRaceTest.java @@ -48,6 +48,7 @@ void closeWhileOperationInFlightStillErasesTheBasicCredential() throws Exception USERNAME, 65001, false, + "|", 0, 0L ); diff --git a/src/test/java/org/metricshub/winrm/light/WsmanClientParityTest.java b/src/test/java/org/metricshub/winrm/light/WsmanClientParityTest.java index a1d6a79..2075504 100644 --- a/src/test/java/org/metricshub/winrm/light/WsmanClientParityTest.java +++ b/src/test/java/org/metricshub/winrm/light/WsmanClientParityTest.java @@ -113,13 +113,42 @@ void itemsCollectedFromBothNamespaceVariants() throws Exception { for (final Document doc : List.of(wsenItems, wsmanItems)) { final List> rows = new ArrayList<>(); - WsmanClient.collectItems(doc, rows); + WsmanClient.collectItems(doc, rows, "|"); assertEquals(1, rows.size()); assertEquals("Spooler", rows.get(0).get("Name")); assertEquals("Running", rows.get(0).get("State")); } } + @Test + void arrayPropertyElementsAreJoinedWithTheSeparator() throws Exception { + // WS-Management sends a WMI array as sibling elements sharing one name; a WMI NULL is xsi:nil. + final Document doc = parse( + "" + + "" + + "" + + "" + + "192.0.2.10" + + "eth0" + + "fe80::1" + + "" + + "" + + "" + + "" + ); + final List> rows = new ArrayList<>(); + WsmanClient.collectItems(doc, rows, ", "); + assertEquals(1, rows.size()); + assertEquals("192.0.2.10, fe80::1", rows.get(0).get("IPAddress")); + assertEquals("eth0", rows.get(0).get("Caption")); + assertEquals("", rows.get(0).get("DefaultIPGateway")); + } + @Test void wmiPropertyNamedItemsIsNotMistakenForTheWrapper() throws Exception { // A structured WMI property called Items (in the class's namespace) must not be read as a @@ -140,7 +169,7 @@ void wmiPropertyNamedItemsIsNotMistakenForTheWrapper() throws Exception { "" ); final List> rows = new ArrayList<>(); - WsmanClient.collectItems(doc, rows); + WsmanClient.collectItems(doc, rows, "|"); assertEquals(1, rows.size()); assertEquals("real-row", rows.get(0).get("Name")); } From 5d7c8f4d250c00cf788f59db5db3e6b256184a70 Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Wed, 30 Sep 2026 00:56:15 +0200 Subject: [PATCH 2/2] Address Codex review: keep the 11-argument createInstance, join arrays linearly Restore the createInstance overload without arraySeparator as a delegating compatibility overload, and build joined array values with StringBuilders so a large array (SMBIOS raw tables) is appended in linear time. Co-Authored-By: Claude Fable 5.1 --- .../winrm/light/LightWinRMService.java | 63 ++++++++++++++++++- .../metricshub/winrm/light/WsmanClient.java | 20 ++++-- 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/metricshub/winrm/light/LightWinRMService.java b/src/main/java/org/metricshub/winrm/light/LightWinRMService.java index 416bbd8..828f80a 100644 --- a/src/main/java/org/metricshub/winrm/light/LightWinRMService.java +++ b/src/main/java/org/metricshub/winrm/light/LightWinRMService.java @@ -206,7 +206,6 @@ public static LightWinRMService createInstance( trustAllCertificates, consoleCodePage, false, - DEFAULT_ARRAY_SEPARATOR, connectRetries, retryDelay ); @@ -215,7 +214,67 @@ public static LightWinRMService createInstance( /** * Create a light WinRM executor that may delegate the caller's Kerberos credentials to the host, * so remote commands can authenticate onward as the caller (the second hop), and may load the - * user profile in the command shell. + * user profile in the command shell. WMI array properties are joined with + * {@link #DEFAULT_ARRAY_SEPARATOR}. + * + * @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, Kerberos, and/or Basic); + * {@code null}/empty means NTLM only + * @param allowDelegation whether Kerberos forwards the caller's ticket-granting ticket to the + * host (which must then be forwardable); requires Kerberos among {@code authentications} + * @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 loadUserProfile whether the command shell loads the user profile (registry hive, + * per-user environment variables); {@code false} is the historical behavior + * @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 + */ + // CPD-OFF — a compatibility overload: its parameter list is the next overload's minus the + // separator, and reordering the parameters to fool the detector would break callers. + public static LightWinRMService createInstance( + final WinRMEndpoint winRMEndpoint, + final long timeout, + final java.nio.file.Path ticketCache, + final List authentications, + final boolean allowDelegation, + final SSLContext sslContext, + final boolean trustAllCertificates, + final int consoleCodePage, + final boolean loadUserProfile, + final int connectRetries, + final long retryDelay + ) throws WinRMException { + return createInstance( + winRMEndpoint, + timeout, + ticketCache, + authentications, + allowDelegation, + sslContext, + trustAllCertificates, + consoleCodePage, + loadUserProfile, + DEFAULT_ARRAY_SEPARATOR, + connectRetries, + retryDelay + ); + // CPD-ON + } + + /** + * Create a light WinRM executor that may delegate the caller's Kerberos credentials to the host, + * so remote commands can authenticate onward as the caller (the second hop), may load the + * user profile in the command shell, and joins WMI array properties with a custom separator. * * @param winRMEndpoint endpoint with credentials (mandatory) * @param timeout timeout in milliseconds (must be > 0) diff --git a/src/main/java/org/metricshub/winrm/light/WsmanClient.java b/src/main/java/org/metricshub/winrm/light/WsmanClient.java index abf8b42..73ed7e5 100644 --- a/src/main/java/org/metricshub/winrm/light/WsmanClient.java +++ b/src/main/java/org/metricshub/winrm/light/WsmanClient.java @@ -1327,16 +1327,26 @@ private static void collectRows( if (instance.getNodeType() != Node.ELEMENT_NODE) { continue; } - final Map row = new LinkedHashMap<>(); + // A WMI array comes back as sibling elements sharing one name: join them. Built with + // StringBuilders so a large array (SMBIOS raw tables: tens of thousands of bytes) is + // appended in linear time rather than re-copied on every element. + final Map values = new LinkedHashMap<>(); final NodeList props = instance.getChildNodes(); for (int k = 0; k < props.getLength(); k++) { final Node prop = props.item(k); - if (prop.getNodeType() == Node.ELEMENT_NODE) { - // A WMI array comes back as sibling elements sharing one name: join them. - row.merge(((Element) prop).getLocalName(), prop.getTextContent(), (a, b) -> a + arraySeparator + b); + if (prop.getNodeType() != Node.ELEMENT_NODE) { + continue; + } + final StringBuilder value = values.get(((Element) prop).getLocalName()); + if (value == null) { + values.put(((Element) prop).getLocalName(), new StringBuilder(prop.getTextContent())); + } else { + value.append(arraySeparator).append(prop.getTextContent()); } } - if (!row.isEmpty()) { + if (!values.isEmpty()) { + final Map row = new LinkedHashMap<>(); + values.forEach((name, value) -> row.put(name, value.toString())); rows.add(row); } }