diff --git a/Cargo.lock b/Cargo.lock index bf03494d29..8addf75467 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -571,9 +571,9 @@ dependencies = [ [[package]] name = "blake3" -version = "1.5.4" +version = "1.5.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d82033247fd8e890df8f740e407ad4d038debb9eb1f40533fffb32e7d17dc6f7" +checksum = "b8ee0c1824c4dea5b5f81736aff91bae041d2c07ee1192bec91054e10e3e601e" dependencies = [ "arrayref", "arrayvec", @@ -984,9 +984,9 @@ dependencies = [ [[package]] name = "constant_time_eq" -version = "0.3.0" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f7144d30dcf0fafbce74250a3963025d8d52177934239851c917d29f1df280c2" +checksum = "7c74b8349d32d297c9134b8c88677813a227df8f779daa29bfc29c183fe3dca6" [[package]] name = "convert_case" @@ -1316,6 +1316,7 @@ dependencies = [ "async-smtp", "async_zip", "base64 0.22.1", + "blake3", "brotli", "bytes", "chrono", diff --git a/Cargo.toml b/Cargo.toml index 72b3f23e7b..469d5d4ac8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -110,6 +110,7 @@ toml = "0.8" url = "2" uuid = { version = "1", features = ["serde", "v4"] } webpki-roots = "0.26.7" +blake3 = "1.5.5" [dev-dependencies] anyhow = { workspace = true, features = ["backtrace"] } # Enable `backtrace` feature in tests. diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index edff3dfb21..153cc9962a 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -4726,6 +4726,25 @@ void dc_msg_set_override_sender_name(dc_msg_t* msg, const char* name) void dc_msg_set_file (dc_msg_t* msg, const char* file, const char* filemime); +/** + * Sets the file associated with a message. + * + * If `name` is non-null, it is used as the file name + * and the actual current name of the file is ignored. + * + * In order to deduplicate files that contain the same data, + * the file will be renamed to a hash of the file data. + * The file must not be modified after this function was called. + * + * @memberof dc_msg_t + * @param msg The message object. + * @param file The file to attach. + * @param name The original filename of the attachment. + * @param filemime The MIME type of the file. NULL if you don't know or don't care. + */ +void dc_msg_set_file_and_deduplicate(dc_msg_t* msg, const char* file, const char* name, const char* filemime); + + /** * Set the dimensions associated with message object. * Typically this is the width and the height of an image or video associated using dc_msg_set_file(). diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index c9c0cf8433..c4e209c7d4 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -3813,6 +3813,33 @@ pub unsafe extern "C" fn dc_msg_set_file( ) } +#[no_mangle] +pub unsafe extern "C" fn dc_msg_set_file_and_deduplicate( + msg: *mut dc_msg_t, + file: *const libc::c_char, + name: *const libc::c_char, + filemime: *const libc::c_char, +) { + if msg.is_null() || file.is_null() { + eprintln!("ignoring careless call to dc_msg_set_file()"); + return; + } + let ffi_msg = &mut *msg; + let ctx = &*ffi_msg.context; + + ffi_msg + .message + .set_file_and_deduplicate( + ctx, + &as_path(file), + to_opt_string_lossy(name).as_deref(), + to_opt_string_lossy(filemime).as_deref(), + ) + .context("failed to set file") + .log_err(&*ffi_msg.context) + .ok(); +} + #[no_mangle] pub unsafe extern "C" fn dc_msg_set_dimension( msg: *mut dc_msg_t, diff --git a/src/blob.rs b/src/blob.rs index 78d53641da..3749249d52 100644 --- a/src/blob.rs +++ b/src/blob.rs @@ -8,7 +8,7 @@ use std::iter::FusedIterator; use std::mem; use std::path::{Path, PathBuf}; -use anyhow::{format_err, Context as _, Result}; +use anyhow::{bail, format_err, Context as _, Result}; use base64::Engine as _; use futures::StreamExt; use image::codecs::jpeg::JpegEncoder; @@ -16,7 +16,7 @@ use image::ImageReader; use image::{DynamicImage, GenericImage, GenericImageView, ImageFormat, Pixel, Rgba}; use num_traits::FromPrimitive; use tokio::io::AsyncWriteExt; -use tokio::{fs, io}; +use tokio::{fs, io, task}; use tokio_stream::wrappers::ReadDirStream; use crate::config::Config; @@ -24,6 +24,7 @@ use crate::constants::{self, MediaQuality}; use crate::context::Context; use crate::events::EventType; use crate::log::LogExt; +use crate::tools::SystemTime; /// Represents a file in the blob directory. /// @@ -74,7 +75,9 @@ impl<'a> BlobObject<'a> { Ok(blob) } - // Creates a new file, returning a tuple of the name and the handle. + /// Creates a new file, returning a tuple of the name and the handle. + /// This uses `create_new(true)` to avoid race conditions + /// when multiple files with the same name are created. async fn create_new_file( context: &Context, dir: &Path, @@ -139,6 +142,96 @@ impl<'a> BlobObject<'a> { Ok(blob) } + /// Creates a blob object from a file that already exists in the blob directory. + /// In order to deduplicate files that contain the same data, + /// the file will be renamed to a hash of the file data. + /// + /// This is done in a in way which avoids race-conditions when multiple files are + /// concurrently created. + pub fn create_and_deduplicate(context: &'a Context, src: &Path) -> Result> { + // `create_and_deduplicate{_from_bytes}()` do blocking I/O, but can still be called + // from an async context thanks to `block_in_place()`. + // Tokio's "async" I/O functions are also just thin wrappers around the blocking I/O syscalls, + // so we are doing essentially the same here. + task::block_in_place(|| { + let blobdir = context.get_blobdir(); + if !(src.starts_with(blobdir) || src.starts_with("$BLOBDIR/")) { + bail!("The file needs to be in the blob dir already and will be renamed. To attach a different file, copy it to the blobdir first."); + } + + let mut hasher = blake3::Hasher::new(); + let mut src_file = std::fs::File::open(src) + .with_context(|| format!("failed to open file {}", src.display()))?; + hasher.update_reader(&mut src_file)?; + drop(src_file); + let blob = BlobObject::from_hash(blobdir, hasher.finalize())?; + let new_path = blob.to_abs_path(); + + // This will also replace an already-existing file. + // Renaming is atomic, so this will avoid race conditions. + if std::fs::rename(src, &new_path).is_err() { + // Try a second time in case there was some temporary error. + // There is no need to try and create the blobdir since create_and_deduplicate() + // only works for files that already are in the blobdir, anyway. + std::fs::rename(src, &new_path)?; + }; + + set_readonly(&new_path).log_err(context).ok(); + context.emit_event(EventType::NewBlobFile(blob.as_name().to_string())); + Ok(blob) + }) + } + + /// Creates a new blob object with the file contents in `data`. + /// In order to deduplicate files that contain the same data, + /// the file will be renamed to a hash of the file data. + /// + /// The `data` will be written into the file without race-conditions. + /// + /// This function does blocking I/O, but it can still be called from an async context + /// because `block_in_place()` is used to leave the async runtime if necessary. + pub fn create_and_deduplicate_from_bytes( + context: &'a Context, + data: &[u8], + ) -> Result> { + task::block_in_place(|| { + let blobdir = context.get_blobdir(); + let blob = BlobObject::from_hash(blobdir, blake3::hash(data))?; + let new_path = blob.to_abs_path(); + + // This call to `std::fs::write` is thread safe because all threads write the same data. + if std::fs::write(&new_path, data).is_err() { + if new_path.exists() { + // Looks like the file is read-only and exists already + // TODO: Maybe we should check if the file contents are the same, + // or at least if the length is the same, and overwrite if not. + + // Set the file to be modified "now", so that it's not deleted during housekeeping + let f = std::fs::File::open(&new_path).context("File::open")?; + f.set_modified(SystemTime::now()).context("set_modified")?; + } else { + // Try to create the blob directory + std::fs::create_dir_all(blobdir).log_err(context).ok(); + std::fs::write(&new_path, data).context("fs::write")?; + } + } + + set_readonly(&new_path).log_err(context).ok(); + context.emit_event(EventType::NewBlobFile(blob.as_name().to_string())); + Ok(blob) + }) + } + + fn from_hash(blobdir: &Path, hash: blake3::Hash) -> Result> { + let hash = hash.to_hex(); + let hash = hash.as_str().get(0..31).context("Too short hash")?; + let blob = BlobObject { + blobdir, + name: format!("$BLOBDIR/{hash}"), + }; + 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 @@ -356,8 +449,6 @@ impl<'a> BlobObject<'a> { } pub async fn recode_to_avatar_size(&mut self, context: &Context) -> Result<()> { - let blob_abs = self.to_abs_path(); - let img_wh = match MediaQuality::from_i32(context.get_config_int(Config::MediaQuality).await?) .unwrap_or_default() @@ -370,16 +461,15 @@ impl<'a> BlobObject<'a> { let strict_limits = true; // max_bytes is 20_000 bytes: Outlook servers don't allow headers larger than 32k. // 32 / 4 * 3 = 24k if you account for base64 encoding. To be safe, we reduced this to 20k. - if let Some(new_name) = self.recode_to_size( + self.recode_to_size( context, - blob_abs, + "".to_string(), // The name of an avatar doesn't matter maybe_sticker, img_wh, 20_000, strict_limits, - )? { - self.name = new_name; - } + )?; + Ok(()) } @@ -393,9 +483,9 @@ impl<'a> BlobObject<'a> { pub async fn recode_to_image_size( &mut self, context: &Context, + name: String, maybe_sticker: &mut bool, - ) -> Result<()> { - let blob_abs = self.to_abs_path(); + ) -> Result { let (img_wh, max_bytes) = match MediaQuality::from_i32(context.get_config_int(Config::MediaQuality).await?) .unwrap_or_default() @@ -407,35 +497,36 @@ impl<'a> BlobObject<'a> { MediaQuality::Worse => (constants::WORSE_IMAGE_SIZE, constants::WORSE_IMAGE_BYTES), }; let strict_limits = false; - if let Some(new_name) = self.recode_to_size( + let new_name = self.recode_to_size( context, - blob_abs, + name, maybe_sticker, img_wh, max_bytes, strict_limits, - )? { - self.name = new_name; - } - Ok(()) + )?; + + Ok(new_name) } /// If `!strict_limits`, then if `max_bytes` is exceeded, reduce the image to `img_wh` and just /// proceed with the result. + /// TODO documentation fn recode_to_size( &mut self, context: &Context, - mut blob_abs: PathBuf, + mut name: String, maybe_sticker: &mut bool, mut img_wh: u32, max_bytes: usize, strict_limits: bool, - ) -> Result> { + ) -> Result { // Add white background only to avatars to spare the CPU. let mut add_white_bg = img_wh <= constants::BALANCED_AVATAR_SIZE; let mut no_exif = false; let no_exif_ref = &mut no_exif; - let res = tokio::task::block_in_place(move || { + let original_name = name.clone(); + let res: Result = tokio::task::block_in_place(move || { let mut file = std::fs::File::open(self.to_abs_path())?; let (nr_bytes, exif) = image_metadata(&file)?; *no_exif_ref = exif.is_none(); @@ -449,7 +540,7 @@ impl<'a> BlobObject<'a> { file.rewind()?; ImageReader::with_format( std::io::BufReader::new(&file), - ImageFormat::from_path(&blob_abs)?, + ImageFormat::from_path(self.to_abs_path())?, ) } }; @@ -457,7 +548,6 @@ impl<'a> BlobObject<'a> { let mut img = imgreader.decode().context("image decode failure")?; let orientation = exif.as_ref().map(|exif| exif_orientation(exif, context)); let mut encoded = Vec::new(); - let mut changed_name = None; if *maybe_sticker { let x_max = img.width().saturating_sub(1); @@ -469,7 +559,7 @@ impl<'a> BlobObject<'a> { || img.get_pixel(x_max, y_max).0[3] == 0); } if *maybe_sticker && exif.is_none() { - return Ok(None); + return Ok(name); } img = match orientation { @@ -566,10 +656,10 @@ impl<'a> BlobObject<'a> { if !matches!(fmt, ImageFormat::Jpeg) && matches!(ofmt, ImageOutputFormat::Jpeg { .. }) { - blob_abs = blob_abs.with_extension("jpg"); - let file_name = blob_abs.file_name().context("No image file name (???)")?; - let file_name = file_name.to_str().context("Filename is no UTF-8 (???)")?; - changed_name = Some(format!("$BLOBDIR/{file_name}")); + name = Path::new(&name) + .with_extension("jpg") + .to_string_lossy() + .into_owned(); } if encoded.is_empty() { @@ -579,11 +669,12 @@ impl<'a> BlobObject<'a> { encode_img(&img, ofmt, &mut encoded)?; } - std::fs::write(&blob_abs, &encoded) - .context("failed to write recoded blob to file")?; + self.name = BlobObject::create_and_deduplicate_from_bytes(context, &encoded) + .context("failed to write recoded blob to file")? + .name; } - Ok(changed_name) + Ok(name) }); match res { Ok(_) => res, @@ -593,7 +684,7 @@ impl<'a> BlobObject<'a> { context, "Cannot recode image, using original data: {err:#}.", ); - Ok(None) + Ok(original_name) } else { Err(err) } @@ -602,6 +693,13 @@ impl<'a> BlobObject<'a> { } } +fn set_readonly(new_path: &Path) -> Result<()> { + let mut perms = std::fs::metadata(new_path)?.permissions(); + perms.set_readonly(true); + std::fs::set_permissions(new_path, perms)?; + Ok(()) +} + /// Returns image file size and Exif. pub fn image_metadata(file: &std::fs::File) -> Result<(u64, Option)> { let len = file.metadata()?.len(); @@ -762,15 +860,21 @@ fn add_white_bg(img: &mut DynamicImage) { #[cfg(test)] mod tests { - use fs::File; + use std::time::Duration; use super::*; use crate::message::{Message, Viewtype}; + use crate::sql; use crate::test_utils::{self, TestContext}; fn check_image_size(path: impl AsRef, width: u32, height: u32) -> image::DynamicImage { tokio::task::block_in_place(move || { - let img = image::open(path).expect("failed to open image"); + let img = ImageReader::open(path) + .expect("failed to open image") + .with_guessed_format() + .expect("failed to guess format") + .decode() + .expect("failed to decode image"); assert_eq!(img.width(), width, "invalid width"); assert_eq!(img.height(), height, "invalid height"); img @@ -1008,7 +1112,7 @@ mod tests { let strict_limits = true; blob.recode_to_size( &t, - blob.to_abs_path(), + "avatar.png".to_string(), maybe_sticker, img_wh, 20_000, @@ -1016,7 +1120,12 @@ mod tests { ) .unwrap(); tokio::task::block_in_place(move || { - let img = image::open(blob.to_abs_path()).unwrap(); + let img = ImageReader::open(blob.to_abs_path()) + .unwrap() + .with_guessed_format() + .unwrap() + .decode() + .unwrap(); assert!(img.width() == img_wh); assert!(img.height() == img_wh); assert_eq!(img.get_pixel(0, 0), Rgba(color)); @@ -1026,19 +1135,25 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_selfavatar_outside_blobdir() { + async fn file_size(path_buf: &Path) -> u64 { + fs::metadata(path_buf).await.unwrap().len() + } + let t = TestContext::new().await; let avatar_src = t.dir.path().join("avatar.jpg"); let avatar_bytes = include_bytes!("../test-data/image/avatar1000x1000.jpg"); fs::write(&avatar_src, avatar_bytes).await.unwrap(); - let avatar_blob = t.get_blobdir().join("avatar.jpg"); - assert!(!avatar_blob.exists()); t.set_config(Config::Selfavatar, Some(avatar_src.to_str().unwrap())) .await .unwrap(); - assert!(avatar_blob.exists()); - assert!(fs::metadata(&avatar_blob).await.unwrap().len() < avatar_bytes.len() as u64); - let avatar_cfg = t.get_config(Config::Selfavatar).await.unwrap(); - assert_eq!(avatar_cfg, avatar_blob.to_str().map(|s| s.to_string())); + let avatar_blob = t.get_config(Config::Selfavatar).await.unwrap().unwrap(); + let avatar_path = Path::new(&avatar_blob); + assert!( + avatar_blob.ends_with("d98cd30ed8f2129bf3968420208849d"), + "The avatar filename should be its hash, put instead it's {avatar_blob}" + ); + let scaled_avatar_size = file_size(avatar_path).await; + assert!(scaled_avatar_size < avatar_bytes.len() as u64); check_image_size(avatar_src, 1000, 1000); check_image_size( @@ -1047,27 +1162,32 @@ mod tests { constants::BALANCED_AVATAR_SIZE, ); - async fn file_size(path_buf: &Path) -> u64 { - let file = File::open(path_buf).await.unwrap(); - file.metadata().await.unwrap().len() - } - - let mut blob = BlobObject::new_from_path(&t, &avatar_blob).await.unwrap(); + let mut blob = BlobObject::new_from_path(&t, avatar_path).await.unwrap(); let maybe_sticker = &mut false; let strict_limits = true; blob.recode_to_size( &t, - blob.to_abs_path(), + "avatar.jpg".to_string(), maybe_sticker, 1000, 3000, strict_limits, ) .unwrap(); - assert!(file_size(&avatar_blob).await <= 3000); - assert!(file_size(&avatar_blob).await > 2000); + let new_file_size = file_size(&blob.to_abs_path()).await; + assert!(new_file_size <= 3000); + assert!(new_file_size > 2000); + // The new file should be smaller: + assert!(new_file_size < scaled_avatar_size); + // And the original file should not be touched: + assert_eq!(file_size(avatar_path).await, scaled_avatar_size); tokio::task::block_in_place(move || { - let img = image::open(avatar_blob).unwrap(); + let img = ImageReader::open(blob.to_abs_path()) + .unwrap() + .with_guessed_format() + .unwrap() + .decode() + .unwrap(); assert!(img.width() > 130); assert_eq!(img.width(), img.height()); }); @@ -1087,9 +1207,9 @@ mod tests { .await .unwrap(); let avatar_cfg = t.get_config(Config::Selfavatar).await.unwrap().unwrap(); - assert_eq!( - avatar_cfg, - avatar_src.with_extension("png").to_str().unwrap() + assert!( + avatar_cfg.ends_with("9e7f409ac5c92b942cc4f31cee2770a"), + "Avatar file name {avatar_cfg} should end with its hash" ); check_image_size( @@ -1373,6 +1493,7 @@ mod tests { .set_config(Config::MediaQuality, Some(media_quality_config)) .await?; let file = alice.get_blobdir().join("file").with_extension(extension); + let file_name = format!("file.{extension}"); fs::write(&file, &bytes) .await @@ -1388,7 +1509,7 @@ mod tests { } let mut msg = Message::new(viewtype); - msg.set_file(file.to_str().unwrap(), None); + msg.set_file_and_deduplicate(&alice, &file, Some(&file_name), None)?; let chat = alice.create_chat(&bob).await; if set_draft { chat.id.set_draft(&alice, Some(&mut msg)).await.unwrap(); @@ -1405,6 +1526,8 @@ mod tests { alice_msg.save_file(&alice, &file_saved).await?; check_image_size(file_saved, compressed_width, compressed_height); + println!("{}", sent.payload()); + let bob_msg = bob.recv_msg(&sent).await; assert_eq!(bob_msg.get_viewtype(), Viewtype::Image); assert_eq!(bob_msg.get_width() as u32, compressed_width); @@ -1444,7 +1567,7 @@ mod tests { .await .context("failed to write file")?; let mut msg = Message::new(Viewtype::Image); - msg.set_file(file.to_str().unwrap(), None); + msg.set_file_and_deduplicate(&alice, &file, Some("file.gif"), None)?; let chat = alice.create_chat(&bob).await; let sent = alice.send_msg(chat.id, &mut msg).await; let bob_msg = bob.recv_msg(&sent).await; @@ -1480,4 +1603,77 @@ mod tests { assert_eq!(msg.get_viewtype(), Viewtype::Sticker); Ok(()) } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_create_and_deduplicate() -> Result<()> { + let t = TestContext::new().await; + + let path = t.get_blobdir().join("anyfile.dat"); + fs::write(&path, b"bla").await?; + let blob = BlobObject::create_and_deduplicate(&t, &path)?; + assert_eq!(blob.name, "$BLOBDIR/ce940175885d7b78f7b7e9f1396611f"); + assert_eq!(path.exists(), false); + + // The file should be read-only: + fs::write(&blob.to_abs_path(), b"bla blub") + .await + .unwrap_err(); + assert_eq!(fs::read(&blob.to_abs_path()).await?, b"bla"); + + fs::write(&path, b"bla").await?; + let blob2 = BlobObject::create_and_deduplicate(&t, &path)?; + assert_eq!(blob2.name, blob.name); + + let path_outside_blobdir = t.dir.path().join("anyfile.dat"); + fs::write(&path_outside_blobdir, b"bla").await?; + let blob_res = BlobObject::create_and_deduplicate(&t, &path_outside_blobdir); + assert!( + blob_res.is_err(), + "Files outside the blobdir should not be allowed in create_and_deduplicate()" + ); + + fs::write(&path, b"blabla").await?; + let blob3 = BlobObject::create_and_deduplicate(&t, &path)?; + assert_ne!(blob3.name, blob.name); + + Ok(()) + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_create_and_deduplicate_from_bytes() -> Result<()> { + let t = TestContext::new().await; + + fs::remove_dir(t.get_blobdir()).await?; + let blob = BlobObject::create_and_deduplicate_from_bytes(&t, b"bla")?; + assert_eq!(blob.name, "$BLOBDIR/ce940175885d7b78f7b7e9f1396611f"); + + // The file should be read-only: + fs::write(&blob.to_abs_path(), b"bla blub") + .await + .unwrap_err(); + assert_eq!(fs::read(&blob.to_abs_path()).await?, b"bla"); + let modified1 = blob.to_abs_path().metadata()?.modified()?; + + // Create a temporary file & shift the time for 1 hour + // so that we can later test whether everything works fine with housekeeping: + let temp_file = t.get_blobdir().join("temp.txt"); + fs::write(&temp_file, b"temporary data").await?; + SystemTime::shift(Duration::from_secs(65 * 60)); + + let blob2 = BlobObject::create_and_deduplicate_from_bytes(&t, b"bla")?; + assert_eq!(blob2.name, blob.name); + + // The modification time of the file should be updated + // so that it's not deleted during housekeeping: + let modified2 = blob.to_abs_path().metadata()?.modified()?; + assert_ne!(modified1, modified2); + sql::housekeeping(&t).await?; + assert!(blob2.to_abs_path().exists()); + assert_eq!(temp_file.exists(), false); + + let blob3 = BlobObject::create_and_deduplicate_from_bytes(&t, b"blabla")?; + assert_ne!(blob3.name, blob.name); + + Ok(()) + } } diff --git a/src/chat.rs b/src/chat.rs index 99769fbcf0..063abd8457 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -896,13 +896,12 @@ impl ChatId { .context("no file stored in params")?; msg.param.set(Param::File, blob.as_name()); if msg.viewtype == Viewtype::File { - if let Some((better_type, _)) = - message::guess_msgtype_from_suffix(&blob.to_abs_path()) - // We do not do an automatic conversion to other viewtypes here so that - // users can send images as "files" to preserve the original quality - // (usually we compress images). The remaining conversions are done by - // `prepare_msg_blob()` later. - .filter(|&(vt, _)| vt == Viewtype::Webxdc || vt == Viewtype::Vcard) + if let Some((better_type, _)) = message::guess_msgtype_from_suffix(msg) + // We do not do an automatic conversion to other viewtypes here so that + // users can send images as "files" to preserve the original quality + // (usually we compress images). The remaining conversions are done by + // `prepare_msg_blob()` later. + .filter(|&(vt, _)| vt == Viewtype::Webxdc || vt == Viewtype::Vcard) { msg.viewtype = better_type; } @@ -2698,8 +2697,7 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> { // Typical conversions: // - from FILE to AUDIO/VIDEO/IMAGE // - from FILE/IMAGE to GIF */ - if let Some((better_type, _)) = message::guess_msgtype_from_suffix(&blob.to_abs_path()) - { + if let Some((better_type, _)) = message::guess_msgtype_from_suffix(msg) { if msg.viewtype == Viewtype::Sticker { if better_type != Viewtype::Image { // UIs don't want conversions of `Sticker` to anything other than `Image`. @@ -2729,8 +2727,14 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> { && (msg.viewtype == Viewtype::Image || maybe_sticker && !msg.param.exists(Param::ForceSticker)) { - blob.recode_to_image_size(context, &mut maybe_sticker) + let new_name = blob + .recode_to_image_size( + context, + msg.get_filename().unwrap_or_else(|| "file".to_string()), + &mut maybe_sticker, + ) .await?; + msg.param.set(Param::Filename, new_name); if !maybe_sticker { msg.viewtype = Viewtype::Image; @@ -2747,7 +2751,7 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> { } if !msg.param.exists(Param::MimeType) { - if let Some((_, mime)) = message::guess_msgtype_from_suffix(&blob.to_abs_path()) { + if let Some((_, mime)) = message::guess_msgtype_from_suffix(msg) { msg.param.set(Param::MimeType, mime); } } @@ -6466,7 +6470,7 @@ mod tests { tokio::fs::write(&file, bytes).await?; let mut msg = Message::new(Viewtype::Sticker); - msg.set_file(file.to_str().unwrap(), None); + msg.set_file_and_deduplicate(&alice, &file, Some(filename), None)?; let sent_msg = alice.send_msg(alice_chat.id, &mut msg).await; let mime = sent_msg.payload(); @@ -6515,7 +6519,7 @@ mod tests { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn test_sticker_jpeg_force() { + async fn test_sticker_jpeg_force() -> Result<()> { let alice = TestContext::new_alice().await; let bob = TestContext::new_bob().await; let alice_chat = alice.create_chat(&bob).await; @@ -6530,14 +6534,17 @@ mod tests { // Images without force_sticker should be turned into [Viewtype::Image] let mut msg = Message::new(Viewtype::Sticker); - msg.set_file(file.to_str().unwrap(), None); + msg.set_file_and_deduplicate(&alice, &file, Some("sticker.jpg"), None) + .unwrap(); + let file = msg.get_file(&alice).unwrap(); let sent_msg = alice.send_msg(alice_chat.id, &mut msg).await; let msg = bob.recv_msg(&sent_msg).await; assert_eq!(msg.get_viewtype(), Viewtype::Image); // Images with `force_sticker = true` should keep [Viewtype::Sticker] let mut msg = Message::new(Viewtype::Sticker); - msg.set_file(file.to_str().unwrap(), None); + msg.set_file_and_deduplicate(&alice, &file, Some("sticker.jpg"), None) + .unwrap(); msg.force_sticker(); let sent_msg = alice.send_msg(alice_chat.id, &mut msg).await; let msg = bob.recv_msg(&sent_msg).await; @@ -6546,7 +6553,8 @@ mod tests { // Images with `force_sticker = true` should keep [Viewtype::Sticker] // even on drafted messages let mut msg = Message::new(Viewtype::Sticker); - msg.set_file(file.to_str().unwrap(), None); + msg.set_file_and_deduplicate(&alice, &file, Some("sticker.jpg"), None) + .unwrap(); msg.force_sticker(); alice_chat .id @@ -6557,6 +6565,8 @@ mod tests { let sent_msg = alice.send_msg(alice_chat.id, &mut msg).await; let msg = bob.recv_msg(&sent_msg).await; assert_eq!(msg.get_viewtype(), Viewtype::Sticker); + + Ok(()) } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -6585,7 +6595,7 @@ mod tests { let file = alice.get_blobdir().join(file_name); tokio::fs::write(&file, bytes).await?; let mut msg = Message::new(Viewtype::Sticker); - msg.set_file(file.to_str().unwrap(), None); + msg.set_file_and_deduplicate(&alice, &file, Some("sticker.jpg"), None)?; // send sticker to bob let sent_msg = alice.send_msg(alice_chat.get_id(), &mut msg).await; @@ -7180,7 +7190,7 @@ mod tests { let file = t.get_blobdir().join(name); tokio::fs::write(&file, bytes).await?; let mut msg = Message::new(msg_type); - msg.set_file(file.to_str().unwrap(), None); + msg.set_file_and_deduplicate(t, &file, Some(name), None)?; send_msg(t, chat_id, &mut msg).await } @@ -7331,18 +7341,21 @@ mod tests { Contact::create(&alice, "bob", "bob@example.net").await?, ) .await?; - let dir = tempfile::tempdir()?; - let file = dir.path().join("harmless_file.\u{202e}txt.exe"); + let file = alice.get_blobdir().join("harmless_file.\u{202e}txt.exe"); fs::write(&file, "aaa").await?; let mut msg = Message::new(Viewtype::File); - msg.set_file(file.to_str().unwrap(), None); + msg.set_file_and_deduplicate(&alice, &file, Some("harmless_file.\u{202e}txt.exe"), None)?; let msg = bob.recv_msg(&alice.send_msg(chat_id, &mut msg).await).await; // the file bob receives should not contain BIDI-control characters assert_eq!( - Some("$BLOBDIR/harmless_file.txt.exe"), + Some("$BLOBDIR/30c0f9c6a167fc2a91285c85be7ea34"), msg.param.get(Param::File), ); + assert_eq!( + Some("harmless_file.txt.exe"), + msg.param.get(Param::Filename), + ); Ok(()) } @@ -7670,7 +7683,7 @@ mod tests { let file = alice.get_blobdir().join("screenshot.png"); tokio::fs::write(&file, bytes).await?; let mut msg = Message::new(Viewtype::Image); - msg.set_file(file.to_str().unwrap(), None); + msg.set_file_and_deduplicate(&alice, &file, Some("screenshot.png"), None)?; let alice_chat = alice.create_chat(&bob).await; let sent_msg = alice.send_msg(alice_chat.get_id(), &mut msg).await; diff --git a/src/debug_logging.rs b/src/debug_logging.rs index eae36fe3d0..4b0ce441f4 100644 --- a/src/debug_logging.rs +++ b/src/debug_logging.rs @@ -100,7 +100,9 @@ pub async fn maybe_set_logging_xdc( context, msg.get_viewtype(), chat_id, - msg.param.get_path(Param::File, context).unwrap_or_default(), + msg.param + .get_path(Param::Filename, context) + .unwrap_or_default(), msg.get_id(), ) .await?; @@ -113,11 +115,11 @@ pub async fn maybe_set_logging_xdc_inner( context: &Context, viewtype: Viewtype, chat_id: ChatId, - file: Option, + filename: Option, msg_id: MsgId, ) -> anyhow::Result<()> { if viewtype == Viewtype::Webxdc { - if let Some(file) = file { + if let Some(file) = filename { if let Some(file_name) = file.file_name().and_then(|name| name.to_str()) { if file_name.starts_with("debug_logging") && file_name.ends_with(".xdc") diff --git a/src/download.rs b/src/download.rs index 0dd6d81f16..c8a75227a0 100644 --- a/src/download.rs +++ b/src/download.rs @@ -440,7 +440,7 @@ mod tests { let file = alice.get_blobdir().join("minimal.xdc"); tokio::fs::write(&file, include_bytes!("../test-data/webxdc/minimal.xdc")).await?; let mut instance = Message::new(Viewtype::File); - instance.set_file(file.to_str().unwrap(), None); + instance.set_file_and_deduplicate(&alice, &file, None, None)?; let _sent1 = alice.send_msg(chat_id, &mut instance).await; alice diff --git a/src/imex/transfer.rs b/src/imex/transfer.rs index 297bed6af3..0fe28893b1 100644 --- a/src/imex/transfer.rs +++ b/src/imex/transfer.rs @@ -394,7 +394,8 @@ mod tests { let file = ctx0.get_blobdir().join("hello.txt"); fs::write(&file, "i am attachment").await.unwrap(); let mut msg = Message::new(Viewtype::File); - msg.set_file(file.to_str().unwrap(), Some("text/plain")); + msg.set_file_and_deduplicate(&ctx0, &file, Some("hello.txt"), Some("text/plain")) + .unwrap(); send_msg(&ctx0, self_chat.id, &mut msg).await.unwrap(); // Prepare to transfer backup. @@ -428,7 +429,12 @@ mod tests { let msg = Message::load_from_db(&ctx1, *msgid).await.unwrap(); let path = msg.get_file(&ctx1).unwrap(); - assert_eq!(path.with_file_name("hello.txt"), path); + assert_eq!( + // That's the hash of the file: + path.with_file_name("ac1d2d284757656a8d41dc40aae4136"), + path + ); + assert_eq!("hello.txt", msg.get_filename().unwrap()); let text = fs::read_to_string(&path).await.unwrap(); assert_eq!(text, "i am attachment"); diff --git a/src/location.rs b/src/location.rs index babfdc28ff..2b32b19409 100644 --- a/src/location.rs +++ b/src/location.rs @@ -1074,7 +1074,7 @@ Content-Disposition: attachment; filename="location.kml" let file = alice.get_blobdir().join(file_name); tokio::fs::write(&file, bytes).await?; let mut msg = Message::new(Viewtype::Image); - msg.set_file(file.to_str().unwrap(), None); + msg.set_file_and_deduplicate(&alice, &file, Some("logo.png"), None)?; let sent = alice.send_msg(alice_chat.id, &mut msg).await; let alice_msg = Message::load_from_db(&alice, sent.sender_msg_id).await?; assert_eq!(alice_msg.has_location(), false); diff --git a/src/message.rs b/src/message.rs index 07e597955b..1a1503e237 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1,6 +1,7 @@ //! # Messages and their identifiers. use std::collections::BTreeSet; +use std::ffi::OsStr; use std::path::{Path, PathBuf}; use std::str; @@ -620,8 +621,8 @@ impl Message { pub fn get_filemime(&self) -> Option { if let Some(m) = self.param.get(Param::MimeType) { return Some(m.to_string()); - } else if let Some(file) = self.param.get(Param::File) { - if let Some((_, mime)) = guess_msgtype_from_suffix(Path::new(file)) { + } else if self.param.exists(Param::File) { + if let Some((_, mime)) = guess_msgtype_from_suffix(self) { return Some(mime.to_string()); } // we have a file but no mimetype, let's use a generic one @@ -1082,18 +1083,51 @@ impl Message { self.param.set_optional(Param::MimeType, filemime); } + /// Sets the file associated with a message. + /// + /// If `name` is Some, it is used as the file name + /// and the actual current name of the file is ignored. + /// + /// In order to deduplicate files that contain the same data, + /// the file will be renamed to a hash of the file data. + /// The file must not be modified after this function was called. + pub fn set_file_and_deduplicate( + &mut self, + context: &Context, + file: &Path, + name: Option<&str>, + filemime: Option<&str>, + ) -> Result<()> { + let blob = BlobObject::create_and_deduplicate(context, file)?; + if let Some(name) = name { + self.param.set(Param::Filename, name); + } else { + let file_name = file.file_name().map(OsStr::to_string_lossy); + self.param.set_optional(Param::Filename, file_name); + } + self.param.set(Param::File, blob.as_name()); + self.param.set_optional(Param::MimeType, filemime); + + Ok(()) + } + /// Creates a new blob and sets it as a file associated with a message. - pub async fn set_file_from_bytes( + /// + /// In order to deduplicate files that contain the same data, + /// the filename will be a hash of the file data. + /// The file must not be modified after this function was called. + pub fn set_file_from_bytes( &mut self, context: &Context, - suggested_name: &str, + name: &str, data: &[u8], filemime: Option<&str>, ) -> Result<()> { - let blob = BlobObject::create(context, suggested_name, data).await?; - self.param.set(Param::Filename, suggested_name); + let blob = BlobObject::create_and_deduplicate_from_bytes(context, data)?; + self.param.set(Param::Filename, name); self.param.set(Param::File, blob.as_name()); self.param.set_optional(Param::MimeType, filemime); + Ok(()) } @@ -1106,7 +1140,6 @@ impl Message { ); let vcard = contact::make_vcard(context, contacts).await?; self.set_file_from_bytes(context, "vcard.vcf", vcard.as_bytes(), None) - .await } /// Updates message state from the vCard attachment. @@ -1433,7 +1466,14 @@ pub async fn get_msg_read_receipts( .await } -pub(crate) fn guess_msgtype_from_suffix(path: &Path) -> Option<(Viewtype, &str)> { +pub(crate) fn guess_msgtype_from_suffix(msg: &Message) -> Option<(Viewtype, &'static str)> { + msg.param + .get(Param::Filename) + .or_else(|| msg.param.get(Param::File)) + .and_then(|file| guess_msgtype_from_path_suffix(Path::new(file))) +} + +pub(crate) fn guess_msgtype_from_path_suffix(path: &Path) -> Option<(Viewtype, &'static str)> { let extension: &str = &path.extension()?.to_str()?.to_lowercase(); let info = match extension { // before using viewtype other than Viewtype::File, @@ -2178,15 +2218,15 @@ mod tests { #[test] fn test_guess_msgtype_from_suffix() { assert_eq!( - guess_msgtype_from_suffix(Path::new("foo/bar-sth.mp3")), + guess_msgtype_from_path_suffix(Path::new("foo/bar-sth.mp3")), Some((Viewtype::Audio, "audio/mpeg")) ); assert_eq!( - guess_msgtype_from_suffix(Path::new("foo/file.html")), + guess_msgtype_from_path_suffix(Path::new("foo/file.html")), Some((Viewtype::File, "text/html")) ); assert_eq!( - guess_msgtype_from_suffix(Path::new("foo/file.xdc")), + guess_msgtype_from_path_suffix(Path::new("foo/file.xdc")), Some((Viewtype::Webxdc, "application/webxdc+zip")) ); } @@ -2557,8 +2597,7 @@ mod tests { let file_bytes = include_bytes!("../test-data/image/screenshot.png"); let mut msg = Message::new(Viewtype::Image); - msg.set_file_from_bytes(bob, "a.jpg", file_bytes, None) - .await?; + msg.set_file_from_bytes(bob, "a.jpg", file_bytes, None)?; let sent_msg = bob.send_msg(bob_chat_id, &mut msg).await; let msg = alice.recv_msg(&sent_msg).await; assert_eq!(msg.download_state, DownloadState::Available); @@ -2627,8 +2666,7 @@ mod tests { let file_bytes = include_bytes!("../test-data/image/screenshot.png"); let mut msg = Message::new(Viewtype::Image); - msg.set_file_from_bytes(bob, "a.jpg", file_bytes, None) - .await?; + msg.set_file_from_bytes(bob, "a.jpg", file_bytes, None)?; let sent_msg = bob.send_msg(bob_chat_id, &mut msg).await; let msg = alice.recv_msg(&sent_msg).await; assert_eq!(msg.download_state, DownloadState::Available); diff --git a/src/mimefactory.rs b/src/mimefactory.rs index a1e2876f95..bd2323ca61 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -1,6 +1,7 @@ //! # MIME message production. use std::collections::HashSet; +use std::path::Path; use anyhow::{bail, Context as _, Result}; use base64::Engine as _; @@ -1512,12 +1513,17 @@ pub(crate) fn wrapped_base64_encode(buf: &[u8]) -> String { } async fn build_body_file(context: &Context, msg: &Message) -> Result { + let file_name = msg.get_filename().context("msg has no file")?; + let suffix = Path::new(&file_name) + .extension() + .and_then(|e| e.to_str()) + .unwrap_or("dat"); + let blob = msg .param .get_blob(Param::File, context) .await? .context("msg has no file")?; - 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 @@ -1557,18 +1563,14 @@ async fn build_body_file(context: &Context, msg: &Message) -> Result msg - .param - .get(Param::Filename) - .unwrap_or_else(|| blob.as_file_name()) - .to_string(), + _ => file_name, }; /* check mimetype */ let mimetype: mime::Mime = match msg.param.get(Param::MimeType) { Some(mtype) => mtype.parse()?, None => { - if let Some(res) = message::guess_msgtype_from_suffix(blob.as_rel_path()) { + if let Some(res) = message::guess_msgtype_from_suffix(msg) { res.1.parse()? } else { mime::APPLICATION_OCTET_STREAM @@ -2523,8 +2525,7 @@ mod tests { // Long messages are truncated and MimeMessage::decoded_data is set for them. We need // decoded_data to check presence of the necessary headers. msg.set_text("a".repeat(constants::DC_DESIRED_TEXT_LEN + 1)); - msg.set_file_from_bytes(&bob, "foo.bar", "content".as_bytes(), None) - .await?; + msg.set_file_from_bytes(&bob, "foo.bar", "content".as_bytes(), None)?; let sent = bob.send_msg(chat, &mut msg).await; assert!(msg.get_showpadlock()); assert!(sent.payload.contains("\r\nSubject: [...]\r\n")); diff --git a/src/mimeparser.rs b/src/mimeparser.rs index c027b57664..a3e5998546 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -1371,7 +1371,7 @@ impl MimeMessage { /* we have a regular file attachment, write decoded data to new blob object */ - let blob = match BlobObject::create(context, filename, decoded_data).await { + let blob = match BlobObject::create_and_deduplicate_from_bytes(context, decoded_data) { Ok(blob) => blob, Err(err) => { error!( @@ -2041,10 +2041,12 @@ fn get_mime_type( } mime::APPLICATION => match mimetype.subtype() { mime::OCTET_STREAM => match filename { - Some(filename) => match message::guess_msgtype_from_suffix(Path::new(&filename)) { - Some((viewtype, _)) => viewtype, - None => Viewtype::File, - }, + Some(filename) => { + match message::guess_msgtype_from_path_suffix(Path::new(&filename)) { + Some((viewtype, _)) => viewtype, + None => Viewtype::File, + } + } None => Viewtype::File, }, _ => Viewtype::File, @@ -3917,8 +3919,8 @@ Message. "this is a classic email – I attached the .EML file".to_string() ); assert_eq!( - mime_message.parts[0].param.get(Param::File), - Some("$BLOBDIR/.eml") + mime_message.parts[0].param.get(Param::Filename), + Some(".eml") ); assert_eq!(mime_message.parts[0].org_filename, Some(".eml".to_string())); diff --git a/src/peer_channels.rs b/src/peer_channels.rs index feccc73880..6345fcf2e5 100644 --- a/src/peer_channels.rs +++ b/src/peer_channels.rs @@ -629,7 +629,6 @@ mod tests { include_bytes!("../test-data/webxdc/minimal.xdc"), None, ) - .await .unwrap(); send_msg(alice, alice_chat.id, &mut instance).await.unwrap(); @@ -801,7 +800,6 @@ mod tests { include_bytes!("../test-data/webxdc/minimal.xdc"), None, ) - .await .unwrap(); send_msg(alice, alice_chat.id, &mut instance).await.unwrap(); @@ -985,7 +983,6 @@ mod tests { include_bytes!("../test-data/webxdc/minimal.xdc"), None, ) - .await .unwrap(); send_msg(alice, alice_chat.id, &mut instance).await.unwrap(); let alice_webxdc = alice.get_last_msg().await; diff --git a/src/receive_imf/tests.rs b/src/receive_imf/tests.rs index 398471719e..23e8be6c65 100644 --- a/src/receive_imf/tests.rs +++ b/src/receive_imf/tests.rs @@ -1663,8 +1663,12 @@ async fn test_pdf_filename_simple() { assert_eq!(msg.viewtype, Viewtype::File); assert_eq!(msg.text, "mail body"); let file_path = msg.param.get(Param::File).unwrap(); - assert!(file_path.starts_with("$BLOBDIR/simple")); - assert!(file_path.ends_with(".pdf")); + assert_eq!( + file_path, + // That's the blake3 hash of the file content: + "$BLOBDIR/24a6af459cec5d733374aeaa19a6133" + ); + assert_eq!(msg.param.get(Param::Filename).unwrap(), "simple.pdf"); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -1679,8 +1683,8 @@ async fn test_pdf_filename_continuation() { assert_eq!(msg.viewtype, Viewtype::File); assert_eq!(msg.text, "mail body"); let file_path = msg.param.get(Param::File).unwrap(); - assert!(file_path.starts_with("$BLOBDIR/test pdf äöüß")); - assert!(file_path.ends_with(".pdf")); + assert!(file_path.starts_with("$BLOBDIR/")); + assert_eq!(msg.get_filename().unwrap(), "test pdf äöüß.pdf"); } /// HTML-images may come with many embedded images, eg. tiny icons, corners for formatting, @@ -3243,11 +3247,11 @@ async fn test_weird_and_duplicated_filenames() -> Result<()> { "a. tar.tar.gz", ] { let attachment = alice.blobdir.join(filename_sent); - let content = format!("File content of {filename_sent}"); + let content = "File content of tar.gz archive".to_string(); tokio::fs::write(&attachment, content.as_bytes()).await?; let mut msg_alice = Message::new(Viewtype::File); - msg_alice.set_file(attachment.to_str().unwrap(), None); + msg_alice.set_file_and_deduplicate(&alice, &attachment, None, None)?; let alice_chat = alice.create_chat(&bob).await; let sent = alice.send_msg(alice_chat.id, &mut msg_alice).await; println!("{}", sent.payload()); @@ -3261,9 +3265,10 @@ async fn test_weird_and_duplicated_filenames() -> Result<()> { let path = msg.get_file(t).unwrap(); let path2 = path.with_file_name("saved.txt"); msg.save_file(t, &path2).await.unwrap(); - assert!( - path.to_str().unwrap().ends_with(".tar.gz"), - "path {path:?} doesn't end with .tar.gz" + assert_eq!( + path.file_name().unwrap().to_str().unwrap(), + "79402cb76f44c5761888f9036992a76", + "The hash of the content should always be the same" ); assert_eq!(fs::read_to_string(&path).await.unwrap(), content); assert_eq!(fs::read_to_string(&path2).await.unwrap(), content); @@ -4697,8 +4702,7 @@ async fn test_create_group_with_big_msg() -> Result<()> { let bob_grp_id = create_group_chat(&bob, ProtectionStatus::Unprotected, "Group").await?; add_contact_to_chat(&bob, bob_grp_id, ba_contact).await?; let mut msg = Message::new(Viewtype::Image); - msg.set_file_from_bytes(&bob, "a.jpg", file_bytes, None) - .await?; + msg.set_file_from_bytes(&bob, "a.jpg", file_bytes, None)?; let sent_msg = bob.send_msg(bob_grp_id, &mut msg).await; assert!(!msg.get_showpadlock()); @@ -4734,8 +4738,7 @@ async fn test_create_group_with_big_msg() -> Result<()> { let bob_grp_id = create_group_chat(&bob, ProtectionStatus::Unprotected, "Group1").await?; add_contact_to_chat(&bob, bob_grp_id, ba_contact).await?; let mut msg = Message::new(Viewtype::Image); - msg.set_file_from_bytes(&bob, "a.jpg", file_bytes, None) - .await?; + msg.set_file_from_bytes(&bob, "a.jpg", file_bytes, None)?; let sent_msg = bob.send_msg(bob_grp_id, &mut msg).await; assert!(msg.get_showpadlock()); @@ -5200,8 +5203,7 @@ async fn test_prefer_references_to_downloaded_msgs() -> Result<()> { let file_bytes = include_bytes!("../../test-data/image/screenshot.gif"); let mut msg = Message::new(Viewtype::File); - msg.set_file_from_bytes(alice, "file", file_bytes, None) - .await?; + msg.set_file_from_bytes(alice, "file", file_bytes, None)?; let mut sent = alice.send_msg(alice_chat_id, &mut msg).await; sent.payload = sent .payload @@ -5213,8 +5215,7 @@ async fn test_prefer_references_to_downloaded_msgs() -> Result<()> { assert_eq!(received.chat_id, bob.get_chat(alice).await.id); let mut msg = Message::new(Viewtype::File); - msg.set_file_from_bytes(alice, "file", file_bytes, None) - .await?; + msg.set_file_from_bytes(alice, "file", file_bytes, None)?; let sent = alice.send_msg(alice_chat_id, &mut msg).await; let received = bob.recv_msg(&sent).await; assert_eq!(received.download_state, DownloadState::Available); @@ -5273,7 +5274,6 @@ async fn test_receive_vcard() -> Result<()> { .as_bytes(), None, ) - .await .unwrap(); let alice_bob_chat = alice.create_chat(bob).await; diff --git a/src/sql.rs b/src/sql.rs index 62a0b44b93..3e359d4ce1 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -260,9 +260,13 @@ impl Sql { let mut blob = BlobObject::new_from_path(context, avatar.as_ref()).await?; match blob.recode_to_avatar_size(context).await { Ok(()) => { - context - .set_config_internal(Config::Selfavatar, Some(&avatar)) - .await? + if let Some(path) = blob.to_abs_path().to_str() { + context + .set_config_internal(Config::Selfavatar, Some(path)) + .await?; + } else { + warn!(context, "Setting selfavatar failed: invalid filename (???)"); + } } Err(e) => { warn!(context, "Migrations can't recode avatar, removing. {:#}", e); diff --git a/src/summary.rs b/src/summary.rs index 72bf3395d0..d2f55f3053 100644 --- a/src/summary.rs +++ b/src/summary.rs @@ -286,6 +286,8 @@ impl Message { #[cfg(test)] mod tests { + use std::path::PathBuf; + use super::*; use crate::chat::ChatId; use crate::param::Param; @@ -305,62 +307,90 @@ mod tests { .unwrap(); let some_text = " bla \t\n\tbla\n\t".to_string(); + async fn write_file_to_blobdir(d: &TestContext) -> PathBuf { + let bytes = &[38, 209, 39, 29]; // Just some random bytes + let file = d.get_blobdir().join("random_filename_392438"); + tokio::fs::write(&file, bytes).await.unwrap(); + file + } + let msg = Message::new_text(some_text.to_string()); assert_summary_texts(&msg, ctx, "bla bla").await; // for simple text, the type is not added to the summary + let file = write_file_to_blobdir(&d).await; let mut msg = Message::new(Viewtype::Image); - msg.set_file("foo.jpg", None); + msg.set_file_and_deduplicate(&d, &file, Some("foo.jpg"), None) + .unwrap(); assert_summary_texts(&msg, ctx, "📷 Image").await; // file names are not added for images + let file = write_file_to_blobdir(&d).await; let mut msg = Message::new(Viewtype::Image); msg.set_text(some_text.to_string()); - msg.set_file("foo.jpg", None); + msg.set_file_and_deduplicate(&d, &file, Some("foo.jpg"), None) + .unwrap(); assert_summary_texts(&msg, ctx, "📷 bla bla").await; // type is visible by emoji if text is set + let file = write_file_to_blobdir(&d).await; let mut msg = Message::new(Viewtype::Video); - msg.set_file("foo.mp4", None); + msg.set_file_and_deduplicate(&d, &file, Some("foo.mp4"), None) + .unwrap(); assert_summary_texts(&msg, ctx, "🎥 Video").await; // file names are not added for videos + let file = write_file_to_blobdir(&d).await; let mut msg = Message::new(Viewtype::Video); msg.set_text(some_text.to_string()); - msg.set_file("foo.mp4", None); + msg.set_file_and_deduplicate(&d, &file, Some("foo.mp4"), None) + .unwrap(); assert_summary_texts(&msg, ctx, "🎥 bla bla").await; // type is visible by emoji if text is set + let file = write_file_to_blobdir(&d).await; let mut msg = Message::new(Viewtype::Gif); - msg.set_file("foo.gif", None); + msg.set_file_and_deduplicate(&d, &file, Some("foo.gif"), None) + .unwrap(); assert_summary_texts(&msg, ctx, "GIF").await; // file names are not added for GIFs + let file = write_file_to_blobdir(&d).await; let mut msg = Message::new(Viewtype::Gif); msg.set_text(some_text.to_string()); - msg.set_file("foo.gif", None); + msg.set_file_and_deduplicate(&d, &file, Some("foo.gif"), None) + .unwrap(); assert_summary_texts(&msg, ctx, "GIF \u{2013} bla bla").await; // file names are not added for GIFs + let file = write_file_to_blobdir(&d).await; let mut msg = Message::new(Viewtype::Sticker); - msg.set_file("foo.png", None); + msg.set_file_and_deduplicate(&d, &file, Some("foo.png"), None) + .unwrap(); assert_summary_texts(&msg, ctx, "Sticker").await; // file names are not added for stickers + let file = write_file_to_blobdir(&d).await; let mut msg = Message::new(Viewtype::Voice); - msg.set_file("foo.mp3", None); + msg.set_file_and_deduplicate(&d, &file, Some("foo.mp3"), None) + .unwrap(); assert_summary_texts(&msg, ctx, "🎤 Voice message").await; // file names are not added for voice messages + let file = write_file_to_blobdir(&d).await; let mut msg = Message::new(Viewtype::Voice); msg.set_text(some_text.clone()); - msg.set_file("foo.mp3", None); + msg.set_file_and_deduplicate(&d, &file, Some("foo.mp3"), None) + .unwrap(); assert_summary_texts(&msg, ctx, "🎤 bla bla").await; + let file = write_file_to_blobdir(&d).await; let mut msg = Message::new(Viewtype::Audio); - msg.set_file("foo.mp3", None); + msg.set_file_and_deduplicate(&d, &file, Some("foo.mp3"), None) + .unwrap(); assert_summary_texts(&msg, ctx, "🎵 foo.mp3").await; // file name is added for audio + let file = write_file_to_blobdir(&d).await; let mut msg = Message::new(Viewtype::Audio); msg.set_text(some_text.clone()); - msg.set_file("foo.mp3", None); + msg.set_file_and_deduplicate(&d, &file, Some("foo.mp3"), None) + .unwrap(); assert_summary_texts(&msg, ctx, "🎵 foo.mp3 \u{2013} bla bla").await; // file name and text added for audio let mut msg = Message::new(Viewtype::File); let bytes = include_bytes!("../test-data/webxdc/with-minimal-manifest.xdc"); msg.set_file_from_bytes(ctx, "foo.xdc", bytes, None) - .await .unwrap(); chat_id.set_draft(ctx, Some(&mut msg)).await.unwrap(); assert_eq!(msg.viewtype, Viewtype::Webxdc); @@ -369,24 +399,28 @@ mod tests { chat_id.set_draft(ctx, Some(&mut msg)).await.unwrap(); assert_summary_texts(&msg, ctx, "nice app! \u{2013} bla bla").await; + let file = write_file_to_blobdir(&d).await; let mut msg = Message::new(Viewtype::File); - msg.set_file("foo.bar", None); + msg.set_file_and_deduplicate(&d, &file, Some("foo.bar"), None) + .unwrap(); assert_summary_texts(&msg, ctx, "📎 foo.bar").await; // file name is added for files + let file = write_file_to_blobdir(&d).await; let mut msg = Message::new(Viewtype::File); msg.set_text(some_text.clone()); - msg.set_file("foo.bar", None); + msg.set_file_and_deduplicate(&d, &file, Some("foo.bar"), None) + .unwrap(); assert_summary_texts(&msg, ctx, "📎 foo.bar \u{2013} bla bla").await; // file name is added for files + let file = write_file_to_blobdir(&d).await; let mut msg = Message::new(Viewtype::VideochatInvitation); msg.set_text(some_text.clone()); - msg.set_file("foo.bar", None); + msg.set_file_and_deduplicate(&d, &file, Some("foo.bar"), None) + .unwrap(); assert_summary_texts(&msg, ctx, "Video chat invitation").await; // text is not added for videochat invitations let mut msg = Message::new(Viewtype::Vcard); - msg.set_file_from_bytes(ctx, "foo.vcf", b"", None) - .await - .unwrap(); + msg.set_file_from_bytes(ctx, "foo.vcf", b"", None).unwrap(); chat_id.set_draft(ctx, Some(&mut msg)).await.unwrap(); // If a vCard can't be parsed, the message becomes `Viewtype::File`. assert_eq!(msg.viewtype, Viewtype::File); @@ -406,7 +440,6 @@ mod tests { END:VCARD", None, ) - .await .unwrap(); chat_id.set_draft(ctx, Some(&mut msg)).await.unwrap(); assert_eq!(msg.viewtype, Viewtype::Vcard); @@ -419,9 +452,11 @@ mod tests { assert_eq!(msg.get_summary_text(ctx).await, "Forwarded: bla bla"); // for simple text, the type is not added to the summary assert_eq!(msg.get_summary_text_without_prefix(ctx).await, "bla bla"); // skipping prefix used for reactions summaries + let file = write_file_to_blobdir(&d).await; let mut msg = Message::new(Viewtype::File); msg.set_text(some_text.clone()); - msg.set_file("foo.bar", None); + msg.set_file_and_deduplicate(&d, &file, Some("foo.bar"), None) + .unwrap(); msg.param.set_int(Param::Forwarded, 1); assert_eq!( msg.get_summary_text(ctx).await, diff --git a/src/webxdc.rs b/src/webxdc.rs index 0829933008..b5fb828b18 100644 --- a/src/webxdc.rs +++ b/src/webxdc.rs @@ -238,7 +238,9 @@ const STATUS_UPDATE_SIZE_MAX: usize = 100 << 10; impl Context { /// check if a file is an acceptable webxdc for sending or receiving. pub(crate) async fn is_webxdc_file(&self, filename: &str, file: &[u8]) -> Result { + println!("is_webxdc_file"); if !filename.ends_with(WEBXDC_SUFFIX) { + println!("Ends not with suffix"); return Ok(false); } @@ -255,15 +257,13 @@ impl Context { return Ok(false); } + println!("Is webxdc"); Ok(true) } /// Ensure that a file is an acceptable webxdc for sending. pub(crate) async fn ensure_sendable_webxdc_file(&self, path: &Path) -> Result<()> { let filename = path.to_str().unwrap_or_default(); - if !filename.ends_with(WEBXDC_SUFFIX) { - bail!("{} is not a valid webxdc file", filename); - } let valid = match FsZipFileReader::new(path).await { Ok(archive) => { @@ -1049,7 +1049,7 @@ mod tests { async fn create_webxdc_instance(t: &TestContext, name: &str, bytes: &[u8]) -> Result { let mut instance = Message::new(Viewtype::File); - instance.set_file_from_bytes(t, name, bytes, None).await?; + instance.set_file_from_bytes(t, name, bytes, None)?; Ok(instance) } @@ -1078,9 +1078,7 @@ mod tests { // sending using bad extension is not working, even when setting Viewtype to webxdc let mut instance = Message::new(Viewtype::Webxdc); - instance - .set_file_from_bytes(&t, "index.html", b"ola!", None) - .await?; + instance.set_file_from_bytes(&t, "index.html", b"ola!", None)?; assert!(send_msg(&t, chat_id, &mut instance).await.is_err()); Ok(()) @@ -1105,14 +1103,12 @@ mod tests { // sending invalid .xdc as Viewtype::Webxdc should fail already on sending let mut instance = Message::new(Viewtype::Webxdc); - instance - .set_file_from_bytes( - &t, - "invalid2.xdc", - include_bytes!("../test-data/webxdc/invalid-no-zip-but-7z.xdc"), - None, - ) - .await?; + instance.set_file_from_bytes( + &t, + "invalid2.xdc", + include_bytes!("../test-data/webxdc/invalid-no-zip-but-7z.xdc"), + None, + )?; assert!(send_msg(&t, chat_id, &mut instance).await.is_err()); Ok(()) diff --git a/src/webxdc/integration.rs b/src/webxdc/integration.rs index b07600def6..fad7d20962 100644 --- a/src/webxdc/integration.rs +++ b/src/webxdc/integration.rs @@ -190,8 +190,7 @@ mod tests { "mapstest.xdc", include_bytes!("../../test-data/webxdc/mapstest-integration-unset.xdc"), None, - ) - .await?; + )?; t.send_msg(self_chat.id, &mut msg).await; assert_integration(&t, "with some icon").await?; // still the default integration @@ -202,8 +201,7 @@ mod tests { "mapstest.xdc", include_bytes!("../../test-data/webxdc/mapstest-integration-set.xdc"), None, - ) - .await?; + )?; let sent = t.send_msg(self_chat.id, &mut msg).await; let info = msg.get_webxdc_info(&t).await?; assert!(info.summary.contains("Used as map"));