From 9f3bf784fc27f93ca144d90016055ef2003cfe28 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Wed, 7 Oct 2026 17:56:15 +0000 Subject: [PATCH] fix(parser): heredoc backslash escapes; read without -r Unquoted heredoc bodies treat \$, \` and \\ as literals, join \ lines and run backtick substitution. `read` without -r removes backslashes, joins continued lines, and returns 1 when input ends before the delimiter. Pipe consumption follows -d/-n. Claude-Session: https://claude.ai/code/session_019aFikmptPc91Fj4N2iDXQA --- crates/bashkit/docs/compatibility.md | 4 +- crates/bashkit/src/builtins/mod.rs | 1 + crates/bashkit/src/builtins/read.rs | 256 +++++++++++++----- crates/bashkit/src/interpreter/mod.rs | 19 +- crates/bashkit/src/parser/mod.rs | 78 +++++- .../bash/heredoc-read-escapes.test.sh | 57 ++++ knowledge/log.md | 1 + 7 files changed, 338 insertions(+), 78 deletions(-) create mode 100644 crates/bashkit/tests/spec_cases/bash/heredoc-read-escapes.test.sh diff --git a/crates/bashkit/docs/compatibility.md b/crates/bashkit/docs/compatibility.md index d8ff763e0..2d27420d6 100644 --- a/crates/bashkit/docs/compatibility.md +++ b/crates/bashkit/docs/compatibility.md @@ -54,7 +54,7 @@ for sandbox security reasons. See the compliance spec for details. | `test` | `-f`, `-d`, `-e`, `-z`, `-n`, `-eq`, `-ne`, `-lt`, `-gt`, `-le`, `-ge` | Conditionals | | `[` | (same as test) | Alias for test | | `export` | `VAR=value` | Export variables | -| `read` | `VAR` | Read line into variable | +| `read` | `VAR`, `-r`, `-a`, `-d`, `-n`, `-p`, `-s`, `-t` | Read a record; without `-r` backslashes escape and `\` continues; status 1 at end of input | | `set` | `-e`, `+e`, positional | Set options and positional params | | `unset` | `VAR` | Unset variable | | `shift` | `[N]` | Shift positional params | @@ -237,7 +237,7 @@ Chrono's validated strftime implementation, plus GNU `%N`, `%3N`, `%6N`, and | `>>` | ✅ | `cmd >> file` | Append to file | | `<` | ✅ | `cmd < file` | Input from file | | `<<<` | ✅ | `cmd <<< "string"` | Here-string | -| `<` | ✅ | `cmd 2> file` | Stderr redirect | | `2>&1` | ✅ | `cmd 2>&1` | Stderr to stdout | | `&>` | ✅ | `cmd &> file` | Both to file | diff --git a/crates/bashkit/src/builtins/mod.rs b/crates/bashkit/src/builtins/mod.rs index 7cadee453..dacc080e9 100644 --- a/crates/bashkit/src/builtins/mod.rs +++ b/crates/bashkit/src/builtins/mod.rs @@ -232,6 +232,7 @@ pub use pr::Pr; pub use printf::Printf; pub use random::{Openssl, Uuidgen}; pub use read::Read; +pub(crate) use read::consumed_len as read_consumed_len; pub use retry::Retry; pub use rg::Rg; pub use sed::Sed; diff --git a/crates/bashkit/src/builtins/read.rs b/crates/bashkit/src/builtins/read.rs index ec7f67a0b..35bdce2d9 100644 --- a/crates/bashkit/src/builtins/read.rs +++ b/crates/bashkit/src/builtins/read.rs @@ -126,10 +126,21 @@ impl Builtin for Read { #[cfg(not(feature = "terminal"))] let _ = (prompt, silent, timeout); + // Split line by IFS (default: space, tab, newline) + // IFS whitespace chars (space, tab, newline) collapse runs and trim. + // Non-whitespace IFS chars preserve empty fields between consecutive delimiters. + // Check shell variables first (IFS=","), then env, then default. + let ifs = ctx + .variables + .get("IFS") + .or_else(|| ctx.env.get("IFS")) + .map(|s| s.as_str()) + .unwrap_or(" \t\n"); + // EOF with no data: clear all target variables to empty and return 1. // This prevents the common `while read line || [[ -n "$line" ]]` // pattern from looping infinitely on the stale last value. - let input = match input { + let input = match input.filter(|s| !s.is_empty()) { Some(s) => s, None => { let var_names: Vec<&str> = if var_args.is_empty() { @@ -151,40 +162,13 @@ impl Builtin for Read { } }; - // Extract input based on delimiter or nchars - let line = if let Some(n) = nchars { - // -n N: read at most N chars - input.chars().take(n).collect::() - } else if let Some(delim) = delimiter { - // -d delim: read until delimiter - input.split(delim).next().unwrap_or("").to_string() - } else if raw_mode { - // -r: treat backslashes literally - input.lines().next().unwrap_or("").to_string() - } else { - // Without -r: handle backslash line continuation - let mut result = String::new(); - for l in input.lines() { - if let Some(stripped) = l.strip_suffix('\\') { - result.push_str(stripped); - } else { - result.push_str(l); - break; - } - } - result - }; + // One record up to the delimiter (newline by default) or N chars. + // Data that ends before the delimiter is still assigned, but the + // status is 1, as in bash. + let (line, terminated) = + read_record(&input, delimiter.unwrap_or('\n'), nchars, raw_mode, ifs); + let status = if terminated { 0 } else { 1 }; - // Split line by IFS (default: space, tab, newline) - // IFS whitespace chars (space, tab, newline) collapse runs and trim. - // Non-whitespace IFS chars preserve empty fields between consecutive delimiters. - // Check shell variables first (IFS=","), then env, then default. - let ifs = ctx - .variables - .get("IFS") - .or_else(|| ctx.env.get("IFS")) - .map(|s| s.as_str()) - .unwrap_or(" \t\n"); struct ReadField<'a> { text: &'a str, start: usize, @@ -319,18 +303,20 @@ impl Builtin for Read { return Ok(ExecResult::ok(String::new())); } let mut result = ExecResult::ok(String::new()); + result.exit_code = status; result.side_effects.push(BuiltinSideEffect::SetArray { name: arr_name.to_string(), - elements: words.iter().map(|w| w.text.to_string()).collect(), + elements: words.iter().map(|w| unprotect(w.text)).collect(), }); return Ok(result); } if var_args.is_empty() { let mut result = ExecResult::ok(String::new()); + result.exit_code = status; result.side_effects.push(BuiltinSideEffect::SetVariable { name: "REPLY".to_string(), - value: line, + value: unprotect(&line), }); return Ok(result); } @@ -339,6 +325,7 @@ impl Builtin for Read { // Assign words to variables via side effects (respects local scoping) let mut result = ExecResult::ok(String::new()); + result.exit_code = status; for (i, var_name) in var_names.iter().enumerate() { // THREAT[TM-INJ-009]: Block internal variable prefix injection via read if is_internal_variable(var_name) { @@ -351,13 +338,14 @@ impl Builtin for Read { words .get(i) .map(|field| { - line[field.start..] - .trim_end_matches(|ch| ifs.contains(ch) && " \t\n".contains(ch)) - .to_string() + unprotect( + line[field.start..] + .trim_end_matches(|ch| ifs.contains(ch) && " \t\n".contains(ch)), + ) }) .unwrap_or_default() } else if i < words.len() { - words[i].text.to_string() + unprotect(words[i].text) } else { // Not enough words - set to empty String::new() @@ -372,6 +360,152 @@ impl Builtin for Read { } } +/// Bytes of `input` one `read` with these `args` consumes from a shared +/// stdin (pipe feeding a loop): the record plus its delimiter, following +/// `-r`, `-d`, `-n` and `\` continuation like [`read_record`]. +pub(crate) fn consumed_len(input: &[u8], args: &[String]) -> usize { + let mut raw = false; + let mut delim = '\n'; + let mut limit = None::; + let mut iter = args.iter(); + while let Some(arg) = iter.next() { + let Some(flags) = arg.strip_prefix('-').filter(|f| !f.is_empty()) else { + break; + }; + let mut chars = flags.chars(); + while let Some(flag) = chars.next() { + let mut value = || { + let rest: String = chars.clone().collect(); + if rest.is_empty() { + iter.next().cloned().unwrap_or_default() + } else { + rest + } + }; + match flag { + 'r' => raw = true, + 'd' => { + delim = value().chars().next().unwrap_or('\0'); + break; + } + 'n' | 'N' => { + limit = value().parse().ok(); + if flag == 'N' { + delim = '\0'; + } + break; + } + 'p' | 't' | 'u' => { + value(); + break; + } + _ => {} + } + } + } + // Byte scan: keeps non-UTF-8 input intact. A multi-byte delimiter + // matches on its first byte. + let mut delim_buf = [0u8; 4]; + let delim = delim.encode_utf8(&mut delim_buf).as_bytes()[0]; + let mut count = 0usize; + let mut i = 0usize; + while i < input.len() { + let b = input[i]; + // UTF-8 continuation bytes belong to the previous character. + let starts_char = b & 0xC0 != 0x80; + if starts_char && limit.is_some_and(|n| count >= n) { + return i; + } + if b == delim { + return i + 1; + } + if b == b'\\' && !raw { + match input.get(i + 1) { + Some(b'\n') => { + i += 2; + continue; + } + Some(_) => i += 1, + None => return input.len(), + } + } + if starts_char { + count += 1; + } + i += 1; + } + input.len() +} + +/// Escaped IFS characters are carried through field splitting as +/// private-use placeholders, so `read a b <<< 'x\ y z'` keeps `x y` whole. +const PROTECT_BASE: u32 = 0xF0000; + +fn protect(ch: char) -> char { + let c = ch as u32; + if c < 0x10000 { + char::from_u32(PROTECT_BASE + c).unwrap_or(ch) + } else { + ch + } +} + +fn unprotect(s: &str) -> String { + s.chars() + .map(|ch| { + let c = ch as u32; + if (PROTECT_BASE..PROTECT_BASE + 0x10000).contains(&c) { + char::from_u32(c - PROTECT_BASE).unwrap_or(ch) + } else { + ch + } + }) + .collect() +} + +/// Read one record from `input`: up to `delim` (not included) or `limit` +/// characters. Without `raw`, `\` joins lines and `\c` is a +/// literal `c` (protected from IFS splitting). Returns the record and +/// whether it ended at the delimiter or limit rather than at end of input. +fn read_record( + input: &str, + delim: char, + limit: Option, + raw: bool, + ifs: &str, +) -> (String, bool) { + let mut out = String::new(); + let mut count = 0usize; + let mut chars = input.chars(); + loop { + if limit.is_some_and(|n| count >= n) { + return (out, true); + } + let Some(c) = chars.next() else { + return (out, false); + }; + if c == delim { + return (out, true); + } + if c == '\\' && !raw { + match chars.next() { + Some('\n') => continue, + Some(next) => { + out.push(if ifs.contains(next) { + protect(next) + } else { + next + }); + } + None => return (out, false), + } + } else { + out.push(c); + } + count += 1; + } +} + #[cfg(feature = "terminal")] enum TerminalRead { Line(String), @@ -475,7 +609,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("hello world"), + Some("hello world\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -494,7 +628,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("secret "), + Some("secret \n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -515,7 +649,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("test_value"), + Some("test_value\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -536,7 +670,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("one two three four"), + Some("one two three four\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -558,7 +692,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("one"), + Some("one\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -581,7 +715,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("hello\\world"), + Some("hello\\world\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -600,7 +734,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("hello\\\nworld"), + Some("hello\\\nworld\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -641,7 +775,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("hi"), + Some("hi\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -683,7 +817,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("one two three"), + Some("one two three\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -708,7 +842,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("a b"), + Some("a b\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -733,7 +867,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("path\\to\\file"), + Some("path\\to\\file\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -776,7 +910,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("foo:bar:baz"), + Some("foo:bar:baz\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -797,7 +931,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("1,2:3"), + Some("1,2:3\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -818,7 +952,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("a b c "), + Some("a b c \n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -839,7 +973,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("1,2:3 "), + Some("1,2:3 \n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -865,7 +999,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("one::three:"), + Some("one::three:\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -888,7 +1022,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("one two:three"), + Some("one two:three\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -910,7 +1044,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("one : two"), + Some("one : two\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -932,7 +1066,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("no splitting here"), + Some("no splitting here\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -953,7 +1087,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("one,two,three"), + Some("one,two,three\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); @@ -976,7 +1110,7 @@ mod tests { &mut variables, &mut cwd, fs.clone(), - Some("a:b:c"), + Some("a:b:c\n"), ); let result = Read.execute(ctx).await.unwrap(); assert_eq!(result.exit_code, 0); diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index d3a8429a6..782c95197 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -7054,20 +7054,13 @@ impl Interpreter { } else if let Some(ref ps) = self.pipeline_stdin { if !ps.is_empty() { if name == "read" { - // Consume one line from pipeline stdin + // Consume one record (line, `-d` delimiter, `-n` count, + // `\` continuation) from pipeline stdin. let data = ps.clone(); - if let Some(newline_pos) = - data.as_bytes().iter().position(|&byte| byte == b'\n') - { - let line = String::from_utf8_lossy(&data.as_bytes()[..=newline_pos]) - .into_owned(); - self.pipeline_stdin = Some(data.as_bytes()[newline_pos + 1..].into()); - Some(line.into()) - } else { - // Last line without trailing newline - self.pipeline_stdin = Some(crate::StreamData::new()); - Some(data) - } + let bytes = data.as_bytes(); + let used = builtins::read_consumed_len(bytes, &args); + self.pipeline_stdin = Some(bytes[used..].into()); + Some(bytes[..used].into()) } else { Some(ps.clone()) } diff --git a/crates/bashkit/src/parser/mod.rs b/crates/bashkit/src/parser/mod.rs index efde7798e..ff9339351 100644 --- a/crates/bashkit/src/parser/mod.rs +++ b/crates/bashkit/src/parser/mod.rs @@ -721,7 +721,7 @@ impl<'a> Parser<'a> { let target = if quoted { Word::quoted_literal(content) } else { - self.parse_word(content) + self.parse_word(heredoc_body_escapes(&content)) }; let kind = if strip_tabs { RedirectKind::HereDocStrip @@ -2521,7 +2521,7 @@ impl<'a> Parser<'a> { let target = if quoted { Word::quoted_literal(content) } else { - self.parse_word(content) + self.parse_word(heredoc_body_escapes(&content)) }; let kind = if strip_tabs { @@ -4102,6 +4102,80 @@ pub(crate) fn unescape_glob_literal(s: &str) -> String { out } +/// Backslash handling in an unquoted heredoc body, as in double quotes but +/// without `"`: `\$`, `` \` `` and `\\` become literal characters (NUL +/// sentinel for `parse_word`), `\` joins lines, and any other +/// backslash is kept. Text inside `$(...)`, `${...}` and `$((...))` is copied +/// for its own parser (only `\` is removed); backticks become +/// `$(...)`. +fn heredoc_body_escapes(content: &str) -> String { + if !content.contains(['\\', '`']) { + return content.to_string(); + } + let mut out = String::with_capacity(content.len()); + let mut chars = content.chars().peekable(); + while let Some(c) = chars.next() { + match c { + '\\' => match chars.peek() { + Some('$' | '`' | '\\') => { + out.push('\x00'); + out.push(chars.next().unwrap_or_default()); + } + Some('\n') => { + chars.next(); + } + _ => out.push('\\'), + }, + '$' if matches!(chars.peek(), Some('(' | '{')) => { + out.push('$'); + let open = chars.next().unwrap_or_default(); + out.push(open); + let close = if open == '(' { ')' } else { '}' }; + let mut depth = 1usize; + while let Some(ch) = chars.next() { + if ch == '\\' && chars.peek() == Some(&'\n') { + chars.next(); + continue; + } + out.push(ch); + if ch == '\\' { + if let Some(n) = chars.next() { + out.push(n); + } + } else if ch == open { + depth += 1; + } else if ch == close { + depth -= 1; + if depth == 0 { + break; + } + } + } + } + // `cmd` is command substitution, as `$(cmd)`; inside it `\``, + // `\$` and `\\` lose their backslash. + '`' => { + out.push_str("$("); + while let Some(ch) = chars.next() { + match ch { + '`' => break, + '\\' if matches!(chars.peek(), Some('`' | '$' | '\\')) => { + out.push(chars.next().unwrap_or_default()); + } + '\\' if chars.peek() == Some(&'\n') => { + chars.next(); + } + _ => out.push(ch), + } + } + out.push(')'); + } + _ => out.push(c), + } + } + out +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/bashkit/tests/spec_cases/bash/heredoc-read-escapes.test.sh b/crates/bashkit/tests/spec_cases/bash/heredoc-read-escapes.test.sh new file mode 100644 index 000000000..59729fd91 --- /dev/null +++ b/crates/bashkit/tests/spec_cases/bash/heredoc-read-escapes.test.sh @@ -0,0 +1,57 @@ +### heredoc_backslash_escapes +user=agente +cat <` joins lines, and backtick substitution runs (it printed `` `cmd` `` raw). `read` without `-r` removes backslashes (`\ ` keeps a space inside one field) and joins `\` lines; data ending before the delimiter is assigned with status 1; an empty stdin returns 1. Records read from a shared pipe follow `-d`, `-n` and continuation instead of always one line. From the pseudo-linus oracle list (shell/heredoc-expand, shell/read-options). * **Fix**: `cmd N>file` with N≥3 used to send the command's stdout into the file, so `( flock -n 9 && ... ) 9>lock` printed nothing. Such redirects now open fd N only: stdout is untouched, the file is created/truncated, and writes to `>&N` inside a compound command or function land in it. * **Fix**: A backslash escape right after a quoted segment broke lexing: `'a'\''b'` (the standard way to put a quote inside single quotes, as in `sh -c '...'\''...'\''...'`) failed with "unterminated single quote", and `"a"\$x`, `'a'\ b`, `"g"\*` kept the backslash. The quote-continuation reader now treats `\c` as a quoted `c` and `\` as a continuation. Still open: single quotes inside an unquoted `${x:-'d'}` operand are not removed. * **Fix**: `$SECONDS` counts whole seconds since the `Bash` instance was built (it was always 0), and `SECONDS=N` restarts the count from N; subshells keep their own. The rootfs gained `/proc/sys/kernel/random/uuid`, a fresh v4 UUID per read from the same CSPRNG as `uuidgen`. See [VFS](foundations/vfs.md).