diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | mullvad-daemon/src/migrations/account_history.rs | 90 |
2 files changed, 56 insertions, 35 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index ee11ea0dce..ba30e96c3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Line wrap the file at 100 chars. Th - Fix resource leak caused by location check. - Fix issue where sockets didn't close after disconnecting from WireGuard servers over TCP by updating `udp-over-tcp` to 0.2. +- Parse old account history formats correctly when they are empty. #### Windows - Fix "Open Mullvad VPN" tray context menu item not working after toggling unpinned window setting. diff --git a/mullvad-daemon/src/migrations/account_history.rs b/mullvad-daemon/src/migrations/account_history.rs index 5400f04e33..06a8326f04 100644 --- a/mullvad-daemon/src/migrations/account_history.rs +++ b/mullvad-daemon/src/migrations/account_history.rs @@ -5,7 +5,7 @@ use serde::Deserialize; use std::path::Path; use talpid_types::ErrorExt; use tokio::{ - fs, + fs::{self, File}, io::{self, AsyncReadExt, AsyncSeekExt, AsyncWriteExt}, }; @@ -63,29 +63,21 @@ pub async fn migrate_formats(settings_dir: &Path, settings: &mut serde_json::Val if is_format_v3(&bytes) { return Ok(()); } - - let token = migrate_formats_inner(&bytes, settings)?; - - file.set_len(0).await.map_err(Error::WriteHistoryError)?; - file.seek(io::SeekFrom::Start(0)) - .await - .map_err(Error::WriteHistoryError)?; - file.write_all(token.as_bytes()) - .await - .map_err(Error::WriteHistoryError)?; - file.sync_all().await.map_err(Error::WriteHistoryError)?; - - Ok(()) + write_format_v3(file, migrate_formats_inner(&bytes, settings)?).await } fn migrate_formats_inner( account_bytes: &[u8], settings: &mut serde_json::Value, -) -> Result<AccountToken> { - if let Some((token, wg_data)) = try_format_v2(account_bytes) { - settings["wireguard"] = wg_data; - Ok(token) - } else if let Some(token) = try_format_v1(account_bytes) { +) -> Result<Option<AccountToken>> { + if let Ok(result) = try_format_v2(account_bytes) { + if let Some((token, wg_data)) = result { + settings["wireguard"] = wg_data; + Ok(Some(token)) + } else { + Ok(None) + } + } else if let Ok(token) = try_format_v1(account_bytes) { Ok(token) } else { Err(Error::ParseHistoryError) @@ -99,30 +91,42 @@ fn is_format_v3(bytes: &[u8]) -> bool { } } -fn try_format_v2(bytes: &[u8]) -> Option<(AccountToken, serde_json::Value)> { +async fn write_format_v3(mut file: File, token: Option<AccountToken>) -> Result<()> { + file.set_len(0).await.map_err(Error::WriteHistoryError)?; + file.seek(io::SeekFrom::Start(0)) + .await + .map_err(Error::WriteHistoryError)?; + if let Some(token) = token { + file.write_all(token.as_bytes()) + .await + .map_err(Error::WriteHistoryError)?; + } + file.sync_all().await.map_err(Error::WriteHistoryError) +} + +fn try_format_v2(bytes: &[u8]) -> Result<Option<(AccountToken, serde_json::Value)>> { #[derive(Deserialize, Clone)] pub struct AccountEntry { pub account: AccountToken, pub wireguard: serde_json::Value, } - serde_json::from_slice(bytes) - .ok() - .and_then(|entries: Vec<AccountEntry>| { - entries - .into_iter() - .next() - .map(|entry| (entry.account, entry.wireguard)) - }) + Ok(serde_json::from_slice::<'_, Vec<AccountEntry>>(bytes) + .map_err(|_error| Error::ParseHistoryError)? + .into_iter() + .next() + .map(|entry| (entry.account, entry.wireguard))) } -fn try_format_v1(bytes: &[u8]) -> Option<AccountToken> { +fn try_format_v1(bytes: &[u8]) -> Result<Option<AccountToken>> { #[derive(Deserialize)] struct OldFormat { accounts: Vec<AccountToken>, } - serde_json::from_slice(bytes) - .ok() - .and_then(|old_format: OldFormat| old_format.accounts.into_iter().next()) + Ok(serde_json::from_slice::<'_, OldFormat>(bytes) + .map_err(|_error| Error::ParseHistoryError)? + .accounts + .into_iter() + .next()) } #[cfg(test)] @@ -134,6 +138,11 @@ mod test { "accounts": ["1234", "4567"] } "#; + pub const ACCOUNT_HISTORY_V1_EMPTY: &str = r#" +{ + "accounts": [] +} +"#; pub const ACCOUNT_HISTORY_V2: &str = r#" [ { @@ -159,6 +168,7 @@ mod test { } } ]"#; + pub const ACCOUNT_HISTORY_V2_EMPTY: &str = r#"[]"#; pub const ACCOUNT_HISTORY_V3: &str = r#"123456"#; pub const OLD_SETTINGS: &str = r#" @@ -327,7 +337,7 @@ mod test { #[test] fn test_v2() { - assert!(super::try_format_v2(ACCOUNT_HISTORY_V1.as_bytes()).is_none()); + assert!(super::try_format_v2(ACCOUNT_HISTORY_V1.as_bytes()).is_err()); let mut old_settings = serde_json::from_str(OLD_SETTINGS).unwrap(); let new_settings: serde_json::Value = serde_json::from_str(NEW_SETTINGS).unwrap(); @@ -337,12 +347,22 @@ mod test { super::migrate_formats_inner(ACCOUNT_HISTORY_V2.as_bytes(), &mut old_settings).unwrap(); assert_eq!(&old_settings, &new_settings); - assert_eq!(token, "1234"); + assert_eq!(token, Some("1234".to_string())); + + // Test whether empty histories are handled correctly + let mut old_settings = serde_json::from_str(OLD_SETTINGS).unwrap(); + let token = + super::migrate_formats_inner(ACCOUNT_HISTORY_V2_EMPTY.as_bytes(), &mut old_settings) + .unwrap(); + assert_eq!(&old_settings, &old_settings); + assert_eq!(token, None); } #[test] fn test_v1() { - let token = super::try_format_v1(ACCOUNT_HISTORY_V1.as_bytes()); + let token = super::try_format_v1(ACCOUNT_HISTORY_V1.as_bytes()).unwrap(); assert_eq!(token, Some("1234".to_string())); + let token = super::try_format_v1(ACCOUNT_HISTORY_V1_EMPTY.as_bytes()).unwrap(); + assert_eq!(token, None); } } |