Incorrect handling of input preprocessor returning nothing and exiting with 0 #817

Hi!

TL;DR: If open_altfile() returns FAKE_EMPTYFILE and sets the input file descriptor to -1, less tries to read from fd -1 and reports a read error.

First a bit of context: RedHat ship their own pipe input preprocessor in the Fedora 44 less package and sets LESSOPEN='||/usr/bin/lesspipe.sh %s (note the two leading pipe characters). This input preprocessor script checks if it is used in combination with perldoc (the Perl documentation system). In this case the script immediately terminates with exit code 0, the intention probably being for less to simply use the original input file. However, as the less documentation says,

if LESSOPEN starts with two vertical bars, the exit status of the script determines the behavior when the output is empty. If the output is empty and the exit status is zero, the empty output is considered to be replacement text. If the output is empty and the exit status is nonzero, the original file is used.

so the input preprocessor should exit with non-zero in this case. This bug in Fedora is already tracked at https://bugzilla.redhat.com/show_bug.cgi?id=2501990

However, I would also argue that less should handle such misuses more gracefully. I am not familiar with the less source code, so I hope I got this right:

1. open_altfile() is called at

   less/edit.c

   Line 499 in 9aba985

   alt_filename = open_altfile(filename, &f, &altpipe);

2. open_altfile() checks if it can read anything from the input processor pipe. If this is not the case and LESSOPEN used two pipe characters and the exit status of the input processor was 0, it set the caller's file descriptor to -1 and returns FAKE_EMPTYFILE.

   less/filename.c

   Lines 973 to 988 in 9aba985

   	if (read(f, &c, 1) != 1)
   	{
   	/*
   	* Pipe is empty.
   	* If more than 1 pipe char was specified,
   	* the exit status tells whether the file itself
   	* is empty, or if there is no alt file.
   	* If only one pipe char, just assume no alt file.
   	*/
   	int status = pclose(fd);
   	if (returnfd > 1 && status == 0) {
   	/* File is empty. */
   	*pfd = NULL;
   	*pf = -1;
   	return (save(FAKE_EMPTYFILE));
   	}

3. This is AFAICT checked correctly after returning from this function:

   less/edit.c

   Lines 536 to 540 in 9aba985

   	} else if (strcmp(open_filename, FAKE_EMPTYFILE) == 0)
   	{
   	f = -1;
   	chflags |= CH_NODATA;
   	} else if (strcmp(open_filename, FAKE_HELPFILE) == 0)

However, less still tries to find FAKE_EMPTYFILE and reads from the invalid file descriptor as can be seen from the following strace output (36363 is the PID of less, 36364 of the input preprocessor):

36363 <... read resumed>, "", 1)        = 0
36364 +++ exited with 0 +++
36363 --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=36364, si_uid=1000, si_status=0, si_utime=0, si_stime=0} ---
36363 close(4)                          = 0
36363 wait4(36364, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], 0, NULL) = 36364
36363 fstat(-1, 0x7ffcf9129c10)         = -1 EBADF (Bad file descriptor)
36363 lseek(-1, 0, SEEK_END)            = -1 EBADF (Bad file descriptor)
36363 newfstatat(AT_FDCWD, "@/\\less/\\empty/\\file/\\@", 0x7ffcf9129c10, 0) = -1 ENOENT (No such file or directory)
36363 rt_sigprocmask(SIG_BLOCK, NULL, [], 8) = 0
36363 read(-1, 0x55fe24a22990, 8192)    = -1 EBADF (Bad file descriptor)
36363 rt_sigprocmask(SIG_BLOCK, NULL, [], 8) = 0
36363 write(1, "\33[?1049h\33[22;0;0t\33[46;1H\33[?1h\33=\r"..., 51) = 51
36363 read(-1, 0x55fe24a22990, 8192)    = -1 EBADF (Bad file descriptor)
36363 write(1, "\33[1m~\33(B\33[m\n\33[1m~\33(B\33[m\n\33[1m~\33(B"..., 584) = 584

This problem can be easily reproduced with
LESSOPEN='||/bin/sh -c ": %s; exit 0"' less /etc/os-release
(or any other existing file).

The workarounds are to exit with non-zero
LESSOPEN='||/bin/sh -c ": %s; exit 1"' less /etc/os-release
or to use only one pipe character
LESSOPEN='|/bin/sh -c ": %s; exit 0"' less /etc/os-release
(because then the exit code does not matter).

PS: No AI was used for analyzing this problem or writing the issue. All mistakes are mine alone. 😊