Repository navigation
Conversation
5c8b168 to
a6e4746
Compare
|
Mostly the test is left: please make it simpler and make it actually check that --from matched. |
| let mut owner_arg = Arg::new(add_arg_if_not_reference) | ||
| .value_name(add_arg_if_not_reference) | ||
| .required(true); | ||
| if !groups_only { |
|
|
||
| let ts = TestScenario::new(util_name!()); | ||
| let library = ts.fixtures.plus("accounts.so"); | ||
| let compilation = Command::new("cc") |
There was a problem hiding this comment.
compiling a C shim with cc in a test is a lot of machinery, do we really need it?
maybe we can just check that the error is now "invalid user" and not the clap UTF-8 error
| } | ||
| ts.ucmd() | ||
| .env("LD_PRELOAD", &library) | ||
| .arg("--from") |
There was a problem hiding this comment.
the ids map to our own, so this passes even if --from never matches
could you please use -v and check the "retained"/"changed" output?
| .arg("file") | ||
| .succeeds(); | ||
| assert_eq!(ts.fixtures.metadata("file").uid(), unsafe { | ||
| libc::geteuid() |
There was a problem hiding this comment.
unsafe without SAFETY, and the same metadata is read twice, could be simpler
a6e4746 to
cfa4fbf
Compare
| /// this to `':'`. If a separator is given but the group is omitted | ||
| /// (as in `"owner:"`), the group is set to the owner's login group. | ||
| fn parse_spec(spec: &str, sep: char) -> UResult<(Option<u32>, Option<u32>)> { | ||
| fn parse_spec(spec: impl AsRef<OsStr>, sep: char) -> UResult<(Option<u32>, Option<u32>)> { |
There was a problem hiding this comment.
why impl AsRef<OsStr>? &OsStr should be enough
| use std::ffi::OsStr; | ||
| use std::os::unix::ffi::OsStrExt; | ||
|
|
||
| for from in [false, true] { |
There was a problem hiding this comment.
two plain new_ucmd!() calls would be easier to read than this loop with if from three times
|
thanks for tackling this! |
| use std::os::unix::ffi::OsStrExt; | ||
| for (spec, message) in [ | ||
| (b"unknown_\xff".as_slice(), "invalid user:"), | ||
| (b":unknown_\xff".as_slice(), "invalid group:"), |
There was a problem hiding this comment.
could you please add a --from case with a non-UTF-8 value too?
Preserve account-name bytes through owner and --from parsing and account lookups, including the login-group shorthand. Test successful ownership changes with synthetic non-UTF-8 accounts. Closes uutils#9958.
cfa4fbf to
69165ea
Compare
This branch has not been deployed
2 participants
Preserve account-name bytes through owner and --from parsing and account lookups, including the login-group shorthand. Test successful ownership changes with synthetic non-UTF-8 accounts.
Closes #9958.