uutils/sed
0
0
Fork 0
mirror of https://github.com/uutils/sed.git synced 2026-06-10 16:14:15 -07:00
Code Issues Packages Projects Releases Wiki Activity

Improve error reporting for RE errors

Browse Source
This commit is contained in:
Diomidis Spinellis
2025-07-08 17:37:57 +03:00
parent fd3172e6b6
commit 60cba2bf55
5 changed files with 109 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/uu/sed/src/command.rs
+1 -1
View File
@@ -348,7 +348,7 @@ mod tests {
// Return the captures for the RE applied to the specified string
fn caps_for<'a>(re: &str, chunk: &'a mut IOChunk) -> Captures<'a> {
Regex::new(re)
Regex::new(ScriptLocation::default(), re)
.unwrap()
.captures(chunk)
.unwrap()
src/uu/sed/src/compiler.rs
+37 -11
View File
@@ -13,7 +13,7 @@ use crate::command::{
ReplacementTemplate, Substitution, Transliteration,
};
use crate::delimited_parser::{parse_char_escape, parse_regex, parse_transliteration};
use crate::error_handling::{compilation_error, semantic_error};
use crate::error_handling::{ScriptLocation, compilation_error, semantic_error};
use crate::fast_regex::Regex;
use crate::named_writer::NamedWriter;
use crate::script_char_provider::ScriptCharProvider;
@@ -509,6 +509,7 @@ fn compile_address(
match line.current() {
'\\' | '/' => {
// Regular expression
let location = ScriptLocation::at_position(lines, line);
if line.current() == '\\' {
// The next character is an arbitrary delimiter
line.advance();
@@ -525,7 +526,9 @@ fn compile_address(
Ok(Address {
atype: AddressType::Re,
value: AddressValue::Regex(compile_regex(lines, line, &re, context, icase)?),
value: AddressValue::Regex(compile_regex(
lines, line, &location, &re, context, icase,
)?),
})
}
'$' => {
@@ -688,6 +691,7 @@ fn bre_to_ere(pattern: &str) -> String {
fn compile_regex(
lines: &ScriptLineProvider,
line: &ScriptCharProvider,
location: &ScriptLocation,
pattern: &str,
context: &ProcessingContext,
icase: bool,
@@ -711,7 +715,7 @@ fn compile_regex(
};
// Compile into engine.
let compiled = Regex::new(&pattern).map_err(|e| {
let compiled = Regex::new(location.clone(), &pattern).map_err(|e| {
compilation_error::<Regex>(lines, line, format!("invalid regex '{pattern}': {e}"))
.unwrap_err()
})?;
@@ -848,6 +852,7 @@ fn compile_subst_command(
);
}
let location = ScriptLocation::at_position(lines, line);
let pattern = parse_regex(lines, line)?;
let mut subst = Box::new(Substitution {
@@ -867,7 +872,7 @@ fn compile_subst_command(
}
// Compile regex with now known ignore_case flag.
subst.regex = compile_regex(lines, line, &pattern, context, subst.ignore_case)?;
subst.regex = compile_regex(lines, line, &location, &pattern, context, subst.ignore_case)?;
// Catch invalid group references at compile time, if possible.
if let Some(regex) = &subst.regex {
@@ -1475,9 +1480,16 @@ mod tests {
#[test]
fn test_compile_re_basic() {
let (lines, chars) = dummy_providers();
let regex = compile_regex(&lines, &chars, "abc", &ctx(), false)
.unwrap()
.expect("regex should be present");
let regex = compile_regex(
&lines,
&chars,
&ScriptLocation::default(),
"abc",
&ctx(),
false,
)
.unwrap()
.expect("regex should be present");
assert!(regex.is_match(&mut IOChunk::from_str("abc")).unwrap());
assert!(!regex.is_match(&mut IOChunk::from_str("ABC")).unwrap());
}
@@ -1485,9 +1497,16 @@ mod tests {
#[test]
fn test_compile_re_case_insensitive() {
let (lines, chars) = dummy_providers();
let regex = compile_regex(&lines, &chars, "abc", &ctx(), true)
.unwrap()
.expect("regex should be present");
let regex = compile_regex(
&lines,
&chars,
&ScriptLocation::default(),
"abc",
&ctx(),
true,
)
.unwrap()
.expect("regex should be present");
assert!(regex.is_match(&mut IOChunk::from_str("abc")).unwrap());
assert!(regex.is_match(&mut IOChunk::from_str("ABC")).unwrap());
assert!(regex.is_match(&mut IOChunk::from_str("AbC")).unwrap());
@@ -1496,7 +1515,14 @@ mod tests {
#[test]
fn test_compile_re_invalid() {
let (lines, chars) = dummy_providers();
let result = compile_regex(&lines, &chars, "a[d", &ctx(), false);
let result = compile_regex(
&lines,
&chars,
&ScriptLocation::default(),
"a[d",
&ctx(),
false,
);
assert!(result.is_err()); // Should fail due to open bracketed expression
}
src/uu/sed/src/error_handling.rs
+1 -1
View File
@@ -15,7 +15,7 @@ use std::rc::Rc;
use uucore::error::{UResult, USimpleError};
#[derive(Debug)]
#[derive(Clone, Debug)]
/// The location in a script where a command is defined
pub struct ScriptLocation {
pub input_name: Rc<str>, // Shared input name
src/uu/sed/src/fast_regex.rs
+61 -44
View File
@@ -11,6 +11,8 @@
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.
use crate::error_handling::{ScriptLocation, runtime_error};
use fancy_regex::{
CaptureMatches as FancyCaptureMatches, Captures as FancyCaptures, Regex as FancyRegex,
};
@@ -234,69 +236,84 @@ pub fn remove_escapes(pattern: &str) -> String {
#[derive(Clone, Debug)]
/// A regular expression that can be implemented in diverse efficient ways
pub enum Regex {
pub enum RegexEngine {
Literal(LiteralMatcher), // Fastest: literal bytes
Byte(ByteRegex), // Slower: byte-based RE
Fancy(FancyRegex), // Slowest: RE supporting UTF-8 and back-references
}
#[derive(Clone, Debug)]
pub struct Regex {
loc: ScriptLocation,
engine: RegexEngine,
}
impl Regex {
/// Construct the most efficient RE-like matching engine possible.
pub fn new(pattern: &str) -> Result<Self, Box<dyn Error>> {
if NEEDS_FANCY_RE.is_match(pattern) {
Ok(Self::Fancy(FancyRegex::new(pattern)?))
pub fn new(loc: ScriptLocation, pattern: &str) -> Result<Self, Box<dyn Error>> {
let engine = if NEEDS_FANCY_RE.is_match(pattern) {
RegexEngine::Fancy(FancyRegex::new(pattern)?)
} else if NEEDS_RE.is_match(pattern) {
Ok(Self::Byte(ByteRegex::new(pattern)?))
RegexEngine::Byte(ByteRegex::new(pattern)?)
} else {
Ok(Self::Literal(LiteralMatcher::new(&remove_escapes(pattern))))
}
RegexEngine::Literal(LiteralMatcher::new(&remove_escapes(pattern)))
};
Ok(Regex { loc, engine })
}
#[cfg(test)]
/// Construct with a default location
pub fn new_unlocated(pattern: &str) -> Result<Self, Box<dyn Error>> {
Regex::new(ScriptLocation::default(), pattern)
}
/// Check if the regex matches the content of the IOChunk.
pub fn is_match(&self, chunk: &mut IOChunk) -> UResult<bool> {
match self {
Regex::Literal(m) => Ok(m.is_match(chunk.as_bytes())),
Regex::Byte(re) => Ok(re.is_match(chunk.as_bytes())),
Regex::Fancy(re) => {
match &self.engine {
RegexEngine::Literal(m) => Ok(m.is_match(chunk.as_bytes())),
RegexEngine::Byte(re) => Ok(re.is_match(chunk.as_bytes())),
RegexEngine::Fancy(re) => {
let text = chunk.as_str()?;
re.is_match(text)
.map_err(|e| USimpleError::new(2, e.to_string()))
match re.is_match(text) {
Ok(found) => Ok(found),
Err(e) => runtime_error(&self.loc, e.to_string()),
}
}
}
}
/// Return an iterator over capture groups.
pub fn captures_iter<'t>(&'t self, chunk: &'t IOChunk) -> UResult<CaptureMatches<'t>> {
match self {
Regex::Literal(m) => {
match &self.engine {
RegexEngine::Literal(m) => {
let haystack = chunk.as_bytes();
Ok(CaptureMatches::Literal(Box::new(m.iter(haystack).map(
|(start, end, text)| Ok(Captures::Literal(Match { start, end, text })),
))))
}
Regex::Byte(re) => Ok(CaptureMatches::Byte(re.captures_iter(chunk.as_bytes()))),
RegexEngine::Byte(re) => Ok(CaptureMatches::Byte(re.captures_iter(chunk.as_bytes()))),
Regex::Fancy(re) => {
RegexEngine::Fancy(re) => {
let text = chunk.as_str()?;
Ok(CaptureMatches::Fancy(re.captures_iter(text)))
Ok(CaptureMatches::Fancy(re.captures_iter(text), &self.loc))
}
}
}
/// Return the number of capture groups, including group 0.
pub fn captures_len(&self) -> usize {
match self {
Regex::Literal(_) => 1, // Only group 0
Regex::Byte(re) => re.captures_len(),
Regex::Fancy(re) => re.captures_len(),
match &self.engine {
RegexEngine::Literal(_) => 1, // Only group 0
RegexEngine::Byte(re) => re.captures_len(),
RegexEngine::Fancy(re) => re.captures_len(),
}
}
/// Return the elements of the first capture.
pub fn captures<'t>(&self, chunk: &'t IOChunk) -> UResult<Option<Captures<'t>>> {
match self {
Regex::Literal(m) => {
match &self.engine {
RegexEngine::Literal(m) => {
let haystack = chunk.as_bytes();
match m.find(haystack) {
Some((start, end, text)) => {
@@ -306,17 +323,17 @@ impl Regex {
}
}
Regex::Byte(re) => {
RegexEngine::Byte(re) => {
let bytes = chunk.as_bytes();
Ok(re.captures(bytes).map(Captures::Byte))
}
Regex::Fancy(re) => {
RegexEngine::Fancy(re) => {
let text = chunk.as_str()?;
match re.captures(text) {
Ok(Some(caps)) => Ok(Some(Captures::Fancy(caps))),
Ok(None) => Ok(None),
Err(e) => Err(USimpleError::new(2, e.to_string())),
Err(e) => runtime_error(&self.loc, e.to_string()),
}
}
}
@@ -324,8 +341,8 @@ impl Regex {
/// Return a non-capturing result for a single match.
pub fn find<'t>(&self, chunk: &'t IOChunk) -> UResult<Option<Match<'t>>> {
match self {
Regex::Literal(m) => {
match &self.engine {
RegexEngine::Literal(m) => {
let haystack = chunk.as_bytes();
match m.find(haystack) {
Some((start, end, text)) => Ok(Some(Match { start, end, text })),
@@ -333,7 +350,7 @@ impl Regex {
}
}
Regex::Byte(re) => {
RegexEngine::Byte(re) => {
let haystack = chunk.as_bytes();
if let Some(m) = re.find(haystack) {
// Attempt UTF-8 decode for the match region only
@@ -349,7 +366,7 @@ impl Regex {
}
}
Regex::Fancy(re) => {
RegexEngine::Fancy(re) => {
let text = chunk.as_str()?;
match re.find(text) {
Ok(Some(m)) => Ok(Some(Match {
@@ -358,7 +375,7 @@ impl Regex {
text: m.as_str(),
})),
Ok(None) => Ok(None),
Err(e) => Err(USimpleError::new(2, e.to_string())),
Err(e) => runtime_error(&self.loc, e.to_string()),
}
}
}
@@ -369,7 +386,7 @@ impl Regex {
pub enum CaptureMatches<'t> {
Literal(Box<dyn Iterator<Item = UResult<Captures<'t>>> + 't>),
Byte(ByteCaptureMatches<'t, 't>),
Fancy(FancyCaptureMatches<'t, 't>),
Fancy(FancyCaptureMatches<'t, 't>, &'t ScriptLocation),
}
impl<'t> Iterator for CaptureMatches<'t> {
@@ -379,12 +396,12 @@ impl<'t> Iterator for CaptureMatches<'t> {
match self {
CaptureMatches::Literal(iter) => iter.next(),
CaptureMatches::Byte(iter) => iter.next().map(|caps| Ok(Captures::Byte(caps))),
CaptureMatches::Fancy(iter) => match iter.next() {
CaptureMatches::Fancy(iter, loc) => match iter.next() {
Some(Ok(caps)) => Some(Ok(Captures::Fancy(caps))),
Some(Err(e)) => Some(Err(USimpleError::new(
2,
Some(Err(e)) => Some(runtime_error(
loc,
format!("error retrieving RE captures: {e}"),
))),
)),
None => None,
},
}
@@ -588,25 +605,25 @@ mod tests {
// Regex::new
#[test]
fn assert_byte_selection() {
let re = Regex::new(r"x*").unwrap();
assert!(matches!(re, Regex::Byte(_)));
let re = Regex::new_unlocated(r"x*").unwrap();
assert!(matches!(re.engine, RegexEngine::Byte(_)));
}
#[test]
fn assert_fancy() {
let re = Regex::new(r"\d").unwrap();
assert!(matches!(re, Regex::Fancy(_)));
let re = Regex::new_unlocated(r"\d").unwrap();
assert!(matches!(re.engine, RegexEngine::Fancy(_)));
}
#[test]
fn assert_literal() {
let re = Regex::new(r"x\.").unwrap();
assert!(matches!(re, Regex::Literal(_)));
let re = Regex::new_unlocated(r"x\.").unwrap();
assert!(matches!(re.engine, RegexEngine::Literal(_)));
}
#[test]
fn handles_invalid_regex_gracefully() {
let err = Regex::new("(").unwrap_err().to_string();
let err = Regex::new_unlocated("(").unwrap_err().to_string();
assert!(
err.contains("unclosed group") || err.contains("error parsing"),
"Unexpected error: {}",
tests/by-util/test_sed.rs
+9
View File
@@ -926,3 +926,12 @@ fn test_undefined_label() {
.code_is(1)
.stderr_is("sed: <script argument 1>:1:1: error: undefined label `foo'\n");
}
#[test]
fn test_fancy_regex_error() {
new_ucmd!()
.args(&["-E", r"/(\.+)+\1b$/p", "input/dots-4k.txt"])
.fails()
.code_is(2)
.stderr_is("sed: <script argument 1>:1:1: error: Error executing regex: Max limit for backtracking count exceeded\n");
}