Skip to content

Commit

Permalink
Fix critical bug in lifetime of string pointers passed over C FFI
Browse files Browse the repository at this point in the history
  • Loading branch information
@felixc
felixc committed Sep 13, 2015
1 parent 19e7436 commit 976964d
Showing 1 changed file with 49 additions and 46 deletions.
95 changes: 49 additions & 46 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@ impl Metadata {
/// ```
pub fn new_from_path(path: &str) -> Result<Metadata, String> {
let mut err: *mut gexiv2::GError = ptr::null_mut();
let c_str_path = ffi::CString::new(path).unwrap().as_ptr();
let c_str_path = ffi::CString::new(path).unwrap();
unsafe {
let metadata = gexiv2::gexiv2_metadata_new();
let ok = gexiv2::gexiv2_metadata_open_path(metadata, c_str_path, &mut err);
let ok = gexiv2::gexiv2_metadata_open_path(metadata, c_str_path.as_ptr(), &mut err);
if !ok {
let err_msg = str::from_utf8(ffi::CStr::from_ptr((*err).message).to_bytes());
match err_msg {
Expand Down Expand Up @@ -176,9 +176,9 @@ impl Metadata {
/// Save metadata to the file found at the given path, which must already exist.
pub fn save_to_file(&self, path: &str) -> Result<(), String> {
let mut err: *mut gexiv2::GError = ptr::null_mut();
let c_str_path = ffi::CString::new(path).unwrap().as_ptr();
let c_str_path = ffi::CString::new(path).unwrap();
unsafe {
let ok = gexiv2::gexiv2_metadata_save_file(self.raw, c_str_path, &mut err);
let ok = gexiv2::gexiv2_metadata_save_file(self.raw, c_str_path.as_ptr(), &mut err);
if !ok {
let err_msg = str::from_utf8(ffi::CStr::from_ptr((*err).message).to_bytes());
match err_msg {
Expand Down Expand Up @@ -246,14 +246,14 @@ impl Metadata {

/// Indicates whether the given tag is present/populated in the loaded metadata.
pub fn has_tag(&self, tag: &str) -> bool {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
unsafe { gexiv2::gexiv2_metadata_has_tag(self.raw, c_str_tag) }
let c_str_tag = ffi::CString::new(tag).unwrap();
unsafe { gexiv2::gexiv2_metadata_has_tag(self.raw, c_str_tag.as_ptr()) }
}

/// Removes the tag from the metadata if it exists. Returns whether it was there originally.
pub fn clear_tag(&self, tag: &str) -> bool {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
unsafe { gexiv2::gexiv2_metadata_clear_tag(self.raw, c_str_tag) }
let c_str_tag = ffi::CString::new(tag).unwrap();
unsafe { gexiv2::gexiv2_metadata_clear_tag(self.raw, c_str_tag.as_ptr()) }
}

/// Remove all tag values from the metadata.
Expand Down Expand Up @@ -352,9 +352,9 @@ impl Metadata {
///
/// Only safe if the tag is really of a string type.
pub fn get_tag_string(&self, tag: &str) -> Result<String, str::Utf8Error> {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
let c_str_tag = ffi::CString::new(tag).unwrap();
let value = unsafe {
let c_str_val = gexiv2::gexiv2_metadata_get_tag_string(self.raw, c_str_tag);
let c_str_val = gexiv2::gexiv2_metadata_get_tag_string(self.raw, c_str_tag.as_ptr());
str::from_utf8(ffi::CStr::from_ptr(c_str_val).to_bytes())
};
match value {
Expand All @@ -367,9 +367,10 @@ impl Metadata {
///
/// Only safe if the tag is really of a string type.
pub fn set_tag_string(&self, tag: &str, value: &str) -> Result<(), ()> {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
let c_str_val = ffi::CString::new(value).unwrap().as_ptr();
match unsafe { gexiv2::gexiv2_metadata_set_tag_string(self.raw, c_str_tag, c_str_val) } {
let c_str_tag = ffi::CString::new(tag).unwrap();
let c_str_val = ffi::CString::new(value).unwrap();
match unsafe { gexiv2::gexiv2_metadata_set_tag_string(self.raw, c_str_tag.as_ptr(),
c_str_val.as_ptr()) } {
false => Err(()),
true => Ok(())
}
Expand All @@ -379,9 +380,10 @@ impl Metadata {
///
/// Only safe if the tag is really of a string type.
pub fn get_tag_interpreted_string(&self, tag: &str) -> Result<String, str::Utf8Error> {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
let c_str_tag = ffi::CString::new(tag).unwrap();
let value = unsafe {
let c_str_val = gexiv2::gexiv2_metadata_get_tag_interpreted_string(self.raw, c_str_tag);
let c_str_val = gexiv2::gexiv2_metadata_get_tag_interpreted_string(self.raw,
c_str_tag.as_ptr());
str::from_utf8(ffi::CStr::from_ptr(c_str_val).to_bytes())
};
match value {
Expand All @@ -394,10 +396,10 @@ impl Metadata {
///
/// Only safe if the tag is in fact of a string type.
pub fn get_tag_multiple_strings(&self, tag: &str) -> Result<Vec<String>, str::Utf8Error> {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
let c_str_tag = ffi::CString::new(tag).unwrap();
let mut vals = vec![];
unsafe {
let c_vals = gexiv2::gexiv2_metadata_get_tag_multiple(self.raw, c_str_tag);
let c_vals = gexiv2::gexiv2_metadata_get_tag_multiple(self.raw, c_str_tag.as_ptr());
let mut cur_offset = 0;
while !(*c_vals.offset(cur_offset) as i8 == 0) {
let value = str::from_utf8(
Expand All @@ -414,13 +416,13 @@ impl Metadata {

/// Store the given strings as the values of a tag.
pub fn set_tag_multiple_strings(&self, tag: &str, values: &[&str]) -> Result<(), ()> {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
let c_str_tag = ffi::CString::new(tag).unwrap();
let c_strs: Result<Vec<_>, _> = values.iter().map(|&s| ffi::CString::new(s)).collect();
let c_strs = c_strs.unwrap();
let mut ptrs: Vec<_> = c_strs.iter().map(|c| c.as_ptr()).collect();
ptrs.push(ptr::null());
match unsafe { gexiv2::gexiv2_metadata_set_tag_multiple(self.raw,
c_str_tag, ptrs.as_ptr()) } {
match unsafe { gexiv2::gexiv2_metadata_set_tag_multiple(self.raw, c_str_tag.as_ptr(),
ptrs.as_ptr()) } {
false => Err(()),
true => Ok(())
}
Expand All @@ -430,16 +432,16 @@ impl Metadata {
///
/// Only safe if the tag is really of a numeric type.
pub fn get_tag_long(&self, tag: &str) -> i64 {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
unsafe { gexiv2::gexiv2_metadata_get_tag_long(self.raw, c_str_tag) }
let c_str_tag = ffi::CString::new(tag).unwrap();
unsafe { gexiv2::gexiv2_metadata_get_tag_long(self.raw, c_str_tag.as_ptr()) }
}

/// Set the value of a tag to the given number.
///
/// Only safe if the tag is really of a numeric type.
pub fn set_tag_long(&self, tag: &str, value: i64) -> Result<(), ()> {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
match unsafe { gexiv2::gexiv2_metadata_set_tag_long(self.raw, c_str_tag, value) } {
let c_str_tag = ffi::CString::new(tag).unwrap();
match unsafe { gexiv2::gexiv2_metadata_set_tag_long(self.raw, c_str_tag.as_ptr(), value) } {
false => Err(()),
true => Ok(())
}
Expand All @@ -449,11 +451,11 @@ impl Metadata {
///
/// Only safe if the tag is in fact of a rational type.
pub fn get_exif_tag_rational(&self, tag: &str) -> Option<num::rational::Ratio<i32>> {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
let c_str_tag = ffi::CString::new(tag).unwrap();
let ref mut num = 0;
let ref mut den = 0;
match unsafe { gexiv2::gexiv2_metadata_get_exif_tag_rational(self.raw,
c_str_tag, num, den) } {
match unsafe { gexiv2::gexiv2_metadata_get_exif_tag_rational(self.raw, c_str_tag.as_ptr(),
num, den) } {
false => None,
true => Some(num::rational::Ratio::new(*num, *den))
}
Expand All @@ -464,8 +466,8 @@ impl Metadata {
/// Only safe if the tag is in fact of a rational type.
pub fn set_exif_tag_rational(&self,
tag: &str, value: &num::rational::Ratio<i32>) -> Result<(), ()> {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
match unsafe { gexiv2::gexiv2_metadata_set_exif_tag_rational(self.raw, c_str_tag,
let c_str_tag = ffi::CString::new(tag).unwrap();
match unsafe { gexiv2::gexiv2_metadata_set_exif_tag_rational(self.raw, c_str_tag.as_ptr(),
*value.numer(),
*value.denom()) } {
false => Err(()),
Expand Down Expand Up @@ -571,8 +573,8 @@ impl Drop for Metadata {
/// assert_eq!(rexiv2::is_exif_tag("Iptc.Application2.Subject"), false);
/// ```
pub fn is_exif_tag(tag: &str) -> bool {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
unsafe { gexiv2::gexiv2_metadata_is_exif_tag(c_str_tag) }
let c_str_tag = ffi::CString::new(tag).unwrap();
unsafe { gexiv2::gexiv2_metadata_is_exif_tag(c_str_tag.as_ptr()) }
}

/// Indicates whether the given tag is part of the IPTC domain.
Expand All @@ -583,8 +585,8 @@ pub fn is_exif_tag(tag: &str) -> bool {
/// assert_eq!(rexiv2::is_iptc_tag("Xmp.dc.Title"), false);
/// ```
pub fn is_iptc_tag(tag: &str) -> bool {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
unsafe { gexiv2::gexiv2_metadata_is_iptc_tag(c_str_tag) }
let c_str_tag = ffi::CString::new(tag).unwrap();
unsafe { gexiv2::gexiv2_metadata_is_iptc_tag(c_str_tag.as_ptr()) }
}

/// Indicates whether the given tag is from the XMP domain.
Expand All @@ -595,8 +597,8 @@ pub fn is_iptc_tag(tag: &str) -> bool {
/// assert_eq!(rexiv2::is_xmp_tag("Exif.Photo.FocalLength"), false);
/// ```
pub fn is_xmp_tag(tag: &str) -> bool {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
unsafe { gexiv2::gexiv2_metadata_is_xmp_tag(c_str_tag) }
let c_str_tag = ffi::CString::new(tag).unwrap();
unsafe { gexiv2::gexiv2_metadata_is_xmp_tag(c_str_tag.as_ptr()) }
}

/// Get a short label for a tag.
Expand All @@ -606,9 +608,9 @@ pub fn is_xmp_tag(tag: &str) -> bool {
/// assert_eq!(rexiv2::get_tag_label("Iptc.Application2.Subject"), Ok("Subject".to_string()));
/// ```
pub fn get_tag_label(tag: &str) -> Result<String, str::Utf8Error> {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
let c_str_tag = ffi::CString::new(tag).unwrap();
let label = unsafe {
let c_str_label = gexiv2::gexiv2_metadata_get_tag_label(c_str_tag);
let c_str_label = gexiv2::gexiv2_metadata_get_tag_label(c_str_tag.as_ptr());
str::from_utf8(ffi::CStr::from_ptr(c_str_label).to_bytes())
};
match label {
Expand All @@ -625,9 +627,9 @@ pub fn get_tag_label(tag: &str) -> Result<String, str::Utf8Error> {
/// Ok("The Subject Reference is a structured definition of the subject matter.".to_string()))
/// ```
pub fn get_tag_description(tag: &str) -> Result<String, str::Utf8Error> {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
let c_str_tag = ffi::CString::new(tag).unwrap();
let desc = unsafe {
let c_str_desc = gexiv2::gexiv2_metadata_get_tag_description(c_str_tag);
let c_str_desc = gexiv2::gexiv2_metadata_get_tag_description(c_str_tag.as_ptr());
str::from_utf8(ffi::CStr::from_ptr(c_str_desc).to_bytes())
};
match desc {
Expand All @@ -644,9 +646,9 @@ pub fn get_tag_description(tag: &str) -> Result<String, str::Utf8Error> {
/// assert_eq!(rexiv2::get_tag_type("Iptc.Application2.DateCreated"), Ok(rexiv2::TagType::Date));
/// ```
pub fn get_tag_type(tag: &str) -> Result<TagType, str::Utf8Error> {
let c_str_tag = ffi::CString::new(tag).unwrap().as_ptr();
let c_str_tag = ffi::CString::new(tag).unwrap();
let tag_type = unsafe {
let c_str_type = gexiv2::gexiv2_metadata_get_tag_type(c_str_tag);
let c_str_type = gexiv2::gexiv2_metadata_get_tag_type(c_str_tag.as_ptr());
str::from_utf8(ffi::CStr::from_ptr(c_str_type).to_bytes())
};
match tag_type {
Expand Down Expand Up @@ -687,18 +689,19 @@ pub fn get_tag_type(tag: &str) -> Result<TagType, str::Utf8Error> {

/// Add a new XMP namespace for tags to exist under.
pub fn register_xmp_namespace(name: &str, prefix: &str) -> Result<(), ()> {
let c_str_name = ffi::CString::new(name).unwrap().as_ptr();
let c_str_prefix = ffi::CString::new(prefix).unwrap().as_ptr();
match unsafe { gexiv2::gexiv2_metadata_register_xmp_namespace(c_str_name, c_str_prefix) } {
let c_str_name = ffi::CString::new(name).unwrap();
let c_str_prefix = ffi::CString::new(prefix).unwrap();
match unsafe { gexiv2::gexiv2_metadata_register_xmp_namespace(c_str_name.as_ptr(),
c_str_prefix.as_ptr()) } {
false => Err(()),
true => Ok(())
}
}

/// Remove an XMP namespace from the set of known ones.
pub fn unregister_xmp_namespace(name: &str) -> Result<(), ()> {
let c_str_name = ffi::CString::new(name).unwrap().as_ptr();
match unsafe { gexiv2::gexiv2_metadata_unregister_xmp_namespace(c_str_name) } {
let c_str_name = ffi::CString::new(name).unwrap();
match unsafe { gexiv2::gexiv2_metadata_unregister_xmp_namespace(c_str_name.as_ptr()) } {
false => Err(()),
true => Ok(())
}
Expand Down

0 comments on commit 976964d

Please sign in to comment.