PolyMeilex / rfd

Rusty File Dialog
MIT License
564 stars 64 forks source link

FileHandle#read() panics if the FileHandle is a directory #125

Open jcbhmr opened 1 year ago

jcbhmr commented 1 year ago

I think it's this .unwrap() that's unwrapping the return value of std::fs::read() that was stashed in the shared value...

https://github.com/PolyMeilex/rfd/blob/25d879a854b606c2236b72c7c085ae776d413837/src/file_handle/native.rs#L52

...but since https://doc.rust-lang.org/stable/std/fs/fn.read.html returns a Result<>, the unwrap() panics.

jcbhmr commented 1 year ago

Here's where this came up and my temp solution for it:

https://github.com/bindrs/rfd.js/blob/060fac3e750d49020bd5dc7cc94c1c9dff52adad/src/file_handle.rs#L139-L154

    #[napi]
    pub async unsafe fn read(&self) -> Result<Buffer> {
        // TODO: https://stackoverflow.com/a/54432441
        // https://stackoverflow.com/a/35559417
        let hook = panic::take_hook();
        panic::set_hook(Box::new(|_| {}));
        // https://stackoverflow.com/a/63159560/19522682
        let vec = self.0.read().catch_unwind().await;
        panic::set_hook(hook);
        if vec.is_err() {
            return Err(Error::from_reason("Panic"));
        }
        let vec = vec.unwrap();
        let buffer = vec.into();
        return Ok(buffer);
    }
PolyMeilex commented 1 year ago

Oh, yeah that's bad, will take a look at that, in the meantime assuming that you are not building for web browsers you could just call .path() instead of .read(), and read the file yourself