call to external copyprog (rsync) mishandles IPv6 literal address, dropping []

[dlucredativ]

The following happens with Unison 2.53.3 (Debian trixie).

Internal copy mechanism works:

root@host1:~# unison -batch -copythreshold -1 /mnt/ ssh://[2a05:d014:ac8:7e00::ffeb]//mnt/
Unison 2.53.3 (ocaml 4.13.1): Contacting server...
Connected [//host1//mnt -> //host2//mnt]

Looking for changes
  Waiting for changes from server
Reconciling changes
         <---- new file   foo  
local        : absent
host2        : new file           modified on 2023-11-30 at 12:40:16  size 4         rw-r--r--

1 items will be synced, 0 skipped
0 B to be synced from local to host2
4 B to be synced from host2 to local
Propagating updates
Unison 2.53.3 (ocaml 4.13.1) started propagating changes at 12:40:21.26 on 30 Nov 2023
[BGN] Copying foo from //host2//mnt to /mnt
[END] Copying foo                 
Unison 2.53.3 (ocaml 4.13.1) finished propagating changes at 12:40:21.26 on 30 Nov 2023, 0.004 s
Saving synchronizer state            
Synchronization complete at 12:40:21  (1 item transferred, 0 skipped, 0 failed)

External copy mechanism fails:

root@host1:~# strace -zfs 300 -e trace=execve unison -batch -copythreshold 0 /mnt/ ssh://[2a05:d014:ac8:7e00::ffeb]//mnt/
execve("/usr/bin/unison", ["unison", "-batch", "-copythreshold", "0", "/mnt/", "ssh://[2a05:d014:ac8:7e00::ffeb]//mnt/"], 0x7ffcb03d4c88 /* 16 vars */) = 0
Unison 2.53.3 (ocaml 4.13.1): Contacting server...
strace: Process 521 attached
[pid   521] execve("/usr/bin/ssh", ["ssh", "2a05:d014:ac8:7e00::ffeb", "-e", "none", "unison", "-server", "__new-rpc-mode"], 0x55ffc0f5ab10 /* 17 vars */) = 0
Connected [//host1//mnt -> //host2//mnt]

Looking for changes
  Waiting for changes from server
Reconciling changes
         <---- new file   bar  
local        : absent
host2        : new file           modified on 2023-11-30 at 12:40:53  size 4         rw-r--r--

1 items will be synced, 0 skipped
0 B to be synced from local to host2
4 B to be synced from host2 to local
Propagating updates
Unison 2.53.3 (ocaml 4.13.1) started propagating changes at 12:41:15.63 on 30 Nov 2023
[BGN] Copying bar from //host2//mnt to /mnt
rsync --partial --inplace --compress 2a05:d014:ac8:7e00::ffeb:/mnt/bar /mnt/.unison.bar.1553ad73f0cb74feb505c198f28f1b31.unison.tmp
  0%  0/1  (0 B of 4 B)  --:-- ETAstrace: Process 522 attached
[pid   522] execve("/usr/bin/rsync", ["rsync", "--partial", "--inplace", "--compress", "2a05:d014:ac8:7e00::ffeb:/mnt/bar", "/mnt/.unison.bar.1553ad73f0cb74feb505c198f28f1b31.unison.tmp"], 0x55ffc0f5add0 /* 17 vars */) = 0
strace: Process 523 attached
[pid   523] execve("/usr/bin/ssh", ["ssh", "2a05", "rsync", "--server", "--sender", "-ze.LsfxCIvu", "--inplace", ".", "d014:ac8:7e00::ffeb:/mnt/bar"], 0x7ffd1c421100 /* 17 vars */) = 0
[pid   522] +++ exited with 12 +++
[pid   520] --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=522, si_uid=0, si_status=12, si_utime=0, si_stime=0} ---
Failed [bar]: External copy program did not create target file (or bad length): .unison.bar.1553ad73f0cb74feb505c198f28f1b31.unison.tmp
Unison 2.53.3 (ocaml 4.13.1) finished propagating changes at 12:41:19.29 on 30 Nov 2023, 3.664 s
Saving synchronizer state
[pid   523] +++ exited with 255 +++
Synchronization incomplete at 12:41:19  (0 items transferred, 0 skipped, 1 failed)
  failed: bar
[pid   521] --- SIGTERM {si_signo=SIGTERM, si_code=SI_USER, si_pid=520, si_uid=0} ---
[pid   521] +++ exited with 255 +++
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=521, si_uid=0, si_status=255, si_utime=17 /* 0.17 s */, si_stime=1 /* 0.01 s */} ---
+++ exited with 2 +++

This is because rsync should have been invoked with [2a05:d014:ac8:7e00::ffeb]:/mnt/bar instead of 2a05:d014:ac8:7e00::ffeb:/mnt/bar.

[gdt]

Looks like a valid bug, but the world of IPv6 literals is messy. I suspect you are just about the only person using v6 literals and external rsync, vs real DNS or mdns names. Do you know that rsync is ok with [] addrs? Are you up for creating a PR?

We are considering removing external sync support as not useful in #871. While you've shown there is a bug, I wonder if you can explain (in #871) why it is useful to use it, vs just letting the defaults happen, in more than an unusual or edge case.

[tleedjarv]

Assuming rsync is ok with [] addrs (I am not going to test) then I can provide a quick PR with a workaround. There will not be a real fix for the reason of #871.

I strongly believe that "copyprog" is itself a hack and only used to work around historic limitations in Unison. As such, this hack should eventually be removed.

[gdt]

I would like to hear from the OP in #871 before we do anything. Before it even makes sense to think about patching, someone(tm) needs to test rsync as the entire IPv6 literal situation is messy. Leaving external rsync was ok while it didn't take up time, but this is pushing me to just disable it.

[dlucredativ]

I suspect you are just about the only person using v6 literals and external rsync, vs real DNS or mdns names.

You are right, I stumbled upon it during some synthetic tests where DNS records were not important enough.

Do you know that rsync is ok with [] addrs?

I don't know if there are gotchas or edge-cases with [], if that is your question.

Are you up for creating a PR?

No.

[gdt]

I'm adding wontfix for now (but leaving it open as apparently valid) since you aren't even confirming that rsync, when passed IPv6 literals with [], will work correctly. There are very few people and a very small fraction of an FTE in general working on Unison, and we rely on users being willing to dig in and help.

[dlucredativ]

since you aren't even confirming that rsync, when passed IPv6 literals with [], will work correctly.

Doesn't this twist the burden of proof? A user who were invested in getting this fixed wouldn't ask "please add brackets" but "please don't remove brackets". Unison thinks to know better what rsync needs and fails with it.