Skip to content

Commit

Permalink
Merge pull request #3111 from jfinkels/split-suffix-contains-separator
Browse files Browse the repository at this point in the history
split: error when --additional-suffix contains /
  • Loading branch information
sylvestre authored Feb 11, 2022
2 parents e818fd2 + 2f65b29 commit 748e6e7
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 2 deletions.
19 changes: 17 additions & 2 deletions src/uu/split/src/split.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,9 @@ enum SettingsError {
/// Invalid suffix length parameter.
SuffixLength(String),

/// Suffix contains a directory separator, which is not allowed.
SuffixContainsSeparator(String),

/// The `--filter` option is not supported on Windows.
#[cfg(windows)]
NotSupported,
Expand All @@ -272,7 +275,10 @@ enum SettingsError {
impl SettingsError {
/// Whether the error demands a usage message.
fn requires_usage(&self) -> bool {
matches!(self, Self::Strategy(StrategyError::MultipleWays))
matches!(
self,
Self::Strategy(StrategyError::MultipleWays) | Self::SuffixContainsSeparator(_)
)
}
}

Expand All @@ -281,6 +287,11 @@ impl fmt::Display for SettingsError {
match self {
Self::Strategy(e) => e.fmt(f),
Self::SuffixLength(s) => write!(f, "invalid suffix length: {}", s.quote()),
Self::SuffixContainsSeparator(s) => write!(
f,
"invalid suffix {}, contains directory separator",
s.quote()
),
#[cfg(windows)]
Self::NotSupported => write!(
f,
Expand All @@ -294,13 +305,17 @@ impl fmt::Display for SettingsError {
impl Settings {
/// Parse a strategy from the command-line arguments.
fn from(matches: &ArgMatches) -> Result<Self, SettingsError> {
let additional_suffix = matches.value_of(OPT_ADDITIONAL_SUFFIX).unwrap().to_string();
if additional_suffix.contains('/') {
return Err(SettingsError::SuffixContainsSeparator(additional_suffix));
}
let suffix_length_str = matches.value_of(OPT_SUFFIX_LENGTH).unwrap();
let result = Self {
suffix_length: suffix_length_str
.parse()
.map_err(|_| SettingsError::SuffixLength(suffix_length_str.to_string()))?,
numeric_suffix: matches.occurrences_of(OPT_NUMERIC_SUFFIXES) > 0,
additional_suffix: matches.value_of(OPT_ADDITIONAL_SUFFIX).unwrap().to_owned(),
additional_suffix,
verbose: matches.occurrences_of("verbose") > 0,
strategy: Strategy::from(matches).map_err(SettingsError::Strategy)?,
input: matches.value_of(ARG_INPUT).unwrap().to_owned(),
Expand Down
8 changes: 8 additions & 0 deletions tests/by-util/test_split.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,14 @@ fn test_split_additional_suffix() {
assert_eq!(glob.collate(), at.read_bytes(name));
}

#[test]
fn test_additional_suffix_no_slash() {
new_ucmd!()
.args(&["--additional-suffix", "a/b"])
.fails()
.usage_error("invalid suffix 'a/b', contains directory separator");
}

// note: the test_filter* tests below are unix-only
// windows support has been waived for now because of the difficulty of getting
// the `cmd` call right
Expand Down

0 comments on commit 748e6e7

Please sign in to comment.