feat: insert -- between RSH and rsync commands#656
Open
TheoTechnicguy wants to merge 1 commit intoRsyncProject:masterfrom
Open
feat: insert -- between RSH and rsync commands#656TheoTechnicguy wants to merge 1 commit intoRsyncProject:masterfrom
-- between RSH and rsync commands#656TheoTechnicguy wants to merge 1 commit intoRsyncProject:masterfrom
Conversation
charmitro
approved these changes
Nov 16, 2024
Contributor
charmitro
left a comment
There was a problem hiding this comment.
This is safe to merge. IMHO It protects against future remote shell updates.
Before patch:
$ ./rsync/rsync -av --verbose notes charmitro@rpi.local:~/
opening connection using: ssh -l charmitro rpi.local rsync --server -vvlogDtpre.iLsfxCIvu . "~/" (9 args)After patch:
$ ./rsync/rsync -av --verbose notes charmitro@rpi.local:~/
opening connection using: ssh -l charmitro rpi.local -- rsync --server -vvlogDtpre.iLsfxCIvu . "~/" (10 args)Nowadays, CLI program options not always support single-dash syntax and do not require flags to be set before positional arguments. To ensure that future remote shells do not interpret rsync flags, this commit inserts a double dash (`--`) before the transmitted rsync command.
b9e5f07 to
8816c9a
Compare
tridge
approved these changes
Aug 23, 2025
Member
|
@TheoTechnicguy this is failing the hardlinks test: |
tridge
requested changes
Aug 23, 2025
Member
tridge
left a comment
There was a problem hiding this comment.
will need changes to pass CI tests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Hi there!
I ran into some minor issue whilst tinkering with rsync and a new remote shell program. I noticed that rsync does not add the now common
--indicating the end of parsable arguments to the program between the RSH flags and rsync command.Effectively, this changes the generated command from
to
To the best of my knowledge, this practice is a POSIX-compatible convention, in use by virtually all applications in my daily use.
I have tested this change with the test scripts (2 tests skipped: crtimes and protected-regular) and with the current SSH implementation (OpenSSH_9.9p1, OpenSSL 3.3.2 3 Sep 2024; I can assert, however, that I have used this practice for 3+ years with OpenSSH). As such, I do not believe there to be any significant impact; nevertheless, I would be very appreciative of input about situations I did not think about. Do you know any other commonly used remote shells I should test?
Furthermore, I was unable to find contributing guidance, so please do not hesitate to indicate code formatting, commenting or other requirements I did not follow.
Cheers and many thanks for a utility I have had the pleasure of using for years (and will continue to)!
Edit: I did some digging and traced back the origin of the
--terminator.