Compare commits

...

4 Commits

Author SHA1 Message Date
iequidoo
562c4f8fe4 fix: Assume file extensions are 32 chars max and don't contain whitespace (#5338)
Before file extensions were also limited to 32 chars, but extra chars in the beginning were just cut
off, e.g. "file.with_lots_of_characters_behind_point_and_double_ending.tar.gz" was considered to
have an extension "d_point_and_double_ending.tar.gz". Better to take only "tar.gz" then.

Also don't include whitespace-containing parts in extensions. File extensions generally don't
contain whitespaces.
2024-04-27 01:16:01 -03:00
iequidoo
204f747a54 feat: Remove original file stem from filenames in the blobstorage (#4309)
This way filenames in the blobstorage are just random hex numbers. This also allows us to get rid of
the `sanitize-filename` dependency.

This also requires `Param::Filename` to be set to "debug_logging*.xdc" for messages containing
logging webxdc-s, otherwise they are not detected properly. This is done in "fix:
Message::set_file_from_bytes(): Set Param::Filename", so don't forget to update senders as well.
2024-04-27 01:15:56 -03:00
iequidoo
7391c8ddea feat: MsgId::get_info(): Report original filename as well 2024-04-27 01:02:31 -03:00
iequidoo
ed33f30a60 feat: Use random filename suffixes for blobstorage (#4309)
Recently there was an accident with a chatbot that replaced its avatar set from the command line
with an unrelated avatar of a contact. Both the `selfavatar` setting and the contact avatar `i`
param pointed to `$BLOBDIR/avatar.png` at the time it was detected. How this happened is unclear,
but it is possible that `avatar.png` was removed, unmounted or otherwise not detected by the core,
and the core stored avatar received from the contact as `avatar.png`, while `selfavatar` config
still pointed to `$BLOBDIR/avatar.png`.

Such bugs are unavoidable even if the core itself has no bugs as we cannot rely on blobdir not
reside on the faulty network filesystem, being incorrectly backed up and restored etc., so we should
assume that files may be randomly removed. Then there may be dangling `$BLOBDIR/...` references in
the database which may accidentally point to unrelated files, could even be an `avatar.png` file
sent to the bot in private.

To prevent such bugs, we add random filename suffixes for the blobdir objects. Thanks to the added
Param::Filename these random suffixes aren't sent over the network.
2024-04-25 21:18:38 -03:00
18 changed files with 169 additions and 195 deletions

1
Cargo.lock generated
View File

@@ -1214,7 +1214,6 @@ dependencies = [
"reqwest", "reqwest",
"rusqlite", "rusqlite",
"rust-hsluv", "rust-hsluv",
"sanitize-filename",
"serde", "serde",
"serde_json", "serde_json",
"sha-1", "sha-1",

View File

@@ -82,7 +82,6 @@ regex = { workspace = true }
reqwest = { version = "0.12.2", features = ["json"] } reqwest = { version = "0.12.2", features = ["json"] }
rusqlite = { workspace = true, features = ["sqlcipher"] } rusqlite = { workspace = true, features = ["sqlcipher"] }
rust-hsluv = "0.1" rust-hsluv = "0.1"
sanitize-filename = "0.5"
serde_json = "1" serde_json = "1"
serde = { version = "1.0", features = ["derive"] } serde = { version = "1.0", features = ["derive"] }
sha-1 = "0.10" sha-1 = "0.10"

View File

@@ -4094,7 +4094,8 @@ char* dc_msg_get_subject (const dc_msg_t* msg);
* *
* Typically files are associated with images, videos, audios, documents. * Typically files are associated with images, videos, audios, documents.
* Plain text messages do not have a file. * Plain text messages do not have a file.
* File name may be mangled. To obtain the original attachment filename use dc_msg_get_filename(). * The filename isn't meaningful, only the extension is preserved. To obtain the original attachment
* filename use dc_msg_get_filename().
* *
* @memberof dc_msg_t * @memberof dc_msg_t
* @param msg The message object. * @param msg The message object.

View File

@@ -444,7 +444,6 @@ describe('Offline Tests with unconfigured account', function () {
context.setChatProfileImage(chatId, imagePath) context.setChatProfileImage(chatId, imagePath)
const blobPath = context.getChat(chatId).getProfileImage() const blobPath = context.getChat(chatId).getProfileImage()
expect(blobPath.startsWith(blobs)).to.be.true expect(blobPath.startsWith(blobs)).to.be.true
expect(blobPath.includes('image')).to.be.true
expect(blobPath.endsWith('.jpeg')).to.be.true expect(blobPath.endsWith('.jpeg')).to.be.true
context.setChatProfileImage(chatId, null) context.setChatProfileImage(chatId, null)

View File

@@ -108,7 +108,7 @@ class Message:
@props.with_doc @props.with_doc
def filename(self): def filename(self):
"""filename if there was an attachment, otherwise empty string.""" """file path if there was an attachment, otherwise empty string."""
return from_dc_charpointer(lib.dc_msg_get_file(self._dc_msg)) return from_dc_charpointer(lib.dc_msg_get_file(self._dc_msg))
def set_file(self, path, mime_type=None): def set_file(self, path, mime_type=None):
@@ -121,7 +121,6 @@ class Message:
@props.with_doc @props.with_doc
def basename(self) -> str: def basename(self) -> str:
"""basename of the attachment if it exists, otherwise empty string.""" """basename of the attachment if it exists, otherwise empty string."""
# FIXME, it does not return basename
return from_dc_charpointer(lib.dc_msg_get_filename(self._dc_msg)) return from_dc_charpointer(lib.dc_msg_get_filename(self._dc_msg))
@props.with_doc @props.with_doc

View File

@@ -181,13 +181,11 @@ def test_send_file_twice_unicode_filename_mangling(tmp_path, acfactory, lp):
msg = send_and_receive_message() msg = send_and_receive_message()
assert msg.text == "withfile" assert msg.text == "withfile"
assert open(msg.filename).read() == "some data" assert open(msg.filename).read() == "some data"
msg.filename.index(basename)
assert msg.filename.endswith(ext) assert msg.filename.endswith(ext)
msg2 = send_and_receive_message() msg2 = send_and_receive_message()
assert msg2.text == "withfile" assert msg2.text == "withfile"
assert open(msg2.filename).read() == "some data" assert open(msg2.filename).read() == "some data"
msg2.filename.index(basename)
assert msg2.filename.endswith(ext) assert msg2.filename.endswith(ext)
assert msg.filename != msg2.filename assert msg.filename != msg2.filename
@@ -214,7 +212,6 @@ def test_send_file_html_attachment(tmp_path, acfactory, lp):
msg = ac2.get_message_by_id(ev.data2) msg = ac2.get_message_by_id(ev.data2)
assert open(msg.filename).read() == content assert open(msg.filename).read() == content
msg.filename.index(basename)
assert msg.filename.endswith(ext) assert msg.filename.endswith(ext)

View File

@@ -50,7 +50,6 @@ class TestOnlineInCreation:
src = tmp_path / "file.txt" src = tmp_path / "file.txt"
src.write_text("hello there\n") src.write_text("hello there\n")
msg = chat.send_file(str(src)) msg = chat.send_file(str(src))
assert msg.filename.startswith(os.path.join(ac1.get_blobdir(), "file"))
assert msg.filename.endswith(".txt") assert msg.filename.endswith(".txt")
def test_forward_increation(self, acfactory, data, lp): def test_forward_increation(self, acfactory, data, lp):

View File

@@ -444,27 +444,30 @@ class TestOfflineChat:
assert msg.filemime == "image/png" assert msg.filemime == "image/png"
@pytest.mark.parametrize( @pytest.mark.parametrize(
("fn", "typein", "typeout"), ("stem", "ext", "typein", "typeout"),
[ [
("r", None, "application/octet-stream"), ("r", "", None, "application/octet-stream"),
("r.txt", None, "text/plain"), ("r", ".txt", None, "text/plain"),
("r.txt", "text/plain", "text/plain"), ("r", ".txt", "text/plain", "text/plain"),
("r.txt", "image/png", "image/png"), ("r", ".txt", "image/png", "image/png"),
], ],
) )
def test_message_file(self, chat1, data, lp, fn, typein, typeout): def test_message_file(self, chat1, data, lp, stem, ext, typein, typeout):
lp.sec("sending file") lp.sec("sending file")
fn = stem + ext
fp = data.get_path(fn) fp = data.get_path(fn)
msg = chat1.send_file(fp, typein) msg = chat1.send_file(fp, typein)
assert msg assert msg
assert msg.id > 0 assert msg.id > 0
assert msg.is_file() assert msg.is_file()
assert os.path.exists(msg.filename) assert os.path.exists(msg.filename)
assert msg.filename.endswith(msg.basename) assert msg.filename.endswith(ext)
assert msg.basename == fn
assert msg.filemime == typeout assert msg.filemime == typeout
msg2 = chat1.send_file(fp, typein) msg2 = chat1.send_file(fp, typein)
assert msg2 != msg assert msg2 != msg
assert msg2.filename != msg.filename assert msg2.filename != msg.filename
assert msg2.basename == fn
def test_create_contact(self, acfactory): def test_create_contact(self, acfactory):
ac1 = acfactory.get_pseudo_configured_account() ac1 = acfactory.get_pseudo_configured_account()

View File

@@ -19,7 +19,7 @@ use tokio::{fs, io};
use tokio_stream::wrappers::ReadDirStream; use tokio_stream::wrappers::ReadDirStream;
use crate::config::Config; use crate::config::Config;
use crate::constants::{self, MediaQuality}; use crate::constants::{self, MediaQuality, BLOB_CREATE_ATTEMPTS};
use crate::context::Context; use crate::context::Context;
use crate::events::EventType; use crate::events::EventType;
use crate::log::LogExt; use crate::log::LogExt;
@@ -56,8 +56,8 @@ impl<'a> BlobObject<'a> {
data: &[u8], data: &[u8],
) -> Result<BlobObject<'a>> { ) -> Result<BlobObject<'a>> {
let blobdir = context.get_blobdir(); let blobdir = context.get_blobdir();
let (stem, ext) = BlobObject::sanitise_name(suggested_name); let ext = BlobObject::get_extension(suggested_name);
let (name, mut file) = BlobObject::create_new_file(context, blobdir, &stem, &ext).await?; let (name, mut file) = BlobObject::create_new_file(context, blobdir, &ext).await?;
file.write_all(data).await.context("file write failure")?; file.write_all(data).await.context("file write failure")?;
// workaround a bug in async-std // workaround a bug in async-std
@@ -77,13 +77,11 @@ impl<'a> BlobObject<'a> {
async fn create_new_file( async fn create_new_file(
context: &Context, context: &Context,
dir: &Path, dir: &Path,
stem: &str,
ext: &str, ext: &str,
) -> Result<(String, fs::File)> { ) -> Result<(String, fs::File)> {
const MAX_ATTEMPT: u32 = 16;
let mut attempt = 0; let mut attempt = 0;
let mut name = format!("{stem}{ext}");
loop { loop {
let name = format!("{:016x}{}", rand::random::<u64>(), ext);
attempt += 1; attempt += 1;
let path = dir.join(&name); let path = dir.join(&name);
match fs::OpenOptions::new() match fs::OpenOptions::new()
@@ -94,12 +92,10 @@ impl<'a> BlobObject<'a> {
{ {
Ok(file) => return Ok((name, file)), Ok(file) => return Ok((name, file)),
Err(err) => { Err(err) => {
if attempt >= MAX_ATTEMPT { if attempt >= BLOB_CREATE_ATTEMPTS {
return Err(err).context("failed to create file"); return Err(err).context("failed to create file");
} else if attempt == 1 && !dir.exists() { } else if attempt == 1 && !dir.exists() {
fs::create_dir_all(dir).await.log_err(context).ok(); fs::create_dir_all(dir).await.log_err(context).ok();
} else {
name = format!("{}-{}{}", stem, rand::random::<u32>(), ext);
} }
} }
} }
@@ -116,9 +112,9 @@ impl<'a> BlobObject<'a> {
let mut src_file = fs::File::open(src) let mut src_file = fs::File::open(src)
.await .await
.with_context(|| format!("failed to open file {}", src.display()))?; .with_context(|| format!("failed to open file {}", src.display()))?;
let (stem, ext) = BlobObject::sanitise_name(&src.to_string_lossy()); let ext = BlobObject::get_extension(&src.to_string_lossy());
let (name, mut dst_file) = let (name, mut dst_file) =
BlobObject::create_new_file(context, context.get_blobdir(), &stem, &ext).await?; BlobObject::create_new_file(context, context.get_blobdir(), &ext).await?;
let name_for_err = name.clone(); let name_for_err = name.clone();
if let Err(err) = io::copy(&mut src_file, &mut dst_file).await { if let Err(err) = io::copy(&mut src_file, &mut dst_file).await {
// Attempt to remove the failed file, swallow errors resulting from that. // Attempt to remove the failed file, swallow errors resulting from that.
@@ -142,10 +138,8 @@ impl<'a> BlobObject<'a> {
/// ///
/// If the source file is not a path to into the blob directory /// 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 /// the file will be copied into the blob directory first. If the
/// source file is already in the blobdir it will not be copied /// source file is already in the blobdir (but not in a subdirectory)
/// and only be created if it is a valid blobname, that is no /// it will not be copied and only be created if it is a valid blobname.
/// subdirectory is used and [BlobObject::sanitise_name] does not
/// modify the filename.
/// ///
/// Paths into the blob directory may be either defined by an absolute path /// Paths into the blob directory may be either defined by an absolute path
/// or by the relative prefix `$BLOBDIR`. /// or by the relative prefix `$BLOBDIR`.
@@ -162,8 +156,7 @@ impl<'a> BlobObject<'a> {
/// Returns a [BlobObject] for an existing blob from a path. /// Returns a [BlobObject] for an existing blob from a path.
/// ///
/// The path must designate a file directly in the blobdir and /// The path must designate a file directly in the blobdir and
/// must use a valid blob name. That is after sanitisation the /// must use a valid blob name. That means it must be valid UTF-8
/// name must still be the same, that means it must be valid UTF-8
/// and not have any special characters in it. /// and not have any special characters in it.
pub fn from_path(context: &'a Context, path: &Path) -> Result<BlobObject<'a>> { pub fn from_path(context: &'a Context, path: &Path) -> Result<BlobObject<'a>> {
let rel_path = path let rel_path = path
@@ -237,65 +230,46 @@ impl<'a> BlobObject<'a> {
} }
} }
/// Create a safe name based on a messy input string. /// Get a file extension if any, including the dot, in lower case, otherwise an empty string.
/// fn get_extension(name: &str) -> String {
/// The safe name will be a valid filename on Unix and Windows and let mut name = name;
/// 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(name: &str) -> (String, String) {
let mut name = name.to_string();
for part in name.rsplit('/') { for part in name.rsplit('/') {
if !part.is_empty() { if !part.is_empty() {
name = part.to_string(); name = part;
break; break;
} }
} }
for part in name.rsplit('\\') { for part in name.rsplit('\\') {
if !part.is_empty() { if !part.is_empty() {
name = part.to_string(); name = part;
break; break;
} }
} }
let opts = sanitize_filename::Options {
truncate: true,
windows: true,
replacement: "",
};
let clean = sanitize_filename::sanitize_with_options(name, opts);
// Let's take the tricky filename // Let's take the tricky filename
// "file.with_lots_of_characters_behind_point_and_double_ending.tar.gz" as an example. // "file.with_lots_of_characters_behind_point_and_double_ending.tar.gz" as an example.
// Split it into "file" and "with_lots_of_characters_behind_point_and_double_ending.tar.gz": // Assume that the extension is 32 chars maximum.
let mut iter = clean.splitn(2, '.'); let ext: String = name
.chars()
let stem: String = iter.next().unwrap_or_default().chars().take(64).collect();
// stem == "file"
let ext_chars = iter.next().unwrap_or_default().chars();
let ext: String = ext_chars
.rev() .rev()
.take(32) .take_while(|c| !c.is_whitespace())
.take(33)
.collect::<Vec<_>>() .collect::<Vec<_>>()
.iter() .iter()
.rev() .rev()
.collect(); .collect();
// ext == "d_point_and_double_ending.tar.gz" // ext == "nd_point_and_double_ending.tar.gz"
// Split it into "nd_point_and_double_ending" and "tar.gz":
let mut iter = ext.splitn(2, '.');
iter.next();
let ext = iter.next().unwrap_or_default();
if ext.is_empty() { if ext.is_empty() {
(stem, "".to_string()) String::new()
} else { } else {
(stem, format!(".{ext}").to_lowercase()) format!(".{ext}").to_lowercase()
// Return ("file", ".d_point_and_double_ending.tar.gz") // Return ".tar.gz".
// which is not perfect but acceptable.
} }
} }
@@ -743,6 +717,7 @@ fn add_white_bg(img: &mut DynamicImage) {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use fs::File; use fs::File;
use regex::Regex;
use super::*; use super::*;
use crate::chat::{self, create_group_chat, ProtectionStatus}; use crate::chat::{self, create_group_chat, ProtectionStatus};
@@ -762,32 +737,43 @@ mod tests {
async fn test_create() { async fn test_create() {
let t = TestContext::new().await; let t = TestContext::new().await;
let blob = BlobObject::create(&t, "foo", b"hello").await.unwrap(); let blob = BlobObject::create(&t, "foo", b"hello").await.unwrap();
let fname = t.get_blobdir().join("foo"); let re = Regex::new("^[[: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(); let data = fs::read(fname).await.unwrap();
assert_eq!(data, b"hello"); assert_eq!(data, b"hello");
assert_eq!(blob.as_name(), "$BLOBDIR/foo"); assert_eq!(
assert_eq!(blob.to_abs_path(), t.get_blobdir().join("foo")); 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)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_lowercase_ext() { async fn test_lowercase_ext() {
let t = TestContext::new().await; let t = TestContext::new().await;
let blob = BlobObject::create(&t, "foo.TXT", b"hello").await.unwrap(); let blob = BlobObject::create(&t, "foo.TXT", b"hello").await.unwrap();
assert_eq!(blob.as_name(), "$BLOBDIR/foo.txt"); let re = Regex::new("^\\$BLOBDIR/[[:xdigit:]]{16}.txt$").unwrap();
assert!(re.is_match(blob.as_name()));
} }
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_as_file_name() { async fn test_as_file_name() {
let t = TestContext::new().await; let t = TestContext::new().await;
let blob = BlobObject::create(&t, "foo.txt", b"hello").await.unwrap(); let blob = BlobObject::create(&t, "foo.txt", b"hello").await.unwrap();
assert_eq!(blob.as_file_name(), "foo.txt"); let re = Regex::new("^[[:xdigit:]]{16}.txt$").unwrap();
assert!(re.is_match(blob.as_file_name()));
} }
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_as_rel_path() { async fn test_as_rel_path() {
let t = TestContext::new().await; let t = TestContext::new().await;
let blob = BlobObject::create(&t, "foo.txt", b"hello").await.unwrap(); 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("^[[:xdigit:]]{16}.txt$").unwrap();
assert!(re.is_match(blob.as_rel_path().to_str().unwrap()));
} }
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
@@ -802,30 +788,30 @@ mod tests {
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_create_dup() { async fn test_create_dup() {
let t = TestContext::new().await; let t = TestContext::new().await;
BlobObject::create(&t, "foo.txt", b"hello").await.unwrap(); let re = Regex::new("^[[:xdigit:]]{16}.txt$").unwrap();
let foo_path = t.get_blobdir().join("foo.txt");
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()); assert!(foo_path.exists());
BlobObject::create(&t, "foo.txt", b"world").await.unwrap();
let mut dir = fs::read_dir(t.get_blobdir()).await.unwrap(); let blob = BlobObject::create(&t, "foo.txt", b"world").await.unwrap();
while let Ok(Some(dirent)) = dir.next_entry().await { assert!(re.is_match(blob.as_rel_path().to_str().unwrap()));
let fname = dirent.file_name(); let foo_path2 = t.get_blobdir().join(blob.as_file_name());
if fname == foo_path.file_name().unwrap() { assert!(foo_path2.exists());
assert_eq!(fs::read(&foo_path).await.unwrap(), b"hello");
} else { assert!(foo_path != foo_path2);
let name = fname.to_str().unwrap();
assert!(name.starts_with("foo"));
assert!(name.ends_with(".txt"));
}
}
} }
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_double_ext_preserved() { async fn test_double_ext_preserved() {
let t = TestContext::new().await; let t = TestContext::new().await;
BlobObject::create(&t, "foo.tar.gz", b"hello") let blob = BlobObject::create(&t, "foo.tar.gz", b"hello")
.await .await
.unwrap(); .unwrap();
let foo_path = t.get_blobdir().join("foo.tar.gz"); let re = Regex::new("^[[: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()); assert!(foo_path.exists());
BlobObject::create(&t, "foo.tar.gz", b"world") BlobObject::create(&t, "foo.tar.gz", b"world")
.await .await
@@ -838,7 +824,6 @@ mod tests {
} else { } else {
let name = fname.to_str().unwrap(); let name = fname.to_str().unwrap();
println!("{name}"); println!("{name}");
assert!(name.starts_with("foo"));
assert!(name.ends_with(".tar.gz")); assert!(name.ends_with(".tar.gz"));
} }
} }
@@ -859,7 +844,8 @@ mod tests {
let src = t.dir.path().join("src"); let src = t.dir.path().join("src");
fs::write(&src, b"boo").await.unwrap(); fs::write(&src, b"boo").await.unwrap();
let blob = BlobObject::create_and_copy(&t, src.as_ref()).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/[[:xdigit:]]{16}$").unwrap();
assert!(re.is_match(blob.as_name()));
let data = fs::read(blob.to_abs_path()).await.unwrap(); let data = fs::read(blob.to_abs_path()).await.unwrap();
assert_eq!(data, b"boo"); assert_eq!(data, b"boo");
@@ -880,7 +866,8 @@ mod tests {
let blob = BlobObject::new_from_path(&t, src_ext.as_ref()) let blob = BlobObject::new_from_path(&t, src_ext.as_ref())
.await .await
.unwrap(); .unwrap();
assert_eq!(blob.as_name(), "$BLOBDIR/external"); let re = Regex::new("^\\$BLOBDIR/[[:xdigit:]]{16}$").unwrap();
assert!(re.is_match(blob.as_name()));
let data = fs::read(blob.to_abs_path()).await.unwrap(); let data = fs::read(blob.to_abs_path()).await.unwrap();
assert_eq!(data, b"boo"); assert_eq!(data, b"boo");
@@ -891,19 +878,6 @@ mod tests {
let data = fs::read(blob.to_abs_path()).await.unwrap(); let data = fs::read(blob.to_abs_path()).await.unwrap();
assert_eq!(data, b"boo"); 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;
let src_ext = t.dir.path().join("autocrypt-setup-message-4137848473.html");
fs::write(&src_ext, b"boo").await.unwrap();
let blob = BlobObject::new_from_path(&t, src_ext.as_ref())
.await
.unwrap();
assert_eq!(
blob.as_name(),
"$BLOBDIR/autocrypt-setup-message-4137848473.html"
);
}
#[test] #[test]
fn test_is_blob_name() { fn test_is_blob_name() {
@@ -916,42 +890,24 @@ mod tests {
} }
#[test] #[test]
fn test_sanitise_name() { fn test_get_extension() {
let (stem, ext) = let ext = BlobObject::get_extension("Я ЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯ.txt");
BlobObject::sanitise_name("Я ЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯЯ.txt");
assert_eq!(ext, ".txt"); assert_eq!(ext, ".txt");
assert!(!stem.is_empty());
// the extensions are kept together as between stem and extension a number may be added - let ext = BlobObject::get_extension("wot.tar.gz");
// and `foo.tar.gz` should become `foo-1234.tar.gz` and not `foo.tar-1234.gz`
let (stem, ext) = BlobObject::sanitise_name("wot.tar.gz");
assert_eq!(stem, "wot");
assert_eq!(ext, ".tar.gz"); assert_eq!(ext, ".tar.gz");
let (stem, ext) = BlobObject::sanitise_name(".foo.bar"); let ext = BlobObject::get_extension(".foo.bar");
assert_eq!(stem, "");
assert_eq!(ext, ".foo.bar"); assert_eq!(ext, ".foo.bar");
let (stem, ext) = BlobObject::sanitise_name("foo?.bar"); let ext = BlobObject::get_extension("foo?.bar");
assert!(stem.contains("foo"));
assert!(!stem.contains('?'));
assert_eq!(ext, ".bar"); assert_eq!(ext, ".bar");
let (stem, ext) = BlobObject::sanitise_name("no-extension"); let ext = BlobObject::get_extension("no-extension");
assert_eq!(stem, "no-extension");
assert_eq!(ext, ""); assert_eq!(ext, "");
let (stem, ext) = BlobObject::sanitise_name("path/ignored\\this: is* forbidden?.c"); let ext = BlobObject::get_extension("path/ignored\\this: is* forbidden?.c");
assert_eq!(ext, ".c"); assert_eq!(ext, ".c");
assert!(!stem.contains("path"));
assert!(!stem.contains("ignored"));
assert!(stem.contains("this"));
assert!(stem.contains("forbidden"));
assert!(!stem.contains('/'));
assert!(!stem.contains('\\'));
assert!(!stem.contains(':'));
assert!(!stem.contains('*'));
assert!(!stem.contains('?'));
} }
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
@@ -994,19 +950,21 @@ mod tests {
let avatar_src = t.dir.path().join("avatar.jpg"); let avatar_src = t.dir.path().join("avatar.jpg");
let avatar_bytes = include_bytes!("../test-data/image/avatar1000x1000.jpg"); let avatar_bytes = include_bytes!("../test-data/image/avatar1000x1000.jpg");
fs::write(&avatar_src, avatar_bytes).await.unwrap(); 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())) t.set_config(Config::Selfavatar, Some(avatar_src.to_str().unwrap()))
.await .await
.unwrap(); .unwrap();
let avatar_blob = t.get_config(Config::Selfavatar).await.unwrap().unwrap();
let blobdir = t.get_blobdir().to_str().unwrap();
assert!(avatar_blob.starts_with(blobdir));
let re = Regex::new("[[:xdigit:]]{16}.jpg$").unwrap();
assert!(re.is_match(&avatar_blob));
let avatar_blob = Path::new(&avatar_blob);
assert!(avatar_blob.exists()); assert!(avatar_blob.exists());
assert!(fs::metadata(&avatar_blob).await.unwrap().len() < avatar_bytes.len() as u64); 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_src, 1000, 1000);
check_image_size( check_image_size(
&avatar_blob, avatar_blob,
constants::BALANCED_AVATAR_SIZE, constants::BALANCED_AVATAR_SIZE,
constants::BALANCED_AVATAR_SIZE, constants::BALANCED_AVATAR_SIZE,
); );
@@ -1016,7 +974,7 @@ mod tests {
file.metadata().await.unwrap().len() 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_blob).await.unwrap();
let maybe_sticker = &mut false; let maybe_sticker = &mut false;
let strict_limits = true; let strict_limits = true;
blob.recode_to_size( blob.recode_to_size(
@@ -1028,8 +986,8 @@ mod tests {
strict_limits, strict_limits,
) )
.unwrap(); .unwrap();
assert!(file_size(&avatar_blob).await <= 3000); assert!(file_size(avatar_blob).await <= 3000);
assert!(file_size(&avatar_blob).await > 2000); assert!(file_size(avatar_blob).await > 2000);
tokio::task::block_in_place(move || { tokio::task::block_in_place(move || {
let img = image::open(avatar_blob).unwrap(); let img = image::open(avatar_blob).unwrap();
assert!(img.width() > 130); assert!(img.width() > 130);
@@ -1069,18 +1027,19 @@ mod tests {
let avatar_src = t.dir.path().join("avatar.png"); let avatar_src = t.dir.path().join("avatar.png");
let avatar_bytes = include_bytes!("../test-data/image/avatar64x64.png"); let avatar_bytes = include_bytes!("../test-data/image/avatar64x64.png");
fs::write(&avatar_src, avatar_bytes).await.unwrap(); 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())) t.set_config(Config::Selfavatar, Some(avatar_src.to_str().unwrap()))
.await .await
.unwrap(); .unwrap();
assert!(avatar_blob.exists()); let avatar_blob = t.get_config(Config::Selfavatar).await.unwrap().unwrap();
let blobdir = t.get_blobdir().to_str().unwrap();
assert!(avatar_blob.starts_with(blobdir));
let re = Regex::new("[[:xdigit:]]{16}.png$").unwrap();
assert!(re.is_match(&avatar_blob));
assert!(Path::new(&avatar_blob).exists());
assert_eq!( assert_eq!(
fs::metadata(&avatar_blob).await.unwrap().len(), fs::metadata(&avatar_blob).await.unwrap().len(),
avatar_bytes.len() as u64 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)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]

View File

@@ -4520,6 +4520,7 @@ mod tests {
use crate::message::delete_msgs; use crate::message::delete_msgs;
use crate::receive_imf::receive_imf; use crate::receive_imf::receive_imf;
use crate::test_utils::{sync, TestContext, TestContextManager}; use crate::test_utils::{sync, TestContext, TestContextManager};
use regex::Regex;
use strum::IntoEnumIterator; use strum::IntoEnumIterator;
use tokio::fs; use tokio::fs;
@@ -7078,9 +7079,11 @@ mod tests {
// the file bob receives should not contain BIDI-control characters // the file bob receives should not contain BIDI-control characters
assert_eq!( assert_eq!(
Some("$BLOBDIR/harmless_file.txt.exe"), msg.param.get(Param::Filename).unwrap(),
msg.param.get(Param::File), "harmless_file.txt.exe"
); );
let re = Regex::new("^\\$BLOBDIR/[[:xdigit:]]{16}.txt.exe$").unwrap();
assert!(re.is_match(msg.param.get(Param::File).unwrap()));
Ok(()) Ok(())
} }

View File

@@ -211,6 +211,9 @@ pub(crate) const DC_FOLDERS_CONFIGURED_KEY: &str = "folders_configured";
// this value can be increased if the folder configuration is changed and must be redone on next program start // this value can be increased if the folder configuration is changed and must be redone on next program start
pub(crate) const DC_FOLDERS_CONFIGURED_VERSION: i32 = 4; pub(crate) const DC_FOLDERS_CONFIGURED_VERSION: i32 = 4;
// Maximum attemps to create a blob file.
pub(crate) const BLOB_CREATE_ATTEMPTS: u32 = 2;
// If more recipients are needed in SMTP's `RCPT TO:` header, the recipient list is split into // If more recipients are needed in SMTP's `RCPT TO:` header, the recipient list is split into
// chunks. This does not affect MIME's `To:` header. Can be overwritten by setting // chunks. This does not affect MIME's `To:` header. Can be overwritten by setting
// `max_smtp_rcpt_to` in the provider db. // `max_smtp_rcpt_to` in the provider db.

View File

@@ -9,7 +9,6 @@ use crate::tools::time;
use crate::webxdc::StatusUpdateItem; use crate::webxdc::StatusUpdateItem;
use async_channel::{self as channel, Receiver, Sender}; use async_channel::{self as channel, Receiver, Sender};
use serde_json::json; use serde_json::json;
use std::path::PathBuf;
use tokio::task; use tokio::task;
#[derive(Debug)] #[derive(Debug)]
@@ -98,7 +97,7 @@ pub async fn maybe_set_logging_xdc(
context, context,
msg.get_viewtype(), msg.get_viewtype(),
chat_id, chat_id,
msg.param.get_path(Param::File, context).unwrap_or_default(), msg.param.get(Param::Filename),
msg.get_id(), msg.get_id(),
) )
.await?; .await?;
@@ -111,18 +110,16 @@ pub async fn maybe_set_logging_xdc_inner(
context: &Context, context: &Context,
viewtype: Viewtype, viewtype: Viewtype,
chat_id: ChatId, chat_id: ChatId,
file: Option<PathBuf>, file_name: Option<&str>,
msg_id: MsgId, msg_id: MsgId,
) -> anyhow::Result<()> { ) -> anyhow::Result<()> {
if viewtype == Viewtype::Webxdc { if viewtype == Viewtype::Webxdc {
if let Some(file) = file { if let Some(file_name) = file_name {
if let Some(file_name) = file.file_name().and_then(|name| name.to_str()) { if file_name.starts_with("debug_logging")
if file_name.starts_with("debug_logging") && file_name.ends_with(".xdc")
&& file_name.ends_with(".xdc") && chat_id.is_self_talk(context).await?
&& chat_id.is_self_talk(context).await? {
{ set_debug_logging_xdc(context, Some(msg_id)).await?;
set_debug_logging_xdc(context, Some(msg_id)).await?;
}
} }
} }
} }

View File

@@ -651,8 +651,9 @@ mod tests {
}; };
let msg = Message::load_from_db(&ctx1, *msgid).await.unwrap(); let msg = Message::load_from_db(&ctx1, *msgid).await.unwrap();
assert_eq!(&msg.get_filename().unwrap(), "hello.txt");
let path = msg.get_file(&ctx1).unwrap(); let path = msg.get_file(&ctx1).unwrap();
assert_eq!(path.with_file_name("hello.txt"), path); assert_eq!(path.with_extension("txt"), path);
let text = fs::read_to_string(&path).await.unwrap(); let text = fs::read_to_string(&path).await.unwrap();
assert_eq!(text, "i am attachment"); assert_eq!(text, "i am attachment");

View File

@@ -304,7 +304,12 @@ WHERE id=?;
if let Some(path) = msg.get_file(context) { if let Some(path) = msg.get_file(context) {
let bytes = get_filebytes(context, &path).await?; let bytes = get_filebytes(context, &path).await?;
ret += &format!("\nFile: {}, {} bytes\n", path.display(), bytes); ret += &format!(
"\nFile: {}, name: {}, {} bytes\n",
path.display(),
msg.get_filename().unwrap_or_default(),
bytes
);
} }
if msg.viewtype != Viewtype::Text { if msg.viewtype != Viewtype::Text {
@@ -601,7 +606,8 @@ impl Message {
None None
} }
/// Returns the full path to the file associated with a message. /// Returns the full path to the file associated with a message. The filename isn't meaningful,
/// only the extension is preserved.
pub fn get_file(&self, context: &Context) -> Option<PathBuf> { pub fn get_file(&self, context: &Context) -> Option<PathBuf> {
self.param.get_path(Param::File, context).unwrap_or(None) self.param.get_path(Param::File, context).unwrap_or(None)
} }
@@ -754,8 +760,6 @@ impl Message {
} }
/// Returns original filename (as shown in chat). /// Returns original filename (as shown in chat).
///
/// To get the full path, use [`Self::get_file()`].
pub fn get_filename(&self) -> Option<String> { pub fn get_filename(&self) -> Option<String> {
if let Some(name) = self.param.get(Param::Filename) { if let Some(name) = self.param.get(Param::Filename) {
return Some(name.to_string()); return Some(name.to_string());

View File

@@ -2254,6 +2254,7 @@ mod tests {
#![allow(clippy::indexing_slicing)] #![allow(clippy::indexing_slicing)]
use mailparse::ParsedMail; use mailparse::ParsedMail;
use regex::Regex;
use super::*; use super::*;
use crate::{ use crate::{
@@ -3454,7 +3455,7 @@ On 2020-10-25, Bob wrote:
assert_eq!(msg.chat_blocked, Blocked::Request); assert_eq!(msg.chat_blocked, Blocked::Request);
assert_eq!(msg.state, MessageState::InFresh); assert_eq!(msg.state, MessageState::InFresh);
assert_eq!(msg.get_filebytes(&t).await.unwrap().unwrap(), 2115); assert_eq!(msg.get_filebytes(&t).await.unwrap().unwrap(), 2115);
assert!(msg.get_file(&t).is_some()); assert_eq!(msg.get_file(&t).unwrap().extension().unwrap(), "png");
assert_eq!(msg.get_filename().unwrap(), "avatar64x64.png"); assert_eq!(msg.get_filename().unwrap(), "avatar64x64.png");
assert_eq!(msg.get_width(), 64); assert_eq!(msg.get_width(), 64);
assert_eq!(msg.get_height(), 64); assert_eq!(msg.get_height(), 64);
@@ -3800,10 +3801,8 @@ Message.
mime_message.parts[0].msg, mime_message.parts[0].msg,
"this is a classic email I attached the .EML file".to_string() "this is a classic email I attached the .EML file".to_string()
); );
assert_eq!( let re = Regex::new("^\\$BLOBDIR/[[:xdigit:]]{16}.eml$").unwrap();
mime_message.parts[0].param.get(Param::File), assert!(re.is_match(mime_message.parts[0].param.get(Param::File).unwrap()));
Some("$BLOBDIR/.eml")
);
assert_eq!(mime_message.parts[0].org_filename, Some(".eml".to_string())); assert_eq!(mime_message.parts[0].org_filename, Some(".eml".to_string()));

View File

@@ -547,8 +547,6 @@ mod tests {
assert!(p.get_blob(Param::File, &t, false).await.is_err()); assert!(p.get_blob(Param::File, &t, false).await.is_err());
fs::write(fname, b"boo").await.unwrap(); fs::write(fname, b"boo").await.unwrap();
let blob = p.get_blob(Param::File, &t, true).await.unwrap().unwrap();
assert!(blob.as_file_name().starts_with("foo"));
// Blob in blobdir, expect blob. // Blob in blobdir, expect blob.
let bar_path = t.get_blobdir().join("bar"); let bar_path = t.get_blobdir().join("bar");

View File

@@ -1577,9 +1577,7 @@ RETURNING id
context, context,
part.typ, part.typ,
chat_id, chat_id,
part.param part.param.get(Param::Filename),
.get_path(Param::File, context)
.unwrap_or_default(),
*msg_id, *msg_id,
) )
.await?; .await?;

View File

@@ -1,5 +1,6 @@
use std::time::Duration; use std::time::Duration;
use regex::Regex;
use tokio::fs; use tokio::fs;
use super::*; use super::*;
@@ -1593,7 +1594,6 @@ async fn test_pdf_filename_simple() {
assert_eq!(msg.viewtype, Viewtype::File); assert_eq!(msg.viewtype, Viewtype::File);
assert_eq!(msg.text, "mail body"); assert_eq!(msg.text, "mail body");
let file_path = msg.param.get(Param::File).unwrap(); let file_path = msg.param.get(Param::File).unwrap();
assert!(file_path.starts_with("$BLOBDIR/simple"));
assert!(file_path.ends_with(".pdf")); assert!(file_path.ends_with(".pdf"));
} }
@@ -1609,7 +1609,6 @@ async fn test_pdf_filename_continuation() {
assert_eq!(msg.viewtype, Viewtype::File); assert_eq!(msg.viewtype, Viewtype::File);
assert_eq!(msg.text, "mail body"); assert_eq!(msg.text, "mail body");
let file_path = msg.param.get(Param::File).unwrap(); 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.ends_with(".pdf"));
} }
@@ -2962,20 +2961,27 @@ Reply from different address
} }
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_long_and_duplicated_filenames() -> Result<()> { async fn test_weird_and_duplicated_filenames() -> Result<()> {
let mut tcm = TestContextManager::new(); let mut tcm = TestContextManager::new();
let alice = tcm.alice().await; let alice = tcm.alice().await;
let bob = tcm.bob().await; let bob = tcm.bob().await;
for filename_sent in &[ for (filename_sent, expected_ext) in &[
"foo.bar very long file name test baz.tar.gz", ("foo.bar very long file name test baz.tar.gz", "tar.gz"),
"foobarabababababababbababababverylongfilenametestbaz.tar.gz", (
"fooo...tar.gz", "foo.barabababababababbababababverylongfilenametestbaz.tar.gz",
"foo. .tar.gz", "tar.gz",
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.tar.gz", ),
"a.tar.gz", ("fooo...tar.gz", "..tar.gz"),
"a.tar.gz", ("foo. .tar.gz", "tar.gz"),
"a.a..a.a.a.a.tar.gz", (
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.tar.gz",
"tar.gz",
),
("a.tar.gz", "tar.gz"),
("a.tar.gz", "tar.gz"),
("a.a..a.a.a.a.tar.gz", "a..a.a.a.a.tar.gz"),
("a. tar.tar.gz", "tar.gz"),
] { ] {
let attachment = alice.blobdir.join(filename_sent); let attachment = alice.blobdir.join(filename_sent);
let content = format!("File content of {filename_sent}"); let content = format!("File content of {filename_sent}");
@@ -2989,23 +2995,33 @@ async fn test_long_and_duplicated_filenames() -> Result<()> {
let msg_bob = bob.recv_msg(&sent).await; let msg_bob = bob.recv_msg(&sent).await;
async fn check_message(msg: &Message, t: &TestContext, filename: &str, content: &str) { async fn check_message(
msg: &Message,
t: &TestContext,
filename: &str,
expected_ext: &str,
content: &str,
) {
assert_eq!(msg.get_viewtype(), Viewtype::File); assert_eq!(msg.get_viewtype(), Viewtype::File);
let resulting_filename = msg.get_filename().unwrap(); let resulting_filename = msg.get_filename().unwrap();
assert_eq!(resulting_filename, filename); assert_eq!(resulting_filename, filename);
let path = msg.get_file(t).unwrap(); let path = msg.get_file(t).unwrap();
if !msg.get_state().is_outgoing() {
let re =
Regex::new(&("^[[:xdigit:]]{16}.".to_string() + expected_ext + "$")).unwrap();
assert!(
re.is_match(path.file_name().unwrap().to_str().unwrap()),
"invalid path {path:?}"
);
}
let path2 = path.with_file_name("saved.txt"); let path2 = path.with_file_name("saved.txt");
msg.save_file(t, &path2).await.unwrap(); 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!(fs::read_to_string(&path).await.unwrap(), content); assert_eq!(fs::read_to_string(&path).await.unwrap(), content);
assert_eq!(fs::read_to_string(&path2).await.unwrap(), content); assert_eq!(fs::read_to_string(&path2).await.unwrap(), content);
fs::remove_file(path2).await.unwrap(); fs::remove_file(path2).await.unwrap();
} }
check_message(&msg_alice, &alice, filename_sent, &content).await; check_message(&msg_alice, &alice, filename_sent, expected_ext, &content).await;
check_message(&msg_bob, &bob, filename_sent, &content).await; check_message(&msg_bob, &bob, filename_sent, expected_ext, &content).await;
} }
Ok(()) Ok(())