Skip to content

Commit f6451c7

Browse files
eyupcanakmancakebaker
authored andcommitted
chown: warn when '.' is used as owner:group separator
When the spec contains '.' but no ':', emit a warning matching GNU coreutils behavior and re-parse with '.' as separator so both owner and group are applied. Closes #11352
1 parent af81585 commit f6451c7

File tree

4 files changed

+84
-8
lines changed

4 files changed

+84
-8
lines changed

src/uu/chown/locales/en-US.ftl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,6 @@ chown-error-failed-to-get-attributes = failed to get attributes of { $file }
2121
chown-error-invalid-user = invalid user: { $user }
2222
chown-error-invalid-group = invalid group: { $group }
2323
chown-error-invalid-spec = invalid spec: { $spec }
24+
25+
# Warning messages
26+
chown-warning-dot-separator = '.' should be ':': { $spec }

src/uu/chown/locales/fr-FR.ftl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,6 @@ chown-error-failed-to-get-attributes = échec de l'obtention des attributs de {
2121
chown-error-invalid-user = utilisateur invalide : { $user }
2222
chown-error-invalid-group = groupe invalide : { $group }
2323
chown-error-invalid-spec = spécification invalide : { $spec }
24+
25+
# Messages d'avertissement
26+
chown-warning-dot-separator = '.' devrait être ':' : { $spec }

src/uu/chown/src/chown.rs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use uucore::display::Quotable;
99
pub use uucore::entries::{self, Group, Locate, Passwd};
1010
use uucore::format_usage;
1111
use uucore::perms::{GidUidOwnerFilter, IfFrom, chown_base, options};
12+
use uucore::show_warning;
1213
use uucore::translate;
1314

1415
use uucore::error::{FromIo, UResult, USimpleError};
@@ -151,7 +152,7 @@ pub fn uu_app() -> Command {
151152
}
152153

153154
/// Parses the user string to extract the UID.
154-
fn parse_uid(user: &str, spec: &str, sep: char) -> UResult<Option<u32>> {
155+
fn parse_uid(user: &str, spec: &str) -> UResult<Option<u32>> {
155156
if user.is_empty() {
156157
return Ok(None);
157158
}
@@ -160,11 +161,6 @@ fn parse_uid(user: &str, spec: &str, sep: char) -> UResult<Option<u32>> {
160161
return Ok(Some(u.uid));
161162
}
162163

163-
// Handle `username.groupname` syntax (e.g. when sep is ':' but spec contains '.')
164-
if spec.contains('.') && !spec.contains(':') && sep == ':' {
165-
return parse_spec(spec, '.').map(|(uid, _)| uid);
166-
}
167-
168164
// Fallback: `user` string contains a numeric user ID
169165
user.parse().map(Some).map_err(|_| {
170166
USimpleError::new(
@@ -209,7 +205,20 @@ fn parse_spec(spec: &str, sep: char) -> UResult<(Option<u32>, Option<u32>)> {
209205
let user = args.next().unwrap_or("");
210206
let group = args.next().unwrap_or("");
211207

212-
let uid = parse_uid(user, spec, sep)?;
208+
// dot separator: try as username first, fall back to owner.group (like GNU)
209+
if sep == ':' && !spec.contains(':') && spec.contains('.') {
210+
if let Ok(uid) = parse_uid(user, spec) {
211+
let gid = parse_gid(group, spec)?;
212+
return Ok((uid, gid));
213+
}
214+
show_warning!(
215+
"{}",
216+
translate!("chown-warning-dot-separator", "spec" => spec.quote())
217+
);
218+
return parse_spec(spec, '.');
219+
}
220+
221+
let uid = parse_uid(user, spec)?;
213222
let gid = parse_gid(group, spec)?;
214223

215224
if user.chars().next().is_some_and(char::is_numeric) && group.is_empty() && spec != user {

tests/by-util/test_chown.rs

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,8 @@ fn test_chown_only_owner_colon() {
149149
.arg("--verbose")
150150
.arg(file1)
151151
.succeeds()
152-
.stderr_contains("retained as");
152+
.stderr_contains("retained as")
153+
.stderr_contains("warning: '.' should be ':'");
153154

154155
scene
155156
.ucmd()
@@ -160,6 +161,66 @@ fn test_chown_only_owner_colon() {
160161
.stderr_contains("failed to change");
161162
}
162163

164+
#[test]
165+
fn test_chown_dot_separator_warning() {
166+
// test that using '.' as separator emits a warning
167+
168+
let scene = TestScenario::new(util_name!());
169+
let at = &scene.fixtures;
170+
171+
let result = scene.cmd("whoami").run();
172+
if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") {
173+
return;
174+
}
175+
let user_name = String::from(result.stdout_str().trim());
176+
assert!(!user_name.is_empty());
177+
178+
let file1 = "test_chown_dot_warn";
179+
at.touch(file1);
180+
181+
let result = scene.cmd("id").arg("-gn").run();
182+
if skipping_test_is_okay(&result, "id: cannot find name for group ID") {
183+
return;
184+
}
185+
let group_name = String::from(result.stdout_str().trim());
186+
assert!(!group_name.is_empty());
187+
188+
// chown user. file should warn about '.' separator
189+
scene
190+
.ucmd()
191+
.arg(format!("{user_name}."))
192+
.arg(file1)
193+
.succeeds()
194+
.stderr_contains("warning: '.' should be ':'");
195+
196+
// chown user.group file should warn AND apply both owner and group
197+
let result = scene
198+
.ucmd()
199+
.arg(format!("{user_name}.{group_name}"))
200+
.arg("--verbose")
201+
.arg(file1)
202+
.run();
203+
if skipping_test_is_okay(&result, "chown: invalid group:") {
204+
return;
205+
}
206+
result.stderr_contains("warning: '.' should be ':'");
207+
// "retained as" on Linux, "changed ownership" on BSDs (group inherited from parent dir)
208+
assert!(
209+
result.stderr_str().contains("retained as")
210+
|| result.stderr_str().contains("changed ownership"),
211+
"expected verbose ownership output, got: {}",
212+
result.stderr_str()
213+
);
214+
215+
// chown user: file should not warn
216+
scene
217+
.ucmd()
218+
.arg(format!("{user_name}:"))
219+
.arg(file1)
220+
.succeeds()
221+
.stderr_does_not_contain("warning");
222+
}
223+
163224
#[test]
164225
fn test_chown_only_colon() {
165226
// test chown : file.txt

0 commit comments

Comments
 (0)