From 6ff1725b1d2b646cf23ba4f281ef5f9998e46250 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Wed, 7 Oct 2026 21:03:33 +0000 Subject: [PATCH] fix(sed): GNU character escapes, -z joins, unreadable inputs and per-line loop budget Claude-Session: https://claude.ai/code/session_01VscpEzxtqpCKx5eRMpywNj --- crates/bashkit/src/builtins/sed/exec.rs | 27 ++++++-- crates/bashkit/src/builtins/sed/mod.rs | 26 +++++--- crates/bashkit/src/builtins/sed/pattern.rs | 77 ++++++++++++++++++++-- crates/bashkit/src/builtins/sed/script.rs | 21 +++++- crates/bashkit/src/builtins/sed/tests.rs | 56 ++++++++++++++++ knowledge/log.md | 1 + knowledge/operations/limitations.md | 2 +- 7 files changed, 188 insertions(+), 22 deletions(-) diff --git a/crates/bashkit/src/builtins/sed/exec.rs b/crates/bashkit/src/builtins/sed/exec.rs index 5d9056074..22a506cea 100644 --- a/crates/bashkit/src/builtins/sed/exec.rs +++ b/crates/bashkit/src/builtins/sed/exec.rs @@ -226,6 +226,7 @@ impl<'a> Machine<'a> { // without bound. The per-cycle step budget cannot see across restarts, // so they get their own budget, reset whenever a line is read. let mut restarts = 0usize; + let mut restart_cursor = self.cursor; loop { if self.exit_code.is_some() { break; @@ -235,6 +236,13 @@ impl<'a> Machine<'a> { // line is actually read, so the reset lives in `read_next`. if restart { restarts += 1; + if self.cursor != restart_cursor { + // The restarted cycles consumed input (`$!N;P;D`), so + // they are bounded by the input; only count restarts + // that read nothing. + restarts = 0; + restart_cursor = self.cursor; + } if restarts > SED_MAX_CYCLE_STEPS { self.warn_loop_limit(); break; @@ -244,6 +252,7 @@ impl<'a> Machine<'a> { break; } restarts = 0; + restart_cursor = self.cursor; } restart = false; @@ -280,8 +289,15 @@ impl<'a> Machine<'a> { let cmds = &self.prog.cmds; let mut pc = 0usize; let mut steps = 0usize; + // The step budget is per line read: a loop that keeps reading input + // with `N`/`n` (`:b;$!N;...;bb`) is bounded by the input instead. + let mut step_cursor = self.cursor; while pc < cmds.len() { + if self.cursor != step_cursor { + step_cursor = self.cursor; + steps = 0; + } steps += 1; if steps > SED_MAX_CYCLE_STEPS { self.warn_loop_limit(); @@ -359,7 +375,7 @@ impl<'a> Machine<'a> { } Kind::Delete => return Cycle::Silent, Kind::DeleteFirstLine => { - return match self.ps.find('\n') { + return match self.ps.find(self.sep) { Some(idx) => { self.ps.drain(..=idx); Cycle::Restart @@ -374,7 +390,7 @@ impl<'a> Machine<'a> { } Kind::PrintFirstLine => { let ps = std::mem::take(&mut self.ps); - match ps.find('\n') { + match ps.find(self.sep) { Some(idx) => self.out.line(&ps[..idx], true), None => self.out.line(&ps, self.ps_had_newline), } @@ -400,7 +416,8 @@ impl<'a> Machine<'a> { return Cycle::Auto; } let line = &self.lines[self.cursor]; - self.ps.push('\n'); + // GNU joins with the buffer delimiter (NUL under -z). + self.ps.push(self.sep); self.ps.push_str(&line.text); self.ps_had_newline = line.had_newline; self.cur_file = line.file; @@ -436,7 +453,7 @@ impl<'a> Machine<'a> { self.hold_had_newline = self.ps_had_newline; } Kind::HoldAppend => { - self.hold.push('\n'); + self.hold.push(self.sep); self.hold.push_str(&self.ps); self.hold_had_newline = self.ps_had_newline; } @@ -445,7 +462,7 @@ impl<'a> Machine<'a> { self.ps_had_newline = self.hold_had_newline; } Kind::GetAppend => { - self.ps.push('\n'); + self.ps.push(self.sep); self.ps.push_str(&self.hold); self.ps_had_newline = self.hold_had_newline; } diff --git a/crates/bashkit/src/builtins/sed/mod.rs b/crates/bashkit/src/builtins/sed/mod.rs index b397a7901..907b6a010 100644 --- a/crates/bashkit/src/builtins/sed/mod.rs +++ b/crates/bashkit/src/builtins/sed/mod.rs @@ -315,7 +315,7 @@ impl Builtin for Sed { } } - let program = match script::parse(&script, opts.extended) { + let program = match script::parse_with(&script, opts.extended, opts.null_data) { Ok(p) => p, Err(e) => return Ok(ExecResult::err(format!("sed: {}\n", e.message), e.code)), }; @@ -349,6 +349,10 @@ impl Builtin for Sed { }; let mut lines: Vec = Vec::new(); let mut raw: Vec = Vec::new(); + // GNU reports an unreadable input file, goes on with the rest, and + // exits 2 at the end. + let mut read_errors = String::new(); + let mut unreadable: Vec = Vec::new(); for (index, name) in names.iter().enumerate() { let content = if name == "-" || opts.files.is_empty() { ctx.stdin.map(ToString::to_string).unwrap_or_default() @@ -369,13 +373,12 @@ impl Builtin for Sed { String::from_utf8_lossy(e.as_bytes()).into_owned() } }, - // GNU exits 2 when an input file cannot be read. Err(e) => { let reason = crate::error::io_error_reason(&e); - return Ok(ExecResult::err( - format!("sed: can't read {name}: {reason}\n"), - 2, - )); + read_errors.push_str(&format!("sed: can't read {name}: {reason}\n")); + unreadable.push(index); + raw.push(String::new()); + continue; } } }; @@ -430,6 +433,10 @@ impl Builtin for Sed { break; } let name = names.get(index).map(String::as_str).unwrap_or("-"); + // Under -s/-i segments are per operand; skip the unreadable ones. + if opts.separate && unreadable.contains(&index) { + continue; + } let in_place = opts.separate && opts.in_place.is_some() && name != "-"; let produced = match machine.run_segment(segment) { Ok(produced) => produced, @@ -442,8 +449,11 @@ impl Builtin for Sed { } } - let mut stderr = std::mem::take(&mut machine.stderr); - let exit_code = machine.exit_code.unwrap_or(0); + let mut stderr = read_errors; + stderr.push_str(&std::mem::take(&mut machine.stderr)); + let exit_code = machine + .exit_code + .unwrap_or(if unreadable.is_empty() { 0 } else { 2 }); let write_files = std::mem::take(&mut machine.write_files); drop(machine); diff --git a/crates/bashkit/src/builtins/sed/pattern.rs b/crates/bashkit/src/builtins/sed/pattern.rs index 743e1d9e9..3409d693f 100644 --- a/crates/bashkit/src/builtins/sed/pattern.rs +++ b/crates/bashkit/src/builtins/sed/pattern.rs @@ -45,12 +45,15 @@ pub(super) fn translate(pattern: &str, extended: bool) -> String { push_literal(&mut out, next); branch_start = false; } - 'n' => push_raw(&mut out, '\n', &mut branch_start), - 't' => push_raw(&mut out, '\t', &mut branch_start), - 'r' => push_raw(&mut out, '\r', &mut branch_start), - 'f' => push_raw(&mut out, '\x0c', &mut branch_start), - 'v' => push_raw(&mut out, '\x0b', &mut branch_start), - 'a' => push_raw(&mut out, '\x07', &mut branch_start), + c if simple_escape(c).is_some() => { + push_raw(&mut out, simple_escape(c).unwrap_or(c), &mut branch_start) + } + 'd' | 'o' | 'x' | 'c' if numeric_escape(&chars, i - 1).is_some() => { + if let Some((c, used)) = numeric_escape(&chars, i - 1) { + push_raw(&mut out, c, &mut branch_start); + i += used; + } + } // GNU character-class and word-boundary escapes. 'w' | 'W' | 's' | 'S' | 'b' | 'B' => { out.push('\\'); @@ -142,6 +145,48 @@ pub(super) fn translate(pattern: &str, extended: bool) -> String { out } +/// GNU's character-producing escapes `\dNNN`, `\oNNN`, `\xHH` and `\cX`, +/// with `chars[i]` the letter after the backslash. Returns the character and +/// how many characters after the letter it used. The character is always +/// literal: `s/z/\x26/` inserts `&`, not the match (GNU bug 30794). +pub(super) fn numeric_escape(chars: &[char], i: usize) -> Option<(char, usize)> { + let (radix, max) = match chars.get(i)? { + 'd' => (10, 3), + 'o' => (8, 3), + 'x' => (16, 2), + 'c' => { + let x = chars.get(i + 1)?.to_ascii_uppercase(); + if !x.is_ascii() { + return None; + } + return Some((char::from((x as u8) ^ 0x40), 1)); + } + _ => return None, + }; + let digits: String = chars[i + 1..] + .iter() + .take(max) + .take_while(|c| c.is_digit(radix)) + .collect(); + let n = u32::from_str_radix(&digits, radix).ok()?; + let ch = char::from_u32(n).filter(|_| n <= 0xff)?; + Some((ch, digits.len())) +} + +/// The plain one-letter escapes GNU sed converts everywhere in a regex, +/// including inside bracket expressions. +fn simple_escape(c: char) -> Option { + Some(match c { + 'n' => '\n', + 't' => '\t', + 'r' => '\r', + 'f' => '\x0c', + 'v' => '\x0b', + 'a' => '\x07', + _ => return None, + }) +} + fn push_raw(out: &mut String, ch: char, branch_start: &mut bool) { push_literal(out, ch); *branch_start = false; @@ -204,6 +249,24 @@ fn copy_bracket(chars: &[char], start: usize, out: &mut String) -> usize { seen += 1; continue; } + // GNU sed converts `\t`, `\n`, `\xHH`... before compiling, so they + // work inside brackets too (`[ \t]`); any other `\` is a member. + if ch == '\\' + && let Some(&next) = chars.get(i + 1) + { + if let Some(c) = simple_escape(next) { + push_class_literal(&mut content, c); + i += 2; + seen += 1; + continue; + } + if let Some((c, used)) = numeric_escape(chars, i + 1) { + push_class_literal(&mut content, c); + i += 2 + used; + seen += 1; + continue; + } + } push_class_literal(&mut content, ch); i += 1; seen += 1; @@ -275,11 +338,13 @@ impl SedRegex { extended: bool, case_insensitive: bool, multi_line: bool, + line_term: u8, ) -> std::result::Result { let translated = translate(pattern, extended); match RegexBuilder::new(&translated) .case_insensitive(case_insensitive) .multi_line(multi_line) + .line_terminator(line_term) .size_limit(REGEX_SIZE_LIMIT) .dfa_size_limit(REGEX_DFA_SIZE_LIMIT) .build() diff --git a/crates/bashkit/src/builtins/sed/script.rs b/crates/bashkit/src/builtins/sed/script.rs index d8490f1f6..df3074f10 100644 --- a/crates/bashkit/src/builtins/sed/script.rs +++ b/crates/bashkit/src/builtins/sed/script.rs @@ -142,14 +142,17 @@ struct Parser { chars: Vec, pos: usize, extended: bool, + /// `-z`: in `M` mode `^`/`$` also match around NUL, the line separator. + null_data: bool, regex_seen: bool, } -pub(super) fn parse(script: &str, extended: bool) -> ParseResult { +pub(super) fn parse_with(script: &str, extended: bool, null_data: bool) -> ParseResult { let mut parser = Parser { chars: script.chars().collect(), pos: 0, extended, + null_data, regex_seen: false, }; parser.program() @@ -457,7 +460,8 @@ impl Parser { return Ok(None); } self.regex_seen = true; - match SedRegex::new(body, self.extended, case_insensitive, multi_line) { + let line_term = if self.null_data { b'\0' } else { b'\n' }; + match SedRegex::new(body, self.extended, case_insensitive, multi_line, line_term) { Ok(re) => Ok(Some(Arc::new(re))), Err(e) => self.err(format!("invalid pattern: {}", flatten(&e))), } @@ -793,11 +797,19 @@ fn unescape_y(s: &str, delim: char) -> Vec { let mut i = 0; while i < chars.len() { if chars[i] == '\\' && i + 1 < chars.len() { + if let Some((c, used)) = super::pattern::numeric_escape(&chars, i + 1) { + out.push(c); + i += 2 + used; + continue; + } let n = chars[i + 1]; out.push(match n { 'n' => '\n', 't' => '\t', 'r' => '\r', + 'f' => '\x0c', + 'v' => '\x0b', + 'a' => '\x07', '\\' => '\\', c if c == delim => c, c => c, @@ -839,6 +851,11 @@ pub(super) fn parse_replacement(text: &str) -> Vec { continue; } if c == '\\' && i + 1 < chars.len() { + if let Some((ch, used)) = super::pattern::numeric_escape(&chars, i + 1) { + lit.push(ch); + i += 2 + used; + continue; + } let n = chars[i + 1]; i += 2; match n { diff --git a/crates/bashkit/src/builtins/sed/tests.rs b/crates/bashkit/src/builtins/sed/tests.rs index d7b1175d6..bd4fa6760 100644 --- a/crates/bashkit/src/builtins/sed/tests.rs +++ b/crates/bashkit/src/builtins/sed/tests.rs @@ -799,3 +799,59 @@ async fn no_leak_invalid_regex() { &["regex::Error", "ParseError {"], ); } + +#[tokio::test] +async fn gnu_character_escapes_are_literal() { + assert_eq!(out(&["s/z/\\x5cA/"], "z\n").await, "\\A\n"); + assert_eq!(out(&["s/yes/yes\\x26/"], "yes\n").await, "yes&\n"); + assert_eq!(out(&["s/\\x2a/S/"], "a*b\n").await, "aSb\n"); + assert_eq!(out(&["s/b/\\d065\\o101\\cA/"], "abc\n").await, "aAA\x01c\n"); + assert_eq!(out(&["y/b/\\x00/"], "abc\n").await, "a\0c\n"); + // `\t` and `\n` also work inside bracket expressions; other escapes + // stay literal members. + assert_eq!(out(&["s/^[ \\t]*//"], " \t a\n").await, "a\n"); + assert_eq!(out(&["s/[\\.]/X/g"], "a.b\\c\n").await, "aXbXc\n"); +} + +#[tokio::test] +async fn null_data_joins_lines_with_nul() { + assert_eq!( + out( + &["-z", "N;N;s/^/X/g;s/^/X/mg;s/$/Y/g;s/$/Y/mg"], + "a\0b\0c\0" + ) + .await, + "XXaY\0XbY\0XcYY\0" + ); + assert_eq!(out(&["-z", "H;$!d;x"], "a\0b\0").await, "\0a\0b\0"); +} + +#[tokio::test] +async fn unreadable_input_file_is_reported_and_skipped() { + let fs = Arc::new(InMemoryFs::new()); + fs.write_file(std::path::Path::new("/in.txt"), b"still\n") + .await + .unwrap(); + let r = run_with(fs, &["-n", "p", "nope.txt", "in.txt"], None).await; + assert_eq!(r.stdout, "still\n"); + assert_eq!( + r.stderr, + "sed: can't read nope.txt: No such file or directory\n" + ); + assert_eq!(r.exit_code, 2); +} + +#[tokio::test] +async fn loops_that_read_input_are_not_capped() { + // `$!N;P;D` restarts once per line and `:b;$b;N;...;bb` loops once per + // line; both are bounded by the input, so long inputs must finish. + let input: String = (1..=3000).map(|n| format!("{n}\n")).collect(); + assert_eq!(out(&["$!N;P;D"], &input).await, input); + let dups = "x\n".repeat(3000) + "y\n"; + let r = run_sed(&["h;:b;$b;N;/^\\(.*\\)\\n\\1$/{g;bb;};$b;P;D"], Some(&dups)).await; + assert_eq!(r.stdout, "x\ny\n"); + assert_eq!(r.stderr, ""); + // A loop that reads nothing is still stopped. + let r = run_sed(&[":a;s/^/x/;ta"], Some("a\n")).await; + assert!(r.stderr.contains("loop limit"), "{}", r.stderr); +} diff --git a/knowledge/log.md b/knowledge/log.md index 7e12b5185..33fd31208 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -2,6 +2,7 @@ ## 2026-10-07 +* **Fix**: sed follows GNU on character escapes, `-z`, unreadable inputs and long loops. `\xHH`, `\dNNN`, `\oNNN`, `\cX` work in regexes, replacements and `y` and always produce a literal character (`s/z/\x26/` inserts `&`); `\t`/`\n` work inside bracket expressions (`s/^[ \t]*//`). Under `-z`, `N`/`G`/`H` join with NUL, `D`/`P` split on it, and `M` anchors match around it. An unreadable input file is reported and skipped (exit 2 at the end). The loop budget now counts per input line read, so `$!N;P;D` no longer stopped after 1000 lines. Debian-oracle sed score 94.2% to 99.4%. * **Fix**: jq numbers and value errors follow jq 1.7: computed floats print with jq's formatting (`1024`, `1e+17`, `1e-05`, NaN as null, infinities as the largest double), input literals keep their token until computed, division by zero and `% 0` are errors, `%` truncates operands, fractional array indexes truncate, indexing an object with a non-string is an error, `gamma` is log-gamma, and math/index errors use jq's wording (also inside `catch`). Patched in the vendored jaq-json (`jqmsg.rs`, `jq_format_f64`, `defs.jq`); L-JQ-004 records the literal-preservation gap. * **Fix**: jq input stream follows jq 1.7: `-n` leaves the stream to `input`/`inputs`, `-s` on empty input gives `[]`, each FILE is read separately (`input_filename`, `input_line_number` as jq counts lines), a missing file is reported and skipped (exit 2), a runtime error prints `jq: error (at FILE:LINE): ...` and moves on to the next input, `input` fails with `No more inputs`, `-e` uses the last output, values before a JSON syntax error are processed. Debian-oracle jq score 67.5% to 74.2%. * **Fix**: awk `for (k in a)` without `PROCINFO["sorted_in"]` now visits keys in gawk's order (integer arrays ascending with other keys first; string keys in gawk's hash-bucket order), so unsorted loops print like Debian awk. All gawk cases in the Debian-oracle awk set now pass (L-AWK-001 narrowed to post-`delete` order). diff --git a/knowledge/operations/limitations.md b/knowledge/operations/limitations.md index 7de2cb9df..13fef8e12 100644 --- a/knowledge/operations/limitations.md +++ b/knowledge/operations/limitations.md @@ -163,7 +163,7 @@ pass in CI); only divergences and boundaries are recorded here. | L-SED-002 | sed | The `e` command and the `s///e` flag (run the pattern space as a shell command) are refused at compile time, not silently ignored | `the_e_command_is_refused_not_silently_ignored` | | L-SED-003 | sed | `-i` refuses a file that is not valid UTF-8 instead of rewriting it through a lossy decode; without `-i`, non-UTF-8 input is decoded with replacement like every other text builtin (L-STREAM-001) | `in_place_refuses_non_utf8_rather_than_corrupting` | | L-SED-004 | sed | `\<` and `\>` (GNU word-edge operators) both compile to `\b`; the linear-time `regex` engine has no look-around, so a word start and a word end are not distinguished | `translate_is_positional` | -| L-SED-005 | sed | `D` restarts the script without reading input, so a non-progressing loop such as `sed 'G;D'` never terminates — GNU hangs on it. Restarts are capped at `SED_MAX_CYCLE_STEPS`, after which sed stops with the same `loop limit` warning `b`/`t` loops get, and exits 0. A `D` loop that does make progress is unaffected | `a_non_progressing_delete_restart_loop_terminates` | +| L-SED-005 | sed | `D` restarts the script without reading input, so a non-progressing loop such as `sed 'G;D'` never terminates — GNU hangs on it. Restarts are capped at `SED_MAX_CYCLE_STEPS`, after which sed stops with the same `loop limit` warning `b`/`t` loops get, and exits 0. A `D` loop that does make progress is unaffected: restarts and the per-cycle step budget are counted per input line read, so `$!N;P;D` and `:b;$b;N;...;bb` run over any input length | `a_non_progressing_delete_restart_loop_terminates` | | L-SED-006 | sed | Scripts that carry the stream past a range's end with `n`, `N` or `D` can still diverge from GNU's range bookkeeping in rare shapes (e.g. `sed '$!D;1,2!x'`). A randomized differential sweep of 18,000 generated scripts against GNU sed 4.9 finds 3 such cases; all combine a multi-line pattern space with a range address, a shape absent from ordinary scripts | `ranges_that_the_stream_skipped_past` | | L-CURL-001 | curl | Spec-test coverage for methods/headers/auth/redirects not ported (needs `http_client` + allowlist in harness); payload behavior has integration and real-curl differential coverage | stance | | L-CURL-002 | curl/wget | Unknown options are ignored for compatibility, not rejected (real curl/wget error); deliberate leniency | `curl.rs` |