From bee8703a66bc30e1b0e5b6252b9f49d5d4eecc26 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Sun, 16 Jul 2023 14:01:53 -0300 Subject: [PATCH] feat: Use random filename suffixes for blobstorage (#4309) --- src/blob.rs | 110 +++++++++++++++++++++++---------------- src/chat.rs | 11 ++-- src/mimeparser.rs | 10 ++-- src/param.rs | 2 +- src/receive_imf/tests.rs | 11 ++-- src/webxdc.rs | 12 +++-- 6 files changed, 92 insertions(+), 64 deletions(-) diff --git a/src/blob.rs b/src/blob.rs index 579a2ad4e4..3b340f8ee9 100644 --- a/src/blob.rs +++ b/src/blob.rs @@ -11,6 +11,7 @@ use anyhow::{format_err, Context as _, Result}; use futures::StreamExt; use image::{DynamicImage, ImageFormat, ImageOutputFormat}; use num_traits::FromPrimitive; +use regex::Regex; use tokio::io::AsyncWriteExt; use tokio::{fs, io}; use tokio_stream::wrappers::ReadDirStream; @@ -71,10 +72,16 @@ impl<'a> BlobObject<'a> { stem: &str, ext: &str, ) -> Result<(String, fs::File)> { - const MAX_ATTEMPT: u32 = 16; + const MAX_ATTEMPT: u32 = 2; let mut attempt = 0; - let mut name = format!("{stem}{ext}"); + let re = Regex::new("-0x[[:xdigit:]]{16}$")?; + let stem = if re.is_match(stem) { + stem.rsplit_once('-').context("No '-'??")?.0 + } else { + stem + }; loop { + let name = format!("{}-0x{:016x}{}", stem, rand::random::(), ext); attempt += 1; let path = dir.join(&name); match fs::OpenOptions::new() @@ -89,8 +96,6 @@ impl<'a> BlobObject<'a> { return Err(err).context("failed to create file"); } else if attempt == 1 && !dir.exists() { fs::create_dir_all(dir).await.log_err(context).ok(); - } else { - name = format!("{}-{}{}", stem, rand::random::(), ext); } } } @@ -640,32 +645,43 @@ mod tests { async fn test_create() { let t = TestContext::new().await; let blob = BlobObject::create(&t, "foo", b"hello").await.unwrap(); - let fname = t.get_blobdir().join("foo"); + let re = Regex::new("^foo-0x[[:xdigit:]]{16}$").unwrap(); + assert!(re.is_match(blob.as_file_name())); + let fname = t.get_blobdir().join(blob.as_file_name()); let data = fs::read(fname).await.unwrap(); assert_eq!(data, b"hello"); - assert_eq!(blob.as_name(), "$BLOBDIR/foo"); - assert_eq!(blob.to_abs_path(), t.get_blobdir().join("foo")); + assert_eq!( + blob.as_name(), + "$BLOBDIR/".to_string() + blob.as_file_name() + ); + assert_eq!( + blob.to_abs_path(), + t.get_blobdir().join(blob.as_file_name()) + ); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_lowercase_ext() { let t = TestContext::new().await; let blob = BlobObject::create(&t, "foo.TXT", b"hello").await.unwrap(); - assert_eq!(blob.as_name(), "$BLOBDIR/foo.txt"); + let re = Regex::new("^\\$BLOBDIR/foo-0x[[:xdigit:]]{16}.txt$").unwrap(); + assert!(re.is_match(blob.as_name())); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_as_file_name() { let t = TestContext::new().await; let blob = BlobObject::create(&t, "foo.txt", b"hello").await.unwrap(); - assert_eq!(blob.as_file_name(), "foo.txt"); + let re = Regex::new("^foo-0x[[:xdigit:]]{16}.txt$").unwrap(); + assert!(re.is_match(blob.as_file_name())); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_as_rel_path() { let t = TestContext::new().await; let blob = BlobObject::create(&t, "foo.txt", b"hello").await.unwrap(); - assert_eq!(blob.as_rel_path(), Path::new("foo.txt")); + let re = Regex::new("^foo-0x[[:xdigit:]]{16}.txt$").unwrap(); + assert!(re.is_match(blob.as_rel_path().to_str().unwrap())); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -680,30 +696,30 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_create_dup() { let t = TestContext::new().await; - BlobObject::create(&t, "foo.txt", b"hello").await.unwrap(); - let foo_path = t.get_blobdir().join("foo.txt"); + let re = Regex::new("^foo-0x[[:xdigit:]]{16}.txt$").unwrap(); + + let blob = BlobObject::create(&t, "foo.txt", b"hello").await.unwrap(); + assert!(re.is_match(blob.as_rel_path().to_str().unwrap())); + let foo_path = t.get_blobdir().join(blob.as_file_name()); assert!(foo_path.exists()); - BlobObject::create(&t, "foo.txt", b"world").await.unwrap(); - let mut dir = fs::read_dir(t.get_blobdir()).await.unwrap(); - while let Ok(Some(dirent)) = dir.next_entry().await { - let fname = dirent.file_name(); - if fname == foo_path.file_name().unwrap() { - assert_eq!(fs::read(&foo_path).await.unwrap(), b"hello"); - } else { - let name = fname.to_str().unwrap(); - assert!(name.starts_with("foo")); - assert!(name.ends_with(".txt")); - } - } + + let blob = BlobObject::create(&t, "foo.txt", b"world").await.unwrap(); + assert!(re.is_match(blob.as_rel_path().to_str().unwrap())); + let foo_path2 = t.get_blobdir().join(blob.as_file_name()); + assert!(foo_path2.exists()); + + assert!(foo_path != foo_path2); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_double_ext_preserved() { let t = TestContext::new().await; - BlobObject::create(&t, "foo.tar.gz", b"hello") + let blob = BlobObject::create(&t, "foo.tar.gz", b"hello") .await .unwrap(); - let foo_path = t.get_blobdir().join("foo.tar.gz"); + let re = Regex::new("^foo-0x[[:xdigit:]]{16}.tar.gz$").unwrap(); + assert!(re.is_match(blob.as_file_name())); + let foo_path = t.get_blobdir().join(blob.as_file_name()); assert!(foo_path.exists()); BlobObject::create(&t, "foo.tar.gz", b"world") .await @@ -737,7 +753,8 @@ mod tests { let src = t.dir.path().join("src"); fs::write(&src, b"boo").await.unwrap(); let blob = BlobObject::create_and_copy(&t, src.as_ref()).await.unwrap(); - assert_eq!(blob.as_name(), "$BLOBDIR/src"); + let re = Regex::new("^\\$BLOBDIR/src-0x[[:xdigit:]]{16}$").unwrap(); + assert!(re.is_match(blob.as_name())); let data = fs::read(blob.to_abs_path()).await.unwrap(); assert_eq!(data, b"boo"); @@ -758,7 +775,8 @@ mod tests { let blob = BlobObject::new_from_path(&t, src_ext.as_ref()) .await .unwrap(); - assert_eq!(blob.as_name(), "$BLOBDIR/external"); + let re = Regex::new("^\\$BLOBDIR/external-0x[[:xdigit:]]{16}$").unwrap(); + assert!(re.is_match(blob.as_name())); let data = fs::read(blob.to_abs_path()).await.unwrap(); assert_eq!(data, b"boo"); @@ -769,6 +787,7 @@ mod tests { let data = fs::read(blob.to_abs_path()).await.unwrap(); assert_eq!(data, b"boo"); } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_create_from_name_long() { let t = TestContext::new().await; @@ -777,10 +796,10 @@ mod tests { let blob = BlobObject::new_from_path(&t, src_ext.as_ref()) .await .unwrap(); - assert_eq!( - blob.as_name(), - "$BLOBDIR/autocrypt-setup-message-4137848473.html" - ); + let re = + Regex::new("^\\$BLOBDIR/autocrypt-setup-message-4137848473-0x[[:xdigit:]]{16}.html$") + .unwrap(); + assert!(re.is_match(blob.as_name())); } #[test] @@ -838,19 +857,20 @@ mod tests { 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(); + let avatar_blob = t.get_config(Config::Selfavatar).await.unwrap().unwrap(); + let blobdir = t.get_blobdir().to_str().unwrap(); + let re = Regex::new(&format!("^{blobdir}/avatar-0x[[:xdigit:]]{{16}}.jpg$")).unwrap(); + assert!(re.is_match(&avatar_blob)); + let avatar_blob = Path::new(&avatar_blob); 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())); check_image_size(avatar_src, 1000, 1000); check_image_size( - &avatar_blob, + avatar_blob, constants::BALANCED_AVATAR_SIZE, constants::BALANCED_AVATAR_SIZE, ); @@ -860,12 +880,12 @@ mod tests { file.metadata().await.unwrap().len() } - let blob = BlobObject::new_from_path(&t, &avatar_blob).await.unwrap(); + let blob = BlobObject::new_from_path(&t, avatar_blob).await.unwrap(); let strict_limits = true; blob.recode_to_size(&t, blob.to_abs_path(), 1000, 3000, strict_limits) .unwrap(); - assert!(file_size(&avatar_blob).await <= 3000); - assert!(file_size(&avatar_blob).await > 2000); + assert!(file_size(avatar_blob).await <= 3000); + assert!(file_size(avatar_blob).await > 2000); tokio::task::block_in_place(move || { let img = image::open(avatar_blob).unwrap(); assert!(img.width() > 130); @@ -905,18 +925,18 @@ mod tests { let avatar_src = t.dir.path().join("avatar.png"); let avatar_bytes = include_bytes!("../test-data/image/avatar64x64.png"); fs::write(&avatar_src, avatar_bytes).await.unwrap(); - let avatar_blob = t.get_blobdir().join("avatar.png"); - assert!(!avatar_blob.exists()); t.set_config(Config::Selfavatar, Some(avatar_src.to_str().unwrap())) .await .unwrap(); - assert!(avatar_blob.exists()); + let avatar_blob = t.get_config(Config::Selfavatar).await.unwrap().unwrap(); + let blobdir = t.get_blobdir().to_str().unwrap(); + let re = Regex::new(&format!("^{blobdir}/avatar-0x[[:xdigit:]]{{16}}.png$")).unwrap(); + assert!(re.is_match(&avatar_blob)); + assert!(Path::new(&avatar_blob).exists()); assert_eq!( 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())); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] diff --git a/src/chat.rs b/src/chat.rs index 6e44cb8ffd..faabfefffa 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -3840,6 +3840,7 @@ mod tests { use crate::message::delete_msgs; use crate::receive_imf::receive_imf; use crate::test_utils::{TestContext, TestContextManager}; + use regex::Regex; use tokio::fs; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -5505,7 +5506,9 @@ mod tests { let msg = bob.recv_msg(&sent_msg).await; assert_eq!(msg.chat_id, bob_chat.id); assert_eq!(msg.get_viewtype(), Viewtype::Sticker); - assert_eq!(msg.get_filename(), Some(filename.to_string())); + let (filename, ext) = filename.split_once('.').unwrap(); + let re = Regex::new(&format!("^{filename}-0x[[:xdigit:]]{{16}}.{ext}$")).unwrap(); + assert!(re.is_match(&msg.get_filename().unwrap())); assert_eq!(msg.get_width(), w); assert_eq!(msg.get_height(), h); assert!(msg.get_filebytes(&bob).await?.unwrap() > 250); @@ -6235,10 +6238,8 @@ mod tests { 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"), - msg.param.get(Param::File), - ); + let re = Regex::new("^\\$BLOBDIR/harmless_file-0x[[:xdigit:]]{16}.txt.exe$").unwrap(); + assert!(re.is_match(msg.param.get(Param::File).unwrap())); Ok(()) } } diff --git a/src/mimeparser.rs b/src/mimeparser.rs index a0cccd1b22..fc963463f5 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -2191,6 +2191,7 @@ mod tests { #![allow(clippy::indexing_slicing)] use mailparse::ParsedMail; + use regex::Regex; use super::*; use crate::{ @@ -3388,7 +3389,8 @@ On 2020-10-25, Bob wrote: assert_eq!(msg.state, MessageState::InFresh); assert_eq!(msg.get_filebytes(&t).await.unwrap().unwrap(), 2115); assert!(msg.get_file(&t).is_some()); - assert_eq!(msg.get_filename().unwrap(), "avatar64x64.png"); + assert!(msg.get_filename().unwrap().starts_with("avatar64x64")); + assert!(msg.get_filename().unwrap().ends_with(".png")); assert_eq!(msg.get_width(), 64); assert_eq!(msg.get_height(), 64); assert_eq!(msg.get_filemime().unwrap(), "image/png"); @@ -3710,10 +3712,8 @@ Message. mime_message.parts[0].msg, "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") - ); + let re = Regex::new("^\\$BLOBDIR/-0x[[:xdigit:]]{16}.eml$").unwrap(); + assert!(re.is_match(mime_message.parts[0].param.get(Param::File).unwrap())); assert_eq!(mime_message.parts[0].org_filename, Some(".eml".to_string())); diff --git a/src/param.rs b/src/param.rs index ae01d3853f..31a9a57cae 100644 --- a/src/param.rs +++ b/src/param.rs @@ -528,7 +528,7 @@ mod tests { fs::write(fname, b"boo").await.unwrap(); let blob = p.get_blob(Param::File, &t, true).await.unwrap().unwrap(); - assert_eq!(blob, BlobObject::from_name(&t, "foo".to_string()).unwrap()); + assert!(blob.as_file_name().starts_with("foo")); // Blob in blobdir, expect blob. let bar_path = t.get_blobdir().join("bar"); diff --git a/src/receive_imf/tests.rs b/src/receive_imf/tests.rs index 03823b36ab..049398fdac 100644 --- a/src/receive_imf/tests.rs +++ b/src/receive_imf/tests.rs @@ -1456,7 +1456,9 @@ async fn test_pdf_filename_simple() { .await; assert_eq!(msg.viewtype, Viewtype::File); assert_eq!(msg.text, "mail body"); - assert_eq!(msg.param.get(Param::File).unwrap(), "$BLOBDIR/simple.pdf"); + let filename = msg.param.get(Param::File).unwrap(); + assert!(filename.starts_with("$BLOBDIR/simple")); + assert!(filename.ends_with(".pdf")); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -1470,10 +1472,9 @@ async fn test_pdf_filename_continuation() { .await; assert_eq!(msg.viewtype, Viewtype::File); assert_eq!(msg.text, "mail body"); - assert_eq!( - msg.param.get(Param::File).unwrap(), - "$BLOBDIR/test pdf äöüß.pdf" - ); + let filename = msg.param.get(Param::File).unwrap(); + assert!(filename.starts_with("$BLOBDIR/test pdf äöüß")); + assert!(filename.ends_with(".pdf")); } /// HTML-images may come with many embedded images, eg. tiny icons, corners for formatting, diff --git a/src/webxdc.rs b/src/webxdc.rs index 1f32153652..46dc302fb4 100644 --- a/src/webxdc.rs +++ b/src/webxdc.rs @@ -1077,7 +1077,9 @@ mod tests { .await?; let instance = t.get_last_msg().await; assert_eq!(instance.viewtype, Viewtype::Webxdc); - assert_eq!(instance.get_filename(), Some("minimal.xdc".to_string())); + let filename = instance.get_filename().unwrap(); + assert!(filename.starts_with("minimal")); + assert!(filename.ends_with(".xdc")); receive_imf( &t, @@ -1087,7 +1089,9 @@ mod tests { .await?; let instance = t.get_last_msg().await; assert_eq!(instance.viewtype, Viewtype::File); // we require the correct extension, only a mime type is not sufficient - assert_eq!(instance.get_filename(), Some("index.html".to_string())); + let filename = instance.get_filename().unwrap(); + assert!(filename.starts_with("index")); + assert!(filename.ends_with(".html")); Ok(()) } @@ -1715,7 +1719,9 @@ mod tests { // bob receives the instance together with the initial updates in a single message let bob_instance = bob.recv_msg(&sent1).await; assert_eq!(bob_instance.viewtype, Viewtype::Webxdc); - assert_eq!(bob_instance.get_filename(), Some("minimal.xdc".to_string())); + let filename = bob_instance.get_filename().unwrap(); + assert!(filename.starts_with("minimal")); + assert!(filename.ends_with(".xdc")); assert!(sent1.payload().contains("Content-Type: application/json")); assert!(sent1.payload().contains("status-update.json")); assert!(sent1.payload().contains(r#""payload":{"foo":"bar"}"#));