From 2b03a7158c344a821227141414efa5b534dd9813 Mon Sep 17 00:00:00 2001 From: Ofek Shaltiel Date: Sun, 28 Jun 2026 11:26:27 +0300 Subject: [PATCH 1/2] fix(bstr): reconstruct OsStr/Path losslessly from interned bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `BorrowedInterned::as_os_str()`/`as_path()` reconstructed the value via bstr's `to_os_str_lossy()`/`to_path_lossy()`. On non-unix platforms (e.g. Windows) that path falls back to a UTF-8-lossy conversion, so non-UTF-8 OS strings/paths came back corrupted with U+FFFD replacement characters — even though the store side (`Interned`'s `From<&OsStr>` / `From<&Path>` impls) preserves the exact `OsStr::as_encoded_bytes()`. Reconstruct losslessly via `OsStr::from_encoded_bytes_unchecked` on the stored bytes, with a SAFETY comment documenting that those bytes came from `as_encoded_bytes()` on the same platform/build (the function's documented precondition). Return types stay `Cow` (now always `Cow::Borrowed`) so the public API is unchanged. Kept bstr-gated. Adds a unix non-UTF-8 round-trip test asserting the raw bytes survive. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/bstr.rs | 16 ++++++++++++++-- src/tests.rs | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/bstr.rs b/src/bstr.rs index 7fcd92a..89c78fa 100644 --- a/src/bstr.rs +++ b/src/bstr.rs @@ -16,11 +16,23 @@ impl BorrowedInterned { } pub fn as_path(&self) -> Cow<'_, Path> { - self.as_bstr().to_path_lossy() + Cow::Borrowed(Path::new(self.as_os_str_ref())) } pub fn as_os_str(&self) -> Cow<'_, OsStr> { - self.as_bstr().to_os_str_lossy() + Cow::Borrowed(self.as_os_str_ref()) + } + + fn as_os_str_ref(&self) -> &OsStr { + // SAFETY: `self.deref()` returns the exact bytes that were stored when this value was + // interned, unchanged. When the value was interned from an `OsStr`/`OsString`/`Path`/ + // `PathBuf` (`Interned`'s `From<&OsStr>`/`From<&OsString>`/`From<&Path>`/... impls in + // `interned.rs`), those bytes were produced by `OsStr::as_encoded_bytes()` on this same + // platform and Rust build, which is exactly the documented precondition of + // `OsStr::from_encoded_bytes_unchecked` (a self-contained slice from `as_encoded_bytes()`, + // not split across an encoded boundary). This reconstructs the original `OsStr` losslessly, + // including non-UTF-8 contents, instead of the previous UTF-8-lossy conversion. + unsafe { OsStr::from_encoded_bytes_unchecked(self.deref()) } } pub fn as_str(&self) -> Cow<'_, str> { diff --git a/src/tests.rs b/src/tests.rs index 23578f6..e47e2d2 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -239,6 +239,27 @@ fn validate_data_hash() { assert_eq!(data_hash_1, data_hash_2); } +#[test] +#[serial] +#[cfg(all(feature = "bstr", unix))] +fn os_str_non_utf8_round_trip() { + use std::{ffi::OsStr, os::unix::ffi::OsStrExt, path::Path}; + + { + // Bytes that are not valid UTF-8 (a lone 0xff / 0xfe) must survive a round-trip + // through interning, rather than being mangled into U+FFFD. + let raw = b"/tmp/\xff\xfe/file"; + let os_str = OsStr::from_bytes(raw); + + let interned = Interned::from(os_str); + + assert_eq!(interned.as_os_str().as_bytes(), raw); + assert_eq!(interned.as_os_str(), os_str); + assert_eq!(interned.as_path(), Path::new(os_str)); + } + verify_empty(); +} + #[test] #[serial] #[cfg(feature = "serde")] From 09da790d2d887a3a8ef0467762386ec07d9977b4 Mon Sep 17 00:00:00 2001 From: Ofek Shaltiel Date: Sun, 28 Jun 2026 11:33:32 +0300 Subject: [PATCH 2/2] fix(bstr): make as_os_str/as_path sound by cfg-gating, drop unsafe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit reconstructed the value via `OsStr::from_encoded_bytes_unchecked(self.deref())`. That is unsound: `as_os_str()` is public and callable on any `Interned`, including one created from arbitrary bytes (e.g. `Interned::new(b"\xff\xff")`). On Windows `from_encoded_bytes_unchecked` requires valid WTF-8; arbitrary bytes are not, so the call is UB. There is no safe lossless reconstruction from arbitrary bytes in std on Windows. Fix without any `unsafe`, cfg-gated: - unix: `OsStrExt::from_bytes(self.deref())` — a unix `OsStr` is just bytes, so this is lossless AND safe; returns `Cow::Borrowed`. - non-unix (Windows/WASI): keep the existing safe lossy path (`as_bstr().to_os_str_lossy()`). - `as_path()` is now implemented in terms of `as_os_str()` (Borrowed -> `Path::new`, Owned -> `PathBuf::from`) so the two stay consistent. Both remain bstr-gated and keep their `Cow` return types. Document on both methods that the conversion is lossless + zero-copy on unix and best-effort lossy on non-unix, with a note that a lossless-on-Windows path could be added later as an explicit `unsafe` opt-in whose caller guarantees `OsStr` provenance. The `os_str_non_utf8_round_trip` test already carries `#[cfg(unix)]` and stays green. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/bstr.rs | 63 ++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 13 deletions(-) diff --git a/src/bstr.rs b/src/bstr.rs index 89c78fa..ca9e8b4 100644 --- a/src/bstr.rs +++ b/src/bstr.rs @@ -3,7 +3,7 @@ use std::{ ffi::OsStr, fmt::{Debug, Display, Formatter}, ops::Deref, - path::Path, + path::{Path, PathBuf}, }; use bstr::{BStr, BString, ByteSlice}; @@ -15,24 +15,61 @@ impl BorrowedInterned { BStr::new(self.deref()) } + /// Returns the interned bytes as a [`Path`]. + /// + /// This is implemented in terms of [`as_os_str`](Self::as_os_str), so it inherits the same + /// platform behavior: on unix it is a lossless, zero-copy borrow of the interned bytes; on + /// non-unix platforms (Windows/WASI) it is a best-effort lossy conversion. See + /// [`as_os_str`](Self::as_os_str) for details. pub fn as_path(&self) -> Cow<'_, Path> { - Cow::Borrowed(Path::new(self.as_os_str_ref())) + match self.as_os_str() { + Cow::Borrowed(os_str) => Cow::Borrowed(Path::new(os_str)), + Cow::Owned(os_string) => Cow::Owned(PathBuf::from(os_string)), + } } + /// Returns the interned bytes as an [`OsStr`]. + /// + /// On unix this is lossless and zero-copy: a unix [`OsStr`] is just bytes, so the interned + /// bytes are reconstructed exactly (including non-UTF-8 contents) via + /// [`OsStrExt::from_bytes`](std::os::unix::ffi::OsStrExt::from_bytes) and returned as a + /// [`Cow::Borrowed`]. + /// + /// On non-unix platforms (Windows/WASI) the result is a best-effort *lossy* conversion: + /// invalid sequences are replaced with the U+FFFD replacement character. The standard library + /// has no *safe* lossless reconstruction of an [`OsStr`] from arbitrary bytes on those + /// platforms — the only lossless option, `OsStr::from_encoded_bytes_unchecked`, is `unsafe` and + /// would be unsound here because an [`Interned`] may have been created from arbitrary bytes + /// (e.g. `Interned::new(b"\xff\xff")`) that are not valid WTF-8. + /// + /// A lossless-on-Windows path could be offered later as an explicit `unsafe` opt-in method + /// whose caller guarantees the interned bytes have [`OsStr`] provenance. + #[cfg(unix)] pub fn as_os_str(&self) -> Cow<'_, OsStr> { - Cow::Borrowed(self.as_os_str_ref()) + use std::os::unix::ffi::OsStrExt; + + Cow::Borrowed(OsStr::from_bytes(self.deref())) } - fn as_os_str_ref(&self) -> &OsStr { - // SAFETY: `self.deref()` returns the exact bytes that were stored when this value was - // interned, unchanged. When the value was interned from an `OsStr`/`OsString`/`Path`/ - // `PathBuf` (`Interned`'s `From<&OsStr>`/`From<&OsString>`/`From<&Path>`/... impls in - // `interned.rs`), those bytes were produced by `OsStr::as_encoded_bytes()` on this same - // platform and Rust build, which is exactly the documented precondition of - // `OsStr::from_encoded_bytes_unchecked` (a self-contained slice from `as_encoded_bytes()`, - // not split across an encoded boundary). This reconstructs the original `OsStr` losslessly, - // including non-UTF-8 contents, instead of the previous UTF-8-lossy conversion. - unsafe { OsStr::from_encoded_bytes_unchecked(self.deref()) } + /// Returns the interned bytes as an [`OsStr`]. + /// + /// On unix this is lossless and zero-copy: a unix [`OsStr`] is just bytes, so the interned + /// bytes are reconstructed exactly (including non-UTF-8 contents) via + /// [`OsStrExt::from_bytes`](std::os::unix::ffi::OsStrExt::from_bytes) and returned as a + /// [`Cow::Borrowed`]. + /// + /// On non-unix platforms (Windows/WASI) the result is a best-effort *lossy* conversion: + /// invalid sequences are replaced with the U+FFFD replacement character. The standard library + /// has no *safe* lossless reconstruction of an [`OsStr`] from arbitrary bytes on those + /// platforms — the only lossless option, `OsStr::from_encoded_bytes_unchecked`, is `unsafe` and + /// would be unsound here because an [`Interned`] may have been created from arbitrary bytes + /// (e.g. `Interned::new(b"\xff\xff")`) that are not valid WTF-8. + /// + /// A lossless-on-Windows path could be offered later as an explicit `unsafe` opt-in method + /// whose caller guarantees the interned bytes have [`OsStr`] provenance. + #[cfg(not(unix))] + pub fn as_os_str(&self) -> Cow<'_, OsStr> { + self.as_bstr().to_os_str_lossy() } pub fn as_str(&self) -> Cow<'_, str> {