Merge branch 'fix-user-input-string-validation-ios-682'

mullvad · May 13, 2024 · cc2517e · cc2517e
2 parents 2378efb + 9d4c27f
commit cc2517e
Show file tree

Hide file tree

Showing 12 changed files with 93 additions and 19 deletions.
diff --git a/ios/MullvadSettings/AccessMethodRepository.swift b/ios/MullvadSettings/AccessMethodRepository.swift
@@ -9,6 +9,7 @@
 import Combine
 import Foundation
 import MullvadLogging
+import MullvadTypes
 
 public class AccessMethodRepository: AccessMethodRepositoryProtocol {
     private let logger = Logger(label: "AccessMethodRepository")
@@ -54,6 +55,9 @@ public class AccessMethodRepository: AccessMethodRepositoryProtocol {
     public func save(_ method: PersistentAccessMethod) {
         var methodStore = readApiAccessMethodStore()
 
+        var method = method
+        method.name = method.name.trimmingCharacters(in: .whitespaces)
+
         if let index = methodStore.accessMethods.firstIndex(where: { $0.id == method.id }) {
             methodStore.accessMethods[index] = method
         } else {

diff --git a/ios/MullvadSettings/CustomListRepository.swift b/ios/MullvadSettings/CustomListRepository.swift
@@ -11,18 +11,29 @@ import Foundation
 import MullvadLogging
 import MullvadTypes
 
-public enum CustomRelayListError: LocalizedError, Equatable {
+public enum CustomRelayListError: LocalizedError, Hashable {
     case duplicateName
+    case nameTooLong
 
     public var errorDescription: String? {
         switch self {
         case .duplicateName:
             NSLocalizedString(
                 "DUPLICATE_CUSTOM_LISTS_ERROR",
                 tableName: "CustomLists",
-                value: "Name is already taken.",
+                value: "A custom list with this name exists, please choose a unique name.",
                 comment: ""
             )
+        case .nameTooLong:
+            String(
+                format: NSLocalizedString(
+                    "CUSTOM_LIST_NAME_TOO_LONG_ERROR",
+                    tableName: "CustomLists",
+                    value: "Name should be no longer than %i characters.",
+                    comment: ""
+                ),
+                NameInputFormatter.maxLength
+            )
         }
     }
 }
@@ -37,11 +48,15 @@ public struct CustomListRepository: CustomListRepositoryProtocol {
     public init() {}
 
     public func save(list: CustomList) throws {
-        var list = list
-        list.name = list.name.trimmingCharacters(in: .whitespaces)
+        guard list.name.count <= NameInputFormatter.maxLength else {
+            throw CustomRelayListError.nameTooLong
+        }
 
         var lists = fetchAll()
 
+        var list = list
+        list.name = list.name.trimmingCharacters(in: .whitespaces)
+
         if let listWithSameName = lists.first(where: { $0.name.compare(list.name) == .orderedSame }),
            listWithSameName.id != list.id {
             throw CustomRelayListError.duplicateName

diff --git a/ios/MullvadSettings/PersistentAccessMethod.swift b/ios/MullvadSettings/PersistentAccessMethod.swift
@@ -40,6 +40,19 @@ public struct PersistentAccessMethod: Identifiable, Codable, Equatable {
         self.proxyConfiguration = proxyConfiguration
     }
 
+    public init(from decoder: any Decoder) throws {
+        let container = try decoder.container(keyedBy: CodingKeys.self)
+
+        self.id = try container.decode(UUID.self, forKey: .id)
+        self.isEnabled = try container.decode(Bool.self, forKey: .isEnabled)
+        self.proxyConfiguration = try container.decode(PersistentProxyConfiguration.self, forKey: .proxyConfiguration)
+
+        // Added after release of API access methods feature. There was previously no limitation on text input length,
+        // so this formatting has been added to prevent already stored names from being too long when displayed.
+        let name = try container.decode(String.self, forKey: .name)
+        self.name = NameInputFormatter.format(name)
+    }
+
     public static func == (lhs: Self, rhs: Self) -> Bool {
         lhs.id == rhs.id
     }

diff --git a/ios/MullvadTypes/NameInputFormatter.swift b/ios/MullvadTypes/NameInputFormatter.swift
@@ -0,0 +1,15 @@
+//
+//  NameInputFormatter.swift
+//  MullvadVPN
+//
+//  Created by Jon Petersson on 2024-05-13.
+//  Copyright © 2024 Mullvad VPN AB. All rights reserved.
+//
+
+public struct NameInputFormatter {
+    public static let maxLength = 30
+
+    public static func format(_ string: String, maxLength: Int = Self.maxLength) -> String {
+        String(string.trimmingCharacters(in: .whitespaces).prefix(maxLength))
+    }
+}
diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj
@@ -540,6 +540,7 @@
 		7A6F2FAD2AFD3DA7006D0856 /* CustomDNSViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A6F2FAC2AFD3DA7006D0856 /* CustomDNSViewController.swift */; };
 		7A6F2FAF2AFE36E7006D0856 /* VPNSettingsInfoButtonItem.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A6F2FAE2AFE36E7006D0856 /* VPNSettingsInfoButtonItem.swift */; };
 		7A7907332BC0280A00B61F81 /* InterceptibleNavigationController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A7907322BC0280A00B61F81 /* InterceptibleNavigationController.swift */; };
+		7A7AD14F2BF21EF200B30B3C /* NameInputFormatter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A7AD14D2BF21DCE00B30B3C /* NameInputFormatter.swift */; };
 		7A7AD28D29DC677800480EF1 /* FirstTimeLaunch.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A7AD28C29DC677800480EF1 /* FirstTimeLaunch.swift */; };
 		7A818F1F29F0305800C7F0F4 /* RootConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A818F1E29F0305800C7F0F4 /* RootConfiguration.swift */; };
 		7A83A0C62B29A750008B5CE7 /* APIAccessMethodsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A83A0C52B29A750008B5CE7 /* APIAccessMethodsTests.swift */; };
@@ -1850,6 +1851,7 @@
 		7A6F2FAC2AFD3DA7006D0856 /* CustomDNSViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomDNSViewController.swift; sourceTree = "<group>"; };
 		7A6F2FAE2AFE36E7006D0856 /* VPNSettingsInfoButtonItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VPNSettingsInfoButtonItem.swift; sourceTree = "<group>"; };
 		7A7907322BC0280A00B61F81 /* InterceptibleNavigationController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InterceptibleNavigationController.swift; sourceTree = "<group>"; };
+		7A7AD14D2BF21DCE00B30B3C /* NameInputFormatter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NameInputFormatter.swift; sourceTree = "<group>"; };
 		7A7AD28C29DC677800480EF1 /* FirstTimeLaunch.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FirstTimeLaunch.swift; sourceTree = "<group>"; };
 		7A818F1E29F0305800C7F0F4 /* RootConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RootConfiguration.swift; sourceTree = "<group>"; };
 		7A83A0C52B29A750008B5CE7 /* APIAccessMethodsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = APIAccessMethodsTests.swift; sourceTree = "<group>"; };
@@ -2544,6 +2546,7 @@
 				58A1AA8623F43901009F7EA6 /* Location.swift */,
 				5840250322B11AB700E4CFEC /* MullvadEndpoint.swift */,
 				58D223D7294C8E5E0029F5F8 /* MullvadTypes.h */,
+				7A7AD14D2BF21DCE00B30B3C /* NameInputFormatter.swift */,
 				A97FF54F2A0D2FFC00900996 /* NSFileCoordinator+Extensions.swift */,
 				58CAFA01298530DC00BE19F7 /* Promise.swift */,
 				449EBA242B975B7C00DFA4EB /* Protocols */,
@@ -5843,6 +5846,7 @@
 				58D22410294C90210029F5F8 /* Location.swift in Sources */,
 				58D22411294C90210029F5F8 /* MullvadEndpoint.swift in Sources */,
 				58D22412294C90210029F5F8 /* RelayConstraint.swift in Sources */,
+				7A7AD14F2BF21EF200B30B3C /* NameInputFormatter.swift in Sources */,
 				58D22413294C90210029F5F8 /* RelayConstraints.swift in Sources */,
 				7AF9BE8C2A321D1F00DBFEDB /* RelayFilter.swift in Sources */,
 				58D22414294C90210029F5F8 /* RelayLocation.swift in Sources */,

diff --git a/ios/MullvadVPN/Coordinators/CustomLists/CustomListCellConfiguration.swift b/ios/MullvadVPN/Coordinators/CustomLists/CustomListCellConfiguration.swift
@@ -7,6 +7,7 @@
 //
 
 import Combine
+import MullvadTypes
 import UIKit
 
 struct CustomListCellConfiguration {
@@ -75,7 +76,7 @@ struct CustomListCellConfiguration {
         contentConfiguration.setPlaceholder(type: .required)
         contentConfiguration.textFieldProperties = .withSmartFeaturesDisabled()
         contentConfiguration.inputText = subject.value.name
-        contentConfiguration.maxLength = 30
+        contentConfiguration.maxLength = NameInputFormatter.maxLength
         contentConfiguration.editingEvents.onChange = subject.bindTextAction(to: \.name)
 
         cell.accessibilityIdentifier = AccessibilityIdentifier.customListEditNameFieldCell

diff --git a/ios/MullvadVPN/Coordinators/CustomLists/CustomListDataSourceConfiguration.swift b/ios/MullvadVPN/Coordinators/CustomLists/CustomListDataSourceConfiguration.swift
@@ -92,7 +92,12 @@ extension CustomListDataSourceConfiguration: UITableViewDelegate {
         let errorsInSection = itemsWithErrors.filter { itemsInSection.contains($0) }.compactMap { item in
             switch item {
             case .name:
-                CustomListFieldValidationError.name
+                Array(validationErrors).filter { error in
+                    if case .name = error {
+                        return true
+                    }
+                    return false
+                }
             case .addLocations, .editLocations, .deleteList:
                 nil
             }
@@ -102,7 +107,7 @@ extension CustomListDataSourceConfiguration: UITableViewDelegate {
         case .name:
             let view = SettingsFieldValidationErrorContentView(
                 configuration: SettingsFieldValidationErrorConfiguration(
-                    errors: errorsInSection.settingsFieldValidationErrors
+                    errors: errorsInSection.flatMap { $0.settingsFieldValidationErrors }
                 )
             )
             return view

diff --git a/ios/MullvadVPN/Coordinators/CustomLists/CustomListValidationError.swift b/ios/MullvadVPN/Coordinators/CustomLists/CustomListValidationError.swift
@@ -7,19 +7,15 @@
 //
 
 import Foundation
+import MullvadSettings
 
-enum CustomListFieldValidationError: LocalizedError {
-    case name
+enum CustomListFieldValidationError: LocalizedError, Hashable {
+    case name(CustomRelayListError)
 
     var errorDescription: String? {
         switch self {
-        case .name:
-            NSLocalizedString(
-                "CUSTOM_LISTS_VALIDATION_ERROR_EMPTY_FIELD",
-                tableName: "CutstomLists",
-                value: "A custom list with this name exists, please choose a unique name.",
-                comment: ""
-            )
+        case let .name(error):
+            error.errorDescription
         }
     }
 }

diff --git a/ios/MullvadVPN/Coordinators/CustomLists/CustomListViewController.swift b/ios/MullvadVPN/Coordinators/CustomLists/CustomListViewController.swift
@@ -154,8 +154,10 @@ class CustomListViewController: UIViewController {
             try interactor.save(viewModel: subject.value)
             delegate?.customListDidSave(subject.value.customList)
         } catch {
-            validationErrors.insert(.name)
-            dataSourceConfiguration?.set(validationErrors: validationErrors)
+            if let error = error as? CustomRelayListError {
+                validationErrors.insert(.name(error))
+                dataSourceConfiguration?.set(validationErrors: validationErrors)
+            }
         }
     }
 

diff --git a/...Coordinators/Settings/APIAccess/Edit/MethodSettings/MethodSettingsCellConfiguration.swift b/...Coordinators/Settings/APIAccess/Edit/MethodSettings/MethodSettingsCellConfiguration.swift
@@ -7,6 +7,7 @@
 //
 
 import Combine
+import MullvadTypes
 import UIKit
 
 class MethodSettingsCellConfiguration {
@@ -109,6 +110,7 @@ class MethodSettingsCellConfiguration {
         contentConfiguration.setPlaceholder(type: .required)
         contentConfiguration.textFieldProperties = .withSmartFeaturesDisabled()
         contentConfiguration.inputText = subject.value.name
+        contentConfiguration.maxLength = NameInputFormatter.maxLength
         contentConfiguration.editingEvents.onChange = subject.bindTextAction(to: \.name)
 
         cell.accessibilityIdentifier = .accessMethodNameTextField

diff --git a/ios/MullvadVPN/Coordinators/Settings/APIAccess/Models/AccessMethodValidationError.swift b/ios/MullvadVPN/Coordinators/Settings/APIAccess/Models/AccessMethodValidationError.swift
@@ -7,6 +7,7 @@
 //
 
 import Foundation
+import MullvadTypes
 
 /// Access method validation error that holds an array of individual per-field validation errors.
 struct AccessMethodValidationError: LocalizedError, Equatable {
@@ -57,6 +58,9 @@ struct AccessMethodFieldValidationError: LocalizedError, Equatable {
 
         /// Invalid port number, i.e zero.
         case invalidPort
+
+        /// The name input is too long.
+        case nameTooLong
     }
 
     /// Kind of validation error.
@@ -91,6 +95,16 @@ struct AccessMethodFieldValidationError: LocalizedError, Equatable {
                 value: "Please enter a valid port.",
                 comment: ""
             )
+        case .nameTooLong:
+            String(
+                format: NSLocalizedString(
+                    "VALIDATION_ERRORS_NAME_TOO_LONG",
+                    tableName: "APIAccess",
+                    value: "Name should be no longer than %i characters.",
+                    comment: ""
+                ),
+                NameInputFormatter.maxLength
+            )
         }
     }
 }

diff --git a/ios/MullvadVPN/Coordinators/Settings/APIAccess/Models/AccessMethodViewModel+Persistent.swift b/ios/MullvadVPN/Coordinators/Settings/APIAccess/Models/AccessMethodViewModel+Persistent.swift
@@ -66,10 +66,13 @@ extension AccessMethodViewModel {
     }
 
     private func validateName() throws -> String {
+        // Context doesn't matter for name field errors.
         if name.isEmpty {
-            // Context doesn't matter for name field.
             let fieldError = AccessMethodFieldValidationError(kind: .emptyValue, field: .name, context: .shadowsocks)
             throw AccessMethodValidationError(fieldErrors: [fieldError])
+        } else if name.count > NameInputFormatter.maxLength {
+            let fieldError = AccessMethodFieldValidationError(kind: .nameTooLong, field: .name, context: .shadowsocks)
+            throw AccessMethodValidationError(fieldErrors: [fieldError])
         }
 
         return name