Limit the size of custom list names to 30 graphemes

4 files changed, 45 insertions, 65 deletions

diff --git a/mullvad-daemon/src/custom_list.rs b/mullvad-daemon/src/custom_list.rs
index df9f30a519..71c5fc77e9 100644
--- a/mullvad-daemon/src/custom_list.rs
+++ b/mullvad-daemon/src/custom_list.rs
@@ -14,22 +14,11 @@ where
     ///
     /// Returns an error if the name is not unique.
     pub async fn create_custom_list(&mut self, name: String) -> Result<Id, crate::Error> {
-        if self
-            .settings
-            .custom_lists
-            .iter()
-            .any(|list| list.name == name)
-        {
-            return Err(Error::CustomListExists);
-        }
-
-        let new_list = CustomList::new(name);
+        let new_list = CustomList::new(name).map_err(crate::Error::CustomListError)?;
         let id = new_list.id;
 
         self.settings
-            .update(|settings| {
-                settings.custom_lists.add(new_list);
-            })
+            .try_update(|settings| settings.custom_lists.add(new_list))
             .await
             .map_err(Error::SettingsError)?;
 
@@ -40,20 +29,12 @@ where
     ///
     /// Returns an error if the list doesn't exist.
     pub async fn delete_custom_list(&mut self, id: Id) -> Result<(), Error> {
-        let Some(list_index) = self
-            .settings
-            .custom_lists
-            .iter()
-            .position(|elem| elem.id == id)
-        else {
-            return Err(Error::CustomListNotFound);
-        };
         let settings_changed = self
             .settings
-            .update(|settings| {
+            .try_update(|settings| {
                 // NOTE: Not using swap remove because it would make user output slightly
                 // more confusing and the cost is so small.
-                settings.custom_lists.remove(list_index);
+                settings.custom_lists.remove(&id)
             })
             .await
             .map_err(Error::SettingsError);
@@ -78,32 +59,10 @@ where
     /// - there is no existing list with the same ID,
     /// - or the existing list has a different name.
     pub async fn update_custom_list(&mut self, new_list: CustomList) -> Result<(), Error> {
-        let Some((list_index, old_list)) = self
-            .settings
-            .custom_lists
-            .iter()
-            .enumerate()
-            .find(|elem| elem.1.id == new_list.id)
-        else {
-            return Err(Error::CustomListNotFound);
-        };
-        let id = old_list.id;
-
-        if old_list.name != new_list.name
-            && self
-                .settings
-                .custom_lists
-                .iter()
-                .any(|list| list.name == new_list.name)
-        {
-            return Err(Error::CustomListExists);
-        }
-
+        let list_id = new_list.id;
         let settings_changed = self
             .settings
-            .update(|settings| {
-                settings.custom_lists[list_index] = new_list;
-            })
+            .try_update(|settings| settings.custom_lists.update(new_list))
             .await
             .map_err(Error::SettingsError);
 
@@ -111,7 +70,7 @@ where
             self.relay_selector
                 .set_config(new_selector_config(&self.settings));
 
-            if self.change_should_cause_reconnect(Some(id)) {
+            if self.change_should_cause_reconnect(Some(list_id)) {
                 log::info!("Initiating tunnel restart because a selected custom list changed");
                 self.reconnect_tunnel();
             }
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index a6b6794d33..875f797ec3 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -173,12 +173,8 @@ pub enum Error {
     TunnelError(#[source] tunnel_state_machine::Error),
 
     /// Custom list already exists
-    #[error("A list with that name already exists")]
-    CustomListExists,
-
-    /// Custom list does not exist
-    #[error("A list with that name does not exist")]
-    CustomListNotFound,
+    #[error("Custom list error: {0}")]
+    CustomListError(#[source] mullvad_types::custom_list::Error),
 
     #[error("Access method error")]
     AccessMethodError(#[source] access_method::Error),
diff --git a/mullvad-daemon/src/management_interface.rs b/mullvad-daemon/src/management_interface.rs
index ce203e7ba8..0699ab697e 100644
--- a/mullvad-daemon/src/management_interface.rs
+++ b/mullvad-daemon/src/management_interface.rs
@@ -1115,16 +1115,6 @@ fn map_daemon_error(error: crate::Error) -> Status {
         DaemonError::NoAccountToken | DaemonError::NoAccountTokenHistory => {
             Status::unauthenticated(error.to_string())
         }
-        DaemonError::CustomListExists => Status::with_details(
-            Code::AlreadyExists,
-            error.to_string(),
-            mullvad_management_interface::CUSTOM_LIST_LIST_EXISTS_DETAILS.into(),
-        ),
-        DaemonError::CustomListNotFound => Status::with_details(
-            Code::NotFound,
-            error.to_string(),
-            mullvad_management_interface::CUSTOM_LIST_LIST_NOT_FOUND_DETAILS.into(),
-        ),
         error => Status::unknown(error.to_string()),
     }
 }
diff --git a/mullvad-daemon/src/settings/mod.rs b/mullvad-daemon/src/settings/mod.rs
index 243764ec90..1e710eebba 100644
--- a/mullvad-daemon/src/settings/mod.rs
+++ b/mullvad-daemon/src/settings/mod.rs
@@ -1,5 +1,7 @@
 #[cfg(not(target_os = "android"))]
 use futures::TryFutureExt;
+#[cfg(not(target_os = "android"))]
+use mullvad_types::custom_list::Error as CustomListError;
 use mullvad_types::{
     relay_constraints::{RelayConstraints, RelaySettings, WireguardConstraints},
     settings::{DnsState, Settings},
@@ -47,11 +49,18 @@ pub enum Error {
 impl From<Error> for mullvad_management_interface::Status {
     fn from(error: Error) -> mullvad_management_interface::Status {
         use mullvad_management_interface::{Code, Status};
-
         match error {
             Error::DeleteError(..) | Error::WriteError(..) | Error::ReadError(..) => {
                 Status::new(Code::FailedPrecondition, error.to_string())
             }
+            Error::UpdateFailed(err)
+                if err
+                    .downcast_ref::<mullvad_types::custom_list::Error>()
+                    .is_some() =>
+            {
+                let custom_list_err = *err.downcast::<CustomListError>().unwrap();
+                handle_custom_list_error(custom_list_err)
+            }
             Error::SerializeError(..) | Error::ParseError(..) | Error::UpdateFailed(..) => {
                 Status::new(Code::Internal, error.to_string())
             }
@@ -59,6 +68,32 @@ impl From<Error> for mullvad_management_interface::Status {
     }
 }
 
+#[cfg(not(target_os = "android"))]
+fn handle_custom_list_error(
+    custom_list_err: CustomListError,
+) -> mullvad_management_interface::Status {
+    use mullvad_management_interface::{Code, Status};
+    match custom_list_err {
+        error @ CustomListError::ListExists | error @ CustomListError::DuplicateName => {
+            Status::with_details(
+                Code::AlreadyExists,
+                error.to_string(),
+                mullvad_management_interface::CUSTOM_LIST_LIST_EXISTS_DETAILS.into(),
+            )
+        }
+        error @ CustomListError::NameTooLong => Status::with_details(
+            Code::InvalidArgument,
+            error.to_string(),
+            mullvad_management_interface::CUSTOM_LIST_LIST_NAME_TOO_LONG_DETAILS.into(),
+        ),
+        error @ CustomListError::ListNotFound => Status::with_details(
+            Code::NotFound,
+            error.to_string(),
+            mullvad_management_interface::CUSTOM_LIST_LIST_NOT_FOUND_DETAILS.into(),
+        ),
+    }
+}
+
 pub struct SettingsPersister {
     settings: Settings,
     path: PathBuf,