From aa010b71b8665cc7a91b80fc398c68852215d552 Mon Sep 17 00:00:00 2001 From: Joseph Jon Booker Date: Mon, 25 Mar 2024 22:02:04 -0500 Subject: [PATCH 01/10] expr: Change error messages when missing closing parenthesis This situation now has two errors - one if we parse a character that isn't a closing parenthesis, and the other if we don't parse anything. --- src/uu/expr/src/expr.rs | 6 +++++- src/uu/expr/src/syntax_tree.rs | 32 +++++++++++++++++++++++++++----- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/uu/expr/src/expr.rs b/src/uu/expr/src/expr.rs index 4e41a6929e6..371f4377e9a 100644 --- a/src/uu/expr/src/expr.rs +++ b/src/uu/expr/src/expr.rs @@ -34,6 +34,7 @@ pub enum ExprError { DivisionByZero, InvalidRegexExpression, ExpectedClosingBraceAfter(String), + ExpectedClosingBraceInsteadOf(String), } impl Display for ExprError { @@ -50,7 +51,10 @@ impl Display for ExprError { Self::DivisionByZero => write!(f, "division by zero"), Self::InvalidRegexExpression => write!(f, "Invalid regex expression"), Self::ExpectedClosingBraceAfter(s) => { - write!(f, "expected ')' after {}", s.quote()) + write!(f, "syntax error: expecting ')' after {}", s.quote()) + } + Self::ExpectedClosingBraceInsteadOf(s) => { + write!(f, "syntax error: expecting ')' instead of {}", s.quote()) } } } diff --git a/src/uu/expr/src/syntax_tree.rs b/src/uu/expr/src/syntax_tree.rs index 0a947a158a4..30d50fb26d9 100644 --- a/src/uu/expr/src/syntax_tree.rs +++ b/src/uu/expr/src/syntax_tree.rs @@ -442,13 +442,21 @@ impl<'a> Parser<'a> { }, "(" => { let s = self.parse_expression()?; - let close_paren = self.next()?; - if close_paren != ")" { + match self.next() { + Ok(")") => {} // Since we have parsed at least a '(', there will be a token // at `self.index - 1`. So this indexing won't panic. - return Err(ExprError::ExpectedClosingBraceAfter( - self.input[self.index - 1].into(), - )); + Ok(_) => { + return Err(ExprError::ExpectedClosingBraceInsteadOf( + self.input[self.index - 1].into(), + )); + } + Err(ExprError::MissingArgument(_)) => { + return Err(ExprError::ExpectedClosingBraceAfter( + self.input[self.index - 1].into(), + )); + } + Err(e) => return Err(e), } s } @@ -484,6 +492,8 @@ pub fn is_truthy(s: &NumOrStr) -> bool { #[cfg(test)] mod test { + use crate::ExprError; + use super::{AstNode, BinOp, NumericOp, RelationOp, StringOp}; impl From<&str> for AstNode { @@ -587,4 +597,16 @@ mod test { )), ); } + + #[test] + fn missing_closing_parenthesis() { + assert_eq!( + AstNode::parse(&["(", "42"]), + Err(ExprError::ExpectedClosingBraceAfter("42".to_string())) + ); + assert_eq!( + AstNode::parse(&["(", "42", "a"]), + Err(ExprError::ExpectedClosingBraceInsteadOf("a".to_string())) + ); + } } From 0ba9a301b0db81a1c98e34184315c51c18dddf57 Mon Sep 17 00:00:00 2001 From: Joseph Jon Booker Date: Mon, 25 Mar 2024 22:03:26 -0500 Subject: [PATCH 02/10] expr: fix panic for "empty substitution" test case --- src/uu/expr/src/syntax_tree.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/uu/expr/src/syntax_tree.rs b/src/uu/expr/src/syntax_tree.rs index 30d50fb26d9..d333e6c2f5c 100644 --- a/src/uu/expr/src/syntax_tree.rs +++ b/src/uu/expr/src/syntax_tree.rs @@ -148,7 +148,7 @@ impl StringOp { .map_err(|_| ExprError::InvalidRegexExpression)?; Ok(if re.captures_len() > 0 { re.captures(&left) - .map(|captures| captures.at(1).unwrap()) + .and_then(|captures| captures.at(1)) .unwrap_or("") .to_string() } else { @@ -609,4 +609,14 @@ mod test { Err(ExprError::ExpectedClosingBraceInsteadOf("a".to_string())) ); } + + #[test] + fn empty_substitution() { + // causes a panic in 0.0.25 + let result = AstNode::parse(&["a", ":", r"\(b\)*"]) + .unwrap() + .eval() + .unwrap(); + assert_eq!(result.eval_as_string(), ""); + } } From 6a7df7d6c1623114915c4f45f671491ec309fb06 Mon Sep 17 00:00:00 2001 From: Joseph Jon Booker Date: Wed, 27 Mar 2024 01:08:40 -0500 Subject: [PATCH 03/10] expr: Add specific errors for invalid regular expressions --- src/uu/expr/src/expr.rs | 20 +++++++ src/uu/expr/src/syntax_tree.rs | 104 ++++++++++++++++++++++++++++++++- 2 files changed, 122 insertions(+), 2 deletions(-) diff --git a/src/uu/expr/src/expr.rs b/src/uu/expr/src/expr.rs index 371f4377e9a..47fdb2a4ea7 100644 --- a/src/uu/expr/src/expr.rs +++ b/src/uu/expr/src/expr.rs @@ -35,6 +35,11 @@ pub enum ExprError { InvalidRegexExpression, ExpectedClosingBraceAfter(String), ExpectedClosingBraceInsteadOf(String), + UnmatchedOpeningParenthesis, + UnmatchedClosingParenthesis, + UnmatchedOpeningBrace, + UnmatchedClosingBrace, + InvalidContent(String), } impl Display for ExprError { @@ -56,6 +61,21 @@ impl Display for ExprError { Self::ExpectedClosingBraceInsteadOf(s) => { write!(f, "syntax error: expecting ')' instead of {}", s.quote()) } + Self::UnmatchedOpeningParenthesis => { + write!(f, "Unmatched ( or \\(") + } + Self::UnmatchedClosingParenthesis => { + write!(f, "Unmatched ) or \\)") + } + Self::UnmatchedOpeningBrace => { + write!(f, "Unmatched \\{{") + } + Self::UnmatchedClosingBrace => { + write!(f, "Unmatched ) or \\}}") + } + Self::InvalidContent(s) => { + write!(f, "Invalid content of {}", s) + } } } } diff --git a/src/uu/expr/src/syntax_tree.rs b/src/uu/expr/src/syntax_tree.rs index d333e6c2f5c..7036ec01072 100644 --- a/src/uu/expr/src/syntax_tree.rs +++ b/src/uu/expr/src/syntax_tree.rs @@ -6,7 +6,7 @@ // spell-checker:ignore (ToDO) ints paren prec multibytes use num_bigint::{BigInt, ParseBigIntError}; -use num_traits::ToPrimitive; +use num_traits::{ToPrimitive, Zero}; use onig::{Regex, RegexOptions, Syntax}; use crate::{ExprError, ExprResult}; @@ -139,6 +139,7 @@ impl StringOp { Self::Match => { let left = left.eval()?.eval_as_string(); let right = right.eval()?.eval_as_string(); + validate_regex(&right)?; let re_string = format!("^{right}"); let re = Regex::with_options( &re_string, @@ -173,6 +174,65 @@ impl StringOp { } } +/// Check errors with a supplied regular expression +/// +/// GNU coreutils shows messages for invalid regular expressions +/// differently from the oniguruma library used by the regex crate. +/// This method attempts to do these checks manually in one linear pass +/// through the regular expression. +fn validate_regex(pattern: &str) -> ExprResult<()> { + let mut escaped_parens: u64 = 0; + let mut escaped_braces: u64 = 0; + let mut escaped = false; + + let mut comma_in_braces = false; + let mut invalid_content_error = false; + + for c in pattern.chars() { + match (escaped, c) { + (true, ')') => { + escaped_parens = escaped_parens + .checked_sub(1) + .ok_or(ExprError::UnmatchedClosingParenthesis)?; + } + (true, '(') => { + escaped_parens += 1; + } + (true, '}') => { + escaped_braces = escaped_braces + .checked_sub(1) + .ok_or(ExprError::UnmatchedClosingBrace)?; + + if !comma_in_braces { + // Empty repeating patterns are not valid + return Err(ExprError::InvalidContent(r"\{\}".to_string())); + } + } + (true, '{') => { + comma_in_braces = false; + escaped_braces += 1; + } + _ => { + if escaped_braces > 0 && !(c.is_ascii_digit() || c == '\\' || c == ',') { + invalid_content_error = true; + } + } + } + escaped = !escaped && c == '\\'; + comma_in_braces = escaped_braces > 0 && (comma_in_braces || c == ',') + } + match ( + escaped_parens.is_zero(), + escaped_braces.is_zero(), + invalid_content_error, + ) { + (true, true, false) => Ok(()), + (_, false, _) => Err(ExprError::UnmatchedOpeningBrace), + (false, _, _) => Err(ExprError::UnmatchedOpeningParenthesis), + (true, true, true) => Err(ExprError::InvalidContent(r"\{\}".to_string())), + } +} + /// Precedence for infix binary operators const PRECEDENCE: &[&[(&str, BinOp)]] = &[ &[("|", BinOp::String(StringOp::Or))], @@ -493,8 +553,9 @@ pub fn is_truthy(s: &NumOrStr) -> bool { #[cfg(test)] mod test { use crate::ExprError; + use crate::ExprError::InvalidContent; - use super::{AstNode, BinOp, NumericOp, RelationOp, StringOp}; + use super::{validate_regex, AstNode, BinOp, NumericOp, RelationOp, StringOp}; impl From<&str> for AstNode { fn from(value: &str) -> Self { @@ -619,4 +680,43 @@ mod test { .unwrap(); assert_eq!(result.eval_as_string(), ""); } + + #[test] + fn validate_regex_valid() { + assert!(validate_regex(r"(a+b) \(a* b\)").is_ok()); + } + + #[test] + fn validate_regex_missing_closing() { + assert_eq!( + validate_regex(r"\(abc"), + Err(ExprError::UnmatchedOpeningParenthesis) + ); + + assert_eq!( + validate_regex(r"\{1,2"), + Err(ExprError::UnmatchedOpeningBrace) + ); + } + + #[test] + fn validate_regex_missing_opening() { + assert_eq!( + validate_regex(r"abc\)"), + Err(ExprError::UnmatchedClosingParenthesis) + ); + + assert_eq!( + validate_regex(r"abc\}"), + Err(ExprError::UnmatchedClosingBrace) + ); + } + + #[test] + fn validate_regex_empty_repeating_pattern() { + assert_eq!( + validate_regex("ab\\{\\}"), + Err(InvalidContent(r"\{\}".to_string())) + ) + } } From fa0bd722b72460306263fa97fd5a957b4d7ef8fd Mon Sep 17 00:00:00 2001 From: Joseph Jon Booker Date: Wed, 27 Mar 2024 22:28:41 -0500 Subject: [PATCH 04/10] expr: Allow initial asterisk in regular expressions GNU expr appears to treat this as a literal asterisk --- src/uu/expr/src/syntax_tree.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/uu/expr/src/syntax_tree.rs b/src/uu/expr/src/syntax_tree.rs index 7036ec01072..da5b2ccc53d 100644 --- a/src/uu/expr/src/syntax_tree.rs +++ b/src/uu/expr/src/syntax_tree.rs @@ -140,7 +140,8 @@ impl StringOp { let left = left.eval()?.eval_as_string(); let right = right.eval()?.eval_as_string(); validate_regex(&right)?; - let re_string = format!("^{right}"); + let prefix = if right.starts_with('*') { r"^\" } else { "^" }; + let re_string = format!("{prefix}{right}"); let re = Regex::with_options( &re_string, RegexOptions::REGEX_OPTION_NONE, @@ -681,6 +682,21 @@ mod test { assert_eq!(result.eval_as_string(), ""); } + #[test] + fn starting_stars_become_escaped() { + let result = AstNode::parse(&["yolo", ":", r"*yolo"]) + .unwrap() + .eval() + .unwrap(); + assert_eq!(result.eval_as_string(), "0"); + + let result = AstNode::parse(&["*yolo", ":", r"*yolo"]) + .unwrap() + .eval() + .unwrap(); + assert_eq!(result.eval_as_string(), "5"); + } + #[test] fn validate_regex_valid() { assert!(validate_regex(r"(a+b) \(a* b\)").is_ok()); From bff827d9ed8344fb8af3a27ef78ba7f1fdd1966b Mon Sep 17 00:00:00 2001 From: Joseph Jon Booker Date: Thu, 28 Mar 2024 00:00:40 -0500 Subject: [PATCH 05/10] expr: Reject invalid intervals in regular expressions --- src/uu/expr/src/syntax_tree.rs | 73 ++++++++++++++++++++++++++++++---- 1 file changed, 65 insertions(+), 8 deletions(-) diff --git a/src/uu/expr/src/syntax_tree.rs b/src/uu/expr/src/syntax_tree.rs index da5b2ccc53d..4725e020a73 100644 --- a/src/uu/expr/src/syntax_tree.rs +++ b/src/uu/expr/src/syntax_tree.rs @@ -179,14 +179,14 @@ impl StringOp { /// /// GNU coreutils shows messages for invalid regular expressions /// differently from the oniguruma library used by the regex crate. -/// This method attempts to do these checks manually in one linear pass +/// This method attempts to do these checks manually in one pass /// through the regular expression. fn validate_regex(pattern: &str) -> ExprResult<()> { let mut escaped_parens: u64 = 0; let mut escaped_braces: u64 = 0; let mut escaped = false; - let mut comma_in_braces = false; + let mut repeating_pattern_text = String::with_capacity(13); let mut invalid_content_error = false; for c in pattern.chars() { @@ -203,24 +203,46 @@ fn validate_regex(pattern: &str) -> ExprResult<()> { escaped_braces = escaped_braces .checked_sub(1) .ok_or(ExprError::UnmatchedClosingBrace)?; - - if !comma_in_braces { - // Empty repeating patterns are not valid - return Err(ExprError::InvalidContent(r"\{\}".to_string())); + let mut repetition = repeating_pattern_text[..repeating_pattern_text.len() - 1] + .splitn(2, |x| x == ','); + match (repetition.next(), repetition.next()) { + (None, None) => { + // Empty repeating pattern + invalid_content_error = true; + } + (Some(x), None) | (Some(x), Some("")) => { + if !x.parse::().is_ok() { + invalid_content_error = true; + } + } + (None, Some(x)) | (Some(""), Some(x)) => { + if !x.parse::().is_ok() { + invalid_content_error = true; + } + } + (Some(f), Some(l)) => { + if let (Ok(f), Ok(l)) = (f.parse::(), l.parse::()) { + invalid_content_error = invalid_content_error || f > l; + } else { + invalid_content_error = true; + } + } } + repeating_pattern_text.clear(); } (true, '{') => { - comma_in_braces = false; escaped_braces += 1; } _ => { + if escaped_braces > 0 && repeating_pattern_text.len() <= 13 { + repeating_pattern_text.push(c); + } if escaped_braces > 0 && !(c.is_ascii_digit() || c == '\\' || c == ',') { invalid_content_error = true; } } } escaped = !escaped && c == '\\'; - comma_in_braces = escaped_braces > 0 && (comma_in_braces || c == ',') } match ( escaped_parens.is_zero(), @@ -697,11 +719,25 @@ mod test { assert_eq!(result.eval_as_string(), "5"); } + #[test] + fn only_match_in_beginning() { + let result = AstNode::parse(&["cowsay", ":", r"ow"]) + .unwrap() + .eval() + .unwrap(); + assert_eq!(result.eval_as_string(), "0"); + } + #[test] fn validate_regex_valid() { assert!(validate_regex(r"(a+b) \(a* b\)").is_ok()); } + #[test] + fn validate_regex_simple_repeating_pattern() { + assert!(validate_regex(r"(a+b){4}").is_ok()); + } + #[test] fn validate_regex_missing_closing() { assert_eq!( @@ -735,4 +771,25 @@ mod test { Err(InvalidContent(r"\{\}".to_string())) ) } + + #[test] + fn validate_regex_intervals_two_numbers() { + assert_eq!( + // out of order + validate_regex("ab\\{1,0\\}"), + Err(InvalidContent(r"\{\}".to_string())) + ); + assert_eq!( + validate_regex("ab\\{1,a\\}"), + Err(InvalidContent(r"\{\}".to_string())) + ); + assert_eq!( + validate_regex("ab\\{a,3\\}"), + Err(InvalidContent(r"\{\}".to_string())) + ); + assert_eq!( + validate_regex("ab\\{a,b\\}"), + Err(InvalidContent(r"\{\}".to_string())) + ); + } } From 335b13f94020f3b0bf92cbae0f2a72c1bc4653f9 Mon Sep 17 00:00:00 2001 From: Joseph Jon Booker Date: Wed, 3 Apr 2024 23:56:23 -0500 Subject: [PATCH 06/10] expr: Minor linting fix --- src/uu/expr/src/syntax_tree.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/uu/expr/src/syntax_tree.rs b/src/uu/expr/src/syntax_tree.rs index 4725e020a73..8411f82d524 100644 --- a/src/uu/expr/src/syntax_tree.rs +++ b/src/uu/expr/src/syntax_tree.rs @@ -211,12 +211,12 @@ fn validate_regex(pattern: &str) -> ExprResult<()> { invalid_content_error = true; } (Some(x), None) | (Some(x), Some("")) => { - if !x.parse::().is_ok() { + if x.parse::().is_err() { invalid_content_error = true; } } (None, Some(x)) | (Some(""), Some(x)) => { - if !x.parse::().is_ok() { + if x.parse::().is_err() { invalid_content_error = true; } } @@ -706,13 +706,13 @@ mod test { #[test] fn starting_stars_become_escaped() { - let result = AstNode::parse(&["yolo", ":", r"*yolo"]) + let result = AstNode::parse(&["cats", ":", r"*cats"]) .unwrap() .eval() .unwrap(); assert_eq!(result.eval_as_string(), "0"); - let result = AstNode::parse(&["*yolo", ":", r"*yolo"]) + let result = AstNode::parse(&["*cats", ":", r"*cats"]) .unwrap() .eval() .unwrap(); @@ -721,7 +721,7 @@ mod test { #[test] fn only_match_in_beginning() { - let result = AstNode::parse(&["cowsay", ":", r"ow"]) + let result = AstNode::parse(&["budget", ":", r"get"]) .unwrap() .eval() .unwrap(); From cfb539d6725862b0d1dea666a77c3eda9bfdcf32 Mon Sep 17 00:00:00 2001 From: Joseph Jon Booker Date: Tue, 14 May 2024 21:07:26 -0500 Subject: [PATCH 07/10] expr: rename validate_regex to check_posix_regex_errors Also clarified the intent of this function --- src/uu/expr/src/syntax_tree.rs | 37 ++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/src/uu/expr/src/syntax_tree.rs b/src/uu/expr/src/syntax_tree.rs index 8411f82d524..d3ea8507b71 100644 --- a/src/uu/expr/src/syntax_tree.rs +++ b/src/uu/expr/src/syntax_tree.rs @@ -139,7 +139,7 @@ impl StringOp { Self::Match => { let left = left.eval()?.eval_as_string(); let right = right.eval()?.eval_as_string(); - validate_regex(&right)?; + check_posix_regex_errors(&right)?; let prefix = if right.starts_with('*') { r"^\" } else { "^" }; let re_string = format!("{prefix}{right}"); let re = Regex::with_options( @@ -175,13 +175,20 @@ impl StringOp { } } -/// Check errors with a supplied regular expression +/// Check for errors in a supplied regular expression /// /// GNU coreutils shows messages for invalid regular expressions /// differently from the oniguruma library used by the regex crate. /// This method attempts to do these checks manually in one pass /// through the regular expression. -fn validate_regex(pattern: &str) -> ExprResult<()> { +/// +/// This method is not comprehensively checking all cases in which +/// a regular expression could be invalid; any cases not caught will +/// result in a [ExprError::InvalidRegexExpression] when passing the +/// regular expression through the Oniguruma bindings. This method is +/// intended to just identify a few situations for which GNU coreutils +/// has specific error messages. +fn check_posix_regex_errors(pattern: &str) -> ExprResult<()> { let mut escaped_parens: u64 = 0; let mut escaped_braces: u64 = 0; let mut escaped = false; @@ -578,7 +585,7 @@ mod test { use crate::ExprError; use crate::ExprError::InvalidContent; - use super::{validate_regex, AstNode, BinOp, NumericOp, RelationOp, StringOp}; + use super::{check_posix_regex_errors, AstNode, BinOp, NumericOp, RelationOp, StringOp}; impl From<&str> for AstNode { fn from(value: &str) -> Self { @@ -730,23 +737,23 @@ mod test { #[test] fn validate_regex_valid() { - assert!(validate_regex(r"(a+b) \(a* b\)").is_ok()); + assert!(check_posix_regex_errors(r"(a+b) \(a* b\)").is_ok()); } #[test] fn validate_regex_simple_repeating_pattern() { - assert!(validate_regex(r"(a+b){4}").is_ok()); + assert!(check_posix_regex_errors(r"(a+b){4}").is_ok()); } #[test] fn validate_regex_missing_closing() { assert_eq!( - validate_regex(r"\(abc"), + check_posix_regex_errors(r"\(abc"), Err(ExprError::UnmatchedOpeningParenthesis) ); assert_eq!( - validate_regex(r"\{1,2"), + check_posix_regex_errors(r"\{1,2"), Err(ExprError::UnmatchedOpeningBrace) ); } @@ -754,12 +761,12 @@ mod test { #[test] fn validate_regex_missing_opening() { assert_eq!( - validate_regex(r"abc\)"), + check_posix_regex_errors(r"abc\)"), Err(ExprError::UnmatchedClosingParenthesis) ); assert_eq!( - validate_regex(r"abc\}"), + check_posix_regex_errors(r"abc\}"), Err(ExprError::UnmatchedClosingBrace) ); } @@ -767,7 +774,7 @@ mod test { #[test] fn validate_regex_empty_repeating_pattern() { assert_eq!( - validate_regex("ab\\{\\}"), + check_posix_regex_errors("ab\\{\\}"), Err(InvalidContent(r"\{\}".to_string())) ) } @@ -776,19 +783,19 @@ mod test { fn validate_regex_intervals_two_numbers() { assert_eq!( // out of order - validate_regex("ab\\{1,0\\}"), + check_posix_regex_errors("ab\\{1,0\\}"), Err(InvalidContent(r"\{\}".to_string())) ); assert_eq!( - validate_regex("ab\\{1,a\\}"), + check_posix_regex_errors("ab\\{1,a\\}"), Err(InvalidContent(r"\{\}".to_string())) ); assert_eq!( - validate_regex("ab\\{a,3\\}"), + check_posix_regex_errors("ab\\{a,3\\}"), Err(InvalidContent(r"\{\}".to_string())) ); assert_eq!( - validate_regex("ab\\{a,b\\}"), + check_posix_regex_errors("ab\\{a,b\\}"), Err(InvalidContent(r"\{\}".to_string())) ); } From 6701c6ef44169369c6b2d83f07249cff559aba09 Mon Sep 17 00:00:00 2001 From: Joseph Jon Booker Date: Tue, 14 May 2024 21:33:29 -0500 Subject: [PATCH 08/10] expr: Rework branches and add tests for check_posix_regex_errors --- src/uu/expr/src/syntax_tree.rs | 39 ++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/src/uu/expr/src/syntax_tree.rs b/src/uu/expr/src/syntax_tree.rs index d3ea8507b71..4461d6f7a04 100644 --- a/src/uu/expr/src/syntax_tree.rs +++ b/src/uu/expr/src/syntax_tree.rs @@ -193,7 +193,7 @@ fn check_posix_regex_errors(pattern: &str) -> ExprResult<()> { let mut escaped_braces: u64 = 0; let mut escaped = false; - let mut repeating_pattern_text = String::with_capacity(13); + let mut repeating_pattern_text = String::new(); let mut invalid_content_error = false; for c in pattern.chars() { @@ -212,22 +212,27 @@ fn check_posix_regex_errors(pattern: &str) -> ExprResult<()> { .ok_or(ExprError::UnmatchedClosingBrace)?; let mut repetition = repeating_pattern_text[..repeating_pattern_text.len() - 1] .splitn(2, |x| x == ','); - match (repetition.next(), repetition.next()) { - (None, None) => { + match ( + repetition + .next() + .expect("splitn always returns at least one string"), + repetition.next(), + ) { + ("", None) => { // Empty repeating pattern invalid_content_error = true; } - (Some(x), None) | (Some(x), Some("")) => { + (x, None) | (x, Some("")) => { if x.parse::().is_err() { invalid_content_error = true; } } - (None, Some(x)) | (Some(""), Some(x)) => { + ("", Some(x)) => { if x.parse::().is_err() { invalid_content_error = true; } } - (Some(f), Some(l)) => { + (f, Some(l)) => { if let (Ok(f), Ok(l)) = (f.parse::(), l.parse::()) { invalid_content_error = invalid_content_error || f > l; } else { @@ -736,17 +741,17 @@ mod test { } #[test] - fn validate_regex_valid() { + fn check_regex_valid() { assert!(check_posix_regex_errors(r"(a+b) \(a* b\)").is_ok()); } #[test] - fn validate_regex_simple_repeating_pattern() { - assert!(check_posix_regex_errors(r"(a+b){4}").is_ok()); + fn check_regex_simple_repeating_pattern() { + assert!(check_posix_regex_errors(r"\(a+b\)\{4\}").is_ok()); } #[test] - fn validate_regex_missing_closing() { + fn check_regex_missing_closing() { assert_eq!( check_posix_regex_errors(r"\(abc"), Err(ExprError::UnmatchedOpeningParenthesis) @@ -759,7 +764,7 @@ mod test { } #[test] - fn validate_regex_missing_opening() { + fn check_regex_missing_opening() { assert_eq!( check_posix_regex_errors(r"abc\)"), Err(ExprError::UnmatchedClosingParenthesis) @@ -772,7 +777,7 @@ mod test { } #[test] - fn validate_regex_empty_repeating_pattern() { + fn check_regex_empty_repeating_pattern() { assert_eq!( check_posix_regex_errors("ab\\{\\}"), Err(InvalidContent(r"\{\}".to_string())) @@ -780,7 +785,7 @@ mod test { } #[test] - fn validate_regex_intervals_two_numbers() { + fn check_regex_intervals_two_numbers() { assert_eq!( // out of order check_posix_regex_errors("ab\\{1,0\\}"), @@ -798,5 +803,13 @@ mod test { check_posix_regex_errors("ab\\{a,b\\}"), Err(InvalidContent(r"\{\}".to_string())) ); + assert_eq!( + check_posix_regex_errors("ab\\{a,\\}"), + Err(InvalidContent(r"\{\}".to_string())) + ); + assert_eq!( + check_posix_regex_errors("ab\\{,b\\}"), + Err(InvalidContent(r"\{\}".to_string())) + ); } } From 8c27c7f3f2d246bf1104513cbb9d427a6c066ec0 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Tue, 17 Sep 2024 11:47:20 +0200 Subject: [PATCH 09/10] Update src/uu/expr/src/syntax_tree.rs --- src/uu/expr/src/syntax_tree.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/expr/src/syntax_tree.rs b/src/uu/expr/src/syntax_tree.rs index 4461d6f7a04..06e7208f8c7 100644 --- a/src/uu/expr/src/syntax_tree.rs +++ b/src/uu/expr/src/syntax_tree.rs @@ -211,7 +211,7 @@ fn check_posix_regex_errors(pattern: &str) -> ExprResult<()> { .checked_sub(1) .ok_or(ExprError::UnmatchedClosingBrace)?; let mut repetition = repeating_pattern_text[..repeating_pattern_text.len() - 1] - .splitn(2, |x| x == ','); + .splitn(2, ','); match ( repetition .next() From 1e0f697ab0e08a212fd7205b2dd6f37147342a36 Mon Sep 17 00:00:00 2001 From: Joseph Jon Booker Date: Sun, 19 Jan 2025 05:24:41 -0600 Subject: [PATCH 10/10] expr: Fix cargo fmt of src/uu/expr/src/syntax_tree.rs --- src/uu/expr/src/syntax_tree.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/expr/src/syntax_tree.rs b/src/uu/expr/src/syntax_tree.rs index 06e7208f8c7..0288a67361b 100644 --- a/src/uu/expr/src/syntax_tree.rs +++ b/src/uu/expr/src/syntax_tree.rs @@ -210,8 +210,8 @@ fn check_posix_regex_errors(pattern: &str) -> ExprResult<()> { escaped_braces = escaped_braces .checked_sub(1) .ok_or(ExprError::UnmatchedClosingBrace)?; - let mut repetition = repeating_pattern_text[..repeating_pattern_text.len() - 1] - .splitn(2, ','); + let mut repetition = + repeating_pattern_text[..repeating_pattern_text.len() - 1].splitn(2, ','); match ( repetition .next()