From 8a916beb389f84e21ed1aaf0d246801cd6acd73c Mon Sep 17 00:00:00 2001 From: Eric Kuck Date: Wed, 12 Dec 2018 21:09:52 -0600 Subject: More formatting nits and cyclomatic complexity fixes Signed-off-by: Eric Kuck --- .../ConfigFile/WgQuickConfigFileParser.swift | 14 ++--- WireGuard/WireGuard/UI/TunnelViewModel.swift | 34 +++++------ WireGuard/WireGuard/UI/iOS/ErrorPresenter.swift | 10 +-- .../UI/iOS/TunnelDetailTableViewController.swift | 8 +-- .../UI/iOS/TunnelEditTableViewController.swift | 59 ++++++++---------- .../UI/iOS/TunnelsListTableViewController.swift | 2 +- WireGuard/WireGuard/VPN/TunnelsManager.swift | 71 +++++++++++----------- WireGuard/WireGuard/WireGuardAppError.swift | 3 +- WireGuard/WireGuard/ZipArchive/ZipArchive.swift | 4 +- WireGuard/WireGuard/ZipArchive/ZipExporter.swift | 5 +- WireGuard/WireGuard/ZipArchive/ZipImporter.swift | 2 +- 11 files changed, 99 insertions(+), 113 deletions(-) (limited to 'WireGuard/WireGuard') diff --git a/WireGuard/WireGuard/ConfigFile/WgQuickConfigFileParser.swift b/WireGuard/WireGuard/ConfigFile/WgQuickConfigFileParser.swift index 138ca62..14a6dfb 100644 --- a/WireGuard/WireGuard/ConfigFile/WgQuickConfigFileParser.swift +++ b/WireGuard/WireGuard/ConfigFile/WgQuickConfigFileParser.swift @@ -54,13 +54,11 @@ class WgQuickConfigFileParser { } else { attributes[key] = value } - } else { - if lowercasedLine != "[interface]" && lowercasedLine != "[peer]" { - throw ParseError.invalidLine(line) - } + } else if lowercasedLine != "[interface]" && lowercasedLine != "[peer]" { + throw ParseError.invalidLine(line) } - let isLastLine = (lineIndex == lines.count - 1) + let isLastLine = lineIndex == lines.count - 1 if isLastLine || lowercasedLine == "[interface]" || lowercasedLine == "[peer]" { // Previous section has ended; process the attributes collected so far @@ -109,7 +107,7 @@ class WgQuickConfigFileParser { } // wg-quick fields if let addressesString = attributes["address"] { - var addresses: [IPAddressRange] = [] + var addresses = [IPAddressRange]() for addressString in addressesString.split(separator: ",") { let trimmedString = addressString.trimmingCharacters(in: .whitespaces) guard let address = IPAddressRange(from: trimmedString) else { return nil } @@ -118,7 +116,7 @@ class WgQuickConfigFileParser { interface.addresses = addresses } if let dnsString = attributes["dns"] { - var dnsServers: [DNSServer] = [] + var dnsServers = [DNSServer]() for dnsServerString in dnsString.split(separator: ",") { let trimmedString = dnsServerString.trimmingCharacters(in: .whitespaces) guard let dnsServer = DNSServer(from: trimmedString) else { return nil } @@ -144,7 +142,7 @@ class WgQuickConfigFileParser { peer.preSharedKey = preSharedKey } if let allowedIPsString = attributes["allowedips"] { - var allowedIPs: [IPAddressRange] = [] + var allowedIPs = [IPAddressRange]() for allowedIPString in allowedIPsString.split(separator: ",") { let trimmedString = allowedIPString.trimmingCharacters(in: CharacterSet.whitespacesAndNewlines) guard let allowedIP = IPAddressRange(from: trimmedString) else { return nil } diff --git a/WireGuard/WireGuard/UI/TunnelViewModel.swift b/WireGuard/WireGuard/UI/TunnelViewModel.swift index af7ff4c..80a6092 100644 --- a/WireGuard/WireGuard/UI/TunnelViewModel.swift +++ b/WireGuard/WireGuard/UI/TunnelViewModel.swift @@ -37,8 +37,8 @@ class TunnelViewModel { static let keyLengthInBase64 = 44 class InterfaceData { - var scratchpad: [InterfaceField: String] = [:] - var fieldsWithError: Set = [] + var scratchpad = [InterfaceField: String]() + var fieldsWithError = Set() var validatedConfiguration: InterfaceConfiguration? subscript(field: InterfaceField) -> String { @@ -114,9 +114,9 @@ class TunnelViewModel { return .error("Interface's private key must be a 32-byte key in base64 encoding") } var config = InterfaceConfiguration(name: name, privateKey: privateKey) - var errorMessages: [String] = [] + var errorMessages = [String]() if let addressesString = scratchpad[.addresses] { - var addresses: [IPAddressRange] = [] + var addresses = [IPAddressRange]() for addressString in addressesString.split(separator: ",") { let trimmedString = addressString.trimmingCharacters(in: CharacterSet.whitespacesAndNewlines) if let address = IPAddressRange(from: trimmedString) { @@ -145,7 +145,7 @@ class TunnelViewModel { } } if let dnsString = scratchpad[.dns] { - var dnsServers: [DNSServer] = [] + var dnsServers = [DNSServer]() for dnsServerString in dnsString.split(separator: ",") { let trimmedString = dnsServerString.trimmingCharacters(in: CharacterSet.whitespacesAndNewlines) if let dnsServer = DNSServer(from: trimmedString) { @@ -177,14 +177,14 @@ class TunnelViewModel { class PeerData { var index: Int - var scratchpad: [PeerField: String] = [:] - var fieldsWithError: Set = [] + var scratchpad = [PeerField: String]() + var fieldsWithError = Set() var validatedConfiguration: PeerConfiguration? // For exclude private IPs - var shouldAllowExcludePrivateIPsControl: Bool = false /* Read-only from the VC's point of view */ - var excludePrivateIPsValue: Bool = false /* Read-only from the VC's point of view */ - fileprivate var numberOfPeers: Int = 0 + private(set) var shouldAllowExcludePrivateIPsControl = false + private(set) var excludePrivateIPsValue = false + fileprivate var numberOfPeers = 0 init(index: Int) { self.index = index @@ -251,7 +251,7 @@ class TunnelViewModel { return .error("Peer's public key must be a 32-byte key in base64 encoding") } var config = PeerConfiguration(publicKey: publicKey) - var errorMessages: [String] = [] + var errorMessages = [String]() if let preSharedKeyString = scratchpad[.preSharedKey] { if let preSharedKey = Data(base64Encoded: preSharedKeyString), preSharedKey.count == TunnelConfiguration.keyLength { config.preSharedKey = preSharedKey @@ -261,7 +261,7 @@ class TunnelViewModel { } } if let allowedIPsString = scratchpad[.allowedIPs] { - var allowedIPs: [IPAddressRange] = [] + var allowedIPs = [IPAddressRange]() for allowedIPString in allowedIPsString.split(separator: ",") { let trimmedString = allowedIPString.trimmingCharacters(in: CharacterSet.whitespacesAndNewlines) if let allowedIP = IPAddressRange(from: trimmedString) { @@ -352,11 +352,9 @@ class TunnelViewModel { let ipv6Addresses = allowedIPStrings.filter { $0.contains(":") } let modifiedAllowedIPStrings: [String] if isOn { - modifiedAllowedIPStrings = ipv6Addresses + - TunnelViewModel.PeerData.ipv4DefaultRouteModRFC1918String + dnsServerStrings + modifiedAllowedIPStrings = ipv6Addresses + TunnelViewModel.PeerData.ipv4DefaultRouteModRFC1918String + dnsServerStrings } else { - modifiedAllowedIPStrings = ipv6Addresses + - [TunnelViewModel.PeerData.ipv4DefaultRouteString] + modifiedAllowedIPStrings = ipv6Addresses + [TunnelViewModel.PeerData.ipv4DefaultRouteString] } scratchpad[.allowedIPs] = modifiedAllowedIPStrings.joined(separator: ", ") validatedConfiguration = nil // The configuration has been modified, and needs to be saved @@ -374,7 +372,7 @@ class TunnelViewModel { init(tunnelConfiguration: TunnelConfiguration?) { let interfaceData: InterfaceData = InterfaceData() - var peersData: [PeerData] = [] + var peersData = [PeerData]() if let tunnelConfiguration = tunnelConfiguration { interfaceData.validatedConfiguration = tunnelConfiguration.interface for (index, peerConfiguration) in tunnelConfiguration.peers.enumerated() { @@ -423,7 +421,7 @@ class TunnelViewModel { case .error(let errorMessage): return .error(errorMessage) case .saved(let interfaceConfiguration): - var peerConfigurations: [PeerConfiguration] = [] + var peerConfigurations = [PeerConfiguration]() peerConfigurations.reserveCapacity(peerSaveResults.count) for peerSaveResult in peerSaveResults { switch peerSaveResult { diff --git a/WireGuard/WireGuard/UI/iOS/ErrorPresenter.swift b/WireGuard/WireGuard/UI/iOS/ErrorPresenter.swift index 7c28495..2889694 100644 --- a/WireGuard/WireGuard/UI/iOS/ErrorPresenter.swift +++ b/WireGuard/WireGuard/UI/iOS/ErrorPresenter.swift @@ -5,10 +5,10 @@ import UIKit import os.log class ErrorPresenter { - static func showErrorAlert(error: WireGuardAppError, from sourceVC: UIViewController?, - onPresented: (() -> Void)? = nil, onDismissal: (() -> Void)? = nil) { + static func showErrorAlert(error: WireGuardAppError, from sourceVC: UIViewController?, onPresented: (() -> Void)? = nil, onDismissal: (() -> Void)? = nil) { guard let sourceVC = sourceVC else { return } - guard let (title, message) = error.alertText() else { return } + + let (title, message) = error.alertText() let okAction = UIAlertAction(title: "OK", style: .default) { _ in onDismissal?() } @@ -18,9 +18,9 @@ class ErrorPresenter { sourceVC.present(alert, animated: true, completion: onPresented) } - static func showErrorAlert(title: String, message: String, from sourceVC: UIViewController?, - onPresented: (() -> Void)? = nil, onDismissal: (() -> Void)? = nil) { + static func showErrorAlert(title: String, message: String, from sourceVC: UIViewController?, onPresented: (() -> Void)? = nil, onDismissal: (() -> Void)? = nil) { guard let sourceVC = sourceVC else { return } + let okAction = UIAlertAction(title: "OK", style: .default) { _ in onDismissal?() } diff --git a/WireGuard/WireGuard/UI/iOS/TunnelDetailTableViewController.swift b/WireGuard/WireGuard/UI/iOS/TunnelDetailTableViewController.swift index e6ad024..c5816e8 100644 --- a/WireGuard/WireGuard/UI/iOS/TunnelDetailTableViewController.swift +++ b/WireGuard/WireGuard/UI/iOS/TunnelDetailTableViewController.swift @@ -322,9 +322,9 @@ private class KeyValueCell: CopyableLabelTableViewCell { let keyLabel: UILabel let valueLabel: ScrollableLabel - var isStackedHorizontally: Bool = false - var isStackedVertically: Bool = false - var contentSizeBasedConstraints: [NSLayoutConstraint] = [] + var isStackedHorizontally = false + var isStackedVertically = false + var contentSizeBasedConstraints = [NSLayoutConstraint]() override init(style: UITableViewCell.CellStyle, reuseIdentifier: String?) { keyLabel = UILabel() @@ -364,7 +364,7 @@ private class KeyValueCell: CopyableLabelTableViewCell { } func configureForContentSize() { - var constraints: [NSLayoutConstraint] = [] + var constraints = [NSLayoutConstraint]() if self.traitCollection.preferredContentSizeCategory.isAccessibilityCategory { // Stack vertically if !isStackedVertically { diff --git a/WireGuard/WireGuard/UI/iOS/TunnelEditTableViewController.swift b/WireGuard/WireGuard/UI/iOS/TunnelEditTableViewController.swift index b26992d..7cd96a2 100644 --- a/WireGuard/WireGuard/UI/iOS/TunnelEditTableViewController.swift +++ b/WireGuard/WireGuard/UI/iOS/TunnelEditTableViewController.swift @@ -334,52 +334,45 @@ extension TunnelEditTableViewController { switch field { case .publicKey: cell.placeholderText = "Required" - case .preSharedKey, .endpoint, .allowedIPs: + cell.keyboardType = .default + case .preSharedKey, .endpoint: + cell.placeholderText = "Optional" + cell.keyboardType = .default + case .allowedIPs: cell.placeholderText = "Optional" + cell.keyboardType = .numbersAndPunctuation case .persistentKeepAlive: cell.placeholderText = "Off" - case .excludePrivateIPs, .deletePeer: - break - } - - switch field { - case .persistentKeepAlive: cell.keyboardType = .numberPad - case .allowedIPs: - cell.keyboardType = .numbersAndPunctuation - default: + case .excludePrivateIPs, .deletePeer: cell.keyboardType = .default } - // Show erroring fields - cell.isValueValid = (!peerData.fieldsWithError.contains(field)) - // Bind values to view model + cell.isValueValid = !peerData.fieldsWithError.contains(field) cell.value = peerData[field] - if field != .allowedIPs { - cell.onValueChanged = { [weak peerData] value in - peerData?[field] = value - } - } - // Compute state of exclude private IPs live + if field == .allowedIPs { cell.onValueBeingEdited = { [weak self, weak peerData] value in - if let peerData = peerData, let self = self { - let oldValue = peerData.shouldAllowExcludePrivateIPsControl - peerData[.allowedIPs] = value - if oldValue != peerData.shouldAllowExcludePrivateIPsControl { - if let row = self.peerFields.firstIndex(of: .excludePrivateIPs) { - if peerData.shouldAllowExcludePrivateIPsControl { - self.tableView.insertRows(at: [IndexPath(row: row, section: indexPath.section)], with: .fade) - } else { - self.tableView.deleteRows(at: [IndexPath(row: row, section: indexPath.section)], with: .fade) - } + guard let self = self, let peerData = peerData else { return } + + let oldValue = peerData.shouldAllowExcludePrivateIPsControl + peerData[.allowedIPs] = value + if oldValue != peerData.shouldAllowExcludePrivateIPsControl { + if let row = self.peerFields.firstIndex(of: .excludePrivateIPs) { + if peerData.shouldAllowExcludePrivateIPsControl { + self.tableView.insertRows(at: [IndexPath(row: row, section: indexPath.section)], with: .fade) + } else { + self.tableView.deleteRows(at: [IndexPath(row: row, section: indexPath.section)], with: .fade) } } } } } else { - cell.onValueBeingEdited = nil + cell.onValueChanged = { [weak peerData] value in + peerData?[field] = value + } } + return cell } @@ -410,7 +403,7 @@ extension TunnelEditTableViewController { cell.isOn = activateOnDemandSetting.isActivateOnDemandEnabled cell.onSwitchToggled = { [weak self] isOn in guard let self = self else { return } - let indexPaths: [IndexPath] = (1 ..< 4).map { IndexPath(row: $0, section: indexPath.section) } + let indexPaths = (1 ..< 4).map { IndexPath(row: $0, section: indexPath.section) } if isOn { self.activateOnDemandSetting.isActivateOnDemandEnabled = true if self.activateOnDemandSetting.activateOnDemandOption == .none { @@ -529,7 +522,7 @@ private class KeyValueCell: UITableViewCell { var isStackedHorizontally: Bool = false var isStackedVertically: Bool = false - var contentSizeBasedConstraints: [NSLayoutConstraint] = [] + var contentSizeBasedConstraints = [NSLayoutConstraint]() private var textFieldValueOnBeginEditing: String = "" @@ -573,7 +566,7 @@ private class KeyValueCell: UITableViewCell { } func configureForContentSize() { - var constraints: [NSLayoutConstraint] = [] + var constraints = [NSLayoutConstraint]() if self.traitCollection.preferredContentSizeCategory.isAccessibilityCategory { // Stack vertically if !isStackedVertically { diff --git a/WireGuard/WireGuard/UI/iOS/TunnelsListTableViewController.swift b/WireGuard/WireGuard/UI/iOS/TunnelsListTableViewController.swift index e5d5af1..f24b452 100644 --- a/WireGuard/WireGuard/UI/iOS/TunnelsListTableViewController.swift +++ b/WireGuard/WireGuard/UI/iOS/TunnelsListTableViewController.swift @@ -173,7 +173,7 @@ class TunnelsListTableViewController: UIViewController { ErrorPresenter.showErrorAlert(error: error, from: self) return } - let configs: [TunnelConfiguration?] = result.value! + let configs = result.value! tunnelsManager.addMultiple(tunnelConfigurations: configs.compactMap { $0 }) { [weak self] numberSuccessful in if numberSuccessful == configs.count { completionHandler?() diff --git a/WireGuard/WireGuard/VPN/TunnelsManager.swift b/WireGuard/WireGuard/VPN/TunnelsManager.swift index 9716eaf..9d48cdb 100644 --- a/WireGuard/WireGuard/VPN/TunnelsManager.swift +++ b/WireGuard/WireGuard/VPN/TunnelsManager.swift @@ -32,7 +32,8 @@ enum TunnelsManagerError: WireGuardAppError { case tunnelActivationFailedInternalError // startTunnel() succeeded, but activation failed case tunnelActivationFailedNoInternetConnection // startTunnel() succeeded, but activation failed since no internet - func alertText() -> (String, String)? { + //swiftlint:disable:next cyclomatic_complexity + func alertText() -> AlertText { switch self { case .tunnelNameEmpty: return ("No name provided", "Can't create tunnel with an empty name") @@ -46,7 +47,6 @@ enum TunnelsManagerError: WireGuardAppError { return ("Unable to modify tunnel", "Internal error") case .vpnSystemErrorOnRemoveTunnel: return ("Unable to remove tunnel", "Internal error") - case .attemptingActivationWhenTunnelIsNotInactive: return ("Activation failure", "The tunnel is already active or in the process of being activated") case .attemptingActivationWhenAnotherTunnelIsOperational(let otherTunnelName): @@ -267,44 +267,41 @@ class TunnelsManager { } private func startObservingTunnelStatuses() { - if statusObservationToken != nil { return } - statusObservationToken = NotificationCenter.default.addObserver( - forName: .NEVPNStatusDidChange, - object: nil, - queue: OperationQueue.main) { [weak self] statusChangeNotification in - guard let self = self else { return } - guard let session = statusChangeNotification.object as? NETunnelProviderSession else { return } - guard let tunnelProvider = session.manager as? NETunnelProviderManager else { return } - guard let tunnel = self.tunnels.first(where: { $0.tunnelProvider == tunnelProvider }) else { return } - - os_log("Tunnel '%{public}@' connection status changed to '%{public}@'", - log: OSLog.default, type: .debug, tunnel.name, "\(tunnel.tunnelProvider.connection.status)") - - // In case our attempt to start the tunnel, didn't succeed - if tunnel == self.tunnelBeingActivated { - if session.status == .disconnected { - if InternetReachability.currentStatus() == .notReachable { - let error = TunnelsManagerError.tunnelActivationFailedNoInternetConnection - self.activationDelegate?.tunnelActivationFailed(tunnel: tunnel, error: error) - } - self.tunnelBeingActivated = nil - } else if session.status == .connected { - self.tunnelBeingActivated = nil + guard statusObservationToken == nil else { return } + + statusObservationToken = NotificationCenter.default.addObserver(forName: .NEVPNStatusDidChange, object: nil, queue: OperationQueue.main) { [weak self] statusChangeNotification in + guard let self = self else { return } + guard let session = statusChangeNotification.object as? NETunnelProviderSession else { return } + guard let tunnelProvider = session.manager as? NETunnelProviderManager else { return } + guard let tunnel = self.tunnels.first(where: { $0.tunnelProvider == tunnelProvider }) else { return } + + os_log("Tunnel '%{public}@' connection status changed to '%{public}@'", + log: OSLog.default, type: .debug, tunnel.name, "\(tunnel.tunnelProvider.connection.status)") + + // In case our attempt to start the tunnel, didn't succeed + if tunnel == self.tunnelBeingActivated { + if session.status == .disconnected { + if InternetReachability.currentStatus() == .notReachable { + let error = TunnelsManagerError.tunnelActivationFailedNoInternetConnection + self.activationDelegate?.tunnelActivationFailed(tunnel: tunnel, error: error) } + self.tunnelBeingActivated = nil + } else if session.status == .connected { + self.tunnelBeingActivated = nil } - - // In case we're restarting the tunnel - if (tunnel.status == .restarting) && (session.status == .disconnected || session.status == .disconnecting) { - // Don't change tunnel.status when disconnecting for a restart - if session.status == .disconnected { - self.tunnelBeingActivated = tunnel - tunnel.startActivation { _ in } - } - return + } + + // In case we're restarting the tunnel + if (tunnel.status == .restarting) && (session.status == .disconnected || session.status == .disconnecting) { + // Don't change tunnel.status when disconnecting for a restart + if session.status == .disconnected { + self.tunnelBeingActivated = tunnel + tunnel.startActivation { _ in } } - - // Update tunnel status - tunnel.refreshStatus() + return + } + + tunnel.refreshStatus() } } diff --git a/WireGuard/WireGuard/WireGuardAppError.swift b/WireGuard/WireGuard/WireGuardAppError.swift index 3e4707d..ba83ac5 100644 --- a/WireGuard/WireGuard/WireGuardAppError.swift +++ b/WireGuard/WireGuard/WireGuardAppError.swift @@ -2,5 +2,6 @@ // Copyright © 2018 WireGuard LLC. All Rights Reserved. protocol WireGuardAppError: Error { - func alertText() -> (/* title */ String, /* message */ String)? + typealias AlertText = (title: String, message: String) + func alertText() -> AlertText } diff --git a/WireGuard/WireGuard/ZipArchive/ZipArchive.swift b/WireGuard/WireGuard/ZipArchive/ZipArchive.swift index 12cfb69..ad74d0e 100644 --- a/WireGuard/WireGuard/ZipArchive/ZipArchive.swift +++ b/WireGuard/WireGuard/ZipArchive/ZipArchive.swift @@ -8,7 +8,7 @@ enum ZipArchiveError: WireGuardAppError { case cantOpenOutputZipFileForWriting case badArchive - func alertText() -> (String, String)? { + func alertText() -> AlertText { switch self { case .cantOpenInputZipFile: return ("Unable to read zip archive", "The zip archive could not be read.") @@ -41,7 +41,7 @@ class ZipArchive { static func unarchive(url: URL, requiredFileExtensions: [String]) throws -> [(fileBaseName: String, contents: Data)] { - var results: [(fileBaseName: String, contents: Data)] = [] + var results = [(fileBaseName: String, contents: Data)]() guard let zipFile = unzOpen64(url.path) else { throw ZipArchiveError.cantOpenInputZipFile diff --git a/WireGuard/WireGuard/ZipArchive/ZipExporter.swift b/WireGuard/WireGuard/ZipArchive/ZipExporter.swift index b0e6b15..cdc9ac9 100644 --- a/WireGuard/WireGuard/ZipArchive/ZipExporter.swift +++ b/WireGuard/WireGuard/ZipArchive/ZipExporter.swift @@ -6,7 +6,7 @@ import UIKit enum ZipExporterError: WireGuardAppError { case noTunnelsToExport - func alertText() -> (String, String)? { + func alertText() -> AlertText { switch self { case .noTunnelsToExport: return ("Nothing to export", "There are no tunnels to export") @@ -15,8 +15,7 @@ enum ZipExporterError: WireGuardAppError { } class ZipExporter { - static func exportConfigFiles(tunnelConfigurations: [TunnelConfiguration], to url: URL, - completion: @escaping (WireGuardAppError?) -> Void) { + static func exportConfigFiles(tunnelConfigurations: [TunnelConfiguration], to url: URL, completion: @escaping (WireGuardAppError?) -> Void) { guard !tunnelConfigurations.isEmpty else { completion(ZipExporterError.noTunnelsToExport) diff --git a/WireGuard/WireGuard/ZipArchive/ZipImporter.swift b/WireGuard/WireGuard/ZipArchive/ZipImporter.swift index e2767f2..523614b 100644 --- a/WireGuard/WireGuard/ZipArchive/ZipImporter.swift +++ b/WireGuard/WireGuard/ZipArchive/ZipImporter.swift @@ -6,7 +6,7 @@ import UIKit enum ZipImporterError: WireGuardAppError { case noTunnelsInZipArchive - func alertText() -> (String, String)? { + func alertText() -> AlertText { switch self { case .noTunnelsInZipArchive: return ("No tunnels in zip archive", "No .conf tunnel files were found inside the zip archive.") -- cgit v1.2.3-59-g8ed1b