Fix Unicode paths on Windows (& accept more paths on Unix)

src/lib.rs

@@ -239,14 +239,69 @@ impl Drop for File {
     }
 }
 
+/// On Windows, we need to convert the paths into wchar_t*, essentially, and
+/// use the matching APIs.
+#[cfg(target_os = "windows")]
+fn path_to_vec_wchar<P: AsRef<Path>>(path: P) -> Result<Vec<u16>, FileError> {
+    use std::os::windows::ffi::OsStrExt;
+    let as_os_str = path.as_ref().as_os_str();
+
+    let wchar: Vec<_> = as_os_str.encode_wide()
+        .chain(std::iter::once(0))
+        .collect();
+
+    // For the filename to be valid, it must not have any internal 0s (i.e. any
+    // internal nul-terminators).
+    for encoded in &wchar[0..wchar.len() - 1] {
+        if *encoded == 0 {
+            return Err(FileError::InvalidFileName);
+        }
+    }
+
+    Ok(wchar)
+}
+
+/// On Unix platforms, converting a Path to a CString can be done simply by
+/// using the as_bytes() method from OsStrExt.
+/// 
+/// Returns a FileError if the path contains an internal NUL.
+#[cfg(unix)]
+fn path_to_cstring<P: AsRef<Path>>(path: P) -> Result<CString, FileError> {
+    use std::os::unix::ffi::OsStrExt;
+    CString::new(path.as_ref().as_os_str().as_bytes())
+        .map_err(|_| FileError::InvalidFileName)
+}
+
+/// On non-Windows, non-Unix platforms, it's unclear what exactly the byte 
+/// encoding should be. We will assume it is UTF-8 as a common denominator,
+/// and attempt to get a valid UTF-8 string out of the input, returning
+/// FileError::InvalidFileName if it is not UTF-8.
+/// 
+/// HOWEVER! This will NOT work correctly if the internal path encoding on 
+/// that system is NOT actually UTF-8! Beware!
+#[cfg(all(not(unix), not(windows)))]
+fn path_to_cstring<P: AsRef<Path>>(path: P) -> Result<CString, FileError> {
+    let filename = path.as_ref().to_str().ok_or(FileError::InvalidFileName)?;
+    CString::new(filename).ok().ok_or(FileError::InvalidFileName)
+}
+
 impl File {
     /// Creates a new `taglib::File` for the given `filename`.
-    pub fn new<P: AsRef<Path>>(path: P) -> Result<File, FileError> {
-        let filename = path.as_ref().to_str().ok_or(FileError::InvalidFileName)?;
-        let filename_c = CString::new(filename).ok().ok_or(FileError::InvalidFileName)?;
-        let filename_c_ptr = filename_c.as_ptr();
-
-        let f = unsafe { ll::taglib_file_new(filename_c_ptr) };
+    pub fn new<P: AsRef<Path>>(filename: P) -> Result<File, FileError> {
+        let f = {
+            #[cfg(windows)]
+            {
+                let wchar = path_to_vec_wchar(filename)?;
+                unsafe { ll::taglib_file_new_wchar(wchar.as_ptr()) }
+            }
+
+            #[cfg(not(windows))]
+            {
+                let filename_c = path_to_cstring(filename)?;
+                unsafe { ll::taglib_file_new(filename_c.as_ptr()) }
+            }
+        };
+
         if f.is_null() {
             return Err(FileError::InvalidFile);
         }
@@ -255,14 +310,23 @@ impl File {
     }
 
     /// Creates a new `taglib::File` for the given `filename` and type of file.
-    pub fn new_type(filename: &str, filetype: FileType) -> Result<File, FileError> {
-        let filename_c = match CString::new(filename) {
-            Ok(s) => s,
-            _ => return Err(FileError::InvalidFileName),
+    pub fn new_type<P: AsRef<Path>>(filename: P, filetype: FileType) -> Result<File, FileError> {
+        // Convert filetype for ABI.
+        let filetype = filetype as u32;
+        let f = {
+            #[cfg(windows)]
+            {
+                let wchar = path_to_vec_wchar(filename)?;
+                unsafe { ll::taglib_file_new_type_wchar(wchar.as_ptr(), filetype) }
+            }
+
+            #[cfg(not(windows))]
+            {
+                let filename_c = path_to_cstring(filename)?;
+                unsafe { ll::taglib_file_new_type(filename_c.as_ptr(), filetype) }
+            }
         };
 
-        let filename_c_ptr = filename_c.as_ptr();
-        let f = unsafe { ll::taglib_file_new_type(filename_c_ptr, filetype as u32) };
         if f.is_null() {
             return Err(FileError::InvalidFile);
         }
@@ -271,7 +335,7 @@ impl File {
     }
 
     /// Returns the `taglib::Tag` instance for the given file.
-    pub fn tag(&self) -> Result<Tag, FileError> {
+    pub fn tag(&self) -> Result<Tag<'_>, FileError> {
         let res = unsafe { ll::taglib_file_tag(self.raw) };
 
         if res.is_null() {
@@ -290,7 +354,7 @@ impl File {
     }
 
     /// Returns the `taglib::AudioProperties` instance for the given file.
-    pub fn audioproperties(&self) -> Result<AudioProperties, FileError> {
+    pub fn audioproperties(&self) -> Result<AudioProperties<'_>, FileError> {
         let res = unsafe { ll::taglib_file_audioproperties(self.raw) };
 
         if res.is_null() {

taglib-sys/src/lib.rs

@@ -21,7 +21,7 @@
 #![allow(non_camel_case_types)]
 extern crate libc;
 
-use libc::{c_int, c_uint, c_char, c_void};
+use libc::{c_char, c_int, c_uint, c_void, wchar_t};
 
 // Public types; these are all opaque pointer types
 pub type TagLib_File = c_void;
@@ -45,10 +45,17 @@ pub const TAGLIB_FILE_ASF: TagLib_FileType = 9;
 // tag_c.h
 extern "C" {
     pub fn taglib_file_new(filename: *const c_char) -> *mut TagLib_File;
+    #[cfg(target_os = "windows")]
+    pub fn taglib_file_new_wchar(filename: *const wchar_t) -> *mut TagLib_File;
     pub fn taglib_file_new_type(
         filename: *const c_char,
         filetype: TagLib_FileType,
     ) -> *mut TagLib_File;
+    #[cfg(target_os = "windows")]
+     pub fn taglib_file_new_type_wchar(
+        filename: *const wchar_t,
+        filetype: TagLib_FileType,
+    ) -> *mut TagLib_File;
     pub fn taglib_file_is_valid(file: *mut TagLib_File) -> TagLib_Bool;
     pub fn taglib_file_free(file: *mut TagLib_File);
     pub fn taglib_file_save(file: *mut TagLib_File) -> TagLib_Bool;