Skip to content

Add SFTP encoding support#9451

Merged
ncw merged 2 commits into
rclone:masterfrom
puneetdixit200:fix/8574-sftp-encoding
May 30, 2026
Merged

Add SFTP encoding support#9451
ncw merged 2 commits into
rclone:masterfrom
puneetdixit200:fix/8574-sftp-encoding

Conversation

@puneetdixit200
Copy link
Copy Markdown
Contributor

Summary

  • add the standard backend encoding option for SFTP
  • encode outbound SFTP and shell paths using the configured encoder
  • decode listed SFTP entry names back to rclone standard names
  • add no-server regression coverage for encoded SFTP paths

Fixes #8574

Verification

  • GOMODCACHE=/tmp/rclone-8574-gomodcache GOCACHE=/tmp/rclone-8574-gocache go test ./backend/sftp -run '^(TestShellEscapeUnix|TestShellEscapeCmd|TestShellEscapePowerShell|TestRemotePathEncodesRemoteNames|TestRemoteShellPathEncodesRemoteNames|TestRemoteShellPathEncodesPathOverrideNames|TestParseHash|TestParseUsage|TestSSHExternalWaitMultipleCalls|TestSSHExternalCloseMultipleCalls|TestStringLock)$' -count=1\n- GOMODCACHE=/tmp/rclone-8574-gomodcache GOCACHE=/tmp/rclone-8574-gocache go test ./lib/encoder -count=1\n- GOMODCACHE=/tmp/rclone-8574-gomodcache GOCACHE=/tmp/rclone-8574-gocache make backenddocs\n- GOMODCACHE=/tmp/rclone-8574-gomodcache GOCACHE=/tmp/rclone-8574-gocache make compiletest\n- git diff --check\n- test -z "$(gofmt -l backend/sftp/sftp.go backend/sftp/sftp_internal_test.go)"\n\nNote: go test ./backend/sftp -count=1 reaches the integration tests in this environment and fails because fstest/testserver/init.d/run.bash requires a flock binary that is not available locally. The SFTP package unit subset above passed.

@puneetdixit200
Copy link
Copy Markdown
Contributor Author

Updated the branch with the same golang.org/x dependency bump now on upstream master to address the govulncheck failure from the previous lint job.

Local verification on the new head (1f2c39f):

  • go run golang.org/x/vuln/cmd/govulncheck@latest ./... -> no vulnerabilities found
  • go test ./backend/sftp -run 'TestRemotePathEncodesRemoteNames|TestRemoteShellPathEncodesRemoteNames|TestRemoteShellPathEncodesPathOverrideNames|TestShellEscapeUnix|TestShellEscapeCmd|TestShellEscapePowerShell|TestParseHash|TestParseUsage' -count=1
  • go test ./lib/encoder -count=1
  • git diff --cached --check before commit

The new upstream Actions run is currently action_required, so it looks like it needs maintainer approval before CI can execute on this updated head.

Copy link
Copy Markdown
Member

@ncw ncw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looking great - thank you :-)

I found one minor nit which the lint checks will surface when I run them.

Otherwise perfect!

Comment thread docs/content/sftp.md Outdated
@puneetdixit200 puneetdixit200 force-pushed the fix/8574-sftp-encoding branch from c8981d1 to 311e672 Compare May 26, 2026 07:57
@puneetdixit200
Copy link
Copy Markdown
Contributor Author

Follow-up pushed in 311e6729 and the generated-doc review thread is resolved. The PR diff is now limited to backend/sftp, go.mod, and go.sum; docs/content/sftp.md is no longer changed.

GitHub Actions for the new head are back to action_required after the force-push, so this needs maintainer approval again before CI can run.

@ncw ncw force-pushed the fix/8574-sftp-encoding branch from 311e672 to ce2a9c5 Compare May 26, 2026 11:21
@puneetdixit200
Copy link
Copy Markdown
Contributor Author

Checked the current head ce2a9c50 against the lint note.

Local verification:

  • golangci-lint run ./backend/sftp --timeout=10m -> 0 issues
  • same package with GOOS=linux, windows, freebsd, and openbsd -> 0 issues
  • golangci-lint run --new-from-rev=upstream/master ./... --timeout=10m -> 0 issues
  • focused SFTP unit tests -> pass
  • go test ./lib/encoder -count=1 -> pass

go test ./backend/sftp -count=1 still reaches the integration tests locally, but those are blocked here by missing flock in fstest/testserver. I do not see a branch-local lint patch needed on this head.

@ncw
Copy link
Copy Markdown
Member

ncw commented May 26, 2026

Github actions is down at the moment :-) I think this is ready to merge but I want to see it run through the actions first!

@ncw ncw force-pushed the fix/8574-sftp-encoding branch from ce2a9c5 to 73af29d Compare May 26, 2026 17:12
Copy link
Copy Markdown
Member

@ncw ncw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you - this is great :-)

I tested this locally since Github actions is not picking up this PR.

Will merge now.

@ncw ncw merged commit 1c92cec into rclone:master May 30, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Support for Encoding to sftp Connections

2 participants