Merge branch 'trim-precedingtrailing-whitespace-in-custom-list-name-and-des-930'

3 files changed, 37 insertions, 13 deletions

diff --git a/gui/src/renderer/components/select-location/CustomListDialogs.tsx b/gui/src/renderer/components/select-location/CustomListDialogs.tsx
index 56244e73eb..4f97706210 100644
--- a/gui/src/renderer/components/select-location/CustomListDialogs.tsx
+++ b/gui/src/renderer/components/select-location/CustomListDialogs.tsx
@@ -147,21 +147,25 @@ export function EditListDialog(props: EditListProps) {
   const { updateCustomList } = useAppContext();
 
   const [newName, setNewName] = useState(props.list.name);
+  const newNameTrimmed = newName.trim();
+  const newNameValid = newNameTrimmed !== '';
   const [error, setError, unsetError] = useBoolean();
 
   // Update name in list and save it.
   const save = useCallback(async () => {
-    try {
-      const updatedList = { ...props.list, name: newName };
-      const result = await updateCustomList(updatedList);
-      if (result && result.type === 'name already exists') {
-        setError();
-      } else {
-        props.hide();
+    if (newNameValid) {
+      try {
+        const updatedList = { ...props.list, name: newNameTrimmed };
+        const result = await updateCustomList(updatedList);
+        if (result && result.type === 'name already exists') {
+          setError();
+        } else {
+          props.hide();
+        }
+      } catch (e) {
+        const error = e as Error;
+        log.error(`Failed to edit custom list ${props.list.id}: ${error.message}`);
       }
-    } catch (e) {
-      const error = e as Error;
-      log.error(`Failed to edit custom list ${props.list.id}: ${error.message}`);
     }
   }, [props.list, newName, props.hide]);
 
@@ -175,7 +179,7 @@ export function EditListDialog(props: EditListProps) {
     <ModalAlert
       isOpen={props.isOpen}
       buttons={[
-        <AppButton.BlueButton key="save" onClick={save}>
+        <AppButton.BlueButton key="save" disabled={!newNameValid} onClick={save}>
           {messages.gettext('Save')}
         </AppButton.BlueButton>,
         <AppButton.BlueButton key="cancel" onClick={props.hide}>
diff --git a/gui/src/renderer/components/select-location/CustomLists.tsx b/gui/src/renderer/components/select-location/CustomLists.tsx
index eafcb161b2..f335f7ae87 100644
--- a/gui/src/renderer/components/select-location/CustomLists.tsx
+++ b/gui/src/renderer/components/select-location/CustomLists.tsx
@@ -123,7 +123,8 @@ interface AddListFormProps {
 
 function AddListForm(props: AddListFormProps) {
   const [name, setName] = useState('');
-  const nameValid = name.trim() !== '';
+  const nameTrimmed = name.trim();
+  const nameValid = nameTrimmed !== '';
   const [error, setError, unsetError] = useBoolean();
   const containerRef = useStyledRef<HTMLDivElement>();
   const inputRef = useStyledRef<HTMLInputElement>();
@@ -137,7 +138,7 @@ function AddListForm(props: AddListFormProps) {
   const createList = useCallback(async () => {
     if (nameValid) {
       try {
-        const result = await props.onCreateList(name);
+        const result = await props.onCreateList(nameTrimmed);
         if (result) {
           setError();
         }
diff --git a/mullvad-cli/src/cmds/custom_list.rs b/mullvad-cli/src/cmds/custom_list.rs
index 47e48c0c5f..edc6b4b0e2 100644
--- a/mullvad-cli/src/cmds/custom_list.rs
+++ b/mullvad-cli/src/cmds/custom_list.rs
@@ -6,11 +6,15 @@ use mullvad_types::{
     constraints::Constraint, relay_constraints::GeographicLocationConstraint, relay_list::RelayList,
 };
 
+/// Custom list length, expressed as a number of UTF8 codepoints (i.e. chars).
+pub const CUSTOM_LIST_MAX_LEN: usize = 30;
+
 #[derive(Subcommand, Debug)]
 pub enum CustomList {
     /// Create a new custom list
     New {
         /// A name for the new custom list
+        #[clap(value_parser = parse_custom_list_name)]
         name: String,
     },
 
@@ -55,7 +59,9 @@ pub enum EditCommand {
     Rename {
         /// Current name of the custom list
         name: String,
+
         /// A new name for the custom list
+        #[clap(value_parser = parse_custom_list_name)]
         new_name: String,
     },
 }
@@ -259,3 +265,16 @@ pub async fn find_list_by_name(
         .find(|list| list.name == name)
         .ok_or(anyhow!("List not found"))
 }
+
+/// Trim the string and validate the length against [CUSTOM_LIST_MAX_LEN].
+// NOTE: should only be used when *creating* custom lists, as we don't want to make it impossible
+// to reference any custom lists created before the max length and whitespace restrictions were put
+// in place.
+fn parse_custom_list_name(s: &str) -> Result<String> {
+    let s = s.trim();
+    let length = s.chars().count();
+    if length > CUSTOM_LIST_MAX_LEN {
+        bail!("Provided name is too long, {length}/{CUSTOM_LIST_MAX_LEN} characters.");
+    }
+    Ok(s.to_string())
+}