clap_lex::OsStrExt is unsound

[blyxxyz]

use std::ffi::OsStr;

use clap_lex::OsStrExt;

fn main() {
    let s = OsStr::new("🦀");
    dbg!(s.split_at(1));
}

On Windows:

$ cargo run -q --target x86_64-pc-windows-gnu
[src/main.rs:7] s.split_at(1) = (
    "🦀[:]  = \n\0\0\0\0T\u{1a},thread 'main' panicked at 'index out of bounds: the len is 33 but the index is 33', library\core\src\unicode\unicode_data.rs:80:40

While Unix OsStr can contain any byte sequence, a Windows OsStr has to be WTF-8, which is UTF-8-ish and has to be decoded. If the data is split in the wrong place the decoder gets confused.

This use of transmute also isn't 100% sound, since OsStr isn't #[repr(transparent)] all the way down, and OsStr representation just hasn't been guaranteed in general. It works on past and current Rust versions so maybe it'll turn out OK in the end, but it wouldn't be unreasonable to e.g. add a "known valid" boolean field to Windows OsStr and break it. It makes me nervous.

So personally I'd revert #4794, or reimplement it using the existing safe APIs.

I don't think clap itself is currently broken due to this, though.

[epage]

Thanks

• I had missed that OsStr was not transparent all the way down. However, it effectively is and I suspect they will maintain that due to being a dynamically sized type
• Yes, I had forgotten there are invariants for OsStr and am documenting them and deprecating OsStrExt:;split_at