Skip to content

refactor: Don't include email addresses in export filenames #8626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
j-g00da wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor: Don't include email addresses in export filenames #8626

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 36 additions & 21 deletions src/imex.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::chat::delete_and_reset_all_device_msgs;
use crate::config::Config;
use crate::context::Context;
use crate::events::EventType;
use crate::key::{self, DcKey, SignedSecretKey};
use crate::key::{self, DcKey, SignedSecretKey, self_fingerprint};
use crate::log::{LogExt, warn};
use crate::qr::DCBACKUP_VERSION;
use crate::sql;
Expand Down Expand Up @@ -411,32 +411,46 @@ async fn import_backup_stream_inner<R: tokio::io::AsyncRead + Unpin>(
/// Returns Ok((temp_db_path, temp_path, dest_path)) on success. Unencrypted database can be
/// written to temp_db_path. The backup can then be written to temp_path. If the backup succeeded,
/// it can be renamed to dest_path. This guarantees that the backup is complete.
///
/// `addr` is no longer included as part of the file stem, and is only required to calculate

@link2xt link2xt Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work if the user changes the sending address, and this way we do not get rid of the get_primary_self_addr call. There is a date in the backup filename prefix, so it should be ordered correctly already. So i think it's fine to already drop the address even here.

@j-g00da j-g00da Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as long as it's not made on the same day - but maybe it's a negligible edge case

@link2xt link2xt Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User making two backups on the same day, one with old version and another with a new version, is definitely an edge case. Also the goal is getting rid of get_primary_self_addr (we want to get rid of the concept of primary address) and currently the PR still calls it.

@j-g00da j-g00da Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can just do the same check I do right now, but taking into account all addresses, this would cover this edge case and won't rely on "primary" addr. Also would cover the pre-existing bug when changing the sending addr.

@link2xt link2xt Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking against all address will do if you want to do it, but IMO not worth the complexity since this code is not going to be used day after the user upgrades.

/// the right prefix for correct ordering.
fn get_next_backup_path(
folder: &Path,
fingerprint: &str,
addr: &str,
backup_time: i64,
) -> Result<(PathBuf, PathBuf, PathBuf)> {
let folder = PathBuf::from(folder);
let stem = chrono::DateTime::<chrono::Utc>::from_timestamp(backup_time, 0)
let prefix = chrono::DateTime::<chrono::Utc>::from_timestamp(backup_time, 0)
.context("can't get next backup path")?
// Don't change this file name format, in `dc_imex_has_backup` we use string comparison to determine which backup is newer:
.format("delta-chat-backup-%Y-%m-%d")
.to_string();

// 64 backup files per day should be enough for everyone
for i in 0..64 {
let mut tempdbfile = folder.clone();
tempdbfile.push(format!("{stem}-{i:02}-{addr}.db"));
'i: for i in 0..64 {
let stem = format!("{prefix}-{i:02}-{fingerprint}");
// historical format, we check against it too to ensure correct ordering
let stem_old = format!("{prefix}-{i:02}-{addr}");

for ext in ["db", "tar.part", "tar"] {
for current_stem in [&stem, &stem_old] {
let mut path = folder.clone();
path.push(format!("{current_stem}.{ext}"));
if path.exists() {
continue 'i;
}
}
}

let mut tempdbfile = folder.clone();
tempdbfile.push(format!("{stem}.db"));
let mut tempfile = folder.clone();
tempfile.push(format!("{stem}-{i:02}-{addr}.tar.part"));

tempfile.push(format!("{stem}.tar.part"));
let mut destfile = folder.clone();
destfile.push(format!("{stem}-{i:02}-{addr}.tar"));
destfile.push(format!("{stem}.tar"));

if !tempdbfile.exists() && !tempfile.exists() && !destfile.exists() {
return Ok((tempdbfile, tempfile, destfile));
}
return Ok((tempdbfile, tempfile, destfile));
}
bail!("could not create backup file, disk full?");
}
Expand All @@ -448,8 +462,11 @@ fn get_next_backup_path(
async fn export_backup(context: &Context, dir: &Path, passphrase: String) -> Result<()> {
// get a fine backup file name (the name includes the date so that multiple backup instances are possible)
let now = time();
let fingerprint = self_fingerprint(context).await?;
// only used to test against backups created by older versions
let self_addr = context.get_primary_self_addr().await?;
let (temp_db_path, temp_path, dest_path) = get_next_backup_path(dir, &self_addr, now)?;
let (temp_db_path, temp_path, dest_path) =
get_next_backup_path(dir, fingerprint, &self_addr, now)?;
let temp_db_path = TempPathGuard::new(temp_db_path);
let temp_path = TempPathGuard::new(temp_path);

Expand Down Expand Up @@ -671,7 +688,6 @@ async fn export_self_keys(context: &Context, dir: &Path) -> Result<()> {
},
)
.await?;
let self_addr = context.get_primary_self_addr().await?;
for (id, private_key, is_default) in keys {
let id = (is_default == 0).then_some(id);

Expand All @@ -680,14 +696,14 @@ async fn export_self_keys(context: &Context, dir: &Path) -> Result<()> {
continue;
};

if let Err(err) = export_key_to_asc_file(context, dir, &self_addr, id, &private_key).await {
if let Err(err) = export_key_to_asc_file(context, dir, id, &private_key).await {
error!(context, "Failed to export private key: {:#}.", err);
export_errors += 1;
}

let public_key = private_key.to_public_key();

if let Err(err) = export_key_to_asc_file(context, dir, &self_addr, id, &public_key).await {
if let Err(err) = export_key_to_asc_file(context, dir, id, &public_key).await {
error!(context, "Failed to export public key: {:#}.", err);
export_errors += 1;
}
Expand All @@ -701,7 +717,6 @@ async fn export_self_keys(context: &Context, dir: &Path) -> Result<()> {
async fn export_key_to_asc_file<T>(
context: &Context,
dir: &Path,
addr: &str,
id: Option<i64>,
key: &T,
) -> Result<String>
Expand All @@ -715,7 +730,7 @@ where
};
let id = id.map_or("default".into(), |i| i.to_string());
let fp = key.dc_fingerprint().hex();
format!("{kind}-key-{addr}-{id}-{fp}.asc")
format!("{kind}-key-{id}-{fp}.asc")
};
let path = dir.join(&file_name);
info!(context, "Exporting key to {}.", path.display());
Expand Down Expand Up @@ -802,10 +817,10 @@ mod tests {
let context = TestContext::new().await;
let key = alice_keypair().to_public_key();
let blobdir = Path::new("$BLOBDIR");
let filename = export_key_to_asc_file(&context.ctx, blobdir, "a@b", None, &key)
let filename = export_key_to_asc_file(&context.ctx, blobdir, None, &key)
.await
.unwrap();
assert!(filename.starts_with("public-key-a@b-default-"));
assert!(filename.starts_with("public-key-default-"));
assert!(filename.ends_with(".asc"));
let blobdir = context.ctx.get_blobdir().to_str().unwrap();
let filename = format!("{blobdir}/{filename}");
Expand All @@ -819,11 +834,11 @@ mod tests {
let context = TestContext::new().await;
let key = alice_keypair();
let blobdir = Path::new("$BLOBDIR");
let filename = export_key_to_asc_file(&context.ctx, blobdir, "a@b", None, &key)
let filename = export_key_to_asc_file(&context.ctx, blobdir, None, &key)
.await
.unwrap();
let fingerprint = filename
.strip_prefix("private-key-a@b-default-")
.strip_prefix("private-key-default-")
.unwrap()
.strip_suffix(".asc")
.unwrap();
Expand Down