fix(sftp): Fix SFTP file download failures on Windows #993

GT-610 · 2026-01-06T12:26:54Z

Fix multiple path handling issues that caused SFTP file download failures on Windows:

Invalid folder names: Replaced spi.oldId (format: user@ip:port) with spi.id (format: ptPQHje00) for folder naming, as the former contains characters illegal on Windows (@ and :)
Path separator mismatch: Fixed inconsistent use of path separators by normalizing all path operations to use platform-specific separators (Pfs.seperator)
RangeError: Fixed substring operations that used hardcoded / separator while localPath used platform-specific separators
Triple separators: Fixed path joining logic that resulted in malformed paths with triple separators (\\\)

This bug has been reported in #992.

Changes

Modified Files

lib/view/page/storage/sftp.dart
- Changed spi.oldId to spi.id for folder naming
- Added logic to strip leading separator from normalizedPath before joining paths
lib/data/model/sftp/worker.dart
- Changed hardcoded / to Pfs.seperator in lastIndexOf operation
lib/data/model/sftp/req.dart
- Changed hardcoded / to Pfs.seperator in split operation

This change only affects internal path handling logic and should not impact the public API.

The application already generates server IDs using ShortId.generate() (e.g., ptPQHje00), which are valid folder names on all platforms. The spi.oldId (format: user@ip:port) is still available for display purposes but should not be used for file system operations on Windows.

Summary by CodeRabbit

Bug Fixes
- Improved file path handling to use platform-appropriate path separators across file and directory operations, ensuring proper functionality across different operating systems.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Unify the use of Pfs.seperator for handling file path separators to enhance cross-platform compatibility.

coderabbitai · 2026-01-06T12:27:05Z

Walkthrough

Path separator handling is standardized across the codebase by replacing hard-coded forward slashes with a platform-aware separator constant (Pfs.seperator). Changes affect four files in the SFTP data layer and storage UI components, updating how file paths are parsed, normalized, and displayed. Control flow logic remains unaffected; only path construction mechanics are updated.

Changes

Cohort / File(s)	Summary
SFTP Data Model Path Handling `lib/data/model/sftp/req.dart`, `lib/data/model/sftp/worker.dart`	Updated `fileName` getter and directory path derivation to use `Pfs.seperator` instead of hard-coded `'/'` when splitting/parsing `localPath`. Enables platform-specific path separator handling in SFTP request processing.
Storage UI Path Extraction `lib/view/page/storage/local.dart`	Replaced hard-coded `'/'` with `Pfs.seperator` in three locations: file list-building logic, file action dialog, and delete dialog. Affects how file/directory names are extracted for UI labels.
SFTP Remote-to-Local Path Resolution `lib/view/page/storage/sftp.dart`	Modified `_getLocalPath` function to normalize `remotePath` using `Pfs.seperator`, strip leading separators, and join with `widget.args.spi.id` (changed from `oldId`). Updates local storage path construction for remote SFTP paths.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

On Windows, SFTP file download fails with PathNotFoundException and RangeError #992 — The changes directly address cross-platform path-separator and path-normalization issues by replacing hard-coded '/' with platform-aware separators across the SFTP and storage modules.

Poem

🐰 A slash here, a slash there—
Platform paths beyond compare!
From '/' to Pfs we hop with care,
Windows, Unix, all treated fair. ✨

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately captures the primary change: fixing SFTP file download failures on Windows through path separator and character handling improvements.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

lib/data/model/sftp/worker.dart (1)
73-73: Consider adding a defensive check for edge cases.

While localPath should always contain at least one separator (since it's constructed using joinPath), lastIndexOf returns -1 if the separator is not found, which would cause substring(0, -1) to throw a RangeError.
🔎 Proposed defensive check
-    final dirPath = req.localPath.substring(0, req.localPath.lastIndexOf(Pfs.seperator));
+    final lastSepIndex = req.localPath.lastIndexOf(Pfs.seperator);
+    if (lastSepIndex == -1) {
+      mainSendPort.send(Exception('Invalid local path: ${req.localPath}'));
+      return;
+    }
+    final dirPath = req.localPath.substring(0, lastSepIndex);

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7c35abe and 9373924.

📒 Files selected for processing (4)

lib/data/model/sftp/req.dart
lib/data/model/sftp/worker.dart
lib/view/page/storage/local.dart
lib/view/page/storage/sftp.dart

🧰 Additional context used

📓 Path-based instructions (3)

lib/data/model/**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

After modifying annotated model files, run: dart run build_runner build --delete-conflicting-outputs

Files:

lib/data/model/sftp/worker.dart
lib/data/model/sftp/req.dart

lib/data/model/sftp/**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

Place SFTP models under lib/data/model/sftp/

Files:

lib/data/model/sftp/worker.dart
lib/data/model/sftp/req.dart

lib/view/**/*.dart

📄 CodeRabbit inference engine (CLAUDE.md)

lib/view/**/*.dart: Prefer widgets/utilities from fl_lib for common UI (e.g., CustomAppBar, context.showRoundDialog, Input, Btnx.cancelOk)
Prefer using libL10n strings before adding new ones to project l10n
Split UI into build, actions, and utils; use extension on to separate concerns

Files:

lib/view/page/storage/local.dart
lib/view/page/storage/sftp.dart

🧠 Learnings (6)

📓 Common learnings

Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/model/sftp/**/*.dart : Place SFTP models under lib/data/model/sftp/