Xeiron / sndfile.rs

A safe rust wrapper of libsndfile
MIT License
10 stars 7 forks source link

`get_tag` causes segmentation fault for wav file #3

Closed wrist closed 2 years ago

wrist commented 2 years ago

According to this, the function sf_get_string returns NULL if the result string doesn't exist for given TagType . So get_tag function using sf_get_string has a possibility to cause a segmentation fault due to the NULL for sound files such as .wav whose a valid tag doesn't exist in their metadata. This segmentation fault happens at the below line. https://github.com/Xeiron/sndfile.rs/blob/33219f56cf52ac261468f5d8a82ed25718ed9076/src/lib.rs#L746

I think it's better to use s_ptr.as_ref() instead of s_ptr directly like the below code.

  pub fn get_tag(&self, t: TagType) -> String {
    let s_ptr =
      unsafe { sndfile_sys::sf_get_string(self.unsafe_fields.sndfile_ptr, tag_type_to_flags(t)) };

    unsafe {
      if let Some(ptr) = s_ptr.as_ref() {
        let c_str = std::ffi::CStr::from_ptr(ptr);
        c_str.to_string_lossy().into_owned()
      } else {
        String::new()
      }
    }
  }

I also think get_tag may return Option<String> to express the case sf_get_string fails. How about this?

tuxzz commented 2 years ago

Thanks for your PR.😆 I'm sorry that I was a bit busy recently and have not noticed this. I will review it within the next day.

tuxzz commented 2 years ago

And one more thing, I will likely not be able to squeeze out much free time for a long time. Do you mind if I transfer the ownership of this project to you? Don't worry if you have no interest in it.