From 620f5b8b90aed68d27a7a06b0d0e84a5dc35ca2e Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Fri, 25 Sep 2026 00:03:11 +0200 Subject: [PATCH 1/5] Download remote files: downloadFile() and downloadTo(), digest-verified and atomic (#147) client.downloadFile(remote, local) and client.file(remote).downloadTo(local) copy a remote file to a local one through the WinRM connection: the counterpart of uploadFile(), with the same guarantees. - A PowerShell probe reports the size and the SHA-256 digest of the file, opening it with the share mode of the reads: certutil -hashfile, which the uploads use, fails with a sharing violation on a file another process holds open for writing (measured). - An identical local file is not transferred again: the call returns 0. - The content streams into ..part next to the destination, with bounded memory, is verified against the probed digest, fsync'ed, and moved onto the destination with ATOMIC_MOVE: a failure or a timeout never leaves a truncated file, and the timeout message tells how many bytes were transferred. - An existing directory as the destination receives the file under its remote name. A remote-file script whose start is rejected by the host's operation quota is now retried like the file-transfer steps, within the timeout (RemoteFiles.start). Co-Authored-By: Claude Opus 5.5 --- .../java/org/metricshub/winrm/RemoteFile.java | 200 ++++++++++++++++- .../org/metricshub/winrm/RemoteFiles.java | 44 +++- .../org/metricshub/winrm/ShellFileCopy.java | 4 +- .../org/metricshub/winrm/WinRMClient.java | 21 ++ src/site/markdown/file-transfers.md | 93 +++++++- src/site/markdown/files.md | 31 ++- src/site/markdown/index.md | 7 +- src/site/markdown/migrating-from-winrm4j.md | 2 +- src/site/markdown/preparing-the-host.md | 2 +- src/site/markdown/timeouts-and-errors.md | 4 +- .../org/metricshub/winrm/RemoteFileTest.java | 201 +++++++++++++++++- .../winrm/RemoteFilesScriptTest.java | 25 +++ .../org/metricshub/winrm/WinRMLiveTest.java | 81 +++++++ 13 files changed, 672 insertions(+), 43 deletions(-) diff --git a/src/main/java/org/metricshub/winrm/RemoteFile.java b/src/main/java/org/metricshub/winrm/RemoteFile.java index ab17a96..9aedf07 100644 --- a/src/main/java/org/metricshub/winrm/RemoteFile.java +++ b/src/main/java/org/metricshub/winrm/RemoteFile.java @@ -20,24 +20,39 @@ * ╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱ */ +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.BufferedReader; +import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.nio.channels.Channels; +import java.nio.channels.FileChannel; import java.nio.charset.Charset; +import java.nio.file.Files; +import java.nio.file.NoSuchFileException; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.time.Duration; import java.util.Optional; import java.util.concurrent.Callable; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.atomic.AtomicLong; import org.metricshub.winrm.exceptions.WinRMClientException; import org.metricshub.winrm.exceptions.WinRMTimeoutException; /** * A file or directory on the remote host, obtained with {@link WinRMClient#file(String)}: read a - * file's content — whole, as a byte range, or as a stream — or compute its digest, get its - * properties ({@link #info()}, {@link #exists()}), or {@link #list()} a directory. Nothing is sent - * until a terminal ({@link #readBytes()}, {@link #readText(Charset)}, {@link #openStream()}, - * {@link #openReader(Charset)}, {@link #digest(String)}, {@link #info()}, {@link #exists()}) is - * called. + * file's content — whole, as a byte range, or as a stream — download it to a local file, or + * compute its digest, get its properties ({@link #info()}, {@link #exists()}), or {@link #list()} a + * directory. Nothing is sent until a terminal ({@link #readBytes()}, {@link #readText(Charset)}, + * {@link #openStream()}, {@link #openReader(Charset)}, {@link #downloadTo(Path)}, + * {@link #digest(String)}, {@link #info()}, {@link #exists()}) is called. * *
{@code
  * byte[] content = client.file("C:\\Windows\\Temp\\collect.bin").readBytes();
@@ -66,6 +81,9 @@
  * 

* A request is not thread-safe; configure it and call its terminals from one thread. */ +@SuppressFBWarnings(value = "AT_NONATOMIC_64BIT_PRIMITIVE", justification = "A request is not thread-safe (documented): it is configured, " + + + "then used, from one thread; a blocking terminal's worker only starts after the configuration") public final class RemoteFile { /** Default cap of {@link #readBytes()} and {@link #readText(Charset)}: 64 MiB. */ @@ -109,7 +127,7 @@ public String path() { * {@code max(0, size - n)}, the size being read by the same remote invocation that seeks (so a * growing log is tailed from its current end); a file shorter than {@code n} is read whole. An * offset past the end of the file reads nothing — it is not an error. Applies to every read - * terminal, not to {@link #digest(String)}. + * terminal, not to {@link #downloadTo(Path)} or {@link #digest(String)}. * * @param offset the byte position; negative counts from the end of the file * @return this request @@ -123,7 +141,7 @@ public RemoteFile offset(final long offset) { * Read at most the given number of bytes (from the {@link #offset(long)}, once resolved) * instead of reading to the end of the file. A length beyond the end of the file reads what * exists; a zero length reads nothing. Applies to every read terminal, not to - * {@link #digest(String)}. + * {@link #downloadTo(Path)} or {@link #digest(String)}. * * @param length how many bytes to read at most (zero or more) * @return this request @@ -159,8 +177,9 @@ public RemoteFile maxBytes(final long maxBytes) { /** * Override the client's timeout for this request. For the blocking terminals - * ({@link #readBytes()}, {@link #readText(Charset)}, {@link #digest(String)}) it is a - * wall-clock deadline for the whole read; for {@link #openStream()} and + * ({@link #readBytes()}, {@link #readText(Charset)}, {@link #downloadTo(Path)}, + * {@link #digest(String)}, {@link #info()}, {@link #exists()}) it is a wall-clock deadline for + * the whole operation; for {@link #openStream()} and * {@link #openReader(Charset)} it is an inactivity timeout, the longest silence * tolerated from the host between two chunks of content. * @@ -260,6 +279,169 @@ public BufferedReader openReader(final Charset charset) { return new BufferedReader(new InputStreamReader(openStream(), charset)); } + /** + * Download the whole file to a local file — the counterpart of + * {@link WinRMClient#uploadFile(Path, String)}, with the same guarantees: + *

    + *
  • verified: the host first reports the size and the SHA-256 digest of the file, and + * the received bytes must carry that digest — a file modified during the download fails; + *
  • no wasted transfer: when the local file already has that digest, nothing is + * transferred; + *
  • atomic: the content goes to a temporary file next to the destination + * ({@code ..part}), which is flushed to disk, then moved onto the destination in + * one step. The destination is never seen truncated: a failure or a timeout leaves it as it + * was, and the temporary file is deleted as the transfer stops. + *
+ * When {@code localFile} is an existing directory, the file is written into it under its remote + * name, like {@code cp}; the destination's directory is created when needed. Memory is bounded + * whatever the size of the file. The {@link #offset(long)}, {@link #length(long)} and + * {@link #maxBytes(long)} settings do not apply. + *

+ * The timeout is a wall-clock deadline for the whole download, and the transfer runs at about + * 1.5 MB/s: a large file needs a raised {@link #timeout(Duration)}. Downloads are not + * resumable — one that fails or times out starts over. + * + * @param localFile the local file to write, or an existing directory to write the file into + * @return the number of bytes transferred: the size of the file, or 0 when the local file + * already had the same content + * @throws WinRMClientException when the file cannot be read, the integrity check fails, or the + * local file cannot be written + * @throws WinRMTimeoutException when the timeout elapses first; the message tells how much was + * transferred + */ + public long downloadTo(final Path localFile) { + Utils.checkNonNull(localFile, "localFile"); + final AtomicLong size = new AtomicLong(-1); + final AtomicLong transferred = new AtomicLong(); + try { + return blocking(() -> { + try { + return download(localFile, size, transferred); + } catch (final IOException e) { + throw new WinRMClientException( + String.format("Download of %s from %s failed: %s", path, client.hostname(), e), + e + ); + } + }); + } catch (final WinRMTimeoutException e) { + throw new WinRMTimeoutException( + String.format( + "Download of %s from %s timed out after %s, %s: raise the timeout for large files", + path, + client.hostname(), + timeout, + size.get() < 0 + ? "while the host computed its digest" + : transferred.get() + " of " + size.get() + " bytes transferred" + ), + e + ); + } + } + + /** + * Probe the size and the digest of the remote file, skip the transfer when the local file + * already has that digest, else stream the file into a temporary file next to the destination, + * verify it, flush it to disk, and move it onto the destination. + * + * @param localFile the local file, or an existing directory to write the file into + * @param size receives the size of the remote file, for the timeout message + * @param transferred counts the bytes transferred, for the timeout message + * @return the number of bytes transferred + */ + private long download(final Path localFile, final AtomicLong size, final AtomicLong transferred) + throws IOException, NoSuchAlgorithmException { + final Path target = Files.isDirectory(localFile) + ? localFile.resolve(path.replaceFirst(".*[\\\\/:]", "")) : localFile; + + final ByteBuffer probe; + try (InputStream in = RemoteFiles.open(client, path, RemoteFiles.probeScript(path), timeout)) { + probe = ByteBuffer.wrap(in.readAllBytes()).order(ByteOrder.LITTLE_ENDIAN); + } + if (probe.remaining() != RemoteFiles.PROBE_LENGTH) { + throw new WinRMClientException( + String.format("Unexpected output while probing remote file %s on %s", path, client.hostname()) + ); + } + size.set(probe.getLong()); + final byte[] digest = new byte[probe.remaining()]; + probe.get(digest); + + if (Files.isRegularFile(target) && Files.size(target) == size.get()) { + try (InputStream in = Files.newInputStream(target)) { + if (MessageDigest.isEqual(copy(in, OutputStream.nullOutputStream(), new AtomicLong()), digest)) { + return 0; + } + } + } + + final Path directory = target.toAbsolutePath().getParent(); + if (directory == null) { + // A root that is not a directory, e.g. a drive with no media + throw new NoSuchFileException(target.toString()); + } + // Not unconditionally: before Java 21, createDirectories() rejects an existing symbolic link + // to a directory (e.g. /tmp on macOS). + if (!Files.isDirectory(directory)) { + Files.createDirectories(directory); + } + final Path part = directory.resolve( + target.getFileName() + "." + Long.toHexString(ThreadLocalRandom.current().nextLong()) + ".part" + ); + try { + try ( + InputStream in = RemoteFiles.open(client, path, RemoteFiles.readScript(path, 0, -1), timeout); + FileChannel out = FileChannel.open(part, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)) { + if (!MessageDigest.isEqual(copy(in, Channels.newOutputStream(out), transferred), digest)) { + throw new WinRMClientException( + String.format( + "Integrity check failed after downloading %s from %s to %s (%d bytes received, %d expected): " + + "the file changed during the transfer, or its content was corrupted", + path, + client.hostname(), + target, + transferred.get(), + size.get() + ) + ); + } + // Interruptible like every channel operation: a download cancelled by its deadline + // fails here, before the move. + out.force(true); + } + Files.move(part, target, StandardCopyOption.ATOMIC_MOVE); + return transferred.get(); + } catch (final IOException | RuntimeException e) { + try { + Files.deleteIfExists(part); + } catch (final IOException ignored) { + // Best effort: the failure that triggered the cleanup matters more + } + throw e; + } + } + + /** + * Copy a stream with bounded memory, counting the bytes copied. + * + * @param in the stream to copy + * @param out where to copy it + * @param copied counts the bytes copied + * @return the SHA-256 digest of the bytes copied + */ + private static byte[] copy(final InputStream in, final OutputStream out, final AtomicLong copied) + throws IOException, NoSuchAlgorithmException { + final MessageDigest sha256 = MessageDigest.getInstance("SHA-256"); + final byte[] buffer = new byte[RemoteFiles.BLOCK_SIZE]; + for (int n = in.read(buffer); n != -1; n = in.read(buffer)) { + sha256.update(buffer, 0, n); + out.write(buffer, 0, n); + copied.addAndGet(n); + } + return sha256.digest(); + } + /** * Compute the digest of the whole file on the host — nothing but the digest is transferred. * The {@link #offset(long)} and {@link #length(long)} settings do not apply. diff --git a/src/main/java/org/metricshub/winrm/RemoteFiles.java b/src/main/java/org/metricshub/winrm/RemoteFiles.java index 1f2c929..73261a4 100644 --- a/src/main/java/org/metricshub/winrm/RemoteFiles.java +++ b/src/main/java/org/metricshub/winrm/RemoteFiles.java @@ -214,6 +214,18 @@ private RemoteFiles() {} private static final String DIGEST = "try{$h=[Security.Cryptography.HashAlgorithm]::Create('%s');" + "[Console]::Out.WriteLine([Convert]::ToBase64String($h.ComputeHash($f)))}catch{fail $_.Exception}finally{$f.Close()}"; + /** + * What a download is verified against, as one base64 line of {@link #PROBE_LENGTH} bytes: the + * size of the file (8 bytes, little-endian), then its SHA-256 digest. + */ + private static final String PROBE = "try{$h=[Security.Cryptography.HashAlgorithm]::Create('SHA256');" + + "[Console]::Out.WriteLine([Convert]::ToBase64String([byte[]]([BitConverter]::GetBytes($f.Length)+$h.ComputeHash($f))))}" + + + "catch{fail $_.Exception}finally{$f.Close()}"; + + /** The length of the {@link #probeScript(String)} output: an 8-byte size and a 32-byte SHA-256 digest. */ + static final int PROBE_LENGTH = 40; + /** * Build the script writing a byte range of the file. * @@ -243,6 +255,16 @@ static String digestScript(final String path, final String algorithm) { return openFile(path) + String.format(DIGEST, algorithm); } + /** + * Build the script writing the size and the SHA-256 digest of the file (see {@link #PROBE}). + * + * @param path the remote file + * @return the PowerShell script + */ + static String probeScript(final String path) { + return openFile(path) + PROBE; + } + private static String openFile(final String path) { return String.format(OPEN_FILE, base64(path)); } @@ -431,6 +453,10 @@ static InputStream open(final WinRMClient client, final String path, final Strin * Start a script. It must fit the command line: {@code powerShell(...)} would transparently * fall back to uploading it as a file, and file access must stay read-only on the host (no * files written, no certutil), so a script too long is refused instead. + *

+ * A start rejected by the host's operation quota ran nothing (see + * {@link ShellFileCopy#isRetryableQuotaRejection(Exception)}): it is retried with the escalating + * delays of the file transfers, as long as the timeout has not elapsed since the first attempt. * * @param client the client to run the script on * @param path the remote path, for the error messages @@ -448,7 +474,23 @@ static RemoteProcess start(final WinRMClient client, final String path, final St ) ); } - return client.powerShell(script).timeout(timeout).start(); + final long begin = Utils.getCurrentTimeMillis(); + for (int attempt = 0;; attempt++) { + try { + return client.powerShell(script).timeout(timeout).start(); + } catch (final WinRMClientException e) { + final long remaining = WinRMClient.toMillis(timeout) - (Utils.getCurrentTimeMillis() - begin); + if (attempt >= ShellFileCopy.QUOTA_RETRIES || remaining <= 0 || !ShellFileCopy.isRetryableQuotaRejection(e)) { + throw e; + } + try { + Utils.sleep(Math.min(ShellFileCopy.QUOTA_RETRY_DELAY_MILLIS * (attempt + 1), remaining)); + } catch (final InterruptedException interrupted) { + Thread.currentThread().interrupt(); + throw e; + } + } + } } /** diff --git a/src/main/java/org/metricshub/winrm/ShellFileCopy.java b/src/main/java/org/metricshub/winrm/ShellFileCopy.java index 388ce89..e6f7607 100644 --- a/src/main/java/org/metricshub/winrm/ShellFileCopy.java +++ b/src/main/java/org/metricshub/winrm/ShellFileCopy.java @@ -114,14 +114,14 @@ private ShellFileCopy() {} private static final String FAULT_OPERATION_QUOTA = "2150859174"; /** How many times a transfer command is retried after an operation-quota rejection. */ - private static final int QUOTA_RETRIES = 4; + static final int QUOTA_RETRIES = 4; /** * Base delay before retrying after an operation-quota rejection; each retry waits one step * longer. Measured on Windows 2008 R2 (quota 15 per user): the budget fully recovers within * 30 seconds, so the escalating delays (5+10+15+20 s) comfortably bridge it. */ - private static final long QUOTA_RETRY_DELAY_MILLIS = 5_000L; + static final long QUOTA_RETRY_DELAY_MILLIS = 5_000L; /** * Copy the specified local files to a temporary directory on the remote host through the diff --git a/src/main/java/org/metricshub/winrm/WinRMClient.java b/src/main/java/org/metricshub/winrm/WinRMClient.java index c17611c..2816ff9 100644 --- a/src/main/java/org/metricshub/winrm/WinRMClient.java +++ b/src/main/java/org/metricshub/winrm/WinRMClient.java @@ -219,6 +219,27 @@ public void uploadFile(final Path localFile, final String remoteFile) { } } + /** + * Copy a file from the remote host to a local file, through the WinRM connection itself — the + * counterpart of {@link #uploadFile(Path, String)}, and a shorthand for + * {@code file(remoteFile).downloadTo(localFile)}. The transfer is digest-verified, skipped when + * the local file already has identical content, and atomic: the local file is replaced in one + * step, never left half-written. When {@code localFile} is an existing directory, the file is + * written into it under its remote name. The client's timeout applies, as a wall-clock deadline: + * raise it for large files (see {@link RemoteFile#downloadTo(Path)}). + * + * @param remoteFile the absolute path of the file on the remote host, e.g. + * {@code C:\Windows\Temp\collect.log} + * @param localFile the local file to write, or an existing directory to write the file into + * @return the number of bytes transferred: the size of the file, or 0 when the local file + * already had the same content + * @throws org.metricshub.winrm.exceptions.WinRMTimeoutException when the timeout elapses first + * @throws org.metricshub.winrm.exceptions.WinRMClientException for any other failure + */ + public long downloadFile(final String remoteFile, final Path localFile) { + return file(remoteFile).downloadTo(localFile); + } + /** * Get the hostname this client connects to. * diff --git a/src/site/markdown/file-transfers.md b/src/site/markdown/file-transfers.md index 8bc5f73..543e9f1 100644 --- a/src/site/markdown/file-transfers.md +++ b/src/site/markdown/file-transfers.md @@ -1,14 +1,16 @@ -keywords: file transfer, file copy, upload, certutil, base64, digest, content-addressed, temporary files -description: How the WinRM Java Client copies local files to the remote host through the WinRM channel itself — destination paths, temporary files, integrity verification, and command-line substitution. +keywords: file transfer, file copy, upload, download, certutil, base64, digest, content-addressed, temporary files, atomic +description: How the WinRM Java Client copies files to and from the remote host through the WinRM channel itself — destination paths, temporary files, integrity verification, atomic downloads, and command-line substitution. # File Transfers -The client can copy local files to the remote host **through the WinRM connection itself** — no -SMB, no TCP port 445, no administrative share — so it works from any client OS and needs no port -beyond the WinRM one. This page explains exactly how the transfer works: where files land, which -temporary files are created, how integrity is guaranteed, and how the command line is rewritten. +The client can copy local files to the remote host, and remote files back, **through the WinRM +connection itself** — no SMB, no TCP port 445, no administrative share — so it works from any +client OS and needs no port beyond the WinRM one. This page explains exactly how the transfers +work: where files land, which temporary files are created, how integrity is guaranteed, and how +the command line is rewritten. [Downloading a file](#downloading-a-file) covers the other +direction. ## The two entry points @@ -175,8 +177,85 @@ Notes: destination directory, and the host must provide `certutil` (transfer) and `forfiles` (housekeeping). See [Preparing the Windows Host](preparing-the-host.html). +## Downloading a file + +[`WinRMClient.downloadFile(...)`](apidocs/org/metricshub/winrm/WinRMClient.html) copies a remote +file to a local one — the counterpart of `uploadFile(...)`, with the same guarantees: the content +is verified, an identical copy is not transferred again, and a failure never leaves a truncated +file behind. The same terminal exists on the per-path request, where the timeout can be set: + +```java +client.downloadFile("C:\\Windows\\Temp\\collect.log", Path.of("collect.log")); + +long bytes = client.file("D:\\exports\\big.csv") + .timeout(Duration.ofMinutes(10)) + .downloadTo(Path.of("big.csv")); // bytes transferred, 0 when the local copy was already identical +``` + +When the local path is an existing directory, the file is written into it under its remote name, +like `cp`: `downloadFile("C:\\Windows\\Temp\\collect.log", Path.of("logs"))` writes +`logs/collect.log`. The destination's directory is created when needed. + +### How a download works + +1. **Probe.** One PowerShell invocation on the host reports the size and the SHA-256 digest of + the file. It opens the file exactly like the [remote file reads](files.html) do, with a share + mode that tolerates other writers — so a log held open by a running service can be downloaded + (`certutil -hashfile`, which the uploads use, fails on such a file with a sharing violation). +2. **Skip check.** If the local destination already exists with the same size and digest, nothing + is transferred: the download returns 0. +3. **Transfer.** The file is read with [`openStream()`](files.html#streaming-large-files) and + written, block by block, to a temporary file **next to the destination**: + `..part`. Memory stays bounded whatever the size of the file — a 64 MiB file was + downloaded by a JVM limited to a 32 MiB heap. +4. **Verify and publish.** The digest of the received bytes must match the probed one; the + temporary file is then flushed to disk (`fsync`) and moved onto the destination in one atomic + step (`ATOMIC_MOVE`), replacing any previous file. + +**The destination is never seen truncated or half-written**: until the final move it keeps its +previous content (or does not exist), and after it, it has the complete, verified content. On any +failure — a digest mismatch, a read error, a timeout — the destination is left as it was and the +temporary file is deleted as the transfer stops; only a process killed in the middle of a download +can leave a `.part` file behind. A file modified on the host between the probe and the end of the +transfer (a log being appended to, for instance) fails the integrity check: the download reports +it rather than delivering a torn copy. + +Downloads are **not resumable**: a download that fails or times out starts over from the first +byte next time. Resuming would mean tracking verified byte ranges across attempts and proving +that the remote file did not change in between — more machinery than the speed of this transport +justifies. + +The file is read-only on the host: a download writes nothing there, and needs what the remote file +reads need — PowerShell 2.0 or later in `FullLanguage` mode, and read access to the file (see +[Remote Files](files.html#errors-and-requirements)). + +### Timeout + +The timeout of a download is a **wall-clock deadline for the whole transfer**: the client's +timeout for `downloadFile(...)` (30 seconds by default), or `timeout(Duration)` on the request. +A large file needs a raised timeout — at the speed below, 30 seconds is about 40 MB. When the +deadline fires, the exception says how far the transfer got: + +```text +Download of D:\exports\big.csv from server01 timed out after PT30S, 41943040 of 104857600 bytes +transferred: raise the timeout for large files +``` + +### Download performance + +Measured over HTTP with NTLM encryption, a download runs at about **1.35–1.45 MB/s**: 20 MiB in +14.7 seconds on Windows Server 2022, 15.6 seconds on Windows Server 2008 R2 (PowerShell 2.0), and +64 MiB in 46 seconds on 2022 — probe included. A small file costs under a second (two PowerShell +invocations: the probe and the read), and skipping an identical copy about 0.4 seconds. The limit +is on the host, in the way the WinRM service forwards a command's output — see +[Read performance](files.html#read-performance). + +That is one to two orders of magnitude slower than SMB on a local network: **downloads are not a +bulk transport either**. They suit logs, configuration files and command results; to move +gigabytes, use SMB (or any file-transfer protocol the host offers). + ## See also -* [Remote Files](files.html) — the other direction: reading and listing remote files through the WinRM channel +* [Remote Files](files.html) — reading remote files (whole, byte ranges, streams) and listing directories through the WinRM channel * [Remote Commands](commands.html) — the command builder that carries the transfer * [Preparing the Windows Host](preparing-the-host.html) — the privileges a transfer needs diff --git a/src/site/markdown/files.md b/src/site/markdown/files.md index 43e9ca0..0279996 100644 --- a/src/site/markdown/files.md +++ b/src/site/markdown/files.md @@ -1,5 +1,5 @@ -keywords: remote file, read file, byte range, tail, digest, base64, powershell, stream, directory listing, file properties, exists, glob -description: How the WinRM Java Client reads files and lists directories on the remote host through the WinRM channel itself — whole files, byte ranges, tails, streams, digests, file properties, and filtered recursive listings. +keywords: remote file, read file, download, byte range, tail, digest, base64, powershell, stream, directory listing, file properties, exists, glob +description: How the WinRM Java Client reads files and lists directories on the remote host through the WinRM channel itself — whole files, byte ranges, tails, streams, downloads, digests, file properties, and filtered recursive listings. # Remote Files @@ -75,11 +75,26 @@ it holds the client's connection until it reaches its end or is closed, and clos the remote read. The file is opened before `openStream()` returns, so a missing file fails there; a failure midway is reported by `read()`, never as a silently short read. +## Downloading to a local file + +`downloadTo(Path)` writes the whole file to a local file — digest-verified, skipped when the local +copy is already identical, and atomic: the destination is replaced in one step, never left +half-written. `client.downloadFile(remote, local)` does the same with the client's timeout: + +```java +long bytes = client.file("C:\\Windows\\Temp\\collect.log").downloadTo(Path.of("collect.log")); +``` + +It streams, so memory stays bounded whatever the size of the file, and its timeout is a +wall-clock deadline for the whole transfer. The mechanics, the guarantees and the measured speed +are described in [File Transfers](file-transfers.html#downloading-a-file). + ## Timeouts -The blocking terminals (`readBytes`, `readText`, `digest`, `info`, `exists`, and `execute()` on a -[directory listing](#listing-a-directory)) run under a **wall-clock deadline**: the client's -timeout, or `timeout(Duration)` on the request — raise it for large reads and big trees. +The blocking terminals (`readBytes`, `readText`, `downloadTo`, `digest`, `info`, `exists`, and +`execute()` on a [directory listing](#listing-a-directory)) run under a **wall-clock deadline**: +the client's timeout, or `timeout(Duration)` on the request — raise it for large reads, downloads +and big trees. `openStream()`/`openReader()` and a listing's `stream()` use the **inactivity** semantics of the other streaming terminals: the timeout bounds the silence between two blocks, not the whole operation. See [Timeouts and Errors](timeouts-and-errors.html). @@ -92,6 +107,10 @@ operation. See [Timeouts and Errors](timeouts-and-errors.html). * Every failure is a `WinRMClientException` naming its cause: path not found, a directory where a file is expected (or a file given to `list()`), access denied, sharing violation, PowerShell not available, or PowerShell in Constrained Language Mode. +* A script rejected at startup by the host's per-user operation quota (as low as 15 concurrent + operations on Windows Server 2008 R2) has not run: it is retried with escalating delays (5, 10, + 15, 20 seconds) for as long as the timeout allows, like the steps of a + [file transfer](file-transfers.html). * The host needs **PowerShell 2.0 or later in `FullLanguage` mode** (Windows Server 2008 R2 and later ship it). When AppLocker or WDAC puts PowerShell in Constrained Language Mode, the .NET calls the scripts rely on are blocked: remote file access is then not available. @@ -255,6 +274,6 @@ DMTF strings. ## See also -* [File Transfers](file-transfers.html) — the other direction: copying local files to the host +* [File Transfers](file-transfers.html) — copying local files to the host, and how downloads work * [Remote Commands](commands.html) — the command shell the reads and listings ride * [WQL Queries](wql.html) — the WMI alternative for file metadata diff --git a/src/site/markdown/index.md b/src/site/markdown/index.md index 060e4ab..69f87b1 100644 --- a/src/site/markdown/index.md +++ b/src/site/markdown/index.md @@ -15,8 +15,9 @@ The **WinRM Java Client** is a small library that talks to the Windows Remote Ma * **execute remote commands** — `cmd.exe` command lines or PowerShell scripts — capturing standard output, standard error and the exit code, optionally copying local script files to the host first ([Remote Commands](commands.html)), and -* **access remote files** — read a file (whole, a byte range, a tail), get its properties, or - list a directory with filters evaluated on the host ([Remote Files](files.html)). +* **access remote files** — read a file (whole, a byte range, a tail), download it to a local + file, get its properties, or list a directory with filters evaluated on the host + ([Remote Files](files.html)). All of them can also **stream**: WQL rows are consumed page by page as they arrive (`stream()`), command output is consumed while the command is still running (`start()`, @@ -134,7 +135,7 @@ remain available and unchanged, with their checked exceptions. and the privileges the account needs * [WQL Queries](wql.html) — query WMI and read the result * [Remote Commands](commands.html) — run commands and copy files to the host -* [File Transfers](file-transfers.html) — how files are copied through the WinRM channel +* [File Transfers](file-transfers.html) — how files are copied to the host and downloaded back through the WinRM channel * [Remote Files](files.html) — read remote files (whole, byte ranges, tails, streams, digests), get file properties, list directories * [Command-Line Client](cli.html) — the standalone jar's manual page * [Authentication](authentication.html) — NTLM and Kerberos diff --git a/src/site/markdown/migrating-from-winrm4j.md b/src/site/markdown/migrating-from-winrm4j.md index 23dba16..c70b228 100644 --- a/src/site/markdown/migrating-from-winrm4j.md +++ b/src/site/markdown/migrating-from-winrm4j.md @@ -181,7 +181,7 @@ Once on the fluent API, features winrm4j never offered are one call away: * **File transfers** — `upload(Path...)` copies local scripts to the host through the WinRM channel itself (no SMB, no port 445) before the command runs ([File Transfers](file-transfers.html)). * **Remote files** — `client.file(path)` reads a file (whole, a byte range, a tail, or streamed), - returns its properties, or lists a directory with filters evaluated on the host + downloads it to a local file, returns its properties, or lists a directory with filters evaluated on the host ([Remote Files](files.html)). * **Standard input** — `stdin(...)` feeds a remote command its input, with real EOF semantics ([Standard input](commands.html#standard-input)). diff --git a/src/site/markdown/preparing-the-host.md b/src/site/markdown/preparing-the-host.md index b2035a6..56ac03d 100644 --- a/src/site/markdown/preparing-the-host.md +++ b/src/site/markdown/preparing-the-host.md @@ -272,7 +272,7 @@ separately: | **Remote commands** (`client.command(...)`) | Remote access to the listener, remote shell access on the host (`AllowRemoteShellAccess`, `True` by default), and whatever rights **the command itself** needs once it runs. | | **Transfer-and-run** (`upload(...)`) | Both of the above, plus write access to `\Temp\winrm-upload-`, and `certutil` and `forfiles` present on the host. See [File Transfers](file-transfers.html). | | **`uploadFile(...)`** to an explicit path | Remote shell access, write access to the destination directory, and `certutil` on the host (the same transfer engine, minus the transfer directory and its `forfiles` housekeeping). | -| **Remote file access** (`client.file(...)`: reads, `info()`, `list()`) | Remote shell access, **PowerShell 2.0 or later in `FullLanguage` mode** (not constrained by AppLocker or WDAC), and read access to the files and directories themselves. Nothing is written on the host. See [Remote Files](files.html). | +| **Remote file access** (`client.file(...)`: reads, downloads, `info()`, `list()`) | Remote shell access, **PowerShell 2.0 or later in `FullLanguage` mode** (not constrained by AppLocker or WDAC), and read access to the files and directories themselves. Nothing is written on the host. See [Remote Files](files.html). | So an account can perfectly well run WQL queries and fail to run commands, or the reverse. When diagnosing, test the two independently — as in [Checking from the client](#Checking_from_the_client) diff --git a/src/site/markdown/timeouts-and-errors.md b/src/site/markdown/timeouts-and-errors.md index 40a2792..fdfac69 100644 --- a/src/site/markdown/timeouts-and-errors.md +++ b/src/site/markdown/timeouts-and-errors.md @@ -24,8 +24,8 @@ try (WinRMClient client = WinRMClient.builder("server.example.com") ``` The timeout is a **wall-clock deadline for the whole operation**: it covers authentication (on the -first operation), every WSMan round trip, and — for commands — the file uploads, all budgeted -against the same deadline. When it elapses, the operation +first operation), every WSMan round trip, and — for commands — the file uploads (for a file +download: the whole transfer), all budgeted against the same deadline. When it elapses, the operation fails with [`WinRMTimeoutException`](apidocs/org/metricshub/winrm/exceptions/WinRMTimeoutException.html) and no part of it (in particular: the command itself) runs afterward. diff --git a/src/test/java/org/metricshub/winrm/RemoteFileTest.java b/src/test/java/org/metricshub/winrm/RemoteFileTest.java index 5e835d4..b3afbde 100644 --- a/src/test/java/org/metricshub/winrm/RemoteFileTest.java +++ b/src/test/java/org/metricshub/winrm/RemoteFileTest.java @@ -22,26 +22,39 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.metricshub.winrm.light.FakeWsmanResponses.commandResponse; import static org.metricshub.winrm.light.FakeWsmanResponses.done; import static org.metricshub.winrm.light.FakeWsmanResponses.envelope; +import static org.metricshub.winrm.light.FakeWsmanResponses.fault; import static org.metricshub.winrm.light.FakeWsmanResponses.receiveResponse; import static org.metricshub.winrm.light.FakeWsmanResponses.resourceCreated; import static org.metricshub.winrm.light.FakeWsmanResponses.signalResponse; import static org.metricshub.winrm.light.FakeWsmanResponses.stream; import java.io.BufferedReader; +import java.io.IOException; import java.io.InputStream; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.MessageDigest; import java.time.Duration; +import java.util.Arrays; import java.util.Base64; +import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.metricshub.winrm.exceptions.WinRMClientException; import org.metricshub.winrm.exceptions.WinRMTimeoutException; import org.metricshub.winrm.light.FakeWsmanServer; @@ -91,7 +104,13 @@ private static String stdout(final String text) { /** Script a whole read: shell creation, command, the given Receive chunks (the last one completes), Signal. */ private void enqueueRead(final int exitCode, final String stderr, final String... chunks) { - server.enqueue(200, envelope(resourceCreated("SHELL-1"))).enqueue(200, envelope(commandResponse(COMMAND_ID))); + server.enqueue(200, envelope(resourceCreated("SHELL-1"))); + enqueueCommand(exitCode, stderr, chunks); + } + + /** Script a command on the existing shell: command, the given Receive chunks (the last one completes), Signal. */ + private void enqueueCommand(final int exitCode, final String stderr, final String... chunks) { + server.enqueue(200, envelope(commandResponse(COMMAND_ID))); for (int i = 0; i < chunks.length; i++) { final boolean last = i == chunks.length - 1; final String streams = stdout(chunks[i]) + @@ -101,17 +120,43 @@ private void enqueueRead(final int exitCode, final String stderr, final String.. server.enqueue(200, envelope(signalResponse())); } - /** The PowerShell script the client sent, decoded from its -EncodedCommand. */ + /** The PowerShell script the client sent first, decoded from its -EncodedCommand. */ private String sentScript() { - final String command = server + final List scripts = sentScripts(); + assertFalse(scripts.isEmpty(), "no PowerShell command sent"); + return scripts.get(0); + } + + /** The PowerShell scripts the client sent, in order, decoded from their -EncodedCommand. */ + private List sentScripts() { + final Pattern encoded = Pattern.compile("-EncodedCommand ([A-Za-z0-9+/=]+)"); + return server .decryptedRequests() .stream() - .filter(r -> r.contains("-EncodedCommand")) - .findFirst() - .orElseThrow(() -> new AssertionError("no PowerShell command sent")); - final Matcher matcher = Pattern.compile("-EncodedCommand ([A-Za-z0-9+/=]+)").matcher(command); - assertTrue(matcher.find(), command); - return new String(Base64.getDecoder().decode(matcher.group(1)), StandardCharsets.UTF_16LE); + .map(encoded::matcher) + .filter(Matcher::find) + .map(m -> new String(Base64.getDecoder().decode(m.group(1)), StandardCharsets.UTF_16LE)) + .collect(Collectors.toList()); + } + + /** The probe output announcing the given content: its size (8 bytes, little-endian), then its SHA-256 digest. */ + private static String probe(final byte[] content) throws Exception { + return b64( + ByteBuffer + .allocate(RemoteFiles.PROBE_LENGTH) + .order(ByteOrder.LITTLE_ENDIAN) + .putLong(content.length) + .put(MessageDigest.getInstance("SHA-256").digest(content)) + .array() + ) + + "\r\n"; + } + + /** The files in the directory, sorted. */ + private static List files(final Path directory) throws IOException { + try (Stream files = Files.list(directory)) { + return files.sorted().collect(Collectors.toList()); + } } @Test @@ -120,8 +165,8 @@ void readBytesDecodesLinesSplitAcrossReceiveChunks() { for (int i = 0; i < content.length; i++) { content[i] = (byte) i; } - final String line1 = b64(java.util.Arrays.copyOfRange(content, 0, 150)); - final String line2 = b64(java.util.Arrays.copyOfRange(content, 150, 300)); + final String line1 = b64(Arrays.copyOfRange(content, 0, 150)); + final String line2 = b64(Arrays.copyOfRange(content, 150, 300)); // The first line is cut in the middle by the protocol chunking. enqueueRead(0, null, line1.substring(0, 17), line1.substring(17) + "\r\n" + line2 + "\r\n"); @@ -279,4 +324,138 @@ void theBlockingTerminalsHonorTheWallClockTimeout() { assertThrows(WinRMTimeoutException.class, file::readBytes); } } + + @Test + void aStartRejectedByTheOperationQuotaIsRetried() { + // The Command is rejected by the host's operation quota before anything runs: retried after 5 s. + server + .enqueue(200, envelope(resourceCreated("SHELL-1"))) + .enqueue( + 500, + fault("2150859174", "The maximum number of concurrent operations for this user has been exceeded.") + ); + enqueueCommand(0, null, b64("ok".getBytes(StandardCharsets.US_ASCII)) + "\r\n"); + try (WinRMClient client = client()) { + assertEquals("ok", client.file(PATH).readText(StandardCharsets.US_ASCII)); + } + assertEquals(2, sentScripts().size()); + } + + @Test + void downloadWritesTheVerifiedContent(@TempDir final Path directory) throws Exception { + final byte[] content = new byte[300]; + for (int i = 0; i < content.length; i++) { + content[i] = (byte) i; + } + enqueueRead(0, null, probe(content)); + enqueueCommand( + 0, + null, + b64(Arrays.copyOfRange(content, 0, 150)) + "\r\n", + b64(Arrays.copyOfRange(content, 150, 300)) + "\r\n" + ); + final Path local = directory.resolve("copy.bin"); + try (WinRMClient client = client()) { + // The range settings do not apply: the whole file is downloaded. + assertEquals(content.length, client.file(PATH).offset(-10).length(5).downloadTo(local)); + } + assertArrayEquals(content, Files.readAllBytes(local)); + assertEquals(List.of(local), files(directory)); + final List scripts = sentScripts(); + assertTrue(scripts.get(0).contains("[BitConverter]::GetBytes($f.Length)"), scripts.get(0)); + assertTrue(scripts.get(1).contains("$n=[long]0;$l=[long]-1"), scripts.get(1)); + } + + @Test + void anIdenticalLocalFileIsNotDownloadedAgain(@TempDir final Path directory) throws Exception { + final byte[] content = "already here".getBytes(StandardCharsets.US_ASCII); + final Path local = Files.write(directory.resolve("copy.bin"), content); + enqueueRead(0, null, probe(content)); + try (WinRMClient client = client()) { + assertEquals(0, client.downloadFile(PATH, local)); + } + // The probe only. + assertEquals(1, sentScripts().size()); + assertArrayEquals(content, Files.readAllBytes(local)); + } + + @Test + void aDirectoryDestinationReceivesTheRemoteFileName(@TempDir final Path directory) throws Exception { + final byte[] content = { 1, 2, 3 }; + enqueueRead(0, null, probe(content)); + enqueueCommand(0, null, b64(content) + "\r\n"); + try (WinRMClient client = client()) { + assertEquals(3, client.downloadFile(PATH, directory)); + } + assertArrayEquals(content, Files.readAllBytes(directory.resolve("collect.bin"))); + } + + @Test + void aDigestMismatchFailsWithoutWritingTheDestination(@TempDir final Path directory) throws Exception { + // The file changed between the probe and the read. + enqueueRead(0, null, probe("expected".getBytes(StandardCharsets.US_ASCII))); + enqueueCommand(0, null, b64("modified".getBytes(StandardCharsets.US_ASCII)) + "\r\n"); + final Path local = directory.resolve("copy.bin"); + try (WinRMClient client = client()) { + final RemoteFile file = client.file(PATH); + final WinRMClientException e = assertThrows(WinRMClientException.class, () -> file.downloadTo(local)); + assertTrue(e.getMessage().contains("Integrity check failed"), e.getMessage()); + } + assertEquals(List.of(), files(directory)); + } + + @Test + void aMalformedProbeIsReported(@TempDir final Path directory) { + enqueueRead(0, null, b64(new byte[] { 1, 2, 3 }) + "\r\n"); + try (WinRMClient client = client()) { + final RemoteFile file = client.file(PATH); + final WinRMClientException e = assertThrows(WinRMClientException.class, () -> file.downloadTo(directory)); + assertTrue(e.getMessage().contains("Unexpected output while probing"), e.getMessage()); + } + } + + @Test + void aLocalWriteFailureNamesTheDownload(@TempDir final Path directory) throws Exception { + // The destination's "directory" is a regular file. + final Path local = Files.write(directory.resolve("not-a-directory"), new byte[0]).resolve("copy.bin"); + enqueueRead(0, null, probe(new byte[] { 1 })); + try (WinRMClient client = client()) { + final RemoteFile file = client.file(PATH); + final WinRMClientException e = assertThrows(WinRMClientException.class, () -> file.downloadTo(local)); + assertTrue(e.getMessage().startsWith("Download of " + PATH + " from 127.0.0.1 failed: "), e.getMessage()); + assertTrue(e.getCause() instanceof IOException, String.valueOf(e.getCause())); + } + } + + @Test + void aTimeoutMidTransferLeavesTheDestinationAsItWas(@TempDir final Path directory) throws Exception { + final byte[] content = new byte[1000]; + final byte[] previous = "previous".getBytes(StandardCharsets.US_ASCII); + final Path local = Files.write(directory.resolve("copy.bin"), previous); + enqueueRead(0, null, probe(content)); + server + .enqueue(200, envelope(commandResponse(COMMAND_ID))) + .enqueue(200, envelope(receiveResponse(stdout(b64(new byte[600]) + "\r\n"), null))) + .enqueueDelayed( + 200, + envelope(receiveResponse(stdout(b64(new byte[400]) + "\r\n"), done(COMMAND_ID, 0))), + 4_000 + ) + .enqueue(200, envelope(signalResponse())); + try (WinRMClient client = client()) { + final RemoteFile file = client.file(PATH).timeout(Duration.ofSeconds(2)); + final WinRMTimeoutException e = assertThrows(WinRMTimeoutException.class, () -> file.downloadTo(local)); + assertTrue(e.getMessage().contains("600 of 1000 bytes transferred"), e.getMessage()); + assertArrayEquals(previous, Files.readAllBytes(local)); + + // The abandoned transfer stops at its next step, deleting its temporary file — and even + // with all the content received by then, it never replaces the destination. + final long deadline = System.nanoTime() + 10_000_000_000L; + while (!files(directory).equals(List.of(local))) { + assertTrue(System.nanoTime() < deadline, () -> "Temporary file left behind: " + directory); + Thread.sleep(50); + } + } + assertArrayEquals(previous, Files.readAllBytes(local)); + } } diff --git a/src/test/java/org/metricshub/winrm/RemoteFilesScriptTest.java b/src/test/java/org/metricshub/winrm/RemoteFilesScriptTest.java index 244fa16..78b3cfe 100644 --- a/src/test/java/org/metricshub/winrm/RemoteFilesScriptTest.java +++ b/src/test/java/org/metricshub/winrm/RemoteFilesScriptTest.java @@ -28,6 +28,8 @@ import java.io.ByteArrayOutputStream; import java.io.FileOutputStream; import java.io.RandomAccessFile; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; import java.nio.channels.FileLock; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -206,6 +208,7 @@ void emptyFile() throws Exception { final Outcome outcome = run(RemoteFiles.readScript(empty.toString(), 0, -1)); assertEquals(0, outcome.exitCode); assertEquals(0, outcome.bytes.length); + assertArrayEquals(probe(new byte[0]), run(RemoteFiles.probeScript(empty.toString())).bytes); } @Test @@ -218,9 +221,31 @@ void aFileOpenForWritingByAnotherProcessCanBeRead() throws Exception { "line 1\r\n".getBytes(StandardCharsets.US_ASCII), run(RemoteFiles.readScript(log.toString(), 0, -1)).bytes ); + // The download probe too — certutil -hashfile fails on such a file with a sharing violation. + assertArrayEquals( + probe("line 1\r\n".getBytes(StandardCharsets.US_ASCII)), + run(RemoteFiles.probeScript(log.toString())).bytes + ); } } + /** The expected probe output: the size (8 bytes, little-endian), then the SHA-256 digest. */ + private static byte[] probe(final byte[] bytes) throws Exception { + return ByteBuffer + .allocate(RemoteFiles.PROBE_LENGTH) + .order(ByteOrder.LITTLE_ENDIAN) + .putLong(bytes.length) + .put(MessageDigest.getInstance("SHA-256").digest(bytes)) + .array(); + } + + @Test + void probeReportsTheSizeAndTheDigest() throws Exception { + final Outcome outcome = run(RemoteFiles.probeScript(file.toString())); + assertEquals(0, outcome.exitCode); + assertArrayEquals(probe(content), outcome.bytes); + } + @Test void failuresMapToTheDocumentedExitCodes() throws Exception { assertEquals( diff --git a/src/test/java/org/metricshub/winrm/WinRMLiveTest.java b/src/test/java/org/metricshub/winrm/WinRMLiveTest.java index d38a618..4ee4a67 100644 --- a/src/test/java/org/metricshub/winrm/WinRMLiveTest.java +++ b/src/test/java/org/metricshub/winrm/WinRMLiveTest.java @@ -5,9 +5,13 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.io.InputStream; +import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.Paths; +import java.security.DigestInputStream; import java.security.MessageDigest; import java.time.Duration; import java.util.List; @@ -16,6 +20,7 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledIfSystemProperty; +import org.junit.jupiter.api.io.TempDir; import org.metricshub.winrm.service.client.auth.AuthenticationEnum; import org.metricshub.winrm.wql.WinRMWqlExecutor; @@ -303,4 +308,80 @@ void remoteDirectoryListing() throws Exception { private static List paths(final List entries) { return entries.stream().map(RemoteFileInfo::path).collect(Collectors.toList()); } + + @Test + void downloadRoundTripsAnUpload(@TempDir final Path directory) throws Exception { + // Every byte value, under a non-ASCII name. Small: an upload costs one command leg per ~8 KB. + final byte[] content = new byte[1024]; + for (int i = 0; i < content.length; i++) { + content[i] = (byte) i; + } + final String name = "winrm-java-live-round-trip-donn\u00e9es-\u6f22\u5b57.bin"; + final Path local = Files.write(directory.resolve(name), content); + final String remote = "C:\\Windows\\Temp\\" + name; + final Path downloads = Files.createDirectory(directory.resolve("downloads")); + + try (WinRMClient client = client()) { + client.uploadFile(local, remote); + try { + // Into a directory: the file keeps its remote name. + final long start = System.nanoTime(); + assertEquals(content.length, client.downloadFile(remote, downloads)); + final long downloaded = System.nanoTime(); + assertArrayEquals(content, Files.readAllBytes(downloads.resolve(name))); + // Identical: nothing transferred. + assertEquals(0, client.downloadFile(remote, downloads.resolve(name))); + System.out.printf( + "Downloaded 1 KiB in %d ms, skipped the identical copy in %d ms%n", + (downloaded - start) / 1_000_000L, + (System.nanoTime() - downloaded) / 1_000_000L + ); + } finally { + client.powerShell("Remove-Item -LiteralPath '" + remote + "' -ErrorAction SilentlyContinue").execute(); + } + } + } + + @Test + void downloadIsDigestVerifiedWithBoundedMemory(@TempDir final Path directory) throws Exception { + // Larger than the heap when run with a small one, e.g. -DargLine=-Xmx32m + // -Dwinrm.live.download.mib=64. Created on the host by one command. + final int mebibytes = Integer.getInteger("winrm.live.download.mib", 20); + final String remote = "C:\\Windows\\Temp\\winrm-java-live-download.bin"; + try (WinRMClient client = client()) { + client + .powerShell( + "$r=New-Object Random 42;$b=New-Object byte[] 1048576;$f=[IO.File]::Create('" + remote + "');" + + "for($i=0;$i -lt " + mebibytes + ";$i++){$r.NextBytes($b);$f.Write($b,0,$b.Length)};$f.Close()" + ) + .execute(); + try { + final Path local = directory.resolve("download.bin"); + final long start = System.nanoTime(); + assertEquals( + mebibytes * 1048576L, + client.file(remote).timeout(Duration.ofMinutes(15)).downloadTo(local) + ); + final long millis = (System.nanoTime() - start) / 1_000_000L; + System.out.printf( + "Downloaded %d MiB in %d ms (%.2f MB/s) with a %d MiB heap%n", + mebibytes, + millis, + mebibytes * 1048.576 / millis, + Runtime.getRuntime().maxMemory() >> 20 + ); + final MessageDigest digest = MessageDigest.getInstance("SHA-256"); + try (InputStream in = new DigestInputStream(Files.newInputStream(local), digest)) { + in.transferTo(OutputStream.nullOutputStream()); + } + final StringBuilder sha256 = new StringBuilder(); + for (final byte b : digest.digest()) { + sha256.append(String.format("%02x", b)); + } + assertEquals(client.file(remote).digest("SHA256"), sha256.toString()); + } finally { + client.command("del /q \"" + remote + "\"").execute(); + } + } + } } From 468e032db9f981d9dcd9ac91c2b7155333a2c8ae Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Fri, 25 Sep 2026 00:27:17 +0200 Subject: [PATCH 2/5] Address Codex review: deadline vs. publication, size check, staging name, retry deadline - A Publication handshake makes the final move and the caller's timeout mutually exclusive: a timeout is only reported when the destination was left untouched, and a deadline that fires while the verified file is moved into place lets the move, and the download, complete. - The probe reports the bytes it hashed (the stream position after ComputeHash) instead of the length read beforehand, so size and digest always describe the same bytes; the integrity check also compares the received byte count. - The staging file keeps at most 64 UTF-16 units of the destination's name, so a long name still fits the 255-unit (NTFS) and 255-byte (ext4) limits. - A quota retry of a remote-file script is only attempted when its pause ends before the timeout: a streaming terminal never starts a command after its deadline. - New test for the common refresh case (an existing, different local file is replaced); ATOMIC_MOVE replaces on Windows too (MoveFileEx with MOVEFILE_REPLACE_EXISTING). Co-Authored-By: Claude Opus 5.5 --- .../java/org/metricshub/winrm/RemoteFile.java | 92 ++++++++++++++++--- .../org/metricshub/winrm/RemoteFiles.java | 22 +++-- .../org/metricshub/winrm/ShellFileCopy.java | 2 +- src/site/markdown/file-transfers.md | 11 ++- .../org/metricshub/winrm/RemoteFileTest.java | 85 ++++++++++++++++- 5 files changed, 186 insertions(+), 26 deletions(-) diff --git a/src/main/java/org/metricshub/winrm/RemoteFile.java b/src/main/java/org/metricshub/winrm/RemoteFile.java index 9aedf07..4127efc 100644 --- a/src/main/java/org/metricshub/winrm/RemoteFile.java +++ b/src/main/java/org/metricshub/winrm/RemoteFile.java @@ -41,6 +41,7 @@ import java.time.Duration; import java.util.Optional; import java.util.concurrent.Callable; +import java.util.concurrent.CancellationException; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicLong; import org.metricshub.winrm.exceptions.WinRMClientException; @@ -92,6 +93,12 @@ public final class RemoteFile { /** The largest byte array the JVM reliably allocates. */ private static final long MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8L; + /** + * How much of the destination's name a download's staging file keeps: 64 UTF-16 units are at + * most 192 bytes in UTF-8, so {@code ..part} stays under the 255-byte limit. + */ + private static final int MAX_PART_PREFIX_LENGTH = 64; + private final WinRMClient client; private final String path; private long offset; @@ -298,8 +305,9 @@ public BufferedReader openReader(final Charset charset) { * {@link #maxBytes(long)} settings do not apply. *

* The timeout is a wall-clock deadline for the whole download, and the transfer runs at about - * 1.5 MB/s: a large file needs a raised {@link #timeout(Duration)}. Downloads are not - * resumable — one that fails or times out starts over. + * 1.5 MB/s: a large file needs a raised {@link #timeout(Duration)}. A deadline that fires while + * the verified file is being moved onto the destination lets that move complete: the download + * then succeeds. Downloads are not resumable — one that fails or times out starts over. * * @param localFile the local file to write, or an existing directory to write the file into * @return the number of bytes transferred: the size of the file, or 0 when the local file @@ -313,10 +321,11 @@ public long downloadTo(final Path localFile) { Utils.checkNonNull(localFile, "localFile"); final AtomicLong size = new AtomicLong(-1); final AtomicLong transferred = new AtomicLong(); + final Publication publication = new Publication(); try { return blocking(() -> { try { - return download(localFile, size, transferred); + return download(localFile, size, transferred, publication); } catch (final IOException e) { throw new WinRMClientException( String.format("Download of %s from %s failed: %s", path, client.hostname(), e), @@ -324,7 +333,15 @@ public long downloadTo(final Path localFile) { ); } }); - } catch (final WinRMTimeoutException e) { + } catch (final WinRMClientException e) { + // The deadline, or an interruption, may fire while the worker moves the verified file + // into place: that move then completes, and so does the download. + if (!publication.abandon()) { + return transferred.get(); + } + if (!(e instanceof WinRMTimeoutException)) { + throw e; + } throw new WinRMTimeoutException( String.format( "Download of %s from %s timed out after %s, %s: raise the timeout for large files", @@ -348,10 +365,15 @@ public long downloadTo(final Path localFile) { * @param localFile the local file, or an existing directory to write the file into * @param size receives the size of the remote file, for the timeout message * @param transferred counts the bytes transferred, for the timeout message + * @param publication decides between moving the file into place and abandoning the download * @return the number of bytes transferred */ - private long download(final Path localFile, final AtomicLong size, final AtomicLong transferred) - throws IOException, NoSuchAlgorithmException { + private long download( + final Path localFile, + final AtomicLong size, + final AtomicLong transferred, + final Publication publication + ) throws IOException, NoSuchAlgorithmException { final Path target = Files.isDirectory(localFile) ? localFile.resolve(path.replaceFirst(".*[\\\\/:]", "")) : localFile; @@ -376,8 +398,10 @@ private long download(final Path localFile, final AtomicLong size, final AtomicL } } - final Path directory = target.toAbsolutePath().getParent(); - if (directory == null) { + final Path absolute = target.toAbsolutePath(); + final Path directory = absolute.getParent(); + final Path name = absolute.getFileName(); + if (directory == null || name == null) { // A root that is not a directory, e.g. a drive with no media throw new NoSuchFileException(target.toString()); } @@ -386,14 +410,18 @@ private long download(final Path localFile, final AtomicLong size, final AtomicL if (!Files.isDirectory(directory)) { Files.createDirectories(directory); } + // The name is cut so the staging name fits the file-name limits (255 UTF-16 units on NTFS, + // 255 bytes on ext4) whatever the length of the destination's name. final Path part = directory.resolve( - target.getFileName() + "." + Long.toHexString(ThreadLocalRandom.current().nextLong()) + ".part" + ShellFileCopy.truncateAtCodePoint(name.toString(), MAX_PART_PREFIX_LENGTH) + "." + + Long.toHexString(ThreadLocalRandom.current().nextLong()) + ".part" ); try { try ( InputStream in = RemoteFiles.open(client, path, RemoteFiles.readScript(path, 0, -1), timeout); FileChannel out = FileChannel.open(part, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)) { - if (!MessageDigest.isEqual(copy(in, Channels.newOutputStream(out), transferred), digest)) { + final byte[] received = copy(in, Channels.newOutputStream(out), transferred); + if (transferred.get() != size.get() || !MessageDigest.isEqual(received, digest)) { throw new WinRMClientException( String.format( "Integrity check failed after downloading %s from %s to %s (%d bytes received, %d expected): " + @@ -406,11 +434,12 @@ private long download(final Path localFile, final AtomicLong size, final AtomicL ) ); } - // Interruptible like every channel operation: a download cancelled by its deadline - // fails here, before the move. out.force(true); } - Files.move(part, target, StandardCopyOption.ATOMIC_MOVE); + if (!publication.publish(part, target)) { + // The caller already reported the failure: the destination must stay as it was. + throw new CancellationException("Download abandoned by its deadline"); + } return transferred.get(); } catch (final IOException | RuntimeException e) { try { @@ -531,4 +560,41 @@ private InputStream open(final long readLength) { private T blocking(final Callable task) { return RemoteFiles.blocking(client, path, timeout, task); } + + /** + * Decides, once, between the worker moving a download's verified file into place and the + * caller abandoning the download when its deadline fires, so the outcome the caller reports + * always matches the destination: a move in progress completes before the caller decides. + */ + static final class Publication { + + private boolean abandoned; + private boolean published; + + /** + * Move the verified file onto the destination, unless the download was abandoned. + * + * @param part the verified staging file + * @param target the destination + * @return whether the file was moved onto the destination + * @throws IOException when the move fails + */ + synchronized boolean publish(final Path part, final Path target) throws IOException { + if (!abandoned) { + Files.move(part, target, StandardCopyOption.ATOMIC_MOVE); + published = true; + } + return published; + } + + /** + * Abandon the download, unless its file was already moved onto the destination. + * + * @return whether the download was abandoned + */ + synchronized boolean abandon() { + abandoned = !published; + return abandoned; + } + } } diff --git a/src/main/java/org/metricshub/winrm/RemoteFiles.java b/src/main/java/org/metricshub/winrm/RemoteFiles.java index 73261a4..5c7f5c1 100644 --- a/src/main/java/org/metricshub/winrm/RemoteFiles.java +++ b/src/main/java/org/metricshub/winrm/RemoteFiles.java @@ -216,11 +216,13 @@ private RemoteFiles() {} /** * What a download is verified against, as one base64 line of {@link #PROBE_LENGTH} bytes: the - * size of the file (8 bytes, little-endian), then its SHA-256 digest. + * number of bytes hashed (8 bytes, little-endian), then their SHA-256 digest. The count is the + * stream position after hashing, not the file length read beforehand, so both describe the same + * bytes even when the file changes during the probe. */ - private static final String PROBE = "try{$h=[Security.Cryptography.HashAlgorithm]::Create('SHA256');" + - "[Console]::Out.WriteLine([Convert]::ToBase64String([byte[]]([BitConverter]::GetBytes($f.Length)+$h.ComputeHash($f))))}" + private static final String PROBE = "try{$h=[Security.Cryptography.HashAlgorithm]::Create('SHA256');$d=$h.ComputeHash($f);" + + "[Console]::Out.WriteLine([Convert]::ToBase64String([byte[]]([BitConverter]::GetBytes($f.Position)+$d)))}" + "catch{fail $_.Exception}finally{$f.Close()}"; /** The length of the {@link #probeScript(String)} output: an 8-byte size and a 32-byte SHA-256 digest. */ @@ -456,7 +458,8 @@ static InputStream open(final WinRMClient client, final String path, final Strin *

* A start rejected by the host's operation quota ran nothing (see * {@link ShellFileCopy#isRetryableQuotaRejection(Exception)}): it is retried with the escalating - * delays of the file transfers, as long as the timeout has not elapsed since the first attempt. + * delays of the file transfers, as long as the retry starts within the timeout, counted from the + * first attempt. * * @param client the client to run the script on * @param path the remote path, for the error messages @@ -479,12 +482,17 @@ static RemoteProcess start(final WinRMClient client, final String path, final St try { return client.powerShell(script).timeout(timeout).start(); } catch (final WinRMClientException e) { - final long remaining = WinRMClient.toMillis(timeout) - (Utils.getCurrentTimeMillis() - begin); - if (attempt >= ShellFileCopy.QUOTA_RETRIES || remaining <= 0 || !ShellFileCopy.isRetryableQuotaRejection(e)) { + final long delay = ShellFileCopy.QUOTA_RETRY_DELAY_MILLIS * (attempt + 1); + // No retry once the pause would reach the timeout: it must start within it. + if (attempt >= ShellFileCopy.QUOTA_RETRIES + || + !ShellFileCopy.isRetryableQuotaRejection(e) + || + Utils.getCurrentTimeMillis() - begin + delay >= WinRMClient.toMillis(timeout)) { throw e; } try { - Utils.sleep(Math.min(ShellFileCopy.QUOTA_RETRY_DELAY_MILLIS * (attempt + 1), remaining)); + Utils.sleep(delay); } catch (final InterruptedException interrupted) { Thread.currentThread().interrupt(); throw e; diff --git a/src/main/java/org/metricshub/winrm/ShellFileCopy.java b/src/main/java/org/metricshub/winrm/ShellFileCopy.java index e6f7607..417d558 100644 --- a/src/main/java/org/metricshub/winrm/ShellFileCopy.java +++ b/src/main/java/org/metricshub/winrm/ShellFileCopy.java @@ -912,7 +912,7 @@ static String contentAddressedName(final String fileName, final byte[] content, * @param maxLength Maximum length, in UTF-16 chars * @return the truncated string */ - private static String truncateAtCodePoint(final String value, final int maxLength) { + static String truncateAtCodePoint(final String value, final int maxLength) { if (value.length() <= maxLength) { return value; } diff --git a/src/site/markdown/file-transfers.md b/src/site/markdown/file-transfers.md index 543e9f1..38a1008 100644 --- a/src/site/markdown/file-transfers.md +++ b/src/site/markdown/file-transfers.md @@ -206,9 +206,10 @@ like `cp`: `downloadFile("C:\\Windows\\Temp\\collect.log", Path.of("logs"))` wri is transferred: the download returns 0. 3. **Transfer.** The file is read with [`openStream()`](files.html#streaming-large-files) and written, block by block, to a temporary file **next to the destination**: - `..part`. Memory stays bounded whatever the size of the file — a 64 MiB file was - downloaded by a JVM limited to a 32 MiB heap. -4. **Verify and publish.** The digest of the received bytes must match the probed one; the + `..part` (the name cut to 64 characters, so a long one still fits the file-name + limits). Memory stays bounded whatever the size of the file — a 64 MiB file was downloaded by a + JVM limited to a 32 MiB heap. +4. **Verify and publish.** The received bytes must match the probed size and digest; the temporary file is then flushed to disk (`fsync`) and moved onto the destination in one atomic step (`ATOMIC_MOVE`), replacing any previous file. @@ -241,6 +242,10 @@ Download of D:\exports\big.csv from server01 timed out after PT30S, 41943040 of transferred: raise the timeout for large files ``` +The deadline and the final move exclude each other: a timeout is only reported when the +destination was left untouched. A deadline that fires while the verified file is being moved into +place lets the move complete, and the download succeeds. + ### Download performance Measured over HTTP with NTLM encryption, a download runs at about **1.35–1.45 MB/s**: 20 MiB in diff --git a/src/test/java/org/metricshub/winrm/RemoteFileTest.java b/src/test/java/org/metricshub/winrm/RemoteFileTest.java index b3afbde..795bb8e 100644 --- a/src/test/java/org/metricshub/winrm/RemoteFileTest.java +++ b/src/test/java/org/metricshub/winrm/RemoteFileTest.java @@ -141,11 +141,16 @@ private List sentScripts() { /** The probe output announcing the given content: its size (8 bytes, little-endian), then its SHA-256 digest. */ private static String probe(final byte[] content) throws Exception { + return probe(content.length, content); + } + + /** The probe output announcing the given size and the SHA-256 digest of the given content. */ + private static String probe(final long size, final byte[] content) throws Exception { return b64( ByteBuffer .allocate(RemoteFiles.PROBE_LENGTH) .order(ByteOrder.LITTLE_ENDIAN) - .putLong(content.length) + .putLong(size) .put(MessageDigest.getInstance("SHA-256").digest(content)) .array() ) + @@ -341,6 +346,82 @@ void aStartRejectedByTheOperationQuotaIsRetried() { assertEquals(2, sentScripts().size()); } + @Test + void noQuotaRetryStartsAfterTheTimeout() throws Exception { + // The first retry would start 5 s later, past the 3 s timeout of this stream: it is not attempted. + server + .enqueue(200, envelope(resourceCreated("SHELL-1"))) + .enqueue( + 500, + fault("2150859174", "The maximum number of concurrent operations for this user has been exceeded.") + ); + try (WinRMClient client = client()) { + final RemoteFile file = client.file(PATH).timeout(Duration.ofSeconds(3)); + final long start = System.nanoTime(); + final WinRMClientException e = assertThrows(WinRMClientException.class, file::openStream); + assertTrue(e.getMessage().contains("2150859174"), e.getMessage()); + assertTrue(System.nanoTime() - start < 2_000_000_000L, "retried after its timeout"); + } + assertEquals(1, sentScripts().size()); + } + + @Test + void downloadReplacesADifferentLocalFile(@TempDir final Path directory) throws Exception { + final byte[] content = "the new content".getBytes(StandardCharsets.US_ASCII); + final Path local = Files.write(directory.resolve("copy.bin"), "previous".getBytes(StandardCharsets.US_ASCII)); + enqueueRead(0, null, probe(content)); + enqueueCommand(0, null, b64(content) + "\r\n"); + try (WinRMClient client = client()) { + assertEquals(content.length, client.downloadFile(PATH, local)); + } + assertArrayEquals(content, Files.readAllBytes(local)); + assertEquals(List.of(local), files(directory)); + } + + @Test + void aSizeMismatchFailsEvenWhenTheDigestMatches(@TempDir final Path directory) throws Exception { + final byte[] content = { 1, 2, 3 }; + enqueueRead(0, null, probe(999, content)); + enqueueCommand(0, null, b64(content) + "\r\n"); + final Path local = directory.resolve("copy.bin"); + try (WinRMClient client = client()) { + final RemoteFile file = client.file(PATH); + final WinRMClientException e = assertThrows(WinRMClientException.class, () -> file.downloadTo(local)); + assertTrue(e.getMessage().contains("(3 bytes received, 999 expected)"), e.getMessage()); + } + assertEquals(List.of(), files(directory)); + } + + @Test + void aLongDestinationNameStillFitsTheStagingFile(@TempDir final Path directory) throws Exception { + // 244 characters fit the 255-character (NTFS) and 255-byte (ext4) limits; with the + // "..part" suffix appended, they would not. + final byte[] content = { 1, 2, 3 }; + final Path local = directory.resolve("a".repeat(240) + ".bin"); + enqueueRead(0, null, probe(content)); + enqueueCommand(0, null, b64(content) + "\r\n"); + try (WinRMClient client = client()) { + assertEquals(3, client.downloadFile(PATH, local)); + } + assertArrayEquals(content, Files.readAllBytes(local)); + } + + @Test + void publishingAndAbandoningExcludeEachOther(@TempDir final Path directory) throws Exception { + final Path part = Files.write(directory.resolve("copy.bin.part"), new byte[] { 1 }); + final Path target = directory.resolve("copy.bin"); + + final RemoteFile.Publication abandoned = new RemoteFile.Publication(); + assertTrue(abandoned.abandon()); + assertFalse(abandoned.publish(part, target)); + assertFalse(Files.exists(target)); + + final RemoteFile.Publication published = new RemoteFile.Publication(); + assertTrue(published.publish(part, target)); + assertFalse(published.abandon()); + assertArrayEquals(new byte[] { 1 }, Files.readAllBytes(target)); + } + @Test void downloadWritesTheVerifiedContent(@TempDir final Path directory) throws Exception { final byte[] content = new byte[300]; @@ -362,7 +443,7 @@ void downloadWritesTheVerifiedContent(@TempDir final Path directory) throws Exce assertArrayEquals(content, Files.readAllBytes(local)); assertEquals(List.of(local), files(directory)); final List scripts = sentScripts(); - assertTrue(scripts.get(0).contains("[BitConverter]::GetBytes($f.Length)"), scripts.get(0)); + assertTrue(scripts.get(0).contains("[BitConverter]::GetBytes($f.Position)"), scripts.get(0)); assertTrue(scripts.get(1).contains("$n=[long]0;$l=[long]-1"), scripts.get(1)); } From 6dbe72199bab0b7391227b6e7ffa0bc129055393 Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Fri, 25 Sep 2026 00:39:32 +0200 Subject: [PATCH 3/5] Recheck the quota retry deadline after the pause (Codex review) The pause before a quota retry can overrun its planned length (a GC pause, a descheduled thread): check the timeout again after it, so a streaming terminal never starts a command after its deadline. Co-Authored-By: Claude Opus 5.5 --- src/main/java/org/metricshub/winrm/RemoteFiles.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/org/metricshub/winrm/RemoteFiles.java b/src/main/java/org/metricshub/winrm/RemoteFiles.java index 5c7f5c1..9762ac7 100644 --- a/src/main/java/org/metricshub/winrm/RemoteFiles.java +++ b/src/main/java/org/metricshub/winrm/RemoteFiles.java @@ -497,6 +497,10 @@ static RemoteProcess start(final WinRMClient client, final String path, final St Thread.currentThread().interrupt(); throw e; } + // The pause itself can overrun (a GC pause, a descheduled thread). + if (Utils.getCurrentTimeMillis() - begin >= WinRMClient.toMillis(timeout)) { + throw e; + } } } } From 090dd7833fb29bf39f3f953658cc355e8c18d175 Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Fri, 25 Sep 2026 01:02:42 +0200 Subject: [PATCH 4/5] Address Codex review: colon names, permissions on replace, precise integrity wording - The local name for a directory destination is taken after the last \ or / only; a name that still contains a colon (an alternate data stream, whose stream name alone would collide, or a drive-relative path such as C:a.txt, which a Windows client resolves outside the directory) is refused before anything is sent. - A replaced file keeps its POSIX permissions: the staging file gets them before any byte is written, so a 0600 file stays private throughout. Windows keeps the directory's inherited ACL (documented). - The docs now state the integrity guarantee precisely: the local copy is exactly the bytes the probe hashed, one consistent version of the file, never a mix of two. Co-Authored-By: Claude Opus 5.5 --- .../java/org/metricshub/winrm/RemoteFile.java | 51 ++++++++++++++++--- src/site/markdown/file-transfers.md | 19 +++++-- .../org/metricshub/winrm/RemoteFileTest.java | 32 ++++++++++++ 3 files changed, 91 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/metricshub/winrm/RemoteFile.java b/src/main/java/org/metricshub/winrm/RemoteFile.java index 4127efc..bbf8a39 100644 --- a/src/main/java/org/metricshub/winrm/RemoteFile.java +++ b/src/main/java/org/metricshub/winrm/RemoteFile.java @@ -36,10 +36,12 @@ import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.PosixFilePermission; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.time.Duration; import java.util.Optional; +import java.util.Set; import java.util.concurrent.Callable; import java.util.concurrent.CancellationException; import java.util.concurrent.ThreadLocalRandom; @@ -291,16 +293,20 @@ public BufferedReader openReader(final Charset charset) { * {@link WinRMClient#uploadFile(Path, String)}, with the same guarantees: *

    *
  • verified: the host first reports the size and the SHA-256 digest of the file, and - * the received bytes must carry that digest — a file modified during the download fails; + * the received bytes must match both, so the local file is exactly one version of the remote + * file, never a mix of two: a change that reaches bytes not transferred yet fails the download; *
  • no wasted transfer: when the local file already has that digest, nothing is * transferred; *
  • atomic: the content goes to a temporary file next to the destination * ({@code ..part}), which is flushed to disk, then moved onto the destination in * one step. The destination is never seen truncated: a failure or a timeout leaves it as it - * was, and the temporary file is deleted as the transfer stops. + * was, and the temporary file is deleted as the transfer stops. A replaced file keeps its POSIX + * permissions; on Windows, the new file gets the permissions the directory gives new files. *
* When {@code localFile} is an existing directory, the file is written into it under its remote - * name, like {@code cp}; the destination's directory is created when needed. Memory is bounded + * name, like {@code cp} — so the remote name must not contain a colon (an alternate data stream, + * or a drive-relative path): name the local file explicitly for those. The destination's + * directory is created when needed. Memory is bounded * whatever the size of the file. The {@link #offset(long)}, {@link #length(long)} and * {@link #maxBytes(long)} settings do not apply. *

@@ -316,6 +322,8 @@ public BufferedReader openReader(final Charset charset) { * local file cannot be written * @throws WinRMTimeoutException when the timeout elapses first; the message tells how much was * transferred + * @throws IllegalArgumentException when {@code localFile} is a directory and the remote name + * contains a colon */ public long downloadTo(final Path localFile) { Utils.checkNonNull(localFile, "localFile"); @@ -374,8 +382,7 @@ private long download( final AtomicLong transferred, final Publication publication ) throws IOException, NoSuchAlgorithmException { - final Path target = Files.isDirectory(localFile) - ? localFile.resolve(path.replaceFirst(".*[\\\\/:]", "")) : localFile; + final Path target = Files.isDirectory(localFile) ? localFile.resolve(localName()) : localFile; final ByteBuffer probe; try (InputStream in = RemoteFiles.open(client, path, RemoteFiles.probeScript(path), timeout)) { @@ -390,13 +397,20 @@ private long download( final byte[] digest = new byte[probe.remaining()]; probe.get(digest); - if (Files.isRegularFile(target) && Files.size(target) == size.get()) { + final boolean replacing = Files.isRegularFile(target); + if (replacing && Files.size(target) == size.get()) { try (InputStream in = Files.newInputStream(target)) { if (MessageDigest.isEqual(copy(in, OutputStream.nullOutputStream(), new AtomicLong()), digest)) { return 0; } } } + // A replaced file keeps its POSIX permissions (a 0600 file must not become 0644): the staging + // file gets them before any byte is written. Windows has no such mode bits. + final Set permissions = replacing + && + target.getFileSystem().supportedFileAttributeViews().contains("posix") + ? Files.getPosixFilePermissions(target) : null; final Path absolute = target.toAbsolutePath(); final Path directory = absolute.getParent(); @@ -420,6 +434,9 @@ private long download( try ( InputStream in = RemoteFiles.open(client, path, RemoteFiles.readScript(path, 0, -1), timeout); FileChannel out = FileChannel.open(part, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)) { + if (permissions != null) { + Files.setPosixFilePermissions(part, permissions); + } final byte[] received = copy(in, Channels.newOutputStream(out), transferred); if (transferred.get() != size.get() || !MessageDigest.isEqual(received, digest)) { throw new WinRMClientException( @@ -451,6 +468,28 @@ private long download( } } + /** + * The name of the file in a local destination directory: the last element of the remote path. + * + * @return the name + * @throws IllegalArgumentException when the name contains a colon: an alternate data stream + * ({@code a.txt:meta}), whose name alone would collide with other streams, or a + * drive-relative path ({@code C:a.txt}), which a Windows client resolves outside the + * directory + */ + private String localName() { + final String name = path.substring(Math.max(path.lastIndexOf('\\'), path.lastIndexOf('/')) + 1); + if (name.indexOf(':') >= 0) { + throw new IllegalArgumentException( + String.format( + "Remote path %s is an alternate data stream or drive-relative: give the local file name, not a directory", + path + ) + ); + } + return name; + } + /** * Copy a stream with bounded memory, counting the bytes copied. * diff --git a/src/site/markdown/file-transfers.md b/src/site/markdown/file-transfers.md index 38a1008..2966683 100644 --- a/src/site/markdown/file-transfers.md +++ b/src/site/markdown/file-transfers.md @@ -194,7 +194,9 @@ long bytes = client.file("D:\\exports\\big.csv") When the local path is an existing directory, the file is written into it under its remote name, like `cp`: `downloadFile("C:\\Windows\\Temp\\collect.log", Path.of("logs"))` writes -`logs/collect.log`. The destination's directory is created when needed. +`logs/collect.log`. A remote name with a colon — an alternate data stream (`a.txt:meta`) or a +drive-relative path (`C:a.txt`) — is refused there: name the local file explicitly. The +destination's directory is created when needed. ### How a download works @@ -211,15 +213,22 @@ like `cp`: `downloadFile("C:\\Windows\\Temp\\collect.log", Path.of("logs"))` wri JVM limited to a 32 MiB heap. 4. **Verify and publish.** The received bytes must match the probed size and digest; the temporary file is then flushed to disk (`fsync`) and moved onto the destination in one atomic - step (`ATOMIC_MOVE`), replacing any previous file. + step (`ATOMIC_MOVE`), replacing any previous file. On Linux and macOS a replaced file keeps its + permissions — the temporary file gets them before any byte is written, so a `0600` file stays + private throughout. On Windows, the new file gets the permissions the directory gives new + files: an explicit ACL set on the replaced file is not carried over. **The destination is never seen truncated or half-written**: until the final move it keeps its previous content (or does not exist), and after it, it has the complete, verified content. On any failure — a digest mismatch, a read error, a timeout — the destination is left as it was and the temporary file is deleted as the transfer stops; only a process killed in the middle of a download -can leave a `.part` file behind. A file modified on the host between the probe and the end of the -transfer (a log being appended to, for instance) fails the integrity check: the download reports -it rather than delivering a torn copy. +can leave a `.part` file behind. + +The local copy is always **exactly the bytes the probe hashed**: one consistent version of the +remote file, never a mix of two. A change that reaches bytes not transferred yet — a log being +appended to, a file rewritten — fails the integrity check instead of delivering a torn copy. A +change confined to bytes already transferred leaves that consistent version in place, like any +copy of a file that changes after it was read. Downloads are **not resumable**: a download that fails or times out starts over from the first byte next time. Resuming would mean tracking verified byte ranges across attempts and proving diff --git a/src/test/java/org/metricshub/winrm/RemoteFileTest.java b/src/test/java/org/metricshub/winrm/RemoteFileTest.java index 795bb8e..19b1fea 100644 --- a/src/test/java/org/metricshub/winrm/RemoteFileTest.java +++ b/src/test/java/org/metricshub/winrm/RemoteFileTest.java @@ -25,6 +25,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; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import static org.metricshub.winrm.light.FakeWsmanResponses.commandResponse; import static org.metricshub.winrm.light.FakeWsmanResponses.done; import static org.metricshub.winrm.light.FakeWsmanResponses.envelope; @@ -42,11 +43,14 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; import java.security.MessageDigest; import java.time.Duration; import java.util.Arrays; import java.util.Base64; import java.util.List; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -378,6 +382,34 @@ void downloadReplacesADifferentLocalFile(@TempDir final Path directory) throws E assertEquals(List.of(local), files(directory)); } + @Test + void aReplacedFileKeepsItsPosixPermissions(@TempDir final Path directory) throws Exception { + assumeTrue(directory.getFileSystem().supportedFileAttributeViews().contains("posix"), "POSIX only"); + final Set ownerOnly = PosixFilePermissions.fromString("rw-------"); + final Path local = Files.write(directory.resolve("secret.conf"), "previous".getBytes(StandardCharsets.US_ASCII)); + Files.setPosixFilePermissions(local, ownerOnly); + final byte[] content = "password=new".getBytes(StandardCharsets.US_ASCII); + enqueueRead(0, null, probe(content)); + enqueueCommand(0, null, b64(content) + "\r\n"); + try (WinRMClient client = client()) { + client.downloadFile(PATH, local); + } + assertArrayEquals(content, Files.readAllBytes(local)); + assertEquals(ownerOnly, Files.getPosixFilePermissions(local)); + } + + @Test + void aStreamPathIsNotDownloadedIntoADirectory(@TempDir final Path directory) { + // "a.txt:meta" and "b.txt:meta" would both land in /meta. + try (WinRMClient client = client()) { + final RemoteFile stream = client.file("C:\\reports\\a.txt:meta"); + assertThrows(IllegalArgumentException.class, () -> stream.downloadTo(directory)); + final RemoteFile driveRelative = client.file("C:a.txt"); + assertThrows(IllegalArgumentException.class, () -> driveRelative.downloadTo(directory)); + } + assertEquals(0, server.decryptedRequests().size()); + } + @Test void aSizeMismatchFailsEvenWhenTheDigestMatches(@TempDir final Path directory) throws Exception { final byte[] content = { 1, 2, 3 }; From 39e5c2d41e56bf0cbd10754bb58f83913d0a3da5 Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Fri, 25 Sep 2026 01:16:59 +0200 Subject: [PATCH 5/5] Create the staging file with the replaced file's permissions (Codex review) A chmod after creation leaves a window in which another user can open the file under the umask-default mode and keep the descriptor, then read what is written later. The POSIX permissions of the replaced file are now a creation attribute: the umask can only narrow that mode, and the exact permissions are restored right after, before any byte is written. Co-Authored-By: Claude Opus 5.5 --- .../java/org/metricshub/winrm/RemoteFile.java | 18 +++++++++++++++--- src/site/markdown/file-transfers.md | 4 ++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/metricshub/winrm/RemoteFile.java b/src/main/java/org/metricshub/winrm/RemoteFile.java index bbf8a39..8a39f15 100644 --- a/src/main/java/org/metricshub/winrm/RemoteFile.java +++ b/src/main/java/org/metricshub/winrm/RemoteFile.java @@ -36,10 +36,13 @@ import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.FileAttribute; import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.time.Duration; +import java.util.EnumSet; import java.util.Optional; import java.util.Set; import java.util.concurrent.Callable; @@ -405,12 +408,16 @@ private long download( } } } - // A replaced file keeps its POSIX permissions (a 0600 file must not become 0644): the staging - // file gets them before any byte is written. Windows has no such mode bits. + // A replaced file keeps its POSIX permissions (a 0600 file must not become 0644). The staging + // file is created with them: set afterward, a reader could open it in between and keep its + // descriptor. Windows has no such mode bits. final Set permissions = replacing && target.getFileSystem().supportedFileAttributeViews().contains("posix") ? Files.getPosixFilePermissions(target) : null; + final FileAttribute[] attributes = permissions == null + ? new FileAttribute[0] : new FileAttribute[] + { PosixFilePermissions.asFileAttribute(permissions) }; final Path absolute = target.toAbsolutePath(); final Path directory = absolute.getParent(); @@ -433,8 +440,13 @@ private long download( try { try ( InputStream in = RemoteFiles.open(client, path, RemoteFiles.readScript(path, 0, -1), timeout); - FileChannel out = FileChannel.open(part, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)) { + FileChannel out = FileChannel.open( + part, + EnumSet.of(StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE), + attributes + )) { if (permissions != null) { + // The umask can only have narrowed the creation mode: restore the exact permissions. Files.setPosixFilePermissions(part, permissions); } final byte[] received = copy(in, Channels.newOutputStream(out), transferred); diff --git a/src/site/markdown/file-transfers.md b/src/site/markdown/file-transfers.md index 2966683..27567c3 100644 --- a/src/site/markdown/file-transfers.md +++ b/src/site/markdown/file-transfers.md @@ -214,8 +214,8 @@ destination's directory is created when needed. 4. **Verify and publish.** The received bytes must match the probed size and digest; the temporary file is then flushed to disk (`fsync`) and moved onto the destination in one atomic step (`ATOMIC_MOVE`), replacing any previous file. On Linux and macOS a replaced file keeps its - permissions — the temporary file gets them before any byte is written, so a `0600` file stays - private throughout. On Windows, the new file gets the permissions the directory gives new + permissions — the temporary file is created with them, so a `0600` file stays private + throughout. On Windows, the new file gets the permissions the directory gives new files: an explicit ACL set on the replaced file is not carried over. **The destination is never seen truncated or half-written**: until the final move it keeps its