Fix regression: Allow FIFO / process substitution for --read-batch

[seks99x]

V3.5.0 in batch.c commit 1604890 introduced a strict S_ISREG check for --read-batch argument paths. This inadvertently breaks bash process substitution (e.g., <(...)), which passes file descriptors as FIFOs (S_IFIFO).

Testing against the 3.4 branch succeeds, but fails on the current 3.5.0dev branch:

Bash

# rsync 3.4.x (Success)
./rsync --read-batch=<(cat reg) /tmp/rsync_test_dir/src/test
skipping non-regular file "test"

# rsync 3.5.0dev (Regression)
./rsync --read-batch=<(cat reg) /tmp/rsync_test_dir/src/test 
Batch file "/dev/fd/63" is not a regular file
rsync error: error in file IO (code 11) at batch.c(281) [Receiver=3.5.0dev-ge8c79d2d]

The Fix:
Modified the check in batch.c open_batch_file() to permit S_ISFIFO alongside S_ISREG.

[seks99x]

the PR wont pass the CI without merging PR #1054

[seks99x]

Actually this would still break on a socket:[ / anon_inode: case. We are only allowing pipes pipe:[.

I think we should not strict a certain file type and only check for do_fstat(batch_fd, &st) == 0 .
@steadytao what do you think?

[steadytao]

Sorry for the late response. Very busy weekend. Best to keep this limited to regular files and FIFOs. Sockets, devices and anonymous inodes are outside the process-substitution use case and removing the type check would undo the original hardening. The test should also clear dest after generating the batch so the read-batch run itself has to recreate the payload. Rebase this after #1054 is resolved of course.

[seks99x]

@steadytao Unfortunately we could break a legit case or just receive another regression issue if we strict the file type. Sockets can be used also with certain shell types like KornShell. Do you have any specific reason why we should strict the file type here on the read-batch?
Take your time mate no worries!

[steadytao]

If KornShell produces a socket for a real --read-batch invocation, please provide the exact shell version and reproducer so we can test that case. Without one, I merely do not want to remove the type restriction and allow every object that happens to pass fstat(). The existing FIFO/process-substitution regression is concrete; sockets and anonymous inodes currently are not.

The test also needs to remove the destination after generating the batch so the --read-batch operation must recreate it.

[steadytao]

Trying to be minimal but that could be a concern so perhaps some testing is justified?

[seks99x]

@steadytao Actually, I just tested KornShell and it uses standard pipes (pipe:[). Looking at the man pages, it says it uses normal pipes.
https://cdesktopenv.sourceforge.io/man1/ksh.html

but on ksh93 it may uses socketpair instead on pipeline not process substitution
https://manpages.debian.org/unstable/ksh93u+m/ksh93.1.en.html

A pipeline is a sequence of one or more commands separated by |. The standard output of each command but the last is connected by a [socketpair(2)](https://manpages.debian.org/unstable/manpages-dev/socketpair.2.en.html) or (if the posix shell option is on) by a [pipe(2)](https://manpages.debian.org/unstable/manpages-dev/pipe.2.en.html) to the standard input of the next command. Each command except the last is run asynchronously in a subshell (see Subshells below). If the monitor or pipefail option is on, or the pipeline is preceded by the reserved word time, then the shell waits for all component commands in the pipeline to terminate; otherwise, the shell only waits for the last component command. The exit status of a pipeline is the exit status of its last component command, unless the pipefail option is enabled. Each pipeline can be preceded by the reserved word ! which causes the exit status of the pipeline to become 0 if the exit status of the last command is non-zero, and 1 if the exit status of the last command is 0.

#set +o posix
# sleep 60 | sleep 61 &                                                                                             
[1]	438284
# ls -l /proc/$!/fd/0                                                                                               
lrwx------ 1 root root 64 Aug 25 14:39 /proc/438284/fd/0 -> 'socket:[6649015]'
# ls -l /proc/$!/fd/ 
total 0
lrwx------ 1 root root 64 Aug 25 14:39 0 -> 'socket:[6649015]'
lrwx------ 1 root root 64 Aug 25 14:39 1 -> /dev/pts/4
lrwx------ 1 root root 64 Aug 25 14:39 10 -> /root/.sh_history
lrwx------ 1 root root 64 Aug 25 14:39 11 -> /dev/pts/4
lrwx------ 1 root root 64 Aug 25 14:39 2 -> /dev/pts/4
# 

But this will be handled well without adding anything in the source due to this check if (!write_batch && batch_fd != STDIN_FILENO) { if we sent a rsync cli like this cat /tmp/mybatch | rsync -av --read-batch=- /tmp/rsync_dest2/ it will succeed normally because it will use standard input.

The only thing could fail is things like sockets or anon_inodes that won't come directly from standard bash process substitution and is being explicitly prepared first.

I think we better keep it stricter to FIFOs if a legit case of socket is already handled.

[seks99x]

I'll rebase and fix the test