Conversation
…ty requests Fixes TerminalStudio#102: Changed SSHChannelController.sendEnv() from void to Future<bool> to properly await environment variable setup responses and prevent the response from being misattributed to the PTY request. BREAKING CHANGE: sendEnv() now returns Future<bool> instead of void. Callers must now await the result and validate it, similar to other channel request methods (sendPtyReq, sendAgentForwardingRequest, etc). This ensures proper ordering and error detection when setting environment variables on shell/execute sessions with PTY allocations.
Add support for unencrypted legacy PEM EC private keys (BEGIN EC PRIVATE KEY) in SSHKeyPair.fromPem and SSHKeyPair.isEncryptedPem.\n\nIncludes regression test using issue TerminalStudio#109 sample and changelog entry.\n\nPartial fix for TerminalStudio#109.
…21-shell-stdio-terminal-guard Update version to 2.16.0 and clarify stdio handling for CLI-only usage
Adds SSHRunResult and SSHClient.runWithResult() so callers can access output together with exitCode and exitSignal. Keeps SSHClient.run() backward compatible as a convenience wrapper that returns combined output bytes.\n\nIncludes integration coverage for successful and failing commands.\n\nCloses TerminalStudio#99.
…denv-race-condition fix(channel): make sendEnv awaitable to prevent race condition with PTY requests
…gacy-ec-private-key-support feat(keys): add legacy EC PRIVATE KEY PEM support (issue TerminalStudio#109)
…-with-result feat(client): add runWithResult API and end-to-end flow example
…ownload-api-124 feat(sftp): add high-level download API and integration coverage
…onutf8-filenames-95 fix(sftp): tolerate malformed UTF-8 filenames
- **Breaking change**: Changed `SSHChannelController.sendEnv()` from `void` to `Future<bool>` to correctly handle responses to environment variable settings - Improved shell stdio handling logic to prevent crashes in terminal-less environments - Added support for the legacy `EC PRIVATE KEY` PEM format - Added the `SSHClient.runWithResult()` method to retrieve command output and exit status - Added an advanced SFTP download API and optimized performance for large file transfers - Made SFTP filename parsing compatible with non-UTF-8 encodings - Improved error messages on the web platform - Added sample programs to demonstrate the complete execution flow - Updated documentation and test cases
…ture type validation Changed the `x11ScreenNumber` from a string type to an integer type to match the actual use case Added RSA signature type validation to `SSHKeyPairAgent` to prevent type mismatches Updated the relevant test cases to reflect the type change
📝 WalkthroughWalkthrough将包版本更新为 2.16.0，更新 CHANGELOG 与 README 并新增示例文件（example/run_flows.dart）。实现/API变更包括：将 SSHChannelController.sendEnv 改为返回 Future 并等待请求回复；新增 SSHRunResult 与 SSHClient.runWithResult，同时调整 SSHClient.run 返回合并输出字节；添加 SftpClient.download 与 SftpFile.downloadTo 并重构 SftpFile.read 为可配置并发管线读取；SFTP 名称解析改为对损坏 UTF‑8 容错；新增对 legacy Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… and stderr were confused during error handling Fixed status error handling during file lookups in SFTP client testing Improved terminal mode recovery logic in the password input example Enhanced terminal detection handling in the shell example Optimized offset handling for SFTP file reading Improved curve ID recognition in EC key pair parsing
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…EC public key fix(sftp_client_test): Ensure the test file is large enough to allow for a partial download refactor(sftp_name): Improve UTF-8 decoding and encoding handling
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/src/ssh_key_pair.dart (1)
827-838: 建议：可考虑在推断曲线时验证长度一致性。当前
_inferCurveId使用||逻辑，只要公钥点长度或私钥长度任一匹配即返回。虽然后续的公钥点验证（lines 806-813）会捕获不一致的情况，但如果在此处显式检查两者一致性，代码意图会更清晰。♻️ 可选的改进方案
String? _inferCurveId(int publicPointLength, int privateKeyLength) { - if (publicPointLength == 65 || privateKeyLength == 32) { + if ((publicPointLength == 0 || publicPointLength == 65) && + (privateKeyLength == 0 || privateKeyLength == 32) && + (publicPointLength != 0 || privateKeyLength != 0)) { return 'nistp256'; } - if (publicPointLength == 97 || privateKeyLength == 48) { + if ((publicPointLength == 0 || publicPointLength == 97) && + (privateKeyLength == 0 || privateKeyLength == 48) && + (publicPointLength != 0 || privateKeyLength != 0)) { return 'nistp384'; } - if (publicPointLength == 133 || privateKeyLength == 66) { + if ((publicPointLength == 0 || publicPointLength == 133) && + (privateKeyLength == 0 || privateKeyLength == 66) && + (publicPointLength != 0 || privateKeyLength != 0)) { return 'nistp521'; } return null; }不过由于公钥点验证已经能捕获不一致的情况，当前实现也是可以接受的。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/src/ssh_key_pair.dart` around lines 827 - 838, The _inferCurveId function currently returns a curve when either publicPointLength or privateKeyLength matches a curve (using ||); change the logic to require both lengths to match for each curve (use && semantics) so _inferCurveId only returns 'nistp256', 'nistp384' or 'nistp521' when publicPointLength AND privateKeyLength correspond to the same curve, leaving existing public-point validation unchanged; update the condition checks in _inferCurveId to enforce length consistency between publicPointLength and privateKeyLength.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/src/ssh_key_pair.dart`:
- Around line 827-838: The _inferCurveId function currently returns a curve when
either publicPointLength or privateKeyLength matches a curve (using ||); change
the logic to require both lengths to match for each curve (use && semantics) so
_inferCurveId only returns 'nistp256', 'nistp384' or 'nistp521' when
publicPointLength AND privateKeyLength correspond to the same curve, leaving
existing public-point validation unchanged; update the condition checks in
_inferCurveId to enforce length consistency between publicPointLength and
privateKeyLength.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4f456589-f91a-43a4-8e57-5c65aba04838
📒 Files selected for processing (3)
lib/src/sftp/sftp_name.dartlib/src/ssh_key_pair.darttest/src/sftp/sftp_client_test.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/src/sftp/sftp_name.dart
- test/src/sftp/sftp_client_test.dart
| [@falrom]: https://github.com/falrom | ||
| [@bradmartin333]: https://github.com/bradmartin333 | ||
| [@Wackymax]: https://github.com/Wackymax | ||
| [@vicajilau]: https://github.com/vicajilau |
There was a problem hiding this comment.
🟡 Removed CHANGELOG link definitions for contributors still referenced in text
The link definitions for [@shihuili1218] and [@isegal] were removed from the CHANGELOG footer, but both are still referenced in the 2.14.0 section (lines 20-21). This results in broken markdown links — the rendered text will show raw [@shihuili1218] and [@isegal] brackets instead of clickable hyperlinks.
| [@vicajilau]: https://github.com/vicajilau | |
| [@vicajilau]: https://github.com/vicajilau | |
| [@shihuili1218]: https://github.com/shihuili1218 | |
| [@isegal]: https://github.com/isegal |
Was this helpful? React with 👍 or 👎 to provide feedback.
| while (bytesRead < length) { | ||
| while ( | ||
| reservedOffset < endOffset && pendingReads.length < maxPendingRequests) { | ||
| final requestLength = min(chunkSize, endOffset - reservedOffset); | ||
| pendingReads.add((reservedOffset, _readChunk(requestLength, reservedOffset))); | ||
| reservedOffset += requestLength; | ||
| } | ||
|
|
||
| if (chunk == null) { | ||
| streamController.close(); | ||
| return; | ||
| if (pendingReads.isEmpty) break; | ||
|
|
||
| final (startOffset, readFuture) = pendingReads.removeAt(0); | ||
| final chunk = await readFuture; | ||
| if (chunk == null) break; | ||
| if (chunk.isEmpty) { | ||
| throw SftpError('Unexpected empty data chunk before EOF'); | ||
| } | ||
|
|
||
| streamController.add(chunk); | ||
| bytessRecieved += chunkLength; | ||
| final remaining = length - bytesRead; | ||
| final outputChunk = chunk.length <= remaining | ||
| ? chunk | ||
| : Uint8List.sublistView(chunk, 0, remaining); | ||
|
|
||
| if (onProgress != null) onProgress(bytessRecieved); | ||
| yield outputChunk; | ||
|
|
||
| if (bytessRecieved >= length) { | ||
| streamController.close(); | ||
| return; | ||
| } | ||
| bytesRead += outputChunk.length; | ||
| onProgress?.call(bytesRead); | ||
| } |
There was a problem hiding this comment.
🟡 SFTP read pipeline abandons in-flight futures without error handlers, causing unhandled async errors
The rewritten SftpFile.read() pipeline dispatches up to maxPendingRequests read operations in parallel but only consumes them one-by-one via pendingReads.removeAt(0). When the loop exits early — via chunk == null (EOF at line 655) or an exception from await readFuture (line 654) — the remaining futures in pendingReads are abandoned without error handlers. If any of these in-flight _readChunk futures complete with an error (e.g. SftpStatusError), they become unhandled async errors that can crash CLI apps or trigger Zone error handlers.
Comparison with old code
The old implementation used a try/catch inside each readChunk callback and routed errors to the StreamController via streamController.addError(e, st), properly handling errors from all in-flight reads even after the stream was closed. The new code lacks equivalent cleanup for abandoned futures.
Prompt for agents
In lib/src/sftp/sftp_client.dart, in the SftpFile.read() method (around lines 643-669), when the while loop breaks early (EOF, error, or stream cancellation), the remaining futures in pendingReads are abandoned. Add cleanup logic after the while loop to await (with error suppression) all remaining pending futures. For example, after the while loop on line 669, add something like:
for (final (_, future) in pendingReads) {
future.then((_) {}, onError: (_) {});
}
pendingReads.clear();
This ensures that errors from in-flight reads don't become unhandled async errors. Alternatively, wrap the whole while loop in a try/finally block to ensure cleanup even when the loop body throws.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Summary by CodeRabbit
发布说明
新功能
修复
文档