coreutils icon indicating copy to clipboard operation
coreutils copied to clipboard

tr: Fix regression causing read error with sockets

Open JohnoKing opened this issue 8 months ago • 1 comments

This pull request fixes a regression introduced in 3e4221a4 that causes tr to fail when used with ksh93 sockets (previous bug report: https://github.com/uutils/coreutils/issues/7658). The test used for determining if a file descriptor tested with fstat points to a directory is traditionally done by using the S_IFMT mask^1, e.g.:

#define S_ISDIR(m)	(((m) & S_IFMT) == S_IFDIR)

In Rust, this translates to m & libc::S_IFMT == libc::S_IFDIR. is_stdin_directory() uses has!(mode, S_IFDIR), which is not equivalent and causes non-directory sockets to be incorrectly recognized as directories. This causes tr to break when used with all sockets created with socketpair, including ksh93 pipes^3. Below is an example Rust program demonstrating why the current check is bogus:

fn main() {
     use nix::sys::socket::{socketpair, SockFlag, AddressFamily, SockType};
     use nix::sys::stat::fstat;
     use libc::{S_IFDIR, S_IFMT, mode_t};
     let (_fd1, fd2) = socketpair(AddressFamily::Unix, SockType::Stream, None, SockFlag::empty()).unwrap();
     let mode = fstat(&fd2).unwrap().st_mode as mode_t;
     if mode & S_IFMT == S_IFDIR {     // Equivalent to S_ISDIR()
           println!("Not reached");    // Not a dir, so unreachable
     }
     if mode & S_IFDIR == S_IFDIR {    // Bogus check for S_IFDIR
           println!("BAD");
     }
}

Changes: - is_stdin_directory(): Fix the regression introduced in 3e4221a4 by replacing the bogus check with one equivalent to S_ISDIR(). - test_tr.rs: Add a regression test for this bug.

Fixes https://github.com/uutils/coreutils/issues/7658

JohnoKing avatar Jun 06 '25 10:06 JohnoKing

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

github-actions[bot] avatar Jun 06 '25 12:06 github-actions[bot]

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

github-actions[bot] avatar Jul 04 '25 09:07 github-actions[bot]

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

github-actions[bot] avatar Jul 09 '25 13:07 github-actions[bot]

Thanks for your PR!

cakebaker avatar Jul 09 '25 13:07 cakebaker