fix(shell): decode UTF-8 in the read builtin (Fixes #6543)

crates/brush-builtins-vendored/src/lib.rs

@@ -60,6 +60,8 @@ mod jobs;
 mod kill;
 #[cfg(feature = "builtin.let")]
 mod let_;
+#[cfg(any(feature = "builtin.mapfile", feature = "builtin.read"))]
+mod line_input;
 #[cfg(feature = "builtin.mapfile")]
 mod mapfile;
 #[cfg(feature = "builtin.popd")]

crates/brush-builtins-vendored/src/line_input.rs

@@ -0,0 +1,233 @@
+// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/line_input.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, a failed refill leaves no read-ahead to give back, and read can take back the bytes past its -n/-N limit.
+//! Byte input for the line-reading builtins (`read`, `mapfile`).
+
+use std::io::{self, Read, Seek, SeekFrom};
+
+use brush_core::openfiles::OpenFile;
+
+/// Bytes per read-ahead.
+const BLOCK: usize = 4096;
+
+/// ASCII control character for Ctrl+C (ETX - End of Text).
+pub(crate) const CTRL_C: u8 = 0x03;
+/// ASCII control character for Ctrl+D (EOT - End of Transmission).
+pub(crate) const CTRL_D: u8 = 0x04;
+
+/// The bytes of a read line as text: UTF-8 decoded once per line, invalid
+/// sequences replaced rather than each byte read as a Latin-1 character.
+pub(crate) fn decode_line(line: Vec<u8>) -> String {
+	String::from_utf8(line).unwrap_or_else(|error| String::from_utf8_lossy(error.as_bytes()).into_owned())
+}
+
+/// When a [`LineInput`] may read past the bytes it hands out.
+#[derive(Clone, Copy)]
+pub(crate) enum ReadAhead {
+	/// The caller consumes everything up to EOF, so reading ahead can never
+	/// take input meant for someone else.
+	ToEof,
+	/// The caller may stop mid-input: read ahead only from a regular file,
+	/// whose offset can be given back.
+	Seekable,
+}
+
+/// A shell descriptor read one byte at a time by its caller.
+///
+/// Where reading ahead is safe, the descriptor is read a block at a time.
+/// On drop, a regular file's shared offset is seeked back over the bytes read
+/// but not handed out, so the next reader of the descriptor resumes right
+/// after the last one consumed (bash's `read` does the same). Elsewhere every
+/// byte is its own read, so nothing past the last consumed byte is taken from
+/// a pipe or terminal another command shares.
+pub(crate) struct LineInput {
+	input:  OpenFile,
+	blocks: bool,
+	block:  Vec<u8>,
+	pos:    usize,
+}
+
+impl LineInput {
+	/// Terminal input is always read byte by byte, so interactive keys
+	/// arrive as typed. Only a plain file handle counts as a regular file:
+	/// std's `Stdin` keeps a buffer of its own, which a seek on the
+	/// descriptor could not give back.
+	pub(crate) fn new(input: OpenFile, read_ahead: ReadAhead) -> Self {
+		let blocks = !input.is_terminal()
+			&& match read_ahead {
+				ReadAhead::ToEof => true,
+				ReadAhead::Seekable => {
+					matches!(&input, OpenFile::File(file) if file.metadata().is_ok_and(|m| m.is_file()))
+				},
+			};
+		Self { input, blocks, block: Vec::new(), pos: 0 }
+	}
+
+	/// The underlying descriptor, e.g. to poll it before a read that could
+	/// block.
+	pub(crate) const fn input(&self) -> &OpenFile {
+		&self.input
+	}
+
+	/// Whether reads come from a read-ahead block, never waiting on input.
+	pub(crate) const fn reads_ahead(&self) -> bool {
+		self.blocks
+	}
+
+	/// The next byte, or `None` at end of input.
+	pub(crate) fn next_byte(&mut self) -> io::Result<Option<u8>> {
+		if !self.blocks {
+			let mut byte = [0_u8; 1];
+			return Ok((self.input.read(&mut byte)? == 1).then_some(byte[0]));
+		}
+		if self.pos == self.block.len() {
+			self.block.resize(BLOCK, 0);
+			self.pos = 0;
+			let n = match self.input.read(&mut self.block) {
+				Ok(n) => n,
+				Err(error) => {
+					// Nothing was read into the block, and everything before it
+					// was handed out; leaving it at full length would make
+					// `give_back` seek back over bytes already consumed.
+					self.block.clear();
+					return Err(error);
+				},
+			};
+			self.block.truncate(n);
+			if n == 0 {
+				return Ok(None);
+			}
+		}
+		self.pos += 1;
+		Ok(Some(self.block[self.pos - 1]))
+	}
+
+	/// Takes back the last `n` bytes handed out, so the next reader of a
+	/// regular file gets them. Only read-ahead from a regular file can do
+	/// this; a byte read from a pipe or terminal is gone, so this returns
+	/// `false` and changes nothing.
+	pub(crate) fn unread(&mut self, n: usize) -> bool {
+		if !self.blocks || !matches!(self.input, OpenFile::File(_)) {
+			return false;
+		}
+		if let Some(pos) = self.pos.checked_sub(n) {
+			self.pos = pos;
+			return true;
+		}
+		// Some of the bytes came from the previous block: seek back over this
+		// block and those bytes, and start over from there.
+		let Ok(back) = i64::try_from(self.block.len() - self.pos + n) else {
+			return false;
+		};
+		if let OpenFile::File(file) = &mut self.input
+			&& file.seek(SeekFrom::Current(-back)).is_ok()
+		{
+			self.block.clear();
+			self.pos = 0;
+			return true;
+		}
+		false
+	}
+
+	/// Gives the bytes read ahead but not handed out back to a regular
+	/// file's shared offset, so another reader of the descriptor resumes
+	/// right after the last byte consumed. Bash's `mapfile` does the same
+	/// before each `-C` callback. Read-ahead that cannot be given back is
+	/// kept and handed out as usual.
+	pub(crate) fn give_back(&mut self) {
+		let unread = self.block.len() - self.pos;
+		if unread > 0
+			&& let OpenFile::File(file) = &mut self.input
+			&& file.seek(SeekFrom::Current(-(unread as i64))).is_ok()
+		{
+			self.block.clear();
+			self.pos = 0;
+		}
+	}
+}
+
+impl Drop for LineInput {
+	fn drop(&mut self) {
+		self.give_back();
+	}
+}
+
+#[cfg(test)]
+#[expect(clippy::panic_in_result_fn)]
+mod tests {
+	use std::io::{Seek, Write};
+
+	use anyhow::Result;
+
+	use super::*;
+
+	/// Regression: a refill that fails must not leave the block at full
+	/// length, or dropping the input seeks the shared offset back over bytes
+	/// that were never read (here, over bytes just written).
+	#[test]
+	fn failed_refill_gives_nothing_back() -> Result<()> {
+		let path = std::env::temp_dir()
+			.join(format!("brush-line-input-failed-refill-{}.txt", std::process::id()));
+		// Write-only, so every read fails.
+		let mut file = std::fs::OpenOptions::new()
+			.write(true)
+			.create(true)
+			.truncate(true)
+			.open(&path)?;
+		file.write_all(&[b'a'; 5000])?;
+		let mut offset = file.try_clone()?;
+
+		let mut input = LineInput::new(OpenFile::File(file), ReadAhead::Seekable);
+		assert!(input.reads_ahead());
+		assert!(input.next_byte().is_err());
+		drop(input);
+
+		let position = offset.stream_position()?;
+		std::fs::remove_file(&path)?;
+		assert_eq!(position, 5000);
+		Ok(())
+	}
+
+	/// Contract: bytes taken back from a regular file go to the next reader
+	/// of the descriptor, also when they came from the previous block.
+	#[test]
+	fn unread_bytes_go_back_to_the_file() -> Result<()> {
+		let path = std::env::temp_dir()
+			.join(format!("brush-line-input-unread-{}.txt", std::process::id()));
+		std::fs::write(&path, vec![b'a'; BLOCK + 10])?;
+		let file = std::fs::File::open(&path)?;
+		let mut offset = file.try_clone()?;
+
+		let mut input = LineInput::new(OpenFile::File(file), ReadAhead::Seekable);
+		for _ in 0..5 {
+			input.next_byte()?;
+		}
+		assert!(input.unread(2));
+		drop(input);
+		let within_block = offset.stream_position()?;
+
+		offset.seek(SeekFrom::Start(0))?;
+		let mut input = LineInput::new(OpenFile::File(offset.try_clone()?), ReadAhead::Seekable);
+		for _ in 0..=BLOCK {
+			input.next_byte()?;
+		}
+		assert!(input.unread(2));
+		drop(input);
+		let across_blocks = offset.stream_position()?;
+
+		std::fs::remove_file(&path)?;
+		assert_eq!(within_block, 3);
+		assert_eq!(across_blocks, u64::try_from(BLOCK)? - 1);
+		Ok(())
+	}
+
+	/// Contract: a pipe byte cannot be taken back.
+	#[test]
+	fn unread_from_a_pipe_does_nothing() -> Result<()> {
+		let (reader, mut writer) = std::io::pipe()?;
+		writer.write_all(b"ab")?;
+		let mut input = LineInput::new(OpenFile::PipeReader(reader), ReadAhead::Seekable);
+		assert_eq!(input.next_byte()?, Some(b'a'));
+		assert!(!input.unread(1));
+		assert_eq!(input.next_byte()?, Some(b'b'));
+		Ok(())
+	}
+}

crates/brush-builtins-vendored/src/mapfile.rs

@@ -1,9 +1,11 @@
-// Vendored from can1357/oh-my-pi@a85bd5228d9f0f619deade1db78fa49420a721e1:crates/pi-builtins/src/mapfile.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, retained local array validation and synchronous file descriptor reads.
-use std::io::{Read, Write};
+// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/mapfile.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, retained local array validation and synchronous file descriptor reads.
+use std::io::Write;
 
 use brush_core::{ErrorKind, ExecutionExitCode, ExecutionResult, builtins, env, escape, variables};
 use clap::Parser;
 
+use crate::line_input::{CTRL_C, CTRL_D, LineInput, ReadAhead, decode_line};
+
 /// Read lines from standard input into an indexed array variable.
 #[derive(Parser)]
 pub(crate) struct MapFileCommand {
@@ -59,6 +61,12 @@ impl builtins::Command for MapFileCommand {
 		}
 
 		if let Some((_, var)) = context.shell.env().get(&self.array_var_name) {
+			// Refused before any input is read, as in bash. Assigning an element
+			// does not check this itself (so -O wrote into a readonly array), and
+			// a failure mid-loop would lose input read ahead of a pipe.
+			if var.is_readonly() {
+				return Err(ErrorKind::ReadonlyVariable.into());
+			}
 			if matches!(
 				var.value(),
 				variables::ShellValue::AssociativeArray(_)
@@ -100,7 +108,7 @@ impl builtins::Command for MapFileCommand {
 impl MapFileCommand {
 	async fn read_entries<SE: brush_core::ShellExtensions>(
 		&self,
-		mut input_file: brush_core::openfiles::OpenFile,
+		input_file: brush_core::openfiles::OpenFile,
 		context: &mut brush_core::ExecutionContext<'_, SE>,
 	) -> Result<Option<ExecutionResult>, brush_core::Error> {
 		let _term_mode = setup_terminal_settings(&input_file)?;
@@ -115,27 +123,30 @@ impl MapFileCommand {
 			None => b'\n',
 		};
 
-		let mut buf = [0u8; 1];
+		let terminal = input_file.is_terminal();
+		// Without -n or -C every byte up to EOF is consumed before anything
+		// else reads the descriptor, so any input is read in blocks. -n may
+		// stop early, and a -C callback may read the descriptor itself or
+		// end the loop; then only a regular file, whose offset is given
+		// back, is (bash's mapfile reads a pipe unbuffered too).
+		let read_ahead =
+			if max_count == 0 && self.callback.is_none() { ReadAhead::ToEof } else { ReadAhead::Seekable };
+		let mut input = LineInput::new(input_file, read_ahead);
 
 		while max_count == 0 || entry_count < max_count {
 			let mut line = vec![];
 			let mut saw_delimiter = false;
 
-			loop {
-				match input_file.read(&mut buf) {
-					Ok(0) => break,                                         // End of input
-					Ok(1) if buf[0] == b'\x03' => break,                    // Ctrl+C
-					Ok(1) if buf[0] == b'\x04' && line.is_empty() => break, // Ctrl+D
-					Ok(1) => {
-						let byte = buf[0];
-						line.push(byte);
-						if byte == delimiter {
-							saw_delimiter = true;
-							break;
-						}
-					},
-					Ok(_) => unreachable!("input can only be 0, 1, or error"),
-					Err(e) => return Err(e.into()),
+			while let Some(byte) = input.next_byte()? {
+				// Interactive keys end input only on a terminal; in a file or
+				// pipe they are data.
+				if terminal && (byte == CTRL_C || byte == CTRL_D && line.is_empty()) {
+					break;
+				}
+				line.push(byte);
+				if byte == delimiter {
+					saw_delimiter = true;
+					break;
 				}
 			}
 
@@ -152,12 +163,13 @@ impl MapFileCommand {
 				line.pop();
 			}
 
-			let line_str = String::from_utf8_lossy(&line).to_string();
+			let line_str = decode_line(line);
 			let array_index = self.origin.unwrap_or(0) + i64::try_from(entry_count)?;
 
 			if let Some(callback) = &self.callback
 				&& (entry_count + 1).is_multiple_of(callback_group_size)
 			{
+				input.give_back();
 				let result = run_callback(callback, array_index, &line_str, context).await?;
 				if !result.is_normal_flow() {
 					return Ok(Some(result));
@@ -204,6 +216,11 @@ async fn run_callback<SE: brush_core::ShellExtensions>(
 fn setup_terminal_settings(
 	file: &brush_core::openfiles::OpenFile,
 ) -> Result<Option<brush_core::terminal::AutoModeGuard>, brush_core::Error> {
+	// The guard dups the descriptor and reads its terminal attributes; a
+	// pipe or file has none.
+	if !file.is_terminal() {
+		return Ok(None);
+	}
 	let mode = brush_core::terminal::AutoModeGuard::new(file.to_owned()).ok();
 	if let Some(mode) = &mode {
 		let config = brush_core::terminal::Settings::builder()