rsync-ssl, testsuite: accept --type=SSL_TYPE anywhere in the args - #1047
rsync-ssl, testsuite: accept --type=SSL_TYPE anywhere in the args#1047Zeno-sole wants to merge 2 commits into
Conversation
|
Before repair： After repair： |
|
I would like to kindly request a code review from the upstream author/maintainer. @tridge |
|
No need to ping after less then 24 hours. 1) a release has just been made with major security concerns, we are resting. 2) its been less then 24 hours :D Let's be respectful. "kindly" doesn't make something respectful. |
The --type=... option was only recognized as the first argument, so a command such as "rsync-ssl --dry-run --type=stunnel host::mod" passed the option through to the underlying rsync, which rejected it as unknown. Scan the full argument list for --type=..., export RSYNC_SSL_TYPE, and drop the option before handing the remaining args to rsync. The manpage no longer says the option must be first. Adds a test that runs rsync-ssl with a fake rsync in PATH and checks that --type= is consumed in first, middle, and last positions.
rsync-ssl consumed a --type=... operand that appeared after a -- argument, even though -- explicitly protects the rest of the command line from option parsing. Stop scanning for --type=... at a -- argument: preserve the -- and every subsequent argument verbatim, passing them through to rsync unchanged. Add regression coverage for this case and document the behavior in the manpage.
| fakebin = base / 'bin' | ||
| fakebin.mkdir(parents=True) | ||
| fake_rsync = fakebin / 'rsync' | ||
| fake_rsync.write_text(f"#!/usr/bin/env bash\nprintf '%s\\n' \"$@\" > {args_capture}\nexit 0\n") |
There was a problem hiding this comment.
Please capture and assert RSYNC_SSL_TYPE as well as argv. The current test proves that --type= is removed but it would also pass if the wrapper silently discarded the requested SSL implementation instead of exporting it. subprocess.run() should also check the wrappers exit status.
2 participants
The --type=... option was only recognized as the first argument, so a command such as "rsync-ssl --dry-run --type=stunnel host::mod" passed the option through to the underlying rsync, which rejected it as unknown. Scan the full argument list for --type=..., export RSYNC_SSL_TYPE, and drop the option before handing the remaining args to rsync. The manpage no longer says the option must be first.
Adds a test that runs rsync-ssl with a fake rsync in PATH and checks that --type= is consumed in first, middle, and last positions.