From 6c9e16d31a6a92804c3fb5e666676652ccf65d28 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Fri, 18 Oct 2019 00:19:49 +0200 Subject: [PATCH] Introduce a BlobObject type for blobs This creates a specific type for blobs, with well defined conversions at the borders. It also introduces a strong type for the Param::File value since that param is often used used by the public API to set filenames using absolute paths, but then core changes the param to a blob before it gets to the database. This eliminates a few more functions with very mallable C-like arguments behaviour which combine a number of operations in one. Because blob filenames are stored so often in arbitrary strings this does add more code when receiving those, until the storage is fixed. File name sanitisation is now deletated to the sanitize-filename crate which should do a slightly better job at this. --- Cargo.lock | 11 + Cargo.toml | 1 + run-integration-tests.sh | 2 +- src/blob.rs | 604 +++++++++++++++++++++++++++++++++++++++ src/chat.rs | 211 +++++++------- src/context.rs | 103 ------- src/dc_mimeparser.rs | 8 +- src/dc_receive_imf.rs | 30 +- src/dc_tools.rs | 61 +--- src/error.rs | 8 + src/imex.rs | 8 +- src/job.rs | 23 +- src/lib.rs | 1 + src/message.rs | 38 ++- src/mimefactory.rs | 63 ++-- src/param.rs | 53 ++++ 16 files changed, 909 insertions(+), 316 deletions(-) create mode 100644 src/blob.rs diff --git a/Cargo.lock b/Cargo.lock index a0b8e2063..51866a4cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -518,6 +518,7 @@ dependencies = [ "reqwest 0.9.22 (registry+https://github.com/rust-lang/crates.io-index)", "rusqlite 0.20.0 (registry+https://github.com/rust-lang/crates.io-index)", "rustyline 4.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "sanitize-filename 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.101 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.41 (registry+https://github.com/rust-lang/crates.io-index)", "sha2 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2028,6 +2029,15 @@ dependencies = [ "winapi-util 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "sanitize-filename" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "regex 1.3.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "schannel" version = "0.1.16" @@ -3009,6 +3019,7 @@ dependencies = [ "checksum ryu 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "bfa8506c1de11c9c4e4c38863ccbe02a305c8188e85a05a784c9e11e1c3910c8" "checksum safemem 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "d2b08423011dae9a5ca23f07cf57dac3857f5c885d352b76f6d95f4aea9434d0" "checksum same-file 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)" = "585e8ddcedc187886a30fa705c47985c3fa88d06624095856b36ca0b82ff4421" +"checksum sanitize-filename 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "23fd0fec94ec480abfd86bb8f4f6c57e0efb36dac5c852add176ea7b04c74801" "checksum schannel 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)" = "87f550b06b6cba9c8b8be3ee73f391990116bf527450d2556e9b9ce263b9a021" "checksum scheduled-thread-pool 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "bd07742e081ff6c077f5f6b283f12f32b9e7cc765b316160d66289b74546fbb3" "checksum scopeguard 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b42e15e59b18a828bbf5c58ea01debb36b9b096346de35d941dcb89009f24a0d" diff --git a/Cargo.toml b/Cargo.toml index 72503bdc8..19d8b86a2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,6 +48,7 @@ escaper = "0.1.0" bitflags = "1.1.0" jetscii = "0.4.4" debug_stub_derive = "0.3.0" +sanitize-filename = "0.2.1" [dev-dependencies] tempfile = "3.0" diff --git a/run-integration-tests.sh b/run-integration-tests.sh index d92f516f7..7aef0babb 100755 --- a/run-integration-tests.sh +++ b/run-integration-tests.sh @@ -23,7 +23,7 @@ if [ $? != 0 ]; then fi pushd python -if [ -e "./liveconfig" && -z "$DCC_PY_LIVECONFIG" ]; then +if [ -e "./liveconfig" -a -z "$DCC_PY_LIVECONFIG" ]; then export DCC_PY_LIVECONFIG=liveconfig fi tox "$@" diff --git a/src/blob.rs b/src/blob.rs new file mode 100644 index 000000000..03e2da572 --- /dev/null +++ b/src/blob.rs @@ -0,0 +1,604 @@ +use std::fmt; +use std::fs; +use std::io::Write; +use std::path::{Path, PathBuf}; + +use crate::context::Context; +use crate::events::Event; + +/// Represents a file in the blob directory. +/// +/// The object has a name, which will always be valid UTF-8. Having a +/// blob object does not imply the respective file exists, however +/// when using one of the `create*()` methods a unique file is +/// created. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct BlobObject<'a> { + blobdir: &'a Path, + name: String, +} + +impl<'a> BlobObject<'a> { + /// Creates a new blob object with a unique name. + /// + /// Creates a new file in the blob directory. The name will be + /// derived from the platform-agnostic basename of the suggested + /// name, followed by a random number and followed by a possible + /// extension. The `data` will be written into the file. + /// + /// # Errors + /// + /// [BlobErrorKind::CreateFailure] is used when the file could not + /// be created. You can expect [BlobError.cause] to contain an + /// underlying error. + /// + /// [BlobErrorKind::WriteFailure] is used when the file could not + /// be written to. You can expect [BlobError.cause] to contain an + /// underlying error. + pub fn create( + context: &'a Context, + suggested_name: impl AsRef, + data: &[u8], + ) -> std::result::Result, BlobError> { + let blobdir = context.get_blobdir(); + let (stem, ext) = BlobObject::sanitise_name(suggested_name.as_ref().to_string()); + let mut name = format!("{}{}", stem, ext); + let max_attempt = 15; + for attempt in 0..max_attempt { + let path = blobdir.join(&name); + match fs::OpenOptions::new() + .create_new(true) + .write(true) + .open(&path) + { + Ok(mut file) => { + file.write_all(data) + .map_err(|err| BlobError::new_write_failure(blobdir, &name, err))?; + let blob = BlobObject { + blobdir, + name: format!("$BLOBDIR/{}", name), + }; + context.call_cb(Event::NewBlobFile(blob.as_name().to_string())); + return Ok(blob); + } + Err(err) => { + if attempt == max_attempt { + return Err(BlobError::new_create_failure(blobdir, &name, err)); + } else { + name = format!("{}-{}{}", stem, rand::random::(), ext); + } + } + } + } + Err(BlobError::new_create_failure( + blobdir, + &name, + format_err!("Unreachable code - supposedly"), + )) + } + + /// Creates a new blob object with unique name by copying an existing file. + /// + /// This creates a new blob as described in [BlobObject::create] + /// but also copies an existing file into it. + /// + /// # Errors + /// + /// In addition to the errors in [BlobObject::create] the + /// [BlobErrorKind::CopyFailure] is used when the data can not be + /// copied. + pub fn create_and_copy( + context: &'a Context, + src: impl AsRef, + ) -> std::result::Result, BlobError> { + let blob = BlobObject::create(context, src.as_ref().to_string_lossy(), b"")?; + fs::copy(src.as_ref(), blob.to_abs_path()).map_err(|err| { + fs::remove_file(blob.to_abs_path()).ok(); + BlobError::new_copy_failure(blob.blobdir, &blob.name, src.as_ref(), err) + })?; + Ok(blob) + } + + /// Creates a blob from a file, possibly copying it to the blobdir. + /// + /// If the source file is not a path to into the blob directory + /// the file will be copied into the blob directory first. If the + /// source file is already in the blobdir it will not be copied + /// and only be created if it is a valid blobname, that is no + /// subdirectory is used and [BlobObject::sanitise_name] does not + /// modify the filename. + /// + /// # Errors + /// + /// This merely delegates to the [BlobObject::create_and_copy] and + /// the [BlobObject::from_path] methods. See those for possible + /// errors. + pub fn create_from_path( + context: &Context, + src: impl AsRef, + ) -> std::result::Result { + match src.as_ref().starts_with(context.get_blobdir()) { + true => BlobObject::from_path(context, src), + false => BlobObject::create_and_copy(context, src), + } + } + + /// Returns a [BlobObject] for an existing blob from a path. + /// + /// The path must designate a file directly in the blobdir and + /// must use a valid blob name. That is after sanitisation the + /// name must still be the same, that means it must be valid UTF-8 + /// and not have any special characters in it. + /// + /// # Errors + /// + /// [BlobErrorKind::WrongBlobdir] is used if the path is not in + /// the blob directory. + /// + /// [BlobErrorKind::WrongName] is used if the file name does not + /// remain identical after sanitisation. + pub fn from_path( + context: &Context, + path: impl AsRef, + ) -> std::result::Result { + let rel_path = path + .as_ref() + .strip_prefix(context.get_blobdir()) + .map_err(|_| BlobError::new_wrong_blobdir(context.get_blobdir(), path.as_ref()))?; + let name = rel_path + .to_str() + .ok_or_else(|| BlobError::new_wrong_name(path.as_ref()))?; + BlobObject::from_name(context, name.to_string()) + } + + /// Returns a [BlobObject] for an existing blob. + /// + /// The `name` may optionally be prefixed with the `$BLOBDIR/` + /// prefixed, as returned by [BlobObject::as_name]. This is how + /// you want to create a [BlobObject] for a filename read from the + /// database. + /// + /// # Errors + /// + /// [BlobErrorKind::WrongName] is used if the name is not a valid + /// blobname, i.e. if [BlobObject::sanitise_name] does modify the + /// provided name. + pub fn from_name( + context: &'a Context, + name: String, + ) -> std::result::Result, BlobError> { + let name: String = match name.starts_with("$BLOBDIR/") { + true => name.splitn(2, '/').last().unwrap().to_string(), + false => name, + }; + let (stem, ext) = BlobObject::sanitise_name(name.clone()); + if format!("{}{}", stem, ext) != name.as_ref() { + return Err(BlobError::new_wrong_name(name)); + } + Ok(BlobObject { + blobdir: context.get_blobdir(), + name: format!("$BLOBDIR/{}", name), + }) + } + + /// Returns the absolute path to the blob in the filesystem. + pub fn to_abs_path(&self) -> PathBuf { + let fname = Path::new(&self.name).strip_prefix("$BLOBDIR/").unwrap(); + self.blobdir.join(fname) + } + + /// Returns the blob name, as stored in the database. + /// + /// This returns the blob in the `$BLOBDIR/` format used in + /// the database. Do not use this unless you're about to store + /// this string in the database or [Params]. Eventually even + /// those conversions should be handled by the type system. + pub fn as_name(&self) -> &str { + &self.name + } + + /// Returns the filename of the blob. + pub fn as_file_name(&self) -> &str { + self.name.rsplitn(2, '/').next().unwrap() + } + + /// The path relative in the blob directory. + pub fn as_rel_path(&self) -> &Path { + Path::new(self.as_file_name()) + } + + /// Returns the extension of the blob. + /// + /// If a blob's filename has an extension, it is always guaranteed + /// to be lowercase. + pub fn suffix(&self) -> Option<&str> { + let ext = self.name.rsplitn(2, '.').next(); + if ext == Some(&self.name) { + None + } else { + ext + } + } + + /// Create a safe name based on a messy input string. + /// + /// The safe name will be a valid filename on Unix and Windows and + /// not contain any path separators. The input can contain path + /// segments separated by either Unix or Windows path separators, + /// the rightmost non-empty segment will be used as name, + /// sanitised for special characters. + /// + /// The resulting name is returned as a tuple, the first part + /// being the stem or basename and the second being an extension, + /// including the dot. E.g. "foo.txt" is returned as `("foo", + /// ".txt")` while "bar" is returned as `("bar", "")`. + /// + /// The extension part will always be lowercased. + fn sanitise_name(mut name: String) -> (String, String) { + for part in name.rsplit('/') { + if part.len() > 0 { + name = part.to_string(); + break; + } + } + for part in name.rsplit('\\') { + if part.len() > 0 { + name = part.to_string(); + break; + } + } + let opts = sanitize_filename::Options { + truncate: true, + windows: true, + replacement: "", + }; + let clean = sanitize_filename::sanitize_with_options(name, opts); + let mut iter = clean.rsplitn(2, '.'); + let mut ext = iter.next().unwrap_or_default().to_string(); + let mut stem = iter.next().unwrap_or_default().to_string(); + ext.truncate(32); + stem.truncate(32); + match stem.len() { + 0 => (ext, "".to_string()), + _ => (stem, format!(".{}", ext).to_lowercase()), + } + } +} + +impl<'a> fmt::Display for BlobObject<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "$BLOBDIR/{}", self.name) + } +} + +/// Errors for the [BlobObject]. +/// +/// To keep the return type small and thus the happy path fast this +/// stores everything on the heap. +#[derive(Debug)] +pub struct BlobError { + inner: Box, +} + +#[derive(Debug)] +struct BlobErrorInner { + kind: BlobErrorKind, + data: BlobErrorData, + backtrace: failure::Backtrace, +} + +/// Error kind for [BlobError]. +/// +/// Each error kind has associated data in the [BlobErrorData]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BlobErrorKind { + /// Failed to create the blob. + CreateFailure, + /// Failed to write data to blob. + WriteFailure, + /// Failed to copy data to blob. + CopyFailure, + /// Blob is not in the blobdir. + WrongBlobdir, + /// Blob has a bad name. + /// + /// E.g. the name is not sanitised correctly or contains a + /// sub-directory. + WrongName, +} + +/// Associated data for each [BlobError] error kind. +/// +/// This is not stored directly on the [BlobErrorKind] so that the +/// kind can stay trivially Copy and Eq. It is however possible to +/// create a [BlobError] with mismatching [BlobErrorKind] and +/// [BlobErrorData], don't do that. +/// +/// Any blobname stored here is the bare name, without the `$BLOBDIR` +/// prefix. All data is owned so that errors do not need to be tied +/// to any lifetimes. +#[derive(Debug)] +enum BlobErrorData { + CreateFailure { + blobdir: PathBuf, + blobname: String, + cause: failure::Error, + }, + WriteFailure { + blobdir: PathBuf, + blobname: String, + cause: failure::Error, + }, + CopyFailure { + blobdir: PathBuf, + blobname: String, + src: PathBuf, + cause: failure::Error, + }, + WrongBlobdir { + blobdir: PathBuf, + src: PathBuf, + }, + WrongName { + blobname: PathBuf, + }, +} + +impl BlobError { + pub fn kind(&self) -> BlobErrorKind { + self.inner.kind + } + + fn new_create_failure( + blobdir: impl Into, + blobname: impl Into, + cause: impl Into, + ) -> BlobError { + BlobError { + inner: Box::new(BlobErrorInner { + kind: BlobErrorKind::CreateFailure, + data: BlobErrorData::CreateFailure { + blobdir: blobdir.into(), + blobname: blobname.into(), + cause: cause.into(), + }, + backtrace: failure::Backtrace::new(), + }), + } + } + + fn new_write_failure( + blobdir: impl Into, + blobname: impl Into, + cause: impl Into, + ) -> BlobError { + BlobError { + inner: Box::new(BlobErrorInner { + kind: BlobErrorKind::WriteFailure, + data: BlobErrorData::WriteFailure { + blobdir: blobdir.into(), + blobname: blobname.into(), + cause: cause.into(), + }, + backtrace: failure::Backtrace::new(), + }), + } + } + + fn new_copy_failure( + blobdir: impl Into, + blobname: impl Into, + src: impl Into, + cause: impl Into, + ) -> BlobError { + BlobError { + inner: Box::new(BlobErrorInner { + kind: BlobErrorKind::CopyFailure, + data: BlobErrorData::CopyFailure { + blobdir: blobdir.into(), + blobname: blobname.into(), + src: src.into(), + cause: cause.into(), + }, + backtrace: failure::Backtrace::new(), + }), + } + } + + fn new_wrong_blobdir(blobdir: impl Into, src: impl Into) -> BlobError { + BlobError { + inner: Box::new(BlobErrorInner { + kind: BlobErrorKind::WrongBlobdir, + data: BlobErrorData::WrongBlobdir { + blobdir: blobdir.into(), + src: src.into(), + }, + backtrace: failure::Backtrace::new(), + }), + } + } + + fn new_wrong_name(blobname: impl Into) -> BlobError { + BlobError { + inner: Box::new(BlobErrorInner { + kind: BlobErrorKind::WrongName, + data: BlobErrorData::WrongName { + blobname: blobname.into(), + }, + backtrace: failure::Backtrace::new(), + }), + } + } +} + +impl fmt::Display for BlobError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // Match on the data rather than kind, they are equivalent for + // identifying purposes but contain the actual data we need. + match &self.inner.data { + BlobErrorData::CreateFailure { + blobdir, blobname, .. + } => write!( + f, + "Failed to create blob {} in {}", + blobname, + blobdir.display() + ), + BlobErrorData::WriteFailure { + blobdir, blobname, .. + } => write!( + f, + "Failed to write data to blob {} in {}", + blobname, + blobdir.display() + ), + BlobErrorData::CopyFailure { + blobdir, + blobname, + src, + .. + } => write!( + f, + "Failed to copy data from {} to blob {} in {}", + src.display(), + blobname, + blobdir.display(), + ), + BlobErrorData::WrongBlobdir { blobdir, src } => write!( + f, + "File path {} is not in blobdir {}", + src.display(), + blobdir.display(), + ), + BlobErrorData::WrongName { blobname } => { + write!(f, "Blob has a bad name: {}", blobname.display(),) + } + } + } +} + +impl failure::Fail for BlobError { + fn cause(&self) -> Option<&dyn failure::Fail> { + match &self.inner.data { + BlobErrorData::CreateFailure { cause, .. } + | BlobErrorData::WriteFailure { cause, .. } + | BlobErrorData::CopyFailure { cause, .. } => Some(cause.as_fail()), + _ => None, + } + } + + fn backtrace(&self) -> Option<&failure::Backtrace> { + Some(&self.inner.backtrace) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + use crate::test_utils::*; + + #[test] + fn test_create() { + let t = dummy_context(); + let blob = BlobObject::create(&t.ctx, "foo", b"hello").unwrap(); + let fname = t.ctx.get_blobdir().join("foo"); + let data = fs::read(fname).unwrap(); + assert_eq!(data, b"hello"); + assert_eq!(blob.as_name(), "$BLOBDIR/foo"); + assert_eq!(blob.to_abs_path(), t.ctx.get_blobdir().join("foo")); + } + + #[test] + fn test_lowercase_ext() { + let t = dummy_context(); + let blob = BlobObject::create(&t.ctx, "foo.TXT", b"hello").unwrap(); + assert_eq!(blob.as_name(), "$BLOBDIR/foo.txt"); + } + + #[test] + fn test_as_file_name() { + let t = dummy_context(); + let blob = BlobObject::create(&t.ctx, "foo.txt", b"hello").unwrap(); + assert_eq!(blob.as_file_name(), "foo.txt"); + } + + #[test] + fn test_as_rel_path() { + let t = dummy_context(); + let blob = BlobObject::create(&t.ctx, "foo.txt", b"hello").unwrap(); + assert_eq!(blob.as_rel_path(), Path::new("foo.txt")); + } + + #[test] + fn test_suffix() { + let t = dummy_context(); + let foo = BlobObject::create(&t.ctx, "foo.txt", b"hello").unwrap(); + assert_eq!(foo.suffix(), Some("txt")); + let bar = BlobObject::create(&t.ctx, "bar", b"world").unwrap(); + assert_eq!(bar.suffix(), None); + } + + #[test] + fn test_create_dup() { + let t = dummy_context(); + BlobObject::create(&t.ctx, "foo.txt", b"hello").unwrap(); + let foo = t.ctx.get_blobdir().join("foo.txt"); + assert!(foo.exists()); + BlobObject::create(&t.ctx, "foo.txt", b"world").unwrap(); + for dirent in fs::read_dir(t.ctx.get_blobdir()).unwrap() { + let fname = dirent.unwrap().file_name(); + if fname == foo.file_name().unwrap() { + assert_eq!(fs::read(&foo).unwrap(), b"hello"); + } else { + let name = fname.to_str().unwrap(); + assert!(name.starts_with("foo")); + assert!(name.ends_with(".txt")); + } + } + } + + #[test] + fn test_create_long_names() { + let t = dummy_context(); + let s = "12312312039182039182039812039810293810293810293810293801293801293123123"; + let blob = BlobObject::create(&t.ctx, s, b"data").unwrap(); + let blobname = blob.as_name().split('/').last().unwrap(); + assert!(s.len() > blobname.len()); + } + + #[test] + fn test_create_and_copy() { + let t = dummy_context(); + let src = t.dir.path().join("src"); + fs::write(&src, b"boo").unwrap(); + let blob = BlobObject::create_and_copy(&t.ctx, &src).unwrap(); + assert_eq!(blob.as_name(), "$BLOBDIR/src"); + let data = fs::read(blob.to_abs_path()).unwrap(); + assert_eq!(data, b"boo"); + + let whoops = t.dir.path().join("whoops"); + assert!(BlobObject::create_and_copy(&t.ctx, &whoops).is_err()); + let whoops = t.ctx.get_blobdir().join("whoops"); + assert!(!whoops.exists()); + } + + #[test] + fn test_create_from_path() { + let t = dummy_context(); + + let src_ext = t.dir.path().join("external"); + fs::write(&src_ext, b"boo").unwrap(); + let blob = BlobObject::create_from_path(&t.ctx, &src_ext).unwrap(); + assert_eq!(blob.as_name(), "$BLOBDIR/external"); + let data = fs::read(blob.to_abs_path()).unwrap(); + assert_eq!(data, b"boo"); + + let src_int = t.ctx.get_blobdir().join("internal"); + fs::write(&src_int, b"boo").unwrap(); + let blob = BlobObject::create_from_path(&t.ctx, &src_int).unwrap(); + assert_eq!(blob.as_name(), "$BLOBDIR/internal"); + let data = fs::read(blob.to_abs_path()).unwrap(); + assert_eq!(data, b"boo"); + } +} diff --git a/src/chat.rs b/src/chat.rs index 580046c88..9d7726055 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -1,5 +1,6 @@ use std::path::{Path, PathBuf}; +use crate::blob::{BlobErrorKind, BlobObject}; use crate::chatlist::*; use crate::config::*; use crate::constants::*; @@ -673,30 +674,24 @@ pub fn msgtype_has_file(msgtype: Viewtype) -> bool { fn prepare_msg_common(context: &Context, chat_id: u32, msg: &mut Message) -> Result { msg.id = 0; - if msg.type_0 == Viewtype::Text { // the caller should check if the message text is empty } else if msgtype_has_file(msg.type_0) { - let path_filename = msg.param.get(Param::File); - - ensure!( - path_filename.is_some(), - "Attachment missing for message of type #{}.", - msg.type_0 - ); - - let mut path_filename = path_filename.unwrap().to_string(); - - if msg.state == MessageState::OutPreparing && !dc_is_blobdir_path(context, &path_filename) { - bail!("Files must be created in the blob-directory."); - } - - ensure!( - dc_make_rel_and_copy(context, &mut path_filename), - "Failed to copy" - ); - - msg.param.set(Param::File, &path_filename); + let param = msg.param.get(Param::File).ok_or_else(|| { + format_err!("Attachment missing for message of type #{}.", msg.type_0) + })?; + let file = ParamsFile::from_param(context, param)?; + let blob = match file { + ParamsFile::FsPath(path) => { + if msg.is_increation() { + BlobObject::from_path(context, path)? + } else { + BlobObject::create_from_path(context, path)? + } + } + ParamsFile::Blob(blob) => blob, + }; + msg.param.set(Param::File, blob.as_name()); if msg.type_0 == Viewtype::File || msg.type_0 == Viewtype::Image { // Correct the type, take care not to correct already very special // formats as GIF or VOICE. @@ -705,19 +700,21 @@ fn prepare_msg_common(context: &Context, chat_id: u32, msg: &mut Message) -> Res // - from FILE to AUDIO/VIDEO/IMAGE // - from FILE/IMAGE to GIF */ if let Some((better_type, better_mime)) = - message::guess_msgtype_from_suffix(Path::new(&path_filename)) + message::guess_msgtype_from_suffix(&blob.to_abs_path()) { msg.type_0 = better_type; msg.param.set(Param::MimeType, better_mime); } } else if !msg.param.exists(Param::MimeType) { - if let Some((_, mime)) = message::guess_msgtype_from_suffix(Path::new(&path_filename)) { + if let Some((_, mime)) = message::guess_msgtype_from_suffix(&blob.to_abs_path()) { msg.param.set(Param::MimeType, mime); } } info!( context, - "Attaching \"{}\" for message type #{}.", &path_filename, msg.type_0 + "Attaching \"{}\" for message type #{}.", + blob.to_abs_path().display(), + msg.type_0 ); } else { bail!("Cannot send messages of type #{}.", msg.type_0); @@ -726,6 +723,11 @@ fn prepare_msg_common(context: &Context, chat_id: u32, msg: &mut Message) -> Res unarchive(context, chat_id)?; let mut chat = Chat::load_from_db(context, chat_id)?; + + // The OutPreparing state is set by dc_prepare_msg() before it + // calls this function and the message is left in the OutPreparing + // state. Otherwise we got called by send_msg() and we change the + // state to OutPending. if msg.state != MessageState::OutPreparing { msg.state = MessageState::OutPending; } @@ -788,6 +790,10 @@ pub fn unarchive(context: &Context, chat_id: u32) -> Result<(), Error> { /// sending may be delayed eg. due to network problems. However, from your /// view, you're done with the message. Sooner or later it will find its way. pub fn send_msg(context: &Context, chat_id: u32, msg: &mut Message) -> Result { + // dc_prepare_msg() leaves the message state to OutPreparing, we + // only have to change the state to OutPending in this case. + // Otherwise we still have to prepare the message, which will set + // the state to OutPending. if msg.state != MessageState::OutPreparing { // automatically prepare normal messages prepare_msg_common(context, chat_id, msg)?; @@ -874,28 +880,36 @@ fn maybe_delete_draft(context: &Context, chat_id: u32) -> bool { /// Set provided message as draft message for specified chat. /// /// Return true on success, false on database error. -fn do_set_draft(context: &Context, chat_id: u32, msg: &mut Message) -> bool { +fn do_set_draft(context: &Context, chat_id: u32, msg: &mut Message) -> Result<(), Error> { match msg.type_0 { - Viewtype::Unknown => return false, - Viewtype::Text => { - if msg.text.as_ref().map_or(false, |s| s.is_empty()) { - return false; + Viewtype::Unknown => bail!("Can not set draft of unknown type."), + Viewtype::Text => match msg.text.as_ref() { + Some(text) => { + if text.is_empty() { + bail!("No text in draft"); + } } - } + None => bail!("No text in draft"), + }, _ => { - if let Some(path_filename) = msg.param.get(Param::File) { - let mut path_filename = path_filename.to_string(); - if msg.is_increation() && !dc_is_blobdir_path(context, &path_filename) { - return false; + let param = msg + .param + .get(Param::File) + .ok_or_else(|| format_err!("No file stored in params"))?; + let file = ParamsFile::from_param(context, param)?; + let blob = match file { + ParamsFile::FsPath(path) => { + if msg.is_increation() { + BlobObject::from_path(context, path)? + } else { + BlobObject::create_from_path(context, path)? + } } - if !dc_make_rel_and_copy(context, &mut path_filename) { - return false; - } - msg.param.set(Param::File, path_filename); - } + ParamsFile::Blob(blob) => blob, + }; + msg.param.set(Param::File, blob.as_name()); } } - sql::execute( context, &context.sql, @@ -912,13 +926,12 @@ fn do_set_draft(context: &Context, chat_id: u32, msg: &mut Message) -> bool { 1, ], ) - .is_ok() } // similar to as dc_set_draft() but does not emit an event fn set_draft_raw(context: &Context, chat_id: u32, msg: &mut Message) -> bool { let deleted = maybe_delete_draft(context, chat_id); - let set = do_set_draft(context, chat_id, msg); + let set = do_set_draft(context, chat_id, msg).is_ok(); // Can't inline. Both functions above must be called, no shortcut! deleted || set @@ -1657,6 +1670,11 @@ pub fn set_chat_name( Ok(()) } +/// Set a new profile image for the chat. +/// +/// The profile image can only be set when you are a member of the +/// chat. To remove the profile image pass an empty string for the +/// `new_image` parameter. #[allow(non_snake_case)] pub fn set_chat_profile_image( context: &Context, @@ -1664,63 +1682,62 @@ pub fn set_chat_profile_image( new_image: impl AsRef, // XXX use PathBuf ) -> Result<(), Error> { ensure!(chat_id > DC_CHAT_ID_LAST_SPECIAL, "Invalid chat ID"); - let mut chat = Chat::load_from_db(context, chat_id)?; - - if real_group_exists(context, chat_id) { - /* we should respect this - whatever we send to the group, it gets discarded anyway! */ - if !is_contact_in_chat(context, chat_id, DC_CONTACT_ID_SELF) { - emit_event!( - context, - Event::ErrorSelfNotInGroup( - "Cannot set chat profile image; self not in group.".into() - ) - ); - bail!("Failed to set profile image"); - } - let mut new_image_rel: String; - if !new_image.as_ref().is_empty() { - new_image_rel = new_image.as_ref().to_string(); - if !dc_make_rel_and_copy(context, &mut new_image_rel) { - bail!("Failed to get relative path for profile image"); - } - } else { - new_image_rel = "".to_string(); - } - - chat.param.set(Param::ProfileImage, &new_image_rel); - if chat.update_param(context).is_ok() { - if chat.is_promoted() { - let mut msg = Message::default(); - msg.param - .set_int(Param::Cmd, SystemMessage::GroupImageChanged as i32); - msg.type_0 = Viewtype::Text; - msg.text = Some(context.stock_system_msg( - if new_image_rel == "" { - msg.param.remove(Param::Arg); - StockMessage::MsgGrpImgDeleted - } else { - msg.param.set(Param::Arg, &new_image_rel); - StockMessage::MsgGrpImgChanged - }, - "", - "", - DC_CONTACT_ID_SELF, - )); - msg.id = send_msg(context, chat_id, &mut msg)?; - emit_event!( - context, - Event::MsgsChanged { - chat_id, - msg_id: msg.id - } - ); - } - emit_event!(context, Event::ChatModified(chat_id)); - return Ok(()); - } + ensure!( + real_group_exists(context, chat_id), + "Failed to set profile image; group does not exist" + ); + /* we should respect this - whatever we send to the group, it gets discarded anyway! */ + if !is_contact_in_chat(context, chat_id, DC_CONTACT_ID_SELF) { + emit_event!( + context, + Event::ErrorSelfNotInGroup("Cannot set chat profile image; self not in group.".into()) + ); + bail!("Failed to set profile image"); } - bail!("Failed to set profile image"); + let mut msg = Message::new(Viewtype::Text); + msg.param + .set_int(Param::Cmd, SystemMessage::GroupImageChanged as i32); + if new_image.as_ref().is_empty() { + chat.param.remove(Param::ProfileImage); + msg.param.remove(Param::Arg); + msg.text = Some(context.stock_system_msg( + StockMessage::MsgGrpImgDeleted, + "", + "", + DC_CONTACT_ID_SELF, + )); + } else { + let image_blob = BlobObject::from_path(context, Path::new(new_image.as_ref())).or_else( + |err| match err.kind() { + BlobErrorKind::WrongBlobdir => { + BlobObject::create_and_copy(context, Path::new(new_image.as_ref())) + } + _ => Err(err), + }, + )?; + chat.param.set(Param::ProfileImage, image_blob.as_name()); + msg.param.set(Param::Arg, image_blob.as_name()); + msg.text = Some(context.stock_system_msg( + StockMessage::MsgGrpImgChanged, + "", + "", + DC_CONTACT_ID_SELF, + )); + } + chat.update_param(context)?; + if chat.is_promoted() { + msg.id = send_msg(context, chat_id, &mut msg)?; + emit_event!( + context, + Event::MsgsChanged { + chat_id, + msg_id: msg.id + } + ); + } + emit_event!(context, Event::ChatModified(chat_id)); + Ok(()) } pub fn forward_msgs(context: &Context, msg_ids: &[u32], chat_id: u32) -> Result<(), Error> { diff --git a/src/context.rs b/src/context.rs index 7fd89e242..0e5753151 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1,7 +1,5 @@ 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}; @@ -11,7 +9,6 @@ use crate::chat::*; use crate::config::Config; use crate::constants::*; use crate::contact::*; -use crate::dc_tools::{dc_copy_file, dc_derive_safe_stem_ext}; use crate::error::*; use crate::events::Event; use crate::imap::*; @@ -24,7 +21,6 @@ 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] /// @@ -165,59 +161,6 @@ impl Context { self.blobdir.as_path() } - pub fn copy_to_blobdir(&self, orig_filename: impl AsRef) -> Result { - // return a $BLOBDIR/ with the content of orig_filename - // copied into it. The will be safely derived from - // orig_filename, and will not clash with existing filenames. - let dest = self.new_blob_file(&orig_filename, b"")?; - if dc_copy_file( - &self, - PathBuf::from(orig_filename.as_ref()), - PathBuf::from(&dest), - ) { - Ok(dest) - } else { - bail!("could not copy {} to {}", orig_filename.as_ref(), dest); - } - } - - pub fn new_blob_file(&self, orig_filename: impl AsRef, data: &[u8]) -> Result { - // return a $BLOBDIR/ 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)?; - let db_entry = format!("$BLOBDIR/{}", candidate_basename); - self.call_cb(Event::NewBlobFile(db_entry.clone())); - return Ok(db_entry); - } - } - bail!("out of luck to create new blob file"); - } - pub fn call_cb(&self, event: Event) -> uintptr_t { (*self.cb)(self, event) } @@ -536,7 +479,6 @@ pub fn get_version_str() -> &'static str { mod tests { use super::*; - use crate::dc_tools::*; use crate::test_utils::*; #[test] @@ -574,51 +516,6 @@ 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/hello-")); - - let x = &context.new_blob_file("xyz/hello.png", b"data").unwrap(); - assert!(dc_file_exist(&context, x)); - assert_eq!(x, "$BLOBDIR/hello.png"); - - let y = &context.new_blob_file("hello\\world.png", b"data").unwrap(); - assert!(dc_file_exist(&context, y)); - assert_eq!(y, "$BLOBDIR/world.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(); diff --git a/src/dc_mimeparser.rs b/src/dc_mimeparser.rs index 39cd38099..e6692faa6 100644 --- a/src/dc_mimeparser.rs +++ b/src/dc_mimeparser.rs @@ -13,6 +13,7 @@ use mmime::mailmime::types::*; use mmime::mailmime::*; use mmime::other::*; +use crate::blob::BlobObject; use crate::constants::Viewtype; use crate::contact::*; use crate::context::Context; @@ -771,8 +772,8 @@ impl<'a> MimeParser<'a> { desired_filename: &str, ) { /* write decoded data to new blob file */ - let bpath = match self.context.new_blob_file(desired_filename, decoded_data) { - Ok(path) => path, + let blob = match BlobObject::create(self.context, desired_filename, decoded_data) { + Ok(blob) => blob, Err(err) => { error!( self.context, @@ -786,7 +787,7 @@ impl<'a> MimeParser<'a> { part.typ = msg_type; part.mimetype = mime_type; part.bytes = decoded_data.len() as libc::c_int; - part.param.set(Param::File, bpath); + part.param.set(Param::File, blob.as_name()); if let Some(raw_mime) = raw_mime { part.param.set(Param::MimeType, raw_mime); } @@ -1220,6 +1221,7 @@ mod tests { } proptest! { + #[ignore] #[test] fn test_dc_mailmime_parse_crash_fuzzy(data in "[!-~\t ]{2000,}") { // this test doesn't exercise much of dc_mimeparser anymore diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index cf46ce15f..452b21bfb 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -10,6 +10,7 @@ use mmime::mailmime::*; use mmime::other::*; use sha2::{Digest, Sha256}; +use crate::blob::BlobObject; use crate::chat::{self, Chat}; use crate::config::Config; use crate::constants::*; @@ -1219,7 +1220,7 @@ unsafe fn create_or_lookup_group( "grp-image-change {} chat {}", X_MrGrpImageChanged, chat_id ); let mut changed = false; - let mut grpimage = "".to_string(); + let mut grpimage: Option = None; if X_MrGrpImageChanged == "0" { changed = true; } else { @@ -1228,21 +1229,32 @@ unsafe fn create_or_lookup_group( grpimage = part .param .get(Param::File) - .map(|s| s.to_string()) - .unwrap_or_else(|| "".to_string()); + .and_then(|param| ParamsFile::from_param(context, param).ok()) + .and_then(|file| match file { + ParamsFile::FsPath(path) => { + BlobObject::create_from_path(context, path).ok() + } + ParamsFile::Blob(blob) => Some(blob), + }); info!(context, "found image {:?}", grpimage); changed = true; } } } if changed { - info!(context, "New group image set to '{}'.", grpimage); + info!( + context, + "New group image set to '{}'.", + grpimage + .as_ref() + .map(|blob| blob.as_name().to_string()) + .unwrap_or_default() + ); if let Ok(mut chat) = Chat::load_from_db(context, chat_id) { - if grpimage.is_empty() { - chat.param.remove(Param::ProfileImage); - } else { - chat.param.set(Param::ProfileImage, grpimage); - } + match grpimage { + Some(blob) => chat.param.set(Param::ProfileImage, blob.as_name()), + None => chat.param.remove(Param::ProfileImage), + }; chat.update_param(context)?; send_EVENT_CHAT_MODIFIED = 1; } diff --git a/src/dc_tools.rs b/src/dc_tools.rs index 6cf4c11de..433805dc9 100644 --- a/src/dc_tools.rs +++ b/src/dc_tools.rs @@ -392,10 +392,6 @@ pub(crate) fn dc_get_abs_path>( } } -pub(crate) fn dc_file_exist(context: &Context, path: impl AsRef) -> bool { - dc_get_abs_path(context, &path).exists() -} - pub(crate) fn dc_get_filebytes(context: &Context, path: impl AsRef) -> u64 { let path_abs = dc_get_abs_path(context, &path); match fs::metadata(&path_abs) { @@ -545,41 +541,6 @@ pub(crate) fn dc_get_next_backup_path( bail!("could not create backup file, disk full?"); } -pub(crate) fn dc_is_blobdir_path(context: &Context, path: impl AsRef) -> bool { - context - .get_blobdir() - .to_str() - .map(|s| path.as_ref().starts_with(s)) - .unwrap_or_default() - || path.as_ref().starts_with("$BLOBDIR") -} - -fn dc_make_rel_path(context: &Context, path: &mut String) { - if context - .get_blobdir() - .to_str() - .map(|s| path.starts_with(s)) - .unwrap_or_default() - { - *path = path.replace( - context.get_blobdir().to_str().unwrap_or_default(), - "$BLOBDIR", - ); - } -} - -pub(crate) fn dc_make_rel_and_copy(context: &Context, path: &mut String) -> bool { - if dc_is_blobdir_path(context, &path) { - dc_make_rel_path(context, path); - return true; - } - if let Ok(blobdir_path) = context.copy_to_blobdir(&path) { - *path = blobdir_path; - return true; - } - false -} - /// Error type for the [OsStrExt] trait #[derive(Debug, Fail, PartialEq)] pub enum CStringError { @@ -1285,19 +1246,6 @@ mod tests { assert_eq!(res, Some("123-45-7@stub".into())); } - #[test] - fn test_dc_make_rel_path() { - let t = dummy_context(); - let mut foo: String = t - .ctx - .get_blobdir() - .join("foo") - .to_string_lossy() - .into_owned(); - dc_make_rel_path(&t.ctx, &mut foo); - assert_eq!(foo, format!("$BLOBDIR{}foo", std::path::MAIN_SEPARATOR)); - } - #[test] fn test_file_get_safe_basename() { assert_eq!(get_safe_basename("12312/hello"), "hello"); @@ -1315,6 +1263,11 @@ mod tests { fn test_file_handling() { let t = dummy_context(); let context = &t.ctx; + let dc_file_exist = |ctx: &Context, fname: &str| { + ctx.get_blobdir() + .join(Path::new(fname).file_name().unwrap()) + .exists() + }; assert!(!dc_delete_file(context, "$BLOBDIR/lkqwjelqkwlje")); if dc_file_exist(context, "$BLOBDIR/foobar") @@ -1338,10 +1291,6 @@ mod tests { .to_string_lossy() .to_string(); - assert!(dc_is_blobdir_path(context, &abs_path)); - - assert!(dc_is_blobdir_path(context, "$BLOBDIR/fofo",)); - assert!(!dc_is_blobdir_path(context, "/BLOBDIR/fofo",)); assert!(dc_file_exist(context, &abs_path)); assert!(dc_copy_file(context, "$BLOBDIR/foobar", "$BLOBDIR/dada",)); diff --git a/src/error.rs b/src/error.rs index bc2ca1d88..7b340e88d 100644 --- a/src/error.rs +++ b/src/error.rs @@ -30,6 +30,8 @@ pub enum Error { Base64Decode(base64::DecodeError), #[fail(display = "{:?}", _0)] FromUtf8(std::string::FromUtf8Error), + #[fail(display = "{}", _0)] + BlobError(#[cause] crate::blob::BlobError), } pub type Result = std::result::Result; @@ -94,6 +96,12 @@ impl From for Error { } } +impl From for Error { + fn from(err: crate::blob::BlobError) -> Error { + Error::BlobError(err) + } +} + #[macro_export] macro_rules! bail { ($e:expr) => { diff --git a/src/imex.rs b/src/imex.rs index 0ba37f563..288421efb 100644 --- a/src/imex.rs +++ b/src/imex.rs @@ -4,6 +4,7 @@ use std::path::{Path, PathBuf}; use num_traits::FromPrimitive; use rand::{thread_rng, Rng}; +use crate::blob::BlobObject; use crate::chat; use crate::config::Config; use crate::configure::*; @@ -122,7 +123,8 @@ fn do_initiate_key_transfer(context: &Context) -> Result { let setup_file_content = render_setup_file(context, &setup_code)?; /* encrypting may also take a while ... */ ensure!(!context.shall_stop_ongoing(), "canceled"); - let setup_file_name = context.new_blob_file( + let setup_file_blob = BlobObject::create( + context, "autocrypt-setup-message.html", setup_file_content.as_bytes(), )?; @@ -130,7 +132,7 @@ fn do_initiate_key_transfer(context: &Context) -> Result { let chat_id = chat::create_by_contact_id(context, 1)?; msg = Message::default(); msg.type_0 = Viewtype::File; - msg.param.set(Param::File, setup_file_name); + msg.param.set(Param::File, setup_file_blob.as_name()); msg.param .set(Param::MimeType, "application/autocrypt-setup"); @@ -402,7 +404,7 @@ fn import_backup(context: &Context, backup_to_import: impl AsRef) -> Resul context.sql.close(&context); dc_delete_file(context, context.get_dbfile()); ensure!( - !dc_file_exist(context, context.get_dbfile()), + !context.get_dbfile().exists(), "Cannot delete old database." ); diff --git a/src/job.rs b/src/job.rs index acaa9e3ed..63820783e 100644 --- a/src/job.rs +++ b/src/job.rs @@ -3,6 +3,7 @@ use std::time::Duration; use deltachat_derive::{FromSql, ToSql}; use rand::{thread_rng, Rng}; +use crate::blob::BlobObject; use crate::chat; use crate::config::Config; use crate::configure::*; @@ -138,8 +139,16 @@ impl Job { } } - if let Some(filename) = self.param.get(Param::File) { - if let Ok(body) = dc_read_file(context, filename) { + if let Some(filename) = self + .param + .get(Param::File) + .and_then(|param| ParamsFile::from_param(context, param).ok()) + .map(|file| match file { + ParamsFile::FsPath(path) => path, + ParamsFile::Blob(blob) => blob.to_abs_path(), + }) + { + if let Ok(body) = dc_read_file(context, &filename) { if let Some(recipients) = self.param.get(Param::Recipients) { let recipients_list = recipients .split('\x1e') @@ -568,7 +577,11 @@ pub fn job_send_msg(context: &Context, msg_id: u32) -> Result<(), Error> { .msg .param .get(Param::File) - .map(|s| s.to_string()); + .and_then(|param| ParamsFile::from_param(context, param).ok()) + .map(|file| match file { + ParamsFile::FsPath(path) => path, + ParamsFile::Blob(blob) => blob.to_abs_path(), + }); if let Some(pathNfilename) = file_param { if (mimefactory.msg.type_0 == Viewtype::Image || mimefactory.msg.type_0 == Viewtype::Gif) @@ -933,9 +946,9 @@ fn add_smtp_job(context: &Context, action: Action, mimefactory: &MimeFactory) -> (*mimefactory.out).len, ) }; - let bpath = context.new_blob_file(&mimefactory.rfc724_mid, bytes)?; + let blob = BlobObject::create(context, &mimefactory.rfc724_mid, bytes)?; let recipients = mimefactory.recipients_addr.join("\x1e"); - param.set(Param::File, &bpath); + param.set(Param::File, blob.as_name()); param.set(Param::Recipients, &recipients); job_add( context, diff --git a/src/lib.rs b/src/lib.rs index 219c07076..07c816e86 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -30,6 +30,7 @@ pub(crate) mod events; pub use events::*; mod aheader; +pub mod blob; pub mod chat; pub mod chatlist; pub mod config; diff --git a/src/message.rs b/src/message.rs index 2be05289a..46eab4e7b 100644 --- a/src/message.rs +++ b/src/message.rs @@ -147,7 +147,11 @@ impl Message { pub fn get_file(&self, context: &Context) -> Option { self.param .get(Param::File) - .map(|f| dc_get_abs_path(context, f)) + .map_or(None, |param| ParamsFile::from_param(context, param).ok()) + .map(|file| match file { + ParamsFile::FsPath(path) => path, + ParamsFile::Blob(blob) => blob.to_abs_path(), + }) } /// Check if a message has a location bound to it. @@ -238,7 +242,12 @@ impl Message { pub fn get_filebytes(&self, context: &Context) -> u64 { self.param .get(Param::File) - .map(|file| dc_get_filebytes(context, &file)) + .and_then(|param| ParamsFile::from_param(context, param).ok()) + .map(|file| match file { + ParamsFile::FsPath(path) => path, + ParamsFile::Blob(blob) => blob.to_abs_path(), + }) + .map(|path| dc_get_filebytes(context, &path)) .unwrap_or_default() } @@ -321,6 +330,14 @@ impl Message { || cmd != SystemMessage::Unknown && cmd != SystemMessage::AutocryptSetupMessage } + /// Whether the message is still being created. + /// + /// Messages with attachments might be created before the + /// attachment is ready. In this case some more restrictions on + /// the attachment apply, e.g. if the file to be attached is still + /// being written to or otherwise will still change it can not be + /// copied to the blobdir. Thus those attachments need to be + /// created immediately in the blobdir with a valid filename. pub fn is_increation(&self) -> bool { chat::msgtype_has_file(self.type_0) && self.state == MessageState::OutPreparing } @@ -812,12 +829,17 @@ pub fn get_summarytext_by_raw( .stock_str(StockMessage::AcSetupMsgSubject) .to_string() } else { - let file_name: String = if let Some(file_path) = param.get(Param::File) { - if let Some(file_name) = Path::new(file_path).file_name() { - Some(file_name.to_string_lossy().into_owned()) - } else { - None - } + let file_name: String = if let Some(param) = param.get(Param::File) { + ParamsFile::from_param(context, param) + .ok() + .map(|file| match file { + ParamsFile::FsPath(path) => path, + ParamsFile::Blob(blob) => blob.to_abs_path(), + }) + .and_then(|path| { + path.file_name() + .map(|fname| fname.to_string_lossy().into_owned()) + }) } else { None } diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 0523307aa..826f10744 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -1,4 +1,3 @@ -use std::path::Path; use std::ptr; use chrono::TimeZone; @@ -12,6 +11,7 @@ use mmime::mailmime::write_mem::*; use mmime::mmapstring::*; use mmime::other::*; +use crate::blob::BlobObject; use crate::chat::{self, Chat}; use crate::config::Config; use crate::constants::*; @@ -794,26 +794,26 @@ fn build_body_file( msg: &Message, base_name: &str, ) -> Result<(*mut Mailmime, String), Error> { - let path_filename = match msg.param.get(Param::File) { - None => { - bail!("msg has no filename"); - } - Some(path) => path, + let param = msg + .param + .get(Param::File) + .ok_or_else(|| format_err!("msg has no filename"))?; + let file = ParamsFile::from_param(context, param)?; + let blob = match file { + ParamsFile::FsPath(path) => BlobObject::create_from_path(context, path)?, + ParamsFile::Blob(blob) => blob, }; - let suffix = dc_get_filesuffix_lc(path_filename).unwrap_or_else(|| "dat".into()); + let suffix = blob.suffix().unwrap_or("dat"); - /* get file name to use for sending - (for privacy purposes, we do not transfer the original filenames eg. for images; - these names are normally not needed and contain timestamps, running numbers etc.) */ - let filename_to_send = match msg.type_0 { + // Get file name to use for sending. For privacy purposes, we do + // not transfer the original filenames eg. for images; these names + // are normally not needed and contain timestamps, running numbers + // etc. + let filename_to_send: String = match msg.type_0 { Viewtype::Voice => chrono::Utc .timestamp(msg.timestamp_sort as i64, 0) - .format(&format!("voice-message_%Y-%m-%d_%H-%M-%S.{}", suffix)) + .format(&format!("voice-message_%Y-%m-%d_%H-%M-%S.{}", &suffix)) .to_string(), - Viewtype::Audio => Path::new(path_filename) - .file_name() - .map(|c| c.to_string_lossy().to_string()) - .unwrap_or_default(), Viewtype::Image | Viewtype::Gif => format!( "{}.{}", if base_name.is_empty() { @@ -824,18 +824,14 @@ fn build_body_file( &suffix, ), Viewtype::Video => format!("video.{}", &suffix), - _ => Path::new(path_filename) - .file_name() - .map(|c| c.to_string_lossy().to_string()) - .unwrap_or_default(), + _ => blob.as_file_name().to_string(), }; /* check mimetype */ let mimetype = match msg.param.get(Param::MimeType) { Some(mtype) => mtype, None => { - let path = Path::new(path_filename); - if let Some(res) = message::guess_msgtype_from_suffix(&path) { + if let Some(res) = message::guess_msgtype_from_suffix(blob.as_rel_path()) { res.1 } else { "application/octet-stream" @@ -895,7 +891,7 @@ fn build_body_file( wrapmime::append_ct_param(content, "name", &filename_encoded)?; let mime_sub = mailmime_new_empty(content, mime_fields); - let abs_path = dc_get_abs_path(context, path_filename).to_c_string()?; + let abs_path = blob.to_abs_path().to_c_string()?; mailmime_set_body_file(mime_sub, dc_strdup(abs_path.as_ptr())); Ok((mime_sub, filename_to_send)) } @@ -912,13 +908,18 @@ pub(crate) fn vec_contains_lowercase(vec: &[String], part: &str) -> bool { } fn is_file_size_okay(context: &Context, msg: &Message) -> bool { - let mut file_size_okay = true; - let path = msg.param.get(Param::File).unwrap_or_default(); - let bytes = dc_get_filebytes(context, &path); - - if bytes > (49 * 1024 * 1024 / 4 * 3) { - file_size_okay = false; + match msg + .param + .get(Param::File) + .and_then(|param| ParamsFile::from_param(context, param).ok()) + .map(|file| match file { + ParamsFile::FsPath(path) => path, + ParamsFile::Blob(blob) => blob.to_abs_path(), + }) { + Some(path) => { + let bytes = dc_get_filebytes(context, &path); + bytes <= (49 * 1024 * 1024 / 4 * 3) + } + None => false, } - - file_size_okay } diff --git a/src/param.rs b/src/param.rs index 3e90c2ff0..e653f80c3 100644 --- a/src/param.rs +++ b/src/param.rs @@ -1,9 +1,12 @@ use std::collections::BTreeMap; use std::fmt; +use std::path::PathBuf; use std::str; use num_traits::FromPrimitive; +use crate::blob::{BlobError, BlobObject}; +use crate::context::Context; use crate::dc_mimeparser::SystemMessage; use crate::error; @@ -204,10 +207,40 @@ impl Params { } } +/// The value contained in [Param::File]. +/// +/// Because the only way to construct this object is from a valid +/// UTF-8 string it is always safe to convert the value contained +/// within the [ParamsFile::FsPath] back to a [String] or [&str]. +/// Despite the type itself does not guarantee this. +#[derive(Debug)] +pub enum ParamsFile<'c> { + FsPath(PathBuf), + Blob(BlobObject<'c>), +} + +impl<'c> ParamsFile<'c> { + /// Parse the [Param::File] value into an object. + /// + /// If the value was stored into the [Params] correctly this + /// should not fail. + pub fn from_param(context: &'c Context, src: &str) -> Result, BlobError> { + let param = match src.starts_with("$BLOBDIR/") { + true => ParamsFile::Blob(BlobObject::from_name(context, src.to_string())?), + false => ParamsFile::FsPath(PathBuf::from(src)), + }; + Ok(param) + } +} + #[cfg(test)] mod tests { use super::*; + use std::path::Path; + + use crate::test_utils::*; + #[test] fn test_dc_param() { let mut p1: Params = "\r\n\r\na=1\nf=2\n\nc = 3 ".parse().unwrap(); @@ -251,4 +284,24 @@ mod tests { .unwrap(); assert_eq!(p1.get(Param::Forwarded).unwrap(), "cli%40deltachat.de"); } + + #[test] + fn test_params_file_fs_path() { + let t = dummy_context(); + if let ParamsFile::FsPath(p) = ParamsFile::from_param(&t.ctx, "/foo/bar/baz").unwrap() { + assert_eq!(p, Path::new("/foo/bar/baz")); + } else { + assert!(false, "Wrong enum variant"); + } + } + + #[test] + fn test_params_file_blob() { + let t = dummy_context(); + if let ParamsFile::Blob(b) = ParamsFile::from_param(&t.ctx, "$BLOBDIR/foo").unwrap() { + assert_eq!(b.as_name(), "$BLOBDIR/foo"); + } else { + assert!(false, "Wrong enum variant"); + } + } }