Skip to content

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

Open
j-g00da wants to merge 1 commit into
mainfrom
j-g00da/refactor-backup-filename
Open

refactor: Don't include email addresses in export filenames#8626
j-g00da wants to merge 1 commit into
mainfrom
j-g00da/refactor-backup-filename

Conversation

@j-g00da

@j-g00da j-g00da commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Changes filenames used in the db backups and key exports,
preferring fingerprint over the email address.

Part of: #8572

@j-g00da
j-g00da requested review from link2xt and removed request for link2xt August 24, 2026 10:58
@j-g00da
j-g00da marked this pull request as draft August 24, 2026 11:11
@j-g00da
j-g00da force-pushed the j-g00da/refactor-backup-filename branch 2 times, most recently from a298f0a to 6ad24a5 Compare August 24, 2026 11:20
@j-g00da j-g00da changed the title refactor: Don't use email address in backup filenames. refactor: Don't include email addresses in export filenames Aug 24, 2026
@j-g00da
j-g00da force-pushed the j-g00da/refactor-backup-filename branch from 6ad24a5 to 78983c2 Compare August 25, 2026 08:53
Changes filenames used in the db backups and key exports,
preferring fingerprint over the email address.

Part of: #8572

Signed-off-by: Jagoda 艢l膮zak <jslazak@jslazak.com>
@j-g00da
j-g00da force-pushed the j-g00da/refactor-backup-filename branch from 78983c2 to 26fbf9e Compare August 25, 2026 08:56
@j-g00da

j-g00da commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Do we want to include display name in the filenames? I didn't do so as this may be problematic. If we were to, it would make sense to at least somewhat sanitize these strings (at least removing slashes, backslashes, and all kinds of whitespaces).

I'm not sure if we actually want it though, display name can be changed and this WILL affect ordering, which can be problematic. Since fingerprint is the actual identity, I think it should be accessible from the UI.

cc: @hpk42 @link2xt

@j-g00da
j-g00da marked this pull request as ready for review August 25, 2026 09:09
@j-g00da
j-g00da requested review from hpk42 and link2xt August 25, 2026 09:09
Comment thread src/imex.rs
/// 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

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.

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

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.

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.

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.

@link2xt

link2xt commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Do we want to include display name in the filenames?

It may be useful in case user exports multiple backups, but i think better not do it now, and not in this PR. Most users probably don't do any backups and having multiple devices is sufficient for backing up profiles, have not even seen any user questions about backups recently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants