add a new tested context.new_blob_dir method to simplify writing out blob files

src/context.rs

@@ -1,5 +1,7 @@
 use std::collections::HashMap;
 use std::ffi::OsString;
+use std::fs;
+use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, Condvar, Mutex, RwLock};
 
@@ -8,6 +10,7 @@ use libc::uintptr_t;
 use crate::chat::*;
 use crate::constants::*;
 use crate::contact::*;
+use crate::dc_tools::dc_derive_safe_stem_ext;
 use crate::error::*;
 use crate::events::Event;
 use crate::imap::*;
@@ -20,6 +23,7 @@ use crate::message::{self, Message};
 use crate::param::Params;
 use crate::smtp::*;
 use crate::sql::Sql;
+use rand::{thread_rng, Rng};
 
 /// Callback function type for [Context]
 ///
@@ -158,6 +162,41 @@ impl Context {
         self.blobdir.as_path()
     }
 
+    pub fn new_blob_file(&self, orig_filename: impl AsRef<str>, data: &[u8]) -> Result<String> {
+        // return a $BLOBDIR/<FILENAME> string which corresponds to the
+        // respective file in the blobdir, and which contains the data.
+        // FILENAME is computed by looking and possibly mangling the
+        // basename of orig_filename. The resulting filenames are meant
+        // to be human-readable.
+        let (stem, ext) = dc_derive_safe_stem_ext(orig_filename.as_ref());
+
+        // ext starts with "." or is empty string, so we can always resconstruct
+
+        for i in 0..3 {
+            let candidate_basename = match i {
+                // first a try to just use the (possibly mangled) original basename
+                0 => format!("{}{}", stem, ext),
+
+                // otherwise extend stem with random numbers
+                _ => {
+                    let mut rng = thread_rng();
+                    let random_id: u32 = rng.gen();
+                    format!("{}-{}{}", stem, random_id, ext)
+                }
+            };
+            let path = self.get_blobdir().join(&candidate_basename);
+            if let Ok(mut file) = fs::OpenOptions::new()
+                .create_new(true)
+                .write(true)
+                .open(&path)
+            {
+                file.write_all(data)?;
+                return Ok(format!("$BLOBDIR/{}", candidate_basename));
+            }
+        }
+        bail!("out of luck to create new blob file");
+    }
+
     pub fn call_cb(&self, event: Event) -> uintptr_t {
         (*self.cb)(self, event)
     }
@@ -438,6 +477,7 @@ pub fn get_version_str() -> &'static str {
 mod tests {
     use super::*;
 
+    use crate::dc_tools::*;
     use crate::test_utils::*;
 
     #[test]
@@ -475,6 +515,52 @@ mod tests {
         assert!(res.is_err());
     }
 
+    #[test]
+    fn test_new_blob_file() {
+        let t = dummy_context();
+        let context = t.ctx;
+        let x = &context.new_blob_file("hello", b"data").unwrap();
+        assert!(dc_file_exist(&context, x));
+        assert!(x.starts_with("$BLOBDIR"));
+        assert!(dc_read_file(&context, x).unwrap() == b"data");
+
+        let y = &context.new_blob_file("hello", b"data").unwrap();
+        assert!(dc_file_exist(&context, y));
+        assert!(y.starts_with("$BLOBDIR/data-"));
+
+        let x = &context.new_blob_file("hello", b"data.png").unwrap();
+        assert!(dc_file_exist(&context, x));
+        assert!(x.starts_with("$BLOBDIR"));
+
+        let y = &context.new_blob_file("hello", b"data.png").unwrap();
+        assert!(dc_file_exist(&context, y));
+        assert!(y.starts_with("$BLOBDIR/data-"));
+        assert!(y.ends_with(".png"));
+    }
+
+    #[test]
+    fn test_new_blob_file_long_names() {
+        let t = dummy_context();
+        let context = t.ctx;
+        let s = "12312312039182039182039812039810293810293810293810293801293801293123123";
+        let x = &context.new_blob_file(s, b"data").unwrap();
+        println!("blobfilename '{}'", x);
+        println!("xxxxfilename '{}'", s);
+        assert!(x.len() < s.len());
+        assert!(dc_file_exist(&context, x));
+        assert!(x.starts_with("$BLOBDIR"));
+    }
+
+    #[test]
+    fn test_new_blob_file_unicode() {
+        let t = dummy_context();
+        let context = t.ctx;
+        let s = "helloäworld.qwe";
+        let x = &context.new_blob_file(s, b"data").unwrap();
+        assert_eq!(x, "$BLOBDIR/hello-world.qwe");
+        assert_eq!(dc_read_file(&context, x).unwrap(), b"data");
+    }
+
     #[test]
     fn test_sqlite_parent_not_exists() {
         let tmp = tempfile::tempdir().unwrap();

src/dc_mimeparser.rs

@@ -776,28 +776,34 @@ impl<'a> MimeParser<'a> {
         decoded_data: &[u8],
         desired_filename: &str,
     ) {
-        /* create a free file name to use */
+        /* write decoded data to new blob file */
-        let path_filename = dc_get_fine_path_filename(self.context, "$BLOBDIR", desired_filename);
+        let bpath = match self.context.new_blob_file(desired_filename, decoded_data) {
-
+            Ok(path) => path,
-        /* copy data to file */
+            Err(err) => {
-        if dc_write_file(self.context, &path_filename, decoded_data) {
+                error!(
-            let mut part = Part::default();
+                    self.context,
-            part.typ = msg_type;
+                    "Could not add blob for mime part {}, error {}", desired_filename, err
-            part.mimetype = mime_type;
+                );
-            part.bytes = decoded_data.len() as libc::c_int;
+                return;
-            part.param.set(Param::File, path_filename.to_string_lossy());
-            if let Some(raw_mime) = raw_mime {
-                part.param.set(Param::MimeType, raw_mime);
             }
+        };
 
-            if mime_type == DC_MIMETYPE_IMAGE {
+        let mut part = Part::default();
-                if let Ok((width, height)) = dc_get_filemeta(decoded_data) {
+        part.typ = msg_type;
-                    part.param.set_int(Param::Width, width as i32);
+        part.mimetype = mime_type;
-                    part.param.set_int(Param::Height, height as i32);
+        part.bytes = decoded_data.len() as libc::c_int;
-                }
+        part.param.set(Param::File, bpath);
-            }
+        if let Some(raw_mime) = raw_mime {
-            self.do_add_single_part(part);
+            part.param.set(Param::MimeType, raw_mime);
         }
+
+        if mime_type == DC_MIMETYPE_IMAGE {
+            if let Ok((width, height)) = dc_get_filemeta(decoded_data) {
+                part.param.set_int(Param::Width, width as i32);
+                part.param.set_int(Param::Height, height as i32);
+            }
+        }
+        self.do_add_single_part(part);
     }
 
     fn do_add_single_part(&mut self, mut part: Part) {

src/dc_tools.rs

@@ -1,6 +1,7 @@
 //! Some tools and enhancements to the used libraries, there should be
 //! no references to Context and other "larger" entities here.
 
+use core::cmp::max;
 use std::borrow::Cow;
 use std::ffi::{CStr, CString, OsString};
 use std::path::{Path, PathBuf};
@@ -415,12 +416,41 @@ pub(crate) fn dc_ensure_no_slash_safe(path: &str) -> &str {
     path
 }
 
-/// Function modifies the given buffer and replaces all characters not valid in filenames by a "-".
+// Function returns a sanitized basename that does not contain
-fn validate_filename(filename: &str) -> String {
+// win/linux path separators and also not any non-ascii chars
-    filename
+fn get_safe_basename(filename: &str) -> String {
-        .replace('/', "-")
+    // return the (potentially mangled) basename of the input filename
-        .replace('\\', "-")
+    // this might be a path that comes in from another operating system
-        .replace(':', "-")
+    let mut index: usize = 0;
+
+    if let Some(unix_index) = filename.rfind("/") {
+        index = unix_index + 1;
+    }
+    if let Some(win_index) = filename.rfind("\\") {
+        index = max(index, win_index + 1);
+    }
+    if index >= filename.len() {
+        "nobasename".to_string()
+    } else {
+        // we don't allow any non-ascii to be super-safe
+        filename[index..].replace(|c: char| !c.is_ascii() || c == ':', "-")
+    }
+}
+
+pub fn dc_derive_safe_stem_ext(filename: &str) -> (String, String) {
+    let basename = get_safe_basename(&filename);
+    let (mut stem, mut ext) = if let Some(index) = basename.rfind(".") {
+        (
+            basename[0..index].to_string(),
+            basename[index..].to_string(),
+        )
+    } else {
+        (basename, "".to_string())
+    };
+    // limit length of stem and ext
+    stem.truncate(32);
+    ext.truncate(32);
+    (stem, ext)
 }
 
 // the returned suffix is lower-case
@@ -611,6 +641,14 @@ pub(crate) fn dc_get_fine_path_filename(
     panic!("Something is really wrong, you need to clean up your disk");
 }
 
+/// Function modifies the given buffer and replaces all characters not valid in filenames by a "-".
+fn validate_filename(filename: &str) -> String {
+    filename
+        .replace('/', "-")
+        .replace('\\', "-")
+        .replace(':', "-")
+}
+
 pub(crate) fn dc_is_blobdir_path(context: &Context, path: impl AsRef<str>) -> bool {
     context
         .get_blobdir()
@@ -1437,6 +1475,19 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_file_get_safe_basename() {
+        assert_eq!(get_safe_basename("12312/hello"), "hello");
+        assert_eq!(get_safe_basename("12312\\hello"), "hello");
+        assert_eq!(get_safe_basename("//12312\\hello"), "hello");
+        assert_eq!(get_safe_basename("//123:12\\hello"), "hello");
+        assert_eq!(get_safe_basename("//123:12/\\\\hello"), "hello");
+        assert_eq!(get_safe_basename("//123:12//hello"), "hello");
+        assert_eq!(get_safe_basename("//123:12//"), "nobasename");
+        assert_eq!(get_safe_basename("//123:12/"), "nobasename");
+        assert!(get_safe_basename("123\x012.hello").ends_with(".hello"));
+    }
+
     #[test]
     fn test_file_handling() {
         let t = dummy_context();
@@ -1483,14 +1534,21 @@ mod tests {
         assert!(dc_create_folder(context, "$BLOBDIR/foobar-folder"));
         assert!(dc_file_exist(context, "$BLOBDIR/foobar-folder",));
         assert!(!dc_delete_file(context, "$BLOBDIR/foobar-folder"));
+
         let fn0 = dc_get_fine_path_filename(context, "$BLOBDIR", "foobar.dadada");
-        assert_eq!(fn0, PathBuf::from("$BLOBDIR/foobar.dadada"));
+        let fn0_s = fn0.to_string_lossy();
+        assert!(fn0_s.starts_with("$BLOBDIR/foobar-"));
+        assert!(fn0.extension().unwrap() == "dadada");
+
+        let fn0 = dc_get_fine_path_filename(context, "$BLOBDIR", "hello\\foobar.x");
+        let fn0_s = fn0.to_string_lossy();
+        assert!(fn0_s.starts_with("$BLOBDIR/foobar-"));
+        assert!(fn0_s.ends_with(".x"));
 
         assert!(dc_write_file(context, &fn0, b"content"));
-        let fn1 = dc_get_fine_path_filename(context, "$BLOBDIR", "foobar.dadada");
-        assert_eq!(fn1, PathBuf::from("$BLOBDIR/foobar-1.dadada"));
 
         assert!(dc_delete_file(context, &fn0));
+        assert!(!dc_file_exist(context, &fn0));
     }
 
     #[test]

src/imex.rs

@@ -129,14 +129,15 @@ pub fn initiate_key_transfer(context: &Context) -> Result<String> {
                 .unwrap()
                 .shall_stop_ongoing
             {
-                let setup_file_name =
+                let setup_file_name = context.new_blob_file(
-                    dc_get_fine_path_filename(context, "$BLOBDIR", "autocrypt-setup-message.html");
+                    "autocrypt-setup-message.html",
-                if dc_write_file(context, &setup_file_name, setup_file_content.as_bytes()) {
+                    setup_file_content.as_bytes(),
+                )?;
+                {
                     if let Ok(chat_id) = chat::create_by_contact_id(context, 1) {
                         msg = Message::default();
                         msg.type_0 = Viewtype::File;
-                        msg.param
+                        msg.param.set(Param::File, setup_file_name);
-                            .set(Param::File, setup_file_name.to_string_lossy());
 
                         msg.param
                             .set(Param::MimeType, "application/autocrypt-setup");
@@ -579,6 +580,7 @@ fn export_backup(context: &Context, dir: impl AsRef<Path>) -> Result<()> {
         .format("delta-chat-%Y-%m-%d.bak")
         .to_string();
 
+    // let dest_path_filename = dc_get_next_backup_file(context, dir, res);
     let dest_path_filename = dc_get_fine_path_filename(context, dir, res);
 
     sql::housekeeping(context);

src/job.rs

@@ -965,41 +965,39 @@ fn send_mdn(context: &Context, msg_id: u32) {
 
 #[allow(non_snake_case)]
 fn add_smtp_job(context: &Context, action: Action, mimefactory: &MimeFactory) -> libc::c_int {
-    let mut success: libc::c_int = 0i32;
     let mut param = Params::new();
-    let path_filename = dc_get_fine_path_filename(context, "$BLOBDIR", &mimefactory.rfc724_mid);
     let bytes = unsafe {
         std::slice::from_raw_parts(
             (*mimefactory.out).str_0 as *const u8,
             (*mimefactory.out).len,
         )
     };
-    if !dc_write_file(context, &path_filename, bytes) {
+    let bpath = match context.new_blob_file(&mimefactory.rfc724_mid, bytes) {
-        error!(
+        Ok(path) => path,
-            context,
+        Err(err) => {
-            "Could not write message <{}> to \"{}\".",
+            error!(
-            mimefactory.rfc724_mid,
+                context,
-            path_filename.display(),
+                "Could not write {} smtp-message, error {}", mimefactory.rfc724_mid, err
-        );
+            );
-    } else {
+            return 0;
-        info!(context, "add_smtp_job file written: {:?}", path_filename);
+        }
-        let recipients = mimefactory.recipients_addr.join("\x1e");
+    };
-        param.set(Param::File, path_filename.to_string_lossy());
+    info!(context, "add_smtp_job file written: {:?}", bpath);
-        param.set(Param::Recipients, &recipients);
+    let recipients = mimefactory.recipients_addr.join("\x1e");
-        job_add(
+    param.set(Param::File, &bpath);
-            context,
+    param.set(Param::Recipients, &recipients);
-            action,
+    job_add(
-            (if mimefactory.loaded == Loaded::Message {
+        context,
-                mimefactory.msg.id
+        action,
-            } else {
+        (if mimefactory.loaded == Loaded::Message {
-                0
+            mimefactory.msg.id
-            }) as libc::c_int,
+        } else {
-            param,
+            0
-            0,
+        }) as libc::c_int,
-        );
+        param,
-        success = 1;
+        0,
-    }
+    );
-    success
+    1
 }
 
 pub fn job_add(