fix(scp): Use less escaping for filenames when not needed

[yedayak]

fix(scp): Use less escaping for filenames when not needed

OpenSSH 9.0 changed the default protocol the "scp" command uses to
sftp, which needs less escaping for special characters in remote
filenames. Check if the command has the option to use the legacy
protocol and do less escaping.

If the command line has the "-O" option to use the legacy protocol,
we revert to the old behaviour.

The extra escaping is necessary for older versions, so we can't do it
unconditionally.

Fixes: #1540

[akinomyoga]

Thank you for updating.

[scop]

Let's also mention remote filenames in the PR title and commit message. I think I'd rephrase it something like "use less remote filename escaping where supported", as the "less ... not needed" reads odd to me.

[scop]

I wonder if the logic with $legacy_scp here is the wrong way around.

I'm assuming the intent is to do less escaping if we found -O was given.

So with that, shouldn't we be doing this instead?

Suggested change

	if [[ $legacy_scp || ! $("$1" --usage 2>&1) =~ scp\ \[-[^]]*O ]]; then
	if [[ ! $legacy_scp && ! $("$1" --usage 2>&1) =~ scp\ \[-[^]]*O ]]; then

...and if logically yes, we could flip the if/else around and remove the negations, so

            if [[ $legacy_scp || $("$1" --usage 2>&1) =~ scp\ \[-[^]]*O ]]; then
                # do the -l here

---

Also, I find this code with $legacy_scp variable name kind of hard to read, would be better if it had the has_ prefix, but just adding that would kind of make the variable read unclear still. How about something like $has_O_given?

[scop]

Er

I'm assuming the intent is to do less escaping if we found -O was given.

...this assumption of mine is false and we want it the other way around, so the original code is fine after all. Will merge with the commit message tweaked.