Skip to content

Commit cb428a2

Browse files
committed
feat(parse_size): Add comprehensive builder validation to prevent fragile configurations
Addresses PR uutils#9653 review feedback about 'fragile commands' by implementing fail-fast validation in Parser builder pattern. Changes: - Add ParserBuilderError enum with 8 validation error variants - Refactor builder methods to return Result<&mut Self, ParserBuilderError> - Implement comprehensive unit validation (57 valid units including k/m/g/t) - Add cross-validation between builder settings (default_unit vs allow_list) - Detect conflicts (b_byte_count with 'b' unit, '%' with size units) - Update all call sites (sort, du, df, od) to handle new error types - Fix invalid '%' unit in sort's parse_byte_count allow_list Benefits: - Configuration errors detected immediately (not during parsing) - Clear error messages listing invalid/conflicting settings - Maintains backward compatibility through explicit error reporting Files modified: - src/uucore/src/lib/features/parser/parse_size.rs (core implementation) - src/uu/sort/src/sort.rs (error handling + fix invalid '%') - src/uu/du/src/du.rs (error handling) - src/uu/df/src/df.rs (error handling) - src/uu/od/src/od.rs (error handling)
1 parent 9ffe95e commit cb428a2

File tree

8 files changed

+447
-51
lines changed

8 files changed

+447
-51
lines changed

.vscode/cspell.dictionaries/workspace.wordlist.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,8 @@ fcntl
152152
vmsplice
153153

154154
# * vars/libc
155+
memsize
156+
sysctlbyname
155157
COMFOLLOW
156158
EXDEV
157159
FILENO

src/uu/df/src/df.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ impl Options {
167167
),
168168
ParseSizeError::ParseFailure(s) => OptionsError::InvalidBlockSize(s),
169169
ParseSizeError::PhysicalMem(s) => OptionsError::InvalidBlockSize(s),
170+
ParseSizeError::BuilderConfig(e) => OptionsError::InvalidBlockSize(e.to_string()),
170171
})?,
171172
header_mode: {
172173
if matches.get_flag(OPT_HUMAN_READABLE_BINARY)

src/uu/du/src/du.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1502,6 +1502,9 @@ fn format_error_message(error: &ParseSizeError, s: &str, option: &str) -> String
15021502
ParseSizeError::SizeTooBig(_) => {
15031503
translate!("du-error-argument-too-large", "option" => option, "value" => s.quote())
15041504
}
1505+
ParseSizeError::BuilderConfig(e) => {
1506+
format!("invalid configuration for {option}: {e}")
1507+
}
15051508
}
15061509
}
15071510

src/uu/od/src/od.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -804,5 +804,8 @@ fn format_error_message(error: &ParseSizeError, s: &str, option: &str) -> String
804804
ParseSizeError::SizeTooBig(_) => {
805805
translate!("od-error-argument-too-large", "option" => option, "value" => s.quote())
806806
}
807+
ParseSizeError::BuilderConfig(e) => {
808+
format!("invalid configuration for {option}: {e}")
809+
}
807810
}
808811
}

src/uu/sort/src/sort.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -310,9 +310,9 @@ impl GlobalSettings {
310310
let size = Parser::default()
311311
.with_allow_list(&[
312312
"b", "k", "K", "m", "M", "g", "G", "t", "T", "P", "E", "Z", "Y", "R", "Q", "%",
313-
])
314-
.with_default_unit("K")
315-
.with_b_byte_count(true)
313+
])?
314+
.with_default_unit("K")?
315+
.with_b_byte_count(true)?
316316
.parse(input.trim())?;
317317

318318
usize::try_from(size).map_err(|_| {
@@ -2955,6 +2955,9 @@ fn format_error_message(error: &ParseSizeError, s: &str, option: &str) -> String
29552955
ParseSizeError::SizeTooBig(_) => {
29562956
translate!("sort-option-arg-too-large", "option" => option, "arg" => s.quote())
29572957
}
2958+
ParseSizeError::BuilderConfig(e) => {
2959+
format!("invalid configuration for {option}: {e}")
2960+
}
29582961
}
29592962
}
29602963

src/uucore/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ xattr = { workspace = true, optional = true }
107107
[dev-dependencies]
108108
tempfile = { workspace = true }
109109

110-
[target.'cfg(target_os = "linux")'.dependencies]
110+
[target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies]
111111
procfs = { workspace = true, optional = true }
112112

113113
[target.'cfg(target_os = "windows")'.dependencies]
@@ -165,7 +165,7 @@ mode = ["libc"]
165165
perms = ["entries", "libc", "walkdir"]
166166
buf-copy = []
167167
parser-num = ["extendedbigdecimal", "num-traits"]
168-
parser-size = ["parser-num", "procfs"]
168+
parser-size = ["parser-num", "procfs", "libc", "windows-sys"]
169169
parser-glob = ["glob"]
170170
parser = ["parser-num", "parser-size", "parser-glob"]
171171
pipes = []

0 commit comments

Comments
 (0)