Skip to content

feat: use bstr::BString for shell values to fix UTF-8 lossy conversion - #1116

Open
lu-zero wants to merge 6 commits into
reubeno:mainfrom
lu-zero:utf8-vs-bytes
Open

feat: use bstr::BString for shell values to fix UTF-8 lossy conversion#1116
lu-zero wants to merge 6 commits into
reubeno:mainfrom
lu-zero:utf8-vs-bytes

Conversation

@lu-zero

@lu-zero lu-zero commented May 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Convert ShellValue, ShellValueLiteral, and ArrayLiteral from String to bstr::BString so arbitrary bytes are preserved throughout the shell variable system
  • Fix silent data corruption in echo -e, mapfile, ANSI-C quoting ($'...'), ${var@E}, PWD/OLDPWD, and From<OsString> for ShellValue

Closes #1115

Details

Bash treats all data (filenames, variable values, command output) as raw byte sequences. Rust's String requires valid UTF-8, so every from_utf8_lossy or to_string_lossy call was a potential silent data corruption point.

The core change replaces String with bstr::BString (backed by Vec<u8>) in ShellValue::String, AssociativeArray, and IndexedArray, plus all related types. Key fixes:

  • echo -e '\x80': now writes raw bytes to stdout instead of lossy-converting
  • mapfile: stores raw bytes as BString
  • ANSI-C quoting ($'\x80'): preserves non-UTF-8 bytes
  • ${var@E}: same as above
  • PWD/OLDPWD: uses OsStrExt::as_bytes() on Unix to preserve raw path bytes
  • From<OsString>: uses into_vec() on Unix instead of to_string_lossy()

24 files changed, all tests passing (49 unit + 224 parser + 25 builtins + 1696 compat).

Co-Authored-by: opencode opencode@anomaly.co
Assisted-by: glm-5.1

@github-actions

github-actions Bot commented May 1, 2026

Copy link
Copy Markdown

Public API changes for crate: brush-core

Added items

+pub use brush_core::BString
+impl core::convert::From<alloc::vec::Vec<bstr::bstring::BString>> for brush_core::variables::ShellValue
+impl core::convert::From<alloc::vec::Vec<bstr::bstring::BString>> for brush_core::variables::ShellValue
+pub fn brush_core::variables::ShellValue::from(alloc::vec::Vec<bstr::bstring::BString>) -> Self
+pub fn brush_core::variables::ShellValue::from(alloc::vec::Vec<bstr::bstring::BString>) -> Self
+impl core::convert::From<bstr::bstring::BString> for brush_core::variables::ShellValue
+impl core::convert::From<bstr::bstring::BString> for brush_core::variables::ShellValue
+pub fn brush_core::variables::ShellValue::from(bstr::bstring::BString) -> Self
+pub fn brush_core::variables::ShellValue::from(bstr::bstring::BString) -> Self
+pub fn brush_core::os_string_to_bstring(std::ffi::os_str::OsString) -> bstr::bstring::BString
+pub fn brush_core::path_to_bstring(impl core::convert::AsRef<std::path::Path>) -> bstr::bstring::BString

Changed items

-pub fn brush_core::env::ShellEnvironment::update_or_add_array_element<N: core::convert::Into<alloc::string::String>>(&mut self, N, alloc::string::String, alloc::string::String, impl core::ops::function::Fn(&mut brush_core::variables::ShellVariable) -> core::result::Result<(), brush_core::error::Error>, brush_core::env::EnvironmentLookup, brush_core::env::EnvironmentScope) -> core::result::Result<(), brush_core::error::Error>
+pub fn brush_core::env::ShellEnvironment::update_or_add_array_element<N: core::convert::Into<alloc::string::String>, I: core::convert::Into<bstr::bstring::BString>, V: core::convert::Into<bstr::bstring::BString>>(&mut self, N, I, V, impl core::ops::function::Fn(&mut brush_core::variables::ShellVariable) -> core::result::Result<(), brush_core::error::Error>, brush_core::env::EnvironmentLookup, brush_core::env::EnvironmentScope) -> core::result::Result<(), brush_core::error::Error>
-pub brush_core::variables::ShellValue::AssociativeArray(alloc::collections::btree::map::BTreeMap<alloc::string::String, alloc::string::String>)
+pub brush_core::variables::ShellValue::AssociativeArray(alloc::collections::btree::map::BTreeMap<bstr::bstring::BString, bstr::bstring::BString>)
-pub brush_core::variables::ShellValue::IndexedArray(alloc::collections::btree::map::BTreeMap<u64, alloc::string::String>)
+pub brush_core::variables::ShellValue::IndexedArray(alloc::collections::btree::map::BTreeMap<u64, bstr::bstring::BString>)
-pub brush_core::variables::ShellValue::String(alloc::string::String)
+pub brush_core::variables::ShellValue::String(bstr::bstring::BString)
-pub fn brush_core::variables::ShellValue::element_keys(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> alloc::vec::Vec<alloc::string::String>
+pub fn brush_core::variables::ShellValue::element_keys(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> alloc::vec::Vec<bstr::bstring::BString>
-pub fn brush_core::variables::ShellValue::element_keys(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> alloc::vec::Vec<alloc::string::String>
+pub fn brush_core::variables::ShellValue::element_keys(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> alloc::vec::Vec<bstr::bstring::BString>
-pub fn brush_core::variables::ShellValue::element_values(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> alloc::vec::Vec<alloc::string::String>
+pub fn brush_core::variables::ShellValue::element_values(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> alloc::vec::Vec<bstr::bstring::BString>
-pub fn brush_core::variables::ShellValue::element_values(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> alloc::vec::Vec<alloc::string::String>
+pub fn brush_core::variables::ShellValue::element_values(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> alloc::vec::Vec<bstr::bstring::BString>
-pub fn brush_core::variables::ShellValue::get_at(&self, &str, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> core::result::Result<core::option::Option<alloc::borrow::Cow<'_, str>>, brush_core::error::Error>
+pub fn brush_core::variables::ShellValue::get_at(&self, &str, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> core::result::Result<core::option::Option<alloc::borrow::Cow<'_, bstr::bstr::BStr>>, brush_core::error::Error>
-pub fn brush_core::variables::ShellValue::get_at(&self, &str, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> core::result::Result<core::option::Option<alloc::borrow::Cow<'_, str>>, brush_core::error::Error>
+pub fn brush_core::variables::ShellValue::get_at(&self, &str, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> core::result::Result<core::option::Option<alloc::borrow::Cow<'_, bstr::bstr::BStr>>, brush_core::error::Error>
-pub fn brush_core::variables::ShellValue::indexed_array_from_strings<S>(S) -> Self where S: core::iter::traits::collect::IntoIterator<Item = alloc::string::String>
+pub fn brush_core::variables::ShellValue::indexed_array_from_strings<S>(S) -> Self where S: core::iter::traits::collect::IntoIterator<Item = bstr::bstring::BString>
-pub fn brush_core::variables::ShellValue::indexed_array_from_strings<S>(S) -> Self where S: core::iter::traits::collect::IntoIterator<Item = alloc::string::String>
+pub fn brush_core::variables::ShellValue::indexed_array_from_strings<S>(S) -> Self where S: core::iter::traits::collect::IntoIterator<Item = bstr::bstring::BString>
-pub fn brush_core::variables::ShellValue::to_cow_str(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> alloc::borrow::Cow<'_, str>
+pub fn brush_core::variables::ShellValue::to_cow_str(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> alloc::borrow::Cow<'_, bstr::bstr::BStr>
-pub fn brush_core::variables::ShellValue::to_cow_str(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> alloc::borrow::Cow<'_, str>
+pub fn brush_core::variables::ShellValue::to_cow_str(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> alloc::borrow::Cow<'_, bstr::bstr::BStr>
-pub fn brush_core::variables::ShellValue::try_get_cow_str(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> core::option::Option<alloc::borrow::Cow<'_, str>>
+pub fn brush_core::variables::ShellValue::try_get_cow_str(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> core::option::Option<alloc::borrow::Cow<'_, bstr::bstr::BStr>>
-pub fn brush_core::variables::ShellValue::try_get_cow_str(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> core::option::Option<alloc::borrow::Cow<'_, str>>
+pub fn brush_core::variables::ShellValue::try_get_cow_str(&self, &brush_core::Shell<impl brush_core::extensions::ShellExtensions>) -> core::option::Option<alloc::borrow::Cow<'_, bstr::bstr::BStr>>
-pub brush_core::variables::ShellValueLiteral::Scalar(alloc::string::String)
+pub brush_core::variables::ShellValueLiteral::Scalar(bstr::bstring::BString)
-pub struct brush_core::variables::ArrayLiteral(pub alloc::vec::Vec<(core::option::Option<alloc::string::String>, alloc::string::String)>)
+pub struct brush_core::variables::ArrayLiteral(pub alloc::vec::Vec<(core::option::Option<bstr::bstring::BString>, bstr::bstring::BString)>)
-pub fn brush_core::variables::ShellVariable::assign_at_index(&mut self, alloc::string::String, alloc::string::String, bool) -> core::result::Result<(), brush_core::error::Error>
+pub fn brush_core::variables::ShellVariable::assign_at_index(&mut self, bstr::bstring::BString, bstr::bstring::BString, bool) -> core::result::Result<(), brush_core::error::Error>
-pub fn brush_core::variables::ShellVariable::assign_at_index(&mut self, alloc::string::String, alloc::string::String, bool) -> core::result::Result<(), brush_core::error::Error>
+pub fn brush_core::variables::ShellVariable::assign_at_index(&mut self, bstr::bstring::BString, bstr::bstring::BString, bool) -> core::result::Result<(), brush_core::error::Error>
-pub brush_core::ShellValue::AssociativeArray(alloc::collections::btree::map::BTreeMap<alloc::string::String, alloc::string::String>)
+pub brush_core::ShellValue::AssociativeArray(alloc::collections::btree::map::BTreeMap<bstr::bstring::BString, bstr::bstring::BString>)
-pub brush_core::ShellValue::IndexedArray(alloc::collections::btree::map::BTreeMap<u64, alloc::string::String>)
+pub brush_core::ShellValue::IndexedArray(alloc::collections::btree::map::BTreeMap<u64, bstr::bstring::BString>)
-pub brush_core::ShellValue::String(alloc::string::String)
+pub brush_core::ShellValue::String(bstr::bstring::BString)

Performance Benchmark Report

Benchmark name Baseline (μs) Test/PR (μs) Delta (μs) Delta %
clone_shell_object 17.61 μs 17.76 μs 0.15 μs 🟠 +0.85%
eval_arithmetic 0.15 μs 0.15 μs 0.00 μs ⚪ Unchanged
expand_one_string 1.87 μs 2.01 μs 0.14 μs ⚪ Unchanged
for_loop 31.89 μs 34.41 μs 2.53 μs 🟠 +7.93%
full_peg_complex 57.29 μs 57.46 μs 0.18 μs ⚪ Unchanged
full_peg_for_loop 6.21 μs 6.23 μs 0.02 μs ⚪ Unchanged
full_peg_nested_expansions 16.23 μs 16.22 μs -0.01 μs ⚪ Unchanged
full_peg_pipeline 4.18 μs 4.15 μs -0.03 μs ⚪ Unchanged
full_peg_simple 1.76 μs 1.75 μs -0.02 μs ⚪ Unchanged
function_call 4.30 μs 4.35 μs 0.05 μs ⚪ Unchanged
instantiate_shell 54.86 μs 54.71 μs -0.15 μs ⚪ Unchanged
instantiate_shell_with_init_scripts 29373.39 μs 30458.37 μs 1084.98 μs ⚪ Unchanged
parse_peg_bash_completion 2208.44 μs 2137.30 μs -71.14 μs ⚪ Unchanged
parse_peg_complex 20.36 μs 20.35 μs -0.01 μs ⚪ Unchanged
parse_peg_for_loop 1.98 μs 2.03 μs 0.05 μs 🟠 +2.62%
parse_peg_pipeline 2.04 μs 2.06 μs 0.02 μs 🟠 +1.13%
parse_peg_simple 1.07 μs 1.07 μs 0.00 μs ⚪ Unchanged
run_echo_builtin_command 16.77 μs 17.96 μs 1.19 μs ⚪ Unchanged
tokenize_sample_script 3.34 μs 3.39 μs 0.05 μs ⚪ Unchanged

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
brush-builtins/src/declare.rs 🟢 87.4% 🟢 87.43% 🟢 0.03%
brush-builtins/src/export.rs 🟠 75% 🟢 75.53% 🟢 0.53%
brush-builtins/src/read.rs 🟢 91.38% 🟢 91.44% 🟢 0.06%
brush-core/src/arithmetic.rs 🟢 98.31% 🟢 98.32% 🟢 0.01%
brush-core/src/commands.rs 🟢 91.59% 🟢 91.61% 🟢 0.02%
brush-core/src/env.rs 🟢 87.16% 🟢 87.46% 🟢 0.3%
brush-core/src/expansion.rs 🟢 96.11% 🟢 95.81% 🔴 -0.3%
brush-core/src/interp.rs 🟢 93.02% 🟢 93.03% 🟢 0.01%
brush-core/src/lib.rs 🔴 0% 🟢 100% 🟢 100%
brush-core/src/patterns.rs 🟢 97.63% 🟢 94.14% 🔴 -3.49%
brush-core/src/regex.rs 🟢 99.12% 🟢 95.83% 🔴 -3.29%
brush-core/src/results.rs 🟢 80.16% 🟢 83.33% 🟢 3.17%
brush-core/src/shell/io.rs 🟢 82.98% 🟢 83.33% 🟢 0.35%
brush-core/src/shell/readline.rs 🟠 61.9% 🔴 46.43% 🔴 -15.47%
brush-core/src/variables.rs 🟢 93.28% 🟢 93.21% 🔴 -0.07%
brush-interactive/src/interactive_shell.rs 🟠 63.83% 🟠 63.03% 🔴 -0.8%
Overall Coverage 🟢 76.12% 🟢 76.09% 🔴 -0.03%

Minimum allowed coverage is 70%, this run produced 76.09%
Maximum allowed coverage difference is -5%, this run produced -0.03%

Test Summary: bash-completion test suite

Outcome Count Percentage
✅ Pass 1582 75.01
❗️ Error 17 0.81
❌ Fail 156 7.40
⏩ Skip 339 16.07
❎ Expected Fail 13 0.62
✔️ Unexpected Pass 2 0.09
📊 Total 2109 100.00

Comment thread brush-builtins/src/echo.rs Outdated
let mut s;
if self.interpret_backslash_escapes {
s = String::new();
s = Vec::new();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
s = Vec::new();
s = Vec::with_capacity(self.args.len());

If you do this you can avoid a couple reallocations.

lu-zero and others added 5 commits July 29, 2026 07:30
0.19.0 adds unconditional bytes support via BytesMode and the
RegexInput trait, which supersedes the fork dependency that was
previously required for byte-level matching.

Also fix the one breaking change: Captures is now generic over the
input type, so name it as Captures<'_, str> in the replace_all
closure.

Assisted-by: opencode:glm-5.2
Convert ShellValue, ShellValueLiteral, and ArrayLiteral from String to
bstr::BString so arbitrary bytes are preserved throughout the shell
variable system. This fixes silent data corruption in:

- PWD/OLDPWD/HISTFILE for directories with non-UTF-8 paths (Unix)
- mapfile reading non-UTF-8 input
- From<OsString> for ShellValue (uses into_vec() on Unix)
- pathcache entries (uses path_to_bstring on Unix)

Added crate-level helpers os_string_to_bstring() and path_to_bstring()
that preserve raw bytes on Unix via OsStrExt, with lossy fallback on
other platforms. env::get_str() and the expansion pipeline retain
String/str at their boundaries (lossy for interpretation/display only);
storage is now fully lossless.

Closes: reubeno#1115
Co-Authored-by: opencode <opencode@anomaly.co>
Assisted-by: opencode:glm-5.2
Enable BytesMode::UnicodeBytes in regex compilation so compiled regexes
can match against both &str and &[u8] input. Update glob pathname
expansion to match against raw filename bytes via OsStrExt::as_bytes()
on Unix, eliminating the to_string_lossy() conversion that previously
corrupted non-UTF-8 filenames.

For valid UTF-8 input the behavior is unchanged (UnicodeBytes mode
preserves Unicode character class semantics while allowing non-UTF-8
byte input).

Assisted-by: opencode:glm-5.2
Convert ExpansionPiece (Unsplittable/Splittable) and WordField from
String to bstr::BString so that non-UTF-8 bytes flow through the
expansion pipeline without lossy conversion. This completes the
lossless data path from variable storage through ${var} expansion.

Key changes:
- ExpansionPiece variants now hold BString instead of String
- Added inherent is_empty(), len(), chars() methods on ExpansionPiece
  using bstr's ByteSlice trait
- Added bstring_to_string() helper for lossless-then-lossy conversion
  at the String boundary (From<ExpansionPiece> for String/PatternPiece)
- polymorphic_subslice uses collect::<String>().into() for char slicing
- ANSI-C quoting ($'...') now preserves raw expanded bytes via
  BString::from(expanded) instead of String::from_utf8_lossy
- element_keys()/element_values() flow directly as BString into
  ExpansionPiece without .to_string()

Assisted-by: opencode:glm-5.2
Convert PatternPiece (Pattern/Literal) and RegexPiece (Pattern/Literal)
from String to bstr::BString so pattern and regex pieces preserve
non-UTF-8 bytes losslessly from the expansion pipeline.

Key changes:
- PatternPiece and RegexPiece variants now hold BString
- From<ExpansionPiece> for PatternPiece/RegexPiece is now lossless
  (BString passed through directly, no bstring_to_string conversion)
- escape_literal_regex_piece now accepts &[u8] and escapes non-UTF-8
  bytes as \xHH regex hex escapes (e.g. byte 0x80 becomes \x80 in the
  regex string). This works correctly with BytesMode::UnicodeBytes.
- PatternPiece::as_str() uses to_str() with unwrap_or fallback
- to_regex_str() in patterns.rs lossy-converts BString pieces to &str
  for the glob-to-regex converter (known limitation: the glob converter
  requires valid UTF-8 input)

This closes the last lossy boundary in the regex matching path. The
only remaining lossy conversion is PatternPiece literal bytes flowing
through the glob converter, and env::get_str() for arithmetic (which
is moot since non-numeric bytes yield 0 regardless).

Assisted-by: opencode:glm-5.2
@lu-zero
lu-zero force-pushed the utf8-vs-bytes branch 2 times, most recently from 4a426bf to d2c8229 Compare July 29, 2026 14:44
The glob-to-regex converter (brush_parser::pattern::pattern_to_regex_str)
requires valid UTF-8 input, which was the last lossy boundary for
PatternPiece literal bytes. Fix it by encoding non-UTF-8 bytes as Private
Use Area characters (U+E000 + byte value) before the converter — these
are valid Unicode but not glob metacharacters, so they pass through
unchanged. After conversion, replace_pua_with_hex_escapes() converts
them to \xHH regex hex escapes that match the original raw bytes under
BytesMode::UnicodeBytes.

This closes the last lossy boundary in pattern matching. The only
remaining lossy conversion is env::get_str() for arithmetic, which is
moot (non-numeric bytes yield 0 in both bash and brush).

Assisted-by: opencode:glm-5.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lossy utf8 conversion usage

2 participants