feat: add native Rebex FTP upload handler and receiver - #190
Conversation
Port the serverless FTP handler and receiver into in-process native adapters (NativeRebexFtpUploadHandler, NativeRebexFtpReceiver), registered only when a Rebex license key is configured. - SshKeyNormalizer: shared, robust private-key normalization used by both adapters (rebuilds flattened single-line PEM keys, leaves well-formed keys untouched) - FtpProtocol.EnsurePasswordProvided: shared password guard — required for ftp/sftp, optional passphrase for sftpssh - Upload handler gains DataEncoding (utf8/base64) to support binary payloads, mirroring the receiver's ResponseEncoding - Receiver drops the broken RenameDuplicateFiles option
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository: simplify9/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds Rebex-based FTP/SFTP native adapters: a receiver (list/get/delete files) and an upload handler, both supporting ftp/sftp/sftpssh protocols with private-key login. Adds SSH key normalization and password-validation utilities, DI registrations, Rebex package upgrades, unit tests, and a ChangesRebex FTP/SFTP Adapters
Estimated code review effort: 3 (Moderate) | ~35 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Security notes: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@SW.Bitween.NativeAdapters/FtpProtocol.cs`:
- Around line 13-17: Add unit tests for EnsurePasswordProvided to cover the
shared auth guard used by the Rebex adapters. Test that it throws for ftp and
sftp when password is null or empty, and that it does not throw for sftpssh or
when a valid password is provided. Place the tests near the existing protocol
validation coverage and reference the FtpProtocol.EnsurePasswordProvided method
so the guard remains protected against regressions.
- Around line 13-17: The protocol connect/login logic is duplicated in both
NativeRebexFtpReceiver.Initialize and NativeRebexFtpUploadHandler.Handle, and
should be consolidated behind a shared factory in FtpProtocol. Add a reusable
Connect(protocol, host, port, username, password, privateKey) method that
performs the protocol switch, key setup, and login once, then update both
adapters to call it instead of maintaining separate sftpssh/sftp/ftp branches.
Keep the existing EnsurePasswordProvided rule in FtpProtocol and ensure the
shared factory preserves the same unknown-protocol behavior and connection
semantics.
In `@SW.Bitween.NativeAdapters/RebexFtpReceiver/NativeRebexFtpReceiver.cs`:
- Around line 21-34: The NativeRebexFtpReceiver.ReceiveAsync flow for the
"sftpssh" branch is missing boundary validation for _options.PrivateKey before
calling SshKeyNormalizer.Normalize and constructing SshPrivateKey. Add an
explicit check alongside the existing FtpProtocol.EnsurePasswordProvided
validation so that empty or null private keys are rejected early with a clear
message before connect/login logic runs. Keep the fix localized to the switch
branch that creates the Sftp instance and uses LoginAsync.
- Around line 25-62: The sftpssh branch in NativeRebexFtpReceiver.ConnectAsync
assigns _ftpOrSftp only after LoginAsync succeeds, unlike the sftp and ftp
branches, so a failed login can leave an open Sftp connection orphaned and
_ftpOrSftp null for Finalize(). Assign the connected Sftp instance to _ftpOrSftp
immediately after ConnectAsync, and make Finalize() safely handle a null or
uninitialized _ftpOrSftp before calling DisconnectAsync/Dispose so cleanup still
works when login fails.
In
`@SW.Bitween.NativeAdapters/RebexFtpUploadHandler/NativeRebexFtpUploadHandler.cs`:
- Around line 20-34: The sftpssh branch in NativeRebexFtpUploadHandler should
validate that _options.PrivateKey is present before creating SshPrivateKey and
calling LoginAsync. Add the same preflight check used in
NativeRebexFtpReceiver.Initialize (or equivalent validation path) near the
FtpProtocol.EnsurePasswordProvided call, and fail fast with a clear validation
error if the private key is missing or empty.
- Line 75: The upload path in NativeRebexFtpUploadHandler currently prefixes the
filename with a slash when _options.TargetPath is unset, which sends files to
the FTP root instead of the current directory. Update the path construction
around ftpOrSftp.PutFileAsync so it only combines TargetPath with filename when
TargetPath is non-empty, and otherwise uploads using just the filename; keep the
behavior aligned with the receiver’s directory handling.
- Around line 22-78: The upload handler leaves FTP/SFTP connections open on
failures because `ftpOrSftp` is only disconnected on the success path and never
disposed. In `NativeRebexFtpUploadHandler`, assign the concrete client to
`ftpOrSftp` before any login/connect branch completes, wrap the whole
connect/login/upload flow in a try/finally, and ensure the finally block always
calls both `DisconnectAsync()` and `Dispose()` (guarding for null/connected
state as needed). Make sure this cleanup runs for all protocol cases, including
`sftpssh`, `sftp`, and `ftp`, so failed logins or `PutFileAsync` exceptions do
not leak sockets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: simplify9/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 775096b8-0194-4ec2-b794-10956d8ae6fe
📒 Files selected for processing (10)
.gitignoreSW.Bitween.NativeAdapters/FtpProtocol.csSW.Bitween.NativeAdapters/RebexFtpReceiver/NativeRebexFtpReceiver.csSW.Bitween.NativeAdapters/RebexFtpReceiver/RebexFtpReceiverInput.csSW.Bitween.NativeAdapters/RebexFtpUploadHandler/NativeRebexFtpUploadHandler.csSW.Bitween.NativeAdapters/RebexFtpUploadHandler/RebexFtpUploadHandlerInput.csSW.Bitween.NativeAdapters/SW.Bitween.NativeAdapters.csprojSW.Bitween.NativeAdapters/ServiceCollectionExtensions.csSW.Bitween.NativeAdapters/SshKeyNormalizer.csSW.Bitween.UnitTests/SshKeyNormalizerTests.cs
📜 Review details
🔇 Additional comments (9)
SW.Bitween.NativeAdapters/ServiceCollectionExtensions.cs (1)
4-5: LGTM!Also applies to: 45-50
.gitignore (1)
361-361: LGTM!SW.Bitween.NativeAdapters/SW.Bitween.NativeAdapters.csproj (1)
20-23: 📐 Maintainability & Code QualityCheck Rebex 8.x compatibility in the native adapters.
Rebex.MailandRebex.Pop3jump from 6.0.8060 to 8.0.9673, andSW.Bitween.NativeAdaptersuses Rebex APIs directly; make sure the existing FTP/POP3 handlers still compile and behave correctly with the new version.SW.Bitween.NativeAdapters/SshKeyNormalizer.cs (1)
1-49: LGTM!SW.Bitween.UnitTests/SshKeyNormalizerTests.cs (1)
1-69: LGTM!SW.Bitween.NativeAdapters/RebexFtpReceiver/RebexFtpReceiverInput.cs (1)
1-37: LGTM!SW.Bitween.NativeAdapters/RebexFtpReceiver/NativeRebexFtpReceiver.cs (1)
64-110: LGTM!SW.Bitween.NativeAdapters/RebexFtpUploadHandler/RebexFtpUploadHandlerInput.cs (1)
1-31: LGTM!SW.Bitween.NativeAdapters/RebexFtpUploadHandler/NativeRebexFtpUploadHandler.cs (1)
81-89: LGTM!
| public static void EnsurePasswordProvided(string protocol, string? password) | ||
| { | ||
| if (protocol.ToLower() is "ftp" or "sftp" && string.IsNullOrEmpty(password)) | ||
| throw new ArgumentException($"Password is required for the '{protocol}' protocol."); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add unit tests for EnsurePasswordProvided.
This guard is the sole validation gate shared by both Rebex adapters (ftp/sftp require password, sftpssh doesn't), but no test file covers it (only SshKeyNormalizerTests.cs was added). A regression here silently breaks auth validation for both adapters.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SW.Bitween.NativeAdapters/FtpProtocol.cs` around lines 13 - 17, Add unit
tests for EnsurePasswordProvided to cover the shared auth guard used by the
Rebex adapters. Test that it throws for ftp and sftp when password is null or
empty, and that it does not throw for sftpssh or when a valid password is
provided. Place the tests near the existing protocol validation coverage and
reference the FtpProtocol.EnsurePasswordProvided method so the guard remains
protected against regressions.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Duplicated connect/login switch across both adapters is a good candidate for consolidation here.
NativeRebexFtpReceiver.Initialize and NativeRebexFtpUploadHandler.Handle both implement a nearly identical sftpssh/sftp/ftp switch (connect, key setup, login, unknown-protocol throw). This file already centralizes the shared password rule — extending it with a shared Connect(protocol, host, port, username, password, privateKey) : Task<IFtp> factory would remove ~30 duplicated lines per adapter and prevent the two implementations from silently diverging (see the assignment-order and target-path issues flagged in the receiver/handler files, which only exist in one of the two copies).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SW.Bitween.NativeAdapters/FtpProtocol.cs` around lines 13 - 17, The protocol
connect/login logic is duplicated in both NativeRebexFtpReceiver.Initialize and
NativeRebexFtpUploadHandler.Handle, and should be consolidated behind a shared
factory in FtpProtocol. Add a reusable Connect(protocol, host, port, username,
password, privateKey) method that performs the protocol switch, key setup, and
login once, then update both adapters to call it instead of maintaining separate
sftpssh/sftp/ftp branches. Keep the existing EnsurePasswordProvided rule in
FtpProtocol and ensure the shared factory preserves the same unknown-protocol
behavior and connection semantics.
| case "sftpssh": | ||
| var sftpssh = new Sftp(); | ||
| await sftpssh.ConnectAsync(_options.Host, _options.Port ?? 22); | ||
|
|
||
| var keyBytes = Encoding.UTF8.GetBytes(SshKeyNormalizer.Normalize(_options.PrivateKey)); | ||
| var privateKey = new SshPrivateKey(keyBytes, _options.Password); | ||
| await sftpssh.LoginAsync(_options.Username, privateKey); | ||
|
|
||
| _ftpOrSftp = sftpssh; | ||
| break; | ||
|
|
||
| case "sftp": | ||
| var sftp = new Sftp(); | ||
| await sftp.ConnectAsync(_options.Host, _options.Port ?? 22); | ||
| _ftpOrSftp = sftp; | ||
| await _ftpOrSftp.LoginAsync(_options.Username, _options.Password); | ||
| break; | ||
|
|
||
| case "ftp": | ||
| var ftp = new Rebex.Net.Ftp(); | ||
| await ftp.ConnectAsync(_options.Host, _options.Port ?? 21); | ||
| _ftpOrSftp = ftp; | ||
| await _ftpOrSftp.LoginAsync(_options.Username, _options.Password); | ||
| break; | ||
|
|
||
| default: | ||
| throw new ArgumentException($"Unknown protocol '{_options.Protocol}'"); | ||
| } | ||
|
|
||
| if (!string.IsNullOrEmpty(_options.TargetPath)) | ||
| await _ftpOrSftp.ChangeDirectoryAsync(_options.TargetPath); | ||
| } | ||
|
|
||
| public async Task Finalize() | ||
| { | ||
| await _ftpOrSftp.DisconnectAsync(); | ||
| _ftpOrSftp.Dispose(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Connection leaked and Finalize() throws NRE if sftpssh login fails.
_ftpOrSftp = sftpssh (line 33) only happens after LoginAsync succeeds (line 31) — unlike the sftp/ftp branches, which assign before login. If login throws (bad credentials, malformed key), the already-connected sftpssh socket is never assigned to _ftpOrSftp, so it's orphaned (never disposed), and _ftpOrSftp stays null!. If the caller then invokes Finalize() in a cleanup path, _ftpOrSftp.DisconnectAsync() (line 60) throws a NullReferenceException that masks the real login failure.
🔒 Proposed fix
case "sftpssh":
var sftpssh = new Sftp();
await sftpssh.ConnectAsync(_options.Host, _options.Port ?? 22);
+ _ftpOrSftp = sftpssh;
var keyBytes = Encoding.UTF8.GetBytes(SshKeyNormalizer.Normalize(_options.PrivateKey));
var privateKey = new SshPrivateKey(keyBytes, _options.Password);
await sftpssh.LoginAsync(_options.Username, privateKey);
-
- _ftpOrSftp = sftpssh;
break; public async Task Finalize()
{
+ if (_ftpOrSftp is null)
+ return;
await _ftpOrSftp.DisconnectAsync();
_ftpOrSftp.Dispose();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case "sftpssh": | |
| var sftpssh = new Sftp(); | |
| await sftpssh.ConnectAsync(_options.Host, _options.Port ?? 22); | |
| var keyBytes = Encoding.UTF8.GetBytes(SshKeyNormalizer.Normalize(_options.PrivateKey)); | |
| var privateKey = new SshPrivateKey(keyBytes, _options.Password); | |
| await sftpssh.LoginAsync(_options.Username, privateKey); | |
| _ftpOrSftp = sftpssh; | |
| break; | |
| case "sftp": | |
| var sftp = new Sftp(); | |
| await sftp.ConnectAsync(_options.Host, _options.Port ?? 22); | |
| _ftpOrSftp = sftp; | |
| await _ftpOrSftp.LoginAsync(_options.Username, _options.Password); | |
| break; | |
| case "ftp": | |
| var ftp = new Rebex.Net.Ftp(); | |
| await ftp.ConnectAsync(_options.Host, _options.Port ?? 21); | |
| _ftpOrSftp = ftp; | |
| await _ftpOrSftp.LoginAsync(_options.Username, _options.Password); | |
| break; | |
| default: | |
| throw new ArgumentException($"Unknown protocol '{_options.Protocol}'"); | |
| } | |
| if (!string.IsNullOrEmpty(_options.TargetPath)) | |
| await _ftpOrSftp.ChangeDirectoryAsync(_options.TargetPath); | |
| } | |
| public async Task Finalize() | |
| { | |
| await _ftpOrSftp.DisconnectAsync(); | |
| _ftpOrSftp.Dispose(); | |
| } | |
| case "sftpssh": | |
| var sftpssh = new Sftp(); | |
| await sftpssh.ConnectAsync(_options.Host, _options.Port ?? 22); | |
| _ftpOrSftp = sftpssh; | |
| var keyBytes = Encoding.UTF8.GetBytes(SshKeyNormalizer.Normalize(_options.PrivateKey)); | |
| var privateKey = new SshPrivateKey(keyBytes, _options.Password); | |
| await sftpssh.LoginAsync(_options.Username, privateKey); | |
| break; | |
| case "sftp": | |
| var sftp = new Sftp(); | |
| await sftp.ConnectAsync(_options.Host, _options.Port ?? 22); | |
| _ftpOrSftp = sftp; | |
| await _ftpOrSftp.LoginAsync(_options.Username, _options.Password); | |
| break; | |
| case "ftp": | |
| var ftp = new Rebex.Net.Ftp(); | |
| await ftp.ConnectAsync(_options.Host, _options.Port ?? 21); | |
| _ftpOrSftp = ftp; | |
| await _ftpOrSftp.LoginAsync(_options.Username, _options.Password); | |
| break; | |
| default: | |
| throw new ArgumentException($"Unknown protocol '{_options.Protocol}'"); | |
| } | |
| if (!string.IsNullOrEmpty(_options.TargetPath)) | |
| await _ftpOrSftp.ChangeDirectoryAsync(_options.TargetPath); | |
| } | |
| public async Task Finalize() | |
| { | |
| if (_ftpOrSftp is null) | |
| return; | |
| await _ftpOrSftp.DisconnectAsync(); | |
| _ftpOrSftp.Dispose(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SW.Bitween.NativeAdapters/RebexFtpReceiver/NativeRebexFtpReceiver.cs` around
lines 25 - 62, The sftpssh branch in NativeRebexFtpReceiver.ConnectAsync assigns
_ftpOrSftp only after LoginAsync succeeds, unlike the sftp and ftp branches, so
a failed login can leave an open Sftp connection orphaned and _ftpOrSftp null
for Finalize(). Assign the connected Sftp instance to _ftpOrSftp immediately
after ConnectAsync, and make Finalize() safely handle a null or uninitialized
_ftpOrSftp before calling DisconnectAsync/Dispose so cleanup still works when
login fails.
| IFtp ftpOrSftp; | ||
| switch (_options.Protocol.ToLower()) | ||
| { | ||
| case "sftpssh": | ||
| var sftpssh = new Sftp(); | ||
| await sftpssh.ConnectAsync(_options.Host, _options.Port ?? 22); | ||
|
|
||
| var keyBytes = Encoding.UTF8.GetBytes(SshKeyNormalizer.Normalize(_options.PrivateKey)); | ||
| var sshPrivateKey = new SshPrivateKey(keyBytes, _options.Password); | ||
| await sftpssh.LoginAsync(_options.Username, sshPrivateKey); | ||
|
|
||
| ftpOrSftp = sftpssh; | ||
| break; | ||
|
|
||
| case "sftp": | ||
| var sftp = new Sftp(); | ||
| await sftp.ConnectAsync(_options.Host, _options.Port ?? 22); | ||
| ftpOrSftp = sftp; | ||
| await ftpOrSftp.LoginAsync(_options.Username, _options.Password); | ||
| break; | ||
|
|
||
| case "ftp": | ||
| var ftp = new Rebex.Net.Ftp(); | ||
| await ftp.ConnectAsync(_options.Host, _options.Port ?? 21); | ||
| ftpOrSftp = ftp; | ||
| await ftpOrSftp.LoginAsync(_options.Username, _options.Password); | ||
| break; | ||
|
|
||
| default: | ||
| throw new ArgumentException($"Unknown protocol '{_options.Protocol}'"); | ||
| } | ||
|
|
||
| var bytes = _options.DataEncoding.ToLower() switch | ||
| { | ||
| "base64" => Convert.FromBase64String(xchangeFile.Data), | ||
| "utf8" => Encoding.UTF8.GetBytes(xchangeFile.Data), | ||
| _ => throw new ArgumentException( | ||
| $"Unknown {nameof(RebexFtpUploadHandlerInput.DataEncoding)} '{_options.DataEncoding}'") | ||
| }; | ||
|
|
||
| await using var stream = new MemoryStream(bytes); | ||
|
|
||
| var filename = xchangeFile.Filename; | ||
| if (string.IsNullOrWhiteSpace(filename)) | ||
| { | ||
| var currentDate = DateTime.UtcNow; | ||
| filename = | ||
| $"{currentDate.Year:0000}{currentDate.Month:00}{currentDate.Day:00}{currentDate.Hour:00}{currentDate.Minute:00}{currentDate.Second:00}{currentDate.Millisecond:000}"; | ||
| } | ||
|
|
||
| if (!string.IsNullOrWhiteSpace(_options.FileNamePrefix)) | ||
| filename = $"{_options.FileNamePrefix}_{filename}"; | ||
|
|
||
| await ftpOrSftp.PutFileAsync(stream, $"{_options.TargetPath}/{filename}"); | ||
|
|
||
| await ftpOrSftp.DisconnectAsync(); | ||
| return new XchangeFile(string.Empty); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Connection is never disposed, and leaks entirely on any failure between connect and upload.
ftpOrSftp is only disconnected on the success path (line 77) — Dispose() is never called at all, and if PutFileAsync (or anything after connect) throws, neither DisconnectAsync nor Dispose runs. Combined with the sftpssh branch assigning ftpOrSftp = sftpssh only after LoginAsync succeeds (line 33, same issue as the receiver), a failed login also leaks the already-open socket with no reference left to clean it up. Under repeated upload failures this accumulates open connections until GC finalizers eventually catch up (unreliable) or the FTP server's connection limit is hit.
🔒 Proposed fix (wrap in try/finally, assign before login)
- IFtp ftpOrSftp;
- switch (_options.Protocol.ToLower())
- {
- case "sftpssh":
- var sftpssh = new Sftp();
- await sftpssh.ConnectAsync(_options.Host, _options.Port ?? 22);
-
- var keyBytes = Encoding.UTF8.GetBytes(SshKeyNormalizer.Normalize(_options.PrivateKey));
- var sshPrivateKey = new SshPrivateKey(keyBytes, _options.Password);
- await sftpssh.LoginAsync(_options.Username, sshPrivateKey);
-
- ftpOrSftp = sftpssh;
- break;
- ...
- }
- ...
- await ftpOrSftp.PutFileAsync(stream, $"{_options.TargetPath}/{filename}");
- await ftpOrSftp.DisconnectAsync();
- return new XchangeFile(string.Empty);
+ IFtp? ftpOrSftp = null;
+ try
+ {
+ switch (_options.Protocol.ToLower())
+ {
+ case "sftpssh":
+ var sftpssh = new Sftp();
+ await sftpssh.ConnectAsync(_options.Host, _options.Port ?? 22);
+ ftpOrSftp = sftpssh;
+
+ var keyBytes = Encoding.UTF8.GetBytes(SshKeyNormalizer.Normalize(_options.PrivateKey));
+ var sshPrivateKey = new SshPrivateKey(keyBytes, _options.Password);
+ await sftpssh.LoginAsync(_options.Username, sshPrivateKey);
+ break;
+ // ... other cases, assign ftpOrSftp before login too
+ }
+ // ... build bytes/stream/filename
+ await ftpOrSftp.PutFileAsync(stream, remotePath);
+ return new XchangeFile(string.Empty);
+ }
+ finally
+ {
+ if (ftpOrSftp is not null)
+ {
+ await ftpOrSftp.DisconnectAsync();
+ ftpOrSftp.Dispose();
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@SW.Bitween.NativeAdapters/RebexFtpUploadHandler/NativeRebexFtpUploadHandler.cs`
around lines 22 - 78, The upload handler leaves FTP/SFTP connections open on
failures because `ftpOrSftp` is only disconnected on the success path and never
disposed. In `NativeRebexFtpUploadHandler`, assign the concrete client to
`ftpOrSftp` before any login/connect branch completes, wrap the whole
connect/login/upload flow in a try/finally, and ensure the finally block always
calls both `DisconnectAsync()` and `Dispose()` (guarding for null/connected
state as needed). Make sure this cleanup runs for all protocol cases, including
`sftpssh`, `sftp`, and `ftp`, so failed logins or `PutFileAsync` exceptions do
not leak sockets.
| if (!string.IsNullOrWhiteSpace(_options.FileNamePrefix)) | ||
| filename = $"{_options.FileNamePrefix}_{filename}"; | ||
|
|
||
| await ftpOrSftp.PutFileAsync(stream, $"{_options.TargetPath}/{filename}"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Uploads land at the FTP root when TargetPath is unset.
$"{_options.TargetPath}/{filename}" evaluates to "/filename" when TargetPath is null (it's optional, no default). This uploads to the server root instead of the connected/current directory, unlike the receiver which only calls ChangeDirectoryAsync when TargetPath is non-empty.
🐛 Proposed fix
- await ftpOrSftp.PutFileAsync(stream, $"{_options.TargetPath}/{filename}");
+ var remotePath = string.IsNullOrWhiteSpace(_options.TargetPath)
+ ? filename
+ : $"{_options.TargetPath.TrimEnd('/')}/{filename}";
+ await ftpOrSftp.PutFileAsync(stream, remotePath);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await ftpOrSftp.PutFileAsync(stream, $"{_options.TargetPath}/{filename}"); | |
| var remotePath = string.IsNullOrWhiteSpace(_options.TargetPath) | |
| ? filename | |
| : $"{_options.TargetPath.TrimEnd('/')}/{filename}"; | |
| await ftpOrSftp.PutFileAsync(stream, remotePath); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@SW.Bitween.NativeAdapters/RebexFtpUploadHandler/NativeRebexFtpUploadHandler.cs`
at line 75, The upload path in NativeRebexFtpUploadHandler currently prefixes
the filename with a slash when _options.TargetPath is unset, which sends files
to the FTP root instead of the current directory. Update the path construction
around ftpOrSftp.PutFileAsync so it only combines TargetPath with filename when
TargetPath is non-empty, and otherwise uploads using just the filename; keep the
behavior aligned with the receiver’s directory handling.
Add FtpProtocol.EnsurePrivateKeyProvided and call it in both the FTP receiver and upload handler before connecting. A missing PrivateKey on sftpssh now fails fast with a clear message instead of surfacing as an opaque Rebex exception from SshPrivateKey deep in the stack.
2 participants
Port the serverless FTP handler and receiver into in-process native adapters (NativeRebexFtpUploadHandler, NativeRebexFtpReceiver), registered only when a Rebex license key is configured.