evdev-rs icon indicating copy to clipboard operation
evdev-rs copied to clipboard

&OsStr vs &str

Open maxbla opened this issue 4 years ago • 1 comments

Some methods on DeviceWrapper should have their signatures changed to use OsStr instead of str. Rust's &str must contain valid UTF-8, but Linux filenames are not necessarily valid UTF-8, rather any byte sequence that doesn't include '/' or '\0'. For example they may contain bytes with value 128-255.

use std::os::unix::ffi::OsStrExt;

fn main() {
    let invalid_utf8:[u8;1] = (128_u8).to_ne_bytes();
    let os_str = std::ffi::OsStr::from_bytes(&invalid_utf8);
    // assert os_str contains invalid UTF-8
    assert!(os_str.to_str().is_none());
    // but you can still make a path and file out of it!
    let path = std::path::Path::new(os_str);
    let file = std::fs::File::create(path).unwrap(); // <- unwrap succeeds
}

But this library deals mainly with devnodes - /dev/input/eventX which are probably never going to contain weird characters (are we willing to make that guarantee forever?). One place where I can imagine non-ascii characters arising is in device names. You can even set a name with uinput that isn't valid UTF-8, and when asking for the name back from this library, it tells you None!

fn test_non_utf8() {
    let mut mouse_name_bytes = b"test name\x80";
    assert!(std::str::from_utf8(mouse_name_bytes).is_err());
    // imagine some C code did this, as that wouldn't require unsafe
    let mouse_name = unsafe{std::str::from_utf8_unchecked(mouse_name_bytes)};
    let uinput = {
        let uninit = UninitDevice::new().unwrap();
        uninit.set_name(mouse_name);
        UInputDevice::create_from_device(&uninit).unwrap()
    };
    let file = File::open(uinput.devnode().unwrap()).unwrap();
    let dev = Device::new_from_file(file).unwrap();
    dbg!(dev.name()); // None!
}

This leaves open the question of which of name, phys, uniq should be changed to OsStr from str.

maxbla avatar Jul 19 '21 19:07 maxbla

I guess it is a corner case which nevertheless could occur. If we were to change from OsStr to str, I would want to change all the instances to OsStr to maintain consistency.

ndesh26 avatar Jul 21 '21 07:07 ndesh26