Skip to content

tr: Fix regression causing read error with sockets #8083

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

JohnoKing
Copy link

This pull request fixes a regression introduced in 3e4221a that causes tr to fail when used with ksh93 sockets (previous bug report: #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 mask12, 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 pipes3. 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 3e4221a by replacing the bogus check with one equivalent to S_ISDIR().
- test_tr.rs: Add a regression test for this bug.

Fixes #7658

Footnotes

  1. https://sourceware.org/git/?p=glibc.git;a=blob;f=io/sys/stat.h;h=4bea9e9a#l123

  2. https://git.musl-libc.org/cgit/musl/tree/include/sys/stat.h?id=047a1639#n51

  3. https://github.com/ksh93/ksh/blob/cc5e0692/src/cmd/ksh93/sh/io.c#L98-L102

JohnoKing added 2 commits June 6, 2025 03:38
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][^2], 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 uu-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");
     }
  }

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

[^1]: https://sourceware.org/git/?p=glibc.git;a=blob;f=io/sys/stat.h;h=4bea9e9a#l123
[^2]: https://git.musl-libc.org/cgit/musl/tree/include/sys/stat.h?id=047a1639#n51
[^3]: https://github.com/ksh93/ksh/blob/cc5e0692/src/cmd/ksh93/sh/io.c#L98-L102

Fixes uutils#7658
JohnoKing added 2 commits June 6, 2025 04:39
Also fix cargo clippy lint.
(In case it isn't obvious, I'm fairly new to Rust)
Copy link

github-actions bot commented Jun 6, 2025

GNU testsuite comparison:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The tr utility fails with read error under ksh93
2 participants