From bdcece800e0861de14d730627b9f6e4f65460665 Mon Sep 17 00:00:00 2001 From: "Victor C." <34163765+vicajilau@users.noreply.github.com> Date: Fri, 4 Sep 2026 07:33:14 +0100 Subject: [PATCH] feat(sftp): list the package as web-compatible, and prove it in CI pub.dev listed android, ios, windows, linux and macos and not web. That also keeps the package out of any search narrowed to web, so it costs discoverability rather than a badge. The cause was one type. `SftpFile.downloadToRandomAccess` takes a `dart:io` `RandomAccessFile`, and naming it from the SFTP library made pana classify the whole package as unavailable there, even though everything else compiled and ran and 4.0.0 had already made the ciphers and SFTP work under dart2js. The method moves to an `SftpFileDownload` extension in its own library, exported conditionally the way `SSHSocket` and dynamic forwarding already are. Nothing changes for a caller on the VM: same import from `package:dartssh2/dartssh2.dart`, same call. On the web the extension is absent, along with the `RandomAccessFile` it would need. The extension reaches back through `isClosed`, already public, and `readChunk`, which was `_readChunk` and is now `@internal`. That one method is the entire seam. `_ReadCompletion` was used by nothing else so it moved too, and the two size constants are redeclared rather than widening the surface further. Saying `platform:web` on pub.dev is a stronger claim than a paragraph in the README, so CI now checks it. Everything that ran under `-p chrome` until now proved the pieces compile and behave, and none of it put a packet on a wire, which is the gap that let every AEAD cipher and the whole of SFTP sit broken on the web until 4.0.0 with a green suite the whole time. So the browser now runs against the same sshd container the VM interop tests use, through `tool/ws_bridge.dart`, because a browser has no TCP socket of its own. Four tests: a handshake, a command, a session pinned to aes256-gcm because its nonce counter is the code that threw under dart2js, and an SFTP round trip because every size and offset on the wire is a 64-bit field. The socket they connect through is the WebSocket one the README tells web users to write, so it doubles as a worked example. They skip when the bridge is not there, the way the VM interop tests skip without the server, decided by probing the bridge rather than by a compile-time define: `dart test` has neither `--define` nor `--dart2js-args`, and a browser has no `Platform.environment`. Verified: pana reports `platform:web` on this branch and not on main, and the four browser tests pass in CI against the real server. Project coverage is unchanged at 4978/6395, the lines codecov reports as new being the same error paths that were already uncovered before the move. --- .github/workflows/dart.yml | 34 +++ CHANGELOG.md | 3 + README.md | 6 +- lib/dartssh2.dart | 5 + lib/src/sftp/sftp_client.dart | 2 +- lib/src/sftp/sftp_file.dart | 209 +---------------- lib/src/sftp/sftp_file_io.dart | 226 +++++++++++++++++++ lib/src/sftp/sftp_file_io_stub.dart | 7 + pubspec.yaml | 1 + test/src/integration/web_interop_test.dart | 213 +++++++++++++++++ test/src/sftp/sftp_client_protocol_test.dart | 1 + tool/ws_bridge.dart | 78 +++++++ 12 files changed, 583 insertions(+), 202 deletions(-) create mode 100644 lib/src/sftp/sftp_file_io.dart create mode 100644 lib/src/sftp/sftp_file_io_stub.dart create mode 100644 test/src/integration/web_interop_test.dart create mode 100644 tool/ws_bridge.dart diff --git a/.github/workflows/dart.yml b/.github/workflows/dart.yml index d1894d41..abc0821d 100644 --- a/.github/workflows/dart.yml +++ b/.github/workflows/dart.yml @@ -112,3 +112,37 @@ jobs: - name: Run unit tests compiled to JavaScript (chrome) run: dart test -p chrome --exclude-tags=integration + + # Everything above proves the pieces compile and behave under dart2js. + # None of it puts a packet on a wire, which is the gap that let every + # AEAD cipher and the whole of SFTP sit broken on the web until 4.0.0. + # The steps below run the browser against the same OpenSSH server the VM + # interop tests use, through a WebSocket bridge, because a browser has no + # TCP socket of its own. That is also what the README tells web users to + # write, so the test doubles as a worked example of it. + - name: Start a local OpenSSH server + if: github.event_name == 'pull_request' || (github.event_name == 'push' && github.ref == 'refs/heads/main') + run: tool/start_test_sshd.sh + + - name: Start the WebSocket bridge + if: github.event_name == 'pull_request' || (github.event_name == 'push' && github.ref == 'refs/heads/main') + run: | + dart run tool/ws_bridge.dart & + for _ in $(seq 1 30); do + if (exec 3<>/dev/tcp/127.0.0.1/8022) 2>/dev/null; then + exec 3<&- 3>&- + echo "bridge is up" + exit 0 + fi + sleep 1 + done + echo "bridge did not come up in time" >&2 + exit 1 + + - name: Run interop tests in a browser against the real server + if: github.event_name == 'pull_request' || (github.event_name == 'push' && github.ref == 'refs/heads/main') + run: dart test -p chrome --tags=integration + + - name: Stop the local OpenSSH server + if: always() + run: docker rm -f dartssh2-test-sshd || true diff --git a/CHANGELOG.md b/CHANGELOG.md index 8267da7c..77a42d4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ ## [4.1.0] - Unreleased - Added `SSHClient.pipelineChannelRequests`, off by default, which sends all of a session's channel requests before reading any reply instead of waiting for each one in turn. `execute` and `shell` send `env`, agent forwarding, `pty-req` and `x11-req` ahead of `exec` or `shell`, and each of them cost a round trip: against a server 40 ms away, `execute` with a pty measured 246 ms before and 206 ms after. RFC 4254 §5.4 permits sending further messages without waiting and §4 requires the peer to answer a channel's requests in the order it received them, which is what `ssh(1)` relies on when it does the same thing. Setting it also adopts OpenSSH's reporting, because it has to: the command is on the wire before a refusal can come back, so a refused `pty-req`, `env`, agent forwarding or `x11-req` is reported through `printDebug` and the command runs, the way `ssh(1)` prints `PTY allocation request failed on channel 0` and carries on, rather than throwing an error that means the command has already run. Only a refused `exec` or `shell` still throws `SSHChannelRequestError`, because nothing has run when that one fails. Leaving it unset changes nothing, down to the order the requests go out and the message of every error [#243]. - Added a `dartssh2-nopty` account to the interop server, which `PermitTTY no` applies to, so both sides of a refused `pty-req` are exercised against a real OpenSSH: by default the command does not run, and with pipelining it does [#243]. +- Fixed the package reading as web-incompatible on pub.dev. `SftpFile.downloadToRandomAccess` takes a `dart:io` `RandomAccessFile`, and naming that type from the SFTP library was enough for pub.dev to drop `platform:web` from the whole package, which also keeps it out of any search filtered to web. Everything else already compiled and ran there, and 4.0.0 made the ciphers and SFTP work. The method moves to an `SftpFileDownload` extension in its own library, exported conditionally the way `SSHSocket` and dynamic forwarding already are, so nothing changes for a caller on the VM: same import, same call. On the web the extension is simply absent, as is the `RandomAccessFile` it would need. Confirmed with pana, the tool pub.dev scores with: `platform:web` is present after and absent before [#248]. +- Added browser interop tests, so "web is supported" is something CI checks rather than something the pieces individually suggest. Everything that ran under `-p chrome` until now proved the AEAD arithmetic, the SFTP encoding and the HTTP parsing compile and behave, and none of it put a packet on a wire, which is the gap that let every AEAD cipher and the whole of SFTP sit broken on the web until 4.0.0. The new tests run the browser against the same OpenSSH server the VM interop tests use, through `tool/ws_bridge.dart`, and cover a handshake, a command, an `aes256-gcm` session and an SFTP round trip. The `SSHSocket` they connect through is the WebSocket one the README tells web users to write, so it doubles as a worked example [#248]. ## [4.0.1] - 2026-09-03 - Fixed a channel stalling permanently when data arrived before the application subscribed to it. `StreamController.isPaused` is true until something listens, which suppressed every `SSH_MSG_CHANNEL_WINDOW_ADJUST`, and Dart delivers the first subscription as `onListen` rather than `onResume` — the only hook wired — so a peer that legally filled the advertised window in that gap was left at zero credit with no further data able to arrive and trigger a grant. Reproduced at the client's own sizes with 64 packets of 32 KiB into a 2 MiB window, before and after the first listener and through `.map()`, in every case zero adjustments. The remote forwarding example in the README reaches it: it awaits `Socket.connect()` for each connection before subscribing, and the forwarded channel is created with no listener attached [#244]. @@ -439,6 +441,7 @@ [#237]: https://github.com/vicajilau/dartssh2/pull/237 [#239]: https://github.com/vicajilau/dartssh2/pull/239 [#243]: https://github.com/vicajilau/dartssh2/issues/243 +[#248]: https://github.com/vicajilau/dartssh2/pull/248 [#244]: https://github.com/vicajilau/dartssh2/pull/244 [@linhanyu]: https://github.com/linhanyu diff --git a/README.md b/README.md index f2b112a4..33c7cce1 100644 --- a/README.md +++ b/README.md @@ -183,11 +183,15 @@ under dart2js, so a browser connection died at the first encrypted packet even with a correct transport. That is fixed, and a `dart test -p chrome` job now guards it. -One caveat remains. `chacha20-poly1305@openssh.com` cannot be used on the web, +Two caveats remain. `chacha20-poly1305@openssh.com` cannot be used on the web, because PointyCastle's Poly1305 requires full-width 64-bit integers. It sits third in the default cipher list, so AES-GCM or AES-CTR is normally negotiated and nothing needs doing. Only pinning ChaCha20-Poly1305 explicitly will fail. +And `SftpFile.downloadToRandomAccess` is not available, since it takes a +`dart:io` `RandomAccessFile` and there is no local file to hand it. Use +`SftpFile.downloadTo`, which takes a sink, or `SftpFile.read`. + ### Customize client SSH identification If your jump host or SSH gateway restricts client versions, you can customize the diff --git a/lib/dartssh2.dart b/lib/dartssh2.dart index 13d39594..e2e9bffc 100644 --- a/lib/dartssh2.dart +++ b/lib/dartssh2.dart @@ -31,6 +31,11 @@ export 'src/sftp/sftp_file_attrs.dart'; export 'src/sftp/sftp_name.dart'; export 'src/sftp/sftp_status_code.dart'; export 'src/sftp/sftp_stream_io.dart'; +// Adds SftpFile.downloadToRandomAccess where dart:io exists. Kept behind a +// conditional export so that naming RandomAccessFile does not make the whole +// package read as web-incompatible on pub.dev. +export 'src/sftp/sftp_file_io_stub.dart' + if (dart.library.io) 'src/sftp/sftp_file_io.dart'; export 'src/http/http_client.dart'; export 'src/http/http_exception.dart'; diff --git a/lib/src/sftp/sftp_client.dart b/lib/src/sftp/sftp_client.dart index 4d420442..9919b3aa 100644 --- a/lib/src/sftp/sftp_client.dart +++ b/lib/src/sftp/sftp_client.dart @@ -1,9 +1,9 @@ import 'dart:async'; -import 'dart:io'; import 'dart:math'; import 'dart:typed_data'; import 'package:convert/convert.dart'; +import 'package:meta/meta.dart'; import 'package:dartssh2/src/sftp/sftp_errors.dart'; import 'package:dartssh2/src/sftp/sftp_file_attrs.dart'; import 'package:dartssh2/src/sftp/sftp_file_open_mode.dart'; diff --git a/lib/src/sftp/sftp_file.dart b/lib/src/sftp/sftp_file.dart index 59188867..107ed79f 100644 --- a/lib/src/sftp/sftp_file.dart +++ b/lib/src/sftp/sftp_file.dart @@ -152,7 +152,7 @@ class SftpFile { void issueRead(int startOffset, int requestLength) { pendingReadCount++; - _readChunk(requestLength, startOffset).then( + readChunk(requestLength, startOffset).then( (chunk) { pendingReadCount--; @@ -301,197 +301,6 @@ class SftpFile { return bytesRead; } - /// Downloads this file into a random-access local file. - /// - /// Unlike [read] and [downloadTo], this method does not require SFTP read - /// replies to be yielded in offset order. Replies are written to - /// [destination] at the same offset, allowing pipelined reads to make - /// progress even when later offsets complete before earlier ones. - /// - /// Returns the total number of bytes written. - Future downloadToRandomAccess( - RandomAccessFile destination, { - int? length, - int offset = 0, - void Function(int bytesRead)? onProgress, - int chunkSize = _kDownloadChunkSize, - int maxPendingRequests = _kDownloadMaxPendingRequests, - }) async { - _mustNotBeClosed(); - if (chunkSize <= 0) { - throw ArgumentError.value(chunkSize, 'chunkSize', 'must be positive'); - } - if (maxPendingRequests <= 0) { - throw ArgumentError.value( - maxPendingRequests, - 'maxPendingRequests', - 'must be positive', - ); - } - - // Whether `length` reflects a real, trustworthy byte count. It starts - // true (an explicit `length` from the caller is always trusted) and is - // only flipped below when we have to fall back to the stat()-size-0 - // sentinel. It gates the truncation check at the end of this method - - // see the comment there for why that check can't just compare against - // the (possibly sentinel) `length` value directly. - var lengthIsKnown = true; - - if (length == null) { - final fileSize = (await stat()).size; - if (fileSize == null) { - throw SftpError('Can not get file size'); - } - length = fileSize - offset; - - // Some filesystems report a size of 0 for files that actually contain - // data (e.g. Linux /proc entries, character/device files). SFTP - // signals end-of-file via the SSH_FX_EOF status code, not via the - // reported size, so don't trust a stat()-derived size of 0 as - // "nothing to download" - see the matching comment in [read] for the - // full reasoning. Fall back to downloading until the server tells us - // we've hit EOF instead. This only applies when we computed `length` - // ourselves; a caller who explicitly passes `length: 0` still gets an - // empty, zero-byte download below. - if (length == 0) { - length = _kUnboundedReadLength; - lengthIsKnown = false; - // See [read]: with the real end unknown, the reservation logic - // below can't tell when to stop fanning out speculative reads, so - // force strictly sequential requests instead of fanning out up to - // [maxPendingRequests] of them into a file that may only be a few - // bytes long. Because writes here go to their own offset (not - // gated on in-order arrival like [read]), concurrency wouldn't be - // *incorrect* - just wasteful for the common case this exists for. - maxPendingRequests = 1; - } - } - - if (length == 0) return 0; - if (length < 0) { - throw SftpError('Length must be positive: $length'); - } - - final endOffset = offset + length; - final completionQueue = <_ReadCompletion>[]; - var reservedOffset = offset; - var bytesWritten = 0; - var pendingReadCount = 0; - var activeReadLimit = 1; - var effectiveChunkSize = chunkSize; - Object? pendingError; - StackTrace? pendingStackTrace; - Completer? completionSignal; - - void notifyReadComplete() { - final signal = completionSignal; - if (signal != null && !signal.isCompleted) { - signal.complete(); - } - } - - Future waitForReadComplete() { - if (completionQueue.isNotEmpty || pendingError != null) { - return Future.value(); - } - final signal = completionSignal = Completer(); - return signal.future.whenComplete(() { - if (identical(completionSignal, signal)) { - completionSignal = null; - } - }); - } - - void issueRead(int startOffset, int requestLength) { - pendingReadCount++; - _readChunk(requestLength, startOffset).then( - (chunk) { - pendingReadCount--; - if (chunk != null && chunk.isNotEmpty) { - activeReadLimit = min(maxPendingRequests, activeReadLimit + 1); - } - completionQueue.add(_ReadCompletion(startOffset, chunk)); - if (chunk != null && - chunk.isNotEmpty && - chunk.length < requestLength && - startOffset + chunk.length < endOffset) { - effectiveChunkSize = max(1, min(effectiveChunkSize, chunk.length)); - issueRead( - startOffset + chunk.length, - min( - requestLength - chunk.length, - endOffset - startOffset - chunk.length, - ), - ); - } - notifyReadComplete(); - }, - onError: (Object error, StackTrace stackTrace) { - pendingReadCount--; - pendingError = error; - pendingStackTrace = stackTrace; - notifyReadComplete(); - }, - ); - } - - void scheduleReads() { - while (reservedOffset < endOffset && pendingReadCount < activeReadLimit) { - final startOffset = reservedOffset; - final requestLength = - min(effectiveChunkSize, endOffset - reservedOffset); - issueRead(startOffset, requestLength); - reservedOffset += requestLength; - } - } - - scheduleReads(); - - while (bytesWritten < length) { - if (pendingError != null) { - Error.throwWithStackTrace(pendingError!, pendingStackTrace!); - } - - if (completionQueue.isEmpty) { - if (pendingReadCount == 0) break; - await waitForReadComplete(); - continue; - } - - final completion = completionQueue.removeAt(0); - final startOffset = completion.startOffset; - final chunk = completion.chunk; - if (chunk == null) break; - if (chunk.isEmpty) { - throw SftpError('Unexpected empty data chunk before EOF'); - } - - final remaining = length - (startOffset - offset); - final outputChunk = chunk.length <= remaining - ? chunk - : Uint8List.sublistView(chunk, 0, remaining); - await destination.setPosition(startOffset); - await destination.writeFrom(outputChunk); - - bytesWritten += outputChunk.length; - onProgress?.call(bytesWritten); - scheduleReads(); - } - - // Only enforce the truncation check when `length` is a real target byte - // count. When it's the unbounded sentinel (stat() reported size 0 but - // EOF is what actually ended the loop above), `bytesWritten` will almost - // never equal the sentinel and this would misfire on every such - // download, including genuinely-complete ones and genuinely-empty ones. - if (lengthIsKnown && bytesWritten != length) { - throw SftpError( - 'Incomplete download: received $bytesWritten of $length bytes', - ); - } - - return bytesWritten; - } - /// Reads at most [length] bytes from the file starting at [offset]. If /// [length] is null, reads until end of the file. /// Use [read] if you want to stream large file in chunks. @@ -628,7 +437,14 @@ class SftpFile { SftpStatusError.check(reply); } - Future _readChunk(int length, [int offset = 0]) async { + /// Reads one chunk from the remote file. + /// + /// The single seam [SftpFileDownload.downloadToRandomAccess] needs. It lives + /// in its own library so that naming `dart:io`'s [RandomAccessFile] does not + /// pull `dart:io` into this one, which is what kept pub.dev from listing the + /// package as available on the web. + @internal + Future readChunk(int length, [int offset = 0]) async { _mustNotBeClosed(); final reply = await _client._sendRead(_handle, offset, length); if (reply is SftpDataPacket) return reply.data; @@ -661,13 +477,6 @@ class SftpHandsake { } /// Tracks a pending SFTP read completion for [SftpFile.downloadToRandomAccess]. -class _ReadCompletion { - _ReadCompletion(this.startOffset, this.chunk); - - final int startOffset; - final Uint8List? chunk; -} - class _WriteCompletion { _WriteCompletion(this.id); diff --git a/lib/src/sftp/sftp_file_io.dart b/lib/src/sftp/sftp_file_io.dart new file mode 100644 index 00000000..1845fdd8 --- /dev/null +++ b/lib/src/sftp/sftp_file_io.dart @@ -0,0 +1,226 @@ +import 'dart:async'; +import 'dart:io'; +import 'dart:math'; +import 'dart:typed_data'; + +import 'package:dartssh2/src/sftp/sftp_client.dart'; +import 'package:dartssh2/src/sftp/sftp_errors.dart'; + +/// Mirrors the defaults the SFTP library uses for [SftpFile.downloadTo]. They +/// are private there, and duplicating two numbers is cheaper than widening the +/// internal surface further. +const _kDownloadChunkSize = 64 * 1024; +const _kDownloadMaxPendingRequests = 128; + +/// See [SftpFile.read]. Kept in step with the same constant there. +const _kUnboundedReadLength = 1099511627776; // 1 TiB + +/// [downloadToRandomAccess] lives here rather than on [SftpFile] itself +/// because it is the only thing in this package that names a `dart:io` type, +/// and naming one from the SFTP library was enough for pub.dev to classify the +/// whole package as unavailable on the web, even though everything else +/// compiles and runs there. +/// +/// It reaches [SftpFile] through [SftpFile.readChunk] and [SftpFile.isClosed], +/// both of which are public, so nothing about the call site changes: the same +/// `package:dartssh2/dartssh2.dart` import brings this in wherever `dart:io` +/// exists. +extension SftpFileDownload on SftpFile { + /// Downloads this file into a random-access local file. + /// + /// Unlike [read] and [downloadTo], this method does not require SFTP read + /// replies to be yielded in offset order. Replies are written to + /// [destination] at the same offset, allowing pipelined reads to make + /// progress even when later offsets complete before earlier ones. + /// + /// Returns the total number of bytes written. + Future downloadToRandomAccess( + RandomAccessFile destination, { + int? length, + int offset = 0, + void Function(int bytesRead)? onProgress, + int chunkSize = _kDownloadChunkSize, + int maxPendingRequests = _kDownloadMaxPendingRequests, + }) async { + if (isClosed) throw SftpError('File is closed'); + if (chunkSize <= 0) { + throw ArgumentError.value(chunkSize, 'chunkSize', 'must be positive'); + } + if (maxPendingRequests <= 0) { + throw ArgumentError.value( + maxPendingRequests, + 'maxPendingRequests', + 'must be positive', + ); + } + + // Whether `length` reflects a real, trustworthy byte count. It starts + // true (an explicit `length` from the caller is always trusted) and is + // only flipped below when we have to fall back to the stat()-size-0 + // sentinel. It gates the truncation check at the end of this method - + // see the comment there for why that check can't just compare against + // the (possibly sentinel) `length` value directly. + var lengthIsKnown = true; + + if (length == null) { + final fileSize = (await stat()).size; + if (fileSize == null) { + throw SftpError('Can not get file size'); + } + length = fileSize - offset; + + // Some filesystems report a size of 0 for files that actually contain + // data (e.g. Linux /proc entries, character/device files). SFTP + // signals end-of-file via the SSH_FX_EOF status code, not via the + // reported size, so don't trust a stat()-derived size of 0 as + // "nothing to download" - see the matching comment in [read] for the + // full reasoning. Fall back to downloading until the server tells us + // we've hit EOF instead. This only applies when we computed `length` + // ourselves; a caller who explicitly passes `length: 0` still gets an + // empty, zero-byte download below. + if (length == 0) { + length = _kUnboundedReadLength; + lengthIsKnown = false; + // See [read]: with the real end unknown, the reservation logic + // below can't tell when to stop fanning out speculative reads, so + // force strictly sequential requests instead of fanning out up to + // [maxPendingRequests] of them into a file that may only be a few + // bytes long. Because writes here go to their own offset (not + // gated on in-order arrival like [read]), concurrency wouldn't be + // *incorrect* - just wasteful for the common case this exists for. + maxPendingRequests = 1; + } + } + + if (length == 0) return 0; + if (length < 0) { + throw SftpError('Length must be positive: $length'); + } + + final endOffset = offset + length; + final completionQueue = <_ReadCompletion>[]; + var reservedOffset = offset; + var bytesWritten = 0; + var pendingReadCount = 0; + var activeReadLimit = 1; + var effectiveChunkSize = chunkSize; + Object? pendingError; + StackTrace? pendingStackTrace; + Completer? completionSignal; + + void notifyReadComplete() { + final signal = completionSignal; + if (signal != null && !signal.isCompleted) { + signal.complete(); + } + } + + Future waitForReadComplete() { + if (completionQueue.isNotEmpty || pendingError != null) { + return Future.value(); + } + final signal = completionSignal = Completer(); + return signal.future.whenComplete(() { + if (identical(completionSignal, signal)) { + completionSignal = null; + } + }); + } + + void issueRead(int startOffset, int requestLength) { + pendingReadCount++; + readChunk(requestLength, startOffset).then( + (chunk) { + pendingReadCount--; + if (chunk != null && chunk.isNotEmpty) { + activeReadLimit = min(maxPendingRequests, activeReadLimit + 1); + } + completionQueue.add(_ReadCompletion(startOffset, chunk)); + if (chunk != null && + chunk.isNotEmpty && + chunk.length < requestLength && + startOffset + chunk.length < endOffset) { + effectiveChunkSize = max(1, min(effectiveChunkSize, chunk.length)); + issueRead( + startOffset + chunk.length, + min( + requestLength - chunk.length, + endOffset - startOffset - chunk.length, + ), + ); + } + notifyReadComplete(); + }, + onError: (Object error, StackTrace stackTrace) { + pendingReadCount--; + pendingError = error; + pendingStackTrace = stackTrace; + notifyReadComplete(); + }, + ); + } + + void scheduleReads() { + while (reservedOffset < endOffset && pendingReadCount < activeReadLimit) { + final startOffset = reservedOffset; + final requestLength = + min(effectiveChunkSize, endOffset - reservedOffset); + issueRead(startOffset, requestLength); + reservedOffset += requestLength; + } + } + + scheduleReads(); + + while (bytesWritten < length) { + if (pendingError != null) { + Error.throwWithStackTrace(pendingError!, pendingStackTrace!); + } + + if (completionQueue.isEmpty) { + if (pendingReadCount == 0) break; + await waitForReadComplete(); + continue; + } + + final completion = completionQueue.removeAt(0); + final startOffset = completion.startOffset; + final chunk = completion.chunk; + if (chunk == null) break; + if (chunk.isEmpty) { + throw SftpError('Unexpected empty data chunk before EOF'); + } + + final remaining = length - (startOffset - offset); + final outputChunk = chunk.length <= remaining + ? chunk + : Uint8List.sublistView(chunk, 0, remaining); + await destination.setPosition(startOffset); + await destination.writeFrom(outputChunk); + + bytesWritten += outputChunk.length; + onProgress?.call(bytesWritten); + scheduleReads(); + } + + // Only enforce the truncation check when `length` is a real target byte + // count. When it's the unbounded sentinel (stat() reported size 0 but + // EOF is what actually ended the loop above), `bytesWritten` will almost + // never equal the sentinel and this would misfire on every such + // download, including genuinely-complete ones and genuinely-empty ones. + if (lengthIsKnown && bytesWritten != length) { + throw SftpError( + 'Incomplete download: received $bytesWritten of $length bytes', + ); + } + + return bytesWritten; + } +} + +class _ReadCompletion { + _ReadCompletion(this.startOffset, this.chunk); + + final int startOffset; + final Uint8List? chunk; +} diff --git a/lib/src/sftp/sftp_file_io_stub.dart b/lib/src/sftp/sftp_file_io_stub.dart new file mode 100644 index 00000000..2f2e42c1 --- /dev/null +++ b/lib/src/sftp/sftp_file_io_stub.dart @@ -0,0 +1,7 @@ +/// Web stand-in for `sftp_file_io.dart`. +/// +/// That library defines `SftpFileDownload.downloadToRandomAccess`, which takes +/// a `dart:io` [RandomAccessFile]. There is no such thing on the web and no +/// local file to hand it, so the extension simply is not there. Use +/// `SftpFile.downloadTo`, which takes a sink, or `SftpFile.read`. +library; diff --git a/pubspec.yaml b/pubspec.yaml index e70057fe..36da4bb4 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -25,6 +25,7 @@ dev_dependencies: build_runner: ^2.4.12 lints: ">=4.0.0 <7.0.0" test: ^1.25.15 + web: ^1.1.1 false_secrets: - test diff --git a/test/src/integration/web_interop_test.dart b/test/src/integration/web_interop_test.dart new file mode 100644 index 00000000..16dea5d6 --- /dev/null +++ b/test/src/integration/web_interop_test.dart @@ -0,0 +1,213 @@ +// Proves that dartssh2 compiled to JavaScript really speaks SSH to a real +// OpenSSH server. +// +// Everything else that runs under `-p chrome` shows that the pieces compile +// and behave: the AEAD nonce arithmetic, the SFTP wire encoding, the HTTP +// parsing. None of it puts a packet on a wire. Without this, "web is +// supported" rested on the parts having been checked individually, which is +// exactly the gap that let every AEAD cipher and the whole of SFTP sit broken +// on the web until 4.0.0. +// +// A browser cannot open a TCP socket, which is why the README tells web users +// to bring their own `SSHSocket`. `tool/ws_bridge.dart` provides the other end +// and CI starts it alongside the sshd container. +@TestOn('browser') +@Tags(['integration']) +library; + +import 'dart:async'; +import 'dart:js_interop'; +import 'dart:typed_data'; + +import 'package:dartssh2/dartssh2.dart'; +import 'package:test/test.dart'; +import 'package:web/web.dart' as web; + +/// Where `tool/ws_bridge.dart` listens. Matches the CI workflow. +const _bridgeUrl = 'ws://127.0.0.1:8022'; + +const _user = 'dartssh2'; +const _password = 'dartssh2-test-password'; + +/// Whether [_bridgeUrl] answered during [setUpAll]. +/// +/// Decided at run time rather than through a compile-time define, because +/// `dart test` has no way to pass one: there is no `--define`, and the browser +/// has no `Platform.environment` for the VM interop tests' `DARTSSH2_LOCAL_SSHD` +/// trick. Probing the bridge is also a truer test of the same thing. +var _bridgeIsUp = false; + +const _skipReason = 'needs tool/ws_bridge.dart and the interop sshd, which CI ' + 'starts. Run tool/start_test_sshd.sh and dart run tool/ws_bridge.dart.'; + +/// Skips the calling test when the bridge is not there, the way the VM interop +/// tests skip without the server. Returns whether the test should go on. +bool _requireBridge() { + if (_bridgeIsUp) return true; + markTestSkipped(_skipReason); + return false; +} + +/// Opens [_bridgeUrl] and closes it again, to see whether anything answers. +Future _probeBridge() async { + final socket = web.WebSocket(_bridgeUrl); + final opened = Completer(); + socket.onopen = ((web.Event _) { + if (!opened.isCompleted) opened.complete(true); + }).toJS; + socket.onerror = ((web.Event _) { + if (!opened.isCompleted) opened.complete(false); + }).toJS; + final up = await opened.future + .timeout(const Duration(seconds: 5), onTimeout: () => false); + socket.close(); + return up; +} + +void main() { + group('dartssh2 compiled to JavaScript, against a real OpenSSH server', () { + late SSHClient client; + + setUpAll(() async { + _bridgeIsUp = await _probeBridge(); + }); + + setUp(() async { + if (!_bridgeIsUp) return; + client = SSHClient( + await _WebSocketSSHSocket.connect(_bridgeUrl), + username: _user, + onPasswordRequest: () => _password, + ); + await client.authenticated; + }); + + tearDown(() { + if (_bridgeIsUp) client.close(); + }); + + test('completes a handshake and reports the server version', () async { + if (!_requireBridge()) return; + + // Getting here at all means the version exchange, key exchange, host + // key signature check and userauth all ran in the browser. + expect(client.remoteVersion, startsWith('SSH-2.0-')); + }); + + test('runs a command and reads its output', () async { + if (!_requireBridge()) return; + + final output = await client.run('echo web-interop'); + expect(String.fromCharCodes(output).trim(), 'web-interop'); + }); + + test('negotiates an AEAD cipher, which is what W-01 broke', () async { + if (!_requireBridge()) return; + + // aes256-gcm is first in the default cipher list and its nonce counter + // is the code that threw under dart2js before 4.0.0. A command that + // returns proves it encrypted and decrypted for real. + final aeadOnly = SSHClient( + await _WebSocketSSHSocket.connect(_bridgeUrl), + username: _user, + onPasswordRequest: () => _password, + algorithms: const SSHAlgorithms(cipher: [SSHCipherType.aes256gcm]), + ); + final output = await aeadOnly.run('echo aead'); + expect(String.fromCharCodes(output).trim(), 'aead'); + aeadOnly.close(); + }); + + test('round-trips a file over SFTP', () async { + if (!_requireBridge()) return; + + // SFTP was the other half of W-01: every size and offset on the wire is + // a 64-bit field, and reading one threw under dart2js. + final sftp = await client.sftp(); + final path = '/tmp/dartssh2-web-interop'; + final payload = Uint8List.fromList('web sftp round trip'.codeUnits); + + final write = await sftp.open( + path, + mode: SftpFileOpenMode.create | + SftpFileOpenMode.write | + SftpFileOpenMode.truncate, + ); + await write.writeBytes(payload); + await write.close(); + + final read = await sftp.open(path); + expect(await read.readBytes(), payload); + expect((await read.stat()).size, payload.length); + await read.close(); + + await sftp.remove(path); + sftp.close(); + }); + }); +} + +/// The `SSHSocket` the README tells web users to write, in its smallest form. +class _WebSocketSSHSocket implements SSHSocket { + _WebSocketSSHSocket._(this._socket); + + final web.WebSocket _socket; + + final _incoming = StreamController(); + final _outgoing = StreamController>(); + final _done = Completer(); + + static Future connect(String url) async { + final socket = web.WebSocket(url)..binaryType = 'arraybuffer'; + final opened = Completer(); + + socket.onopen = ((web.Event _) { + if (!opened.isCompleted) opened.complete(); + }).toJS; + socket.onerror = ((web.Event _) { + if (!opened.isCompleted) { + opened.completeError(StateError('cannot reach $url')); + } + }).toJS; + + await opened.future.timeout(const Duration(seconds: 10)); + return _WebSocketSSHSocket._(socket).._wire(); + } + + void _wire() { + _socket.onmessage = ((web.MessageEvent event) { + final buffer = event.data as JSArrayBuffer; + _incoming.add(buffer.toDart.asUint8List()); + }).toJS; + + _socket.onclose = ((web.CloseEvent _) { + if (!_incoming.isClosed) _incoming.close(); + if (!_done.isCompleted) _done.complete(); + }).toJS; + + _outgoing.stream.listen((data) { + _socket.send(Uint8List.fromList(data).toJS); + }); + } + + @override + Stream get stream => _incoming.stream; + + @override + StreamSink> get sink => _outgoing.sink; + + @override + Future get done => _done.future; + + @override + Future close() async { + _socket.close(); + return done; + } + + @override + void destroy() => _socket.close(); + + @override + Future flush() async {} +} diff --git a/test/src/sftp/sftp_client_protocol_test.dart b/test/src/sftp/sftp_client_protocol_test.dart index 83b1f53b..ab7847fd 100644 --- a/test/src/sftp/sftp_client_protocol_test.dart +++ b/test/src/sftp/sftp_client_protocol_test.dart @@ -7,6 +7,7 @@ import 'dart:typed_data'; import 'package:dartssh2/src/message/msg_channel.dart'; import 'package:dartssh2/src/sftp/sftp_client.dart'; +import 'package:dartssh2/src/sftp/sftp_file_io.dart'; import 'package:dartssh2/src/sftp/sftp_errors.dart'; import 'package:dartssh2/src/sftp/sftp_file_attrs.dart'; import 'package:dartssh2/src/sftp/sftp_packet.dart'; diff --git a/tool/ws_bridge.dart b/tool/ws_bridge.dart new file mode 100644 index 00000000..0f30ea46 --- /dev/null +++ b/tool/ws_bridge.dart @@ -0,0 +1,78 @@ +// A WebSocket to TCP bridge, so a browser can reach the interop sshd. +// +// The web tests exist to prove that dartssh2 compiled to JavaScript really +// speaks SSH to a real OpenSSH server, not just that its pieces compile. +// A browser cannot open a TCP socket, which is the whole reason the README +// tells web users to bring their own `SSHSocket` over a WebSocket. This is +// the smallest possible version of that: every frame received is written to +// the sshd connection, every byte read back is sent as one binary frame. +// +// dart run tool/ws_bridge.dart [--port 8022] [--target-port 2222] +// +// Nothing here is part of the published package. It is test scaffolding, and +// it deliberately binds to the loopback interface only. +import 'dart:async'; +import 'dart:io'; + +Future main(List args) async { + var port = 8022; + var targetPort = 2222; + const targetHost = '127.0.0.1'; + + for (var i = 0; i < args.length - 1; i++) { + if (args[i] == '--port') port = int.parse(args[i + 1]); + if (args[i] == '--target-port') targetPort = int.parse(args[i + 1]); + } + + final server = await HttpServer.bind(InternetAddress.loopbackIPv4, port); + stdout.writeln('ws bridge on ws://127.0.0.1:$port ' + '-> $targetHost:$targetPort'); + + await for (final request in server) { + if (!WebSocketTransformer.isUpgradeRequest(request)) { + request.response.statusCode = HttpStatus.badRequest; + await request.response.close(); + continue; + } + unawaited(_bridge(request, targetHost, targetPort)); + } +} + +Future _bridge(HttpRequest request, String host, int port) async { + final socket = await WebSocketTransformer.upgrade(request); + Socket? tcp; + try { + tcp = await Socket.connect(host, port); + } catch (error) { + stderr.writeln('ws bridge: cannot reach $host:$port: $error'); + await socket.close(); + return; + } + + // Either side closing tears down the other, so a test that ends without + // closing cleanly does not leave the bridge holding a connection open. + final tcpDone = Completer(); + tcp.listen( + socket.add, + onError: (Object error) { + stderr.writeln('ws bridge: tcp error: $error'); + if (!tcpDone.isCompleted) tcpDone.complete(); + }, + onDone: () { + if (!tcpDone.isCompleted) tcpDone.complete(); + }, + cancelOnError: true, + ); + + socket.listen( + (frame) { + if (frame is List) tcp!.add(frame); + }, + onError: (Object error) => stderr.writeln('ws bridge: ws error: $error'), + onDone: () => tcp?.destroy(), + cancelOnError: true, + ); + + await tcpDone.future; + await socket.close(); +}