Skip to content

Commit a5a183a

Browse files
authored
Feature/domain parsing fix (#2)
* Fix parsing of domain to URL, add tests * Clean-up unused code * Reformat and license * Handle non web schemes in URL * URL built from domain can still be invalid, don't force unwrap but throw an error
1 parent 7406992 commit a5a183a

File tree

7 files changed

+274
-24
lines changed

7 files changed

+274
-24
lines changed

ORLib.xcodeproj/project.pbxproj

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@
4343
4CFEC51C2C919AF300DCC936 /* ORLib.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 4CBDF2AA2AE285E400C7D94C /* ORLib.framework */; };
4444
4CFEC51D2C919AF300DCC936 /* ORLib.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = 4CBDF2AA2AE285E400C7D94C /* ORLib.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
4545
914F7D4429215D3500655A22 /* test12.json in Resources */ = {isa = PBXBuildFile; fileRef = 914F7D4329215D3500655A22 /* test12.json */; };
46+
9154E2A02D9EB0D50055E565 /* StringUtilsTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9154E29F2D9EB0D50055E565 /* StringUtilsTest.swift */; };
47+
9154E2A22D9EB3220055E565 /* URLTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9154E2A12D9EB3220055E565 /* URLTest.swift */; };
4648
9156512A28FC6D6700062E16 /* test9.json in Resources */ = {isa = PBXBuildFile; fileRef = 9156512928FC6D6700062E16 /* test9.json */; };
4749
91932F9328C66A3C00BABBA3 /* test1.json in Resources */ = {isa = PBXBuildFile; fileRef = 91932F9228C66A3C00BABBA3 /* test1.json */; };
4850
91932F9628C6715C00BABBA3 /* test2.json in Resources */ = {isa = PBXBuildFile; fileRef = 91932F9528C6715C00BABBA3 /* test2.json */; };
@@ -116,6 +118,8 @@
116118
4CF5D06727143F1F00D705BE /* ORNotificationResource.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ORNotificationResource.swift; sourceTree = "<group>"; };
117119
4CF5D06827143F1F00D705BE /* HttpApiManager.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = HttpApiManager.swift; sourceTree = "<group>"; };
118120
914F7D4329215D3500655A22 /* test12.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = test12.json; sourceTree = "<group>"; };
121+
9154E29F2D9EB0D50055E565 /* StringUtilsTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StringUtilsTest.swift; sourceTree = "<group>"; };
122+
9154E2A12D9EB3220055E565 /* URLTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = URLTest.swift; sourceTree = "<group>"; };
119123
9156512928FC6D6700062E16 /* test9.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = test9.json; sourceTree = "<group>"; };
120124
91658F9028897E55000FF05C /* ORConsoleConfig.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ORConsoleConfig.swift; sourceTree = "<group>"; };
121125
91658F9228897EDD000FF05C /* ORAppInfo.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ORAppInfo.swift; sourceTree = "<group>"; };
@@ -296,6 +300,8 @@
296300
91A9A8FA28BF6A4900DF8928 /* ConfigManagerTest.swift */,
297301
91A9A90028BF6EA000DF8928 /* FileApiManager.swift */,
298302
91AA79F228D628E9005B9913 /* Fixture.swift */,
303+
9154E29F2D9EB0D50055E565 /* StringUtilsTest.swift */,
304+
9154E2A12D9EB3220055E565 /* URLTest.swift */,
299305
);
300306
path = Tests;
301307
sourceTree = "<group>";
@@ -463,6 +469,8 @@
463469
buildActionMask = 2147483647;
464470
files = (
465471
91A9A90128BF6EA000DF8928 /* FileApiManager.swift in Sources */,
472+
9154E2A22D9EB3220055E565 /* URLTest.swift in Sources */,
473+
9154E2A02D9EB0D50055E565 /* StringUtilsTest.swift in Sources */,
466474
91A9A8FB28BF6A4900DF8928 /* ConfigManagerTest.swift in Sources */,
467475
91AA79F328D628E9005B9913 /* Fixture.swift in Sources */,
468476
);

ORLib/ConfigManager.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ public enum ConfigManagerError: Error {
3232
case couldNotLoadAppConfig
3333
}
3434

35-
public typealias ApiManagerFactory = (String) -> ApiManager
35+
public typealias ApiManagerFactory = (String) throws -> ApiManager
3636

3737

3838
public class ConfigManager {
3939

40-
private var apiManagerFactory: ((String) -> ApiManager)
40+
private var apiManagerFactory: ApiManagerFactory
4141
private var apiManager: ApiManager?
4242

4343
public private(set) var globalAppInfos : [String:ORAppInfo] = [:] // app infos from the top level consoleConfig information
@@ -52,11 +52,11 @@ public class ConfigManager {
5252
public func setDomain(domain: String) async throws -> ConfigManagerState {
5353
switch state {
5454
case .selectDomain:
55-
let baseUrl = domain.isValidURL ? domain : "https://\(domain).openremote.app"
55+
let baseUrl = domain.buildBaseUrlFromDomain()
5656
let url = baseUrl.appending("/api/master")
5757

58-
apiManager = apiManagerFactory(url)
59-
58+
apiManager = try apiManagerFactory(url)
59+
6060
guard let api = apiManager else {
6161
throw ConfigManagerError.communicationError
6262
}
@@ -136,7 +136,7 @@ public class ConfigManager {
136136
}
137137

138138
}
139-
139+
140140
private func filterPotentialApps(apiManager: ApiManager, potentialApps: [String]?) async -> [String]? {
141141
var filteredApps : [String]?
142142
if let appNames = potentialApps {

ORLib/Network/ApiManager.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import Foundation
2222
public typealias ResponseBlock<T: Codable> = (_ statusCode: Int, _ object: T?, _ error: Error?) -> ()
2323

2424
public enum ApiManagerError: Error {
25+
case invalidUrl
2526
case notFound
2627
case communicationError(Int)
2728
case parsingError(Int)

ORLib/Network/HttpApiManager.swift

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,11 @@ public class HttpApiManager: NSObject, ApiManager {
3131

3232
private let baseUrl: URL;
3333

34-
public init(baseUrl: String) {
35-
self.baseUrl = URL(string: baseUrl)!
34+
public init(baseUrl: String) throws {
35+
guard let url = URL(string: baseUrl) else {
36+
throw ApiManagerError.invalidUrl
37+
}
38+
self.baseUrl = url
3639
super.init()
3740
}
3841

ORLib/Utils/String+Utils.swift

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,25 +21,46 @@ import Foundation
2121

2222
extension String {
2323

24-
func stringByURLEncoding() -> String? {
25-
26-
let characters = CharacterSet.urlQueryAllowed.union(CharacterSet(charactersIn: "#"))
27-
28-
guard let encodedString = self.addingPercentEncoding(withAllowedCharacters: characters) else {
29-
return nil
24+
func stringByURLEncoding() -> String? {
25+
let characters = CharacterSet.urlQueryAllowed.union(CharacterSet(charactersIn: "#"))
26+
guard let encodedString = self.addingPercentEncoding(withAllowedCharacters: characters) else {
27+
return nil
28+
}
29+
return encodedString
3030
}
3131

32-
return encodedString
33-
}
34-
35-
var isValidURL: Bool {
36-
let detector = try! NSDataDetector(types: NSTextCheckingResult.CheckingType.link.rawValue)
37-
if let match = detector.firstMatch(in: self, options: [], range: NSRange(location: 0, length: self.utf16.count)) {
38-
// it is a link, if the match covers the whole string
39-
return match.range.length == self.utf16.count
40-
} else {
41-
return false
32+
/// There is no validation that the generated string represents a valid URL.
33+
/// For instance, no validation is performed on the port if one is provided.
34+
func buildBaseUrlFromDomain() -> String {
35+
do {
36+
let pattern = "^(?:([0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6}))$"
37+
let ipv6NoSchemeNoPort = try NSRegularExpression(pattern: pattern)
38+
let numberOfMatches = ipv6NoSchemeNoPort.numberOfMatches(in: self, options: [], range: NSRange(location: 0, length: self.utf16.count))
39+
if numberOfMatches == 1 {
40+
return "https://[\(self)]"
41+
}
42+
} catch let error as NSError {
43+
print("Error creating NSRegularExpression: \(error)")
44+
}
45+
46+
let numberOfMatches: Int
47+
do {
48+
let schemePrefix = try NSRegularExpression(pattern: "^[a-zA-Z]+://.*$")
49+
numberOfMatches = schemePrefix.numberOfMatches(in: self, options: [], range: NSRange(location: 0, length: self.utf16.count))
50+
} catch let error as NSError {
51+
numberOfMatches = 0
52+
print("Error creating NSRegularExpression: \(error)")
53+
}
54+
if numberOfMatches == 1 {
55+
if self.firstIndex(of: ".") != nil || self.firstIndex(of: "[") != nil {
56+
return self
57+
} else {
58+
return "\(self).openremote.app"
59+
}
60+
} else if self.firstIndex(of: ".") != nil || self.firstIndex(of: "[") != nil {
61+
return "https://\(self)"
4262
}
63+
return "https://\(self).openremote.app"
4364
}
4465

4566
}

Tests/StringUtilsTest.swift

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*
2+
* Copyright 2017, OpenRemote Inc.
3+
*
4+
* See the CONTRIBUTORS.txt file in the distribution for a
5+
* full listing of individual contributors.
6+
*
7+
* This program is free software: you can redistribute it and/or modify
8+
* it under the terms of the GNU Affero General Public License as
9+
* published by the Free Software Foundation, either version 3 of the
10+
* License, or (at your option) any later version.
11+
*
12+
* This program is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
* GNU Affero General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU Affero General Public License
18+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
19+
*/
20+
21+
import Testing
22+
@testable import ORLib
23+
24+
struct StringUtilsTest {
25+
26+
@Test func fqdnWithScheme() async throws {
27+
#expect("http://www.example.com".buildBaseUrlFromDomain() == "http://www.example.com")
28+
#expect("https://www.example.com".buildBaseUrlFromDomain() == "https://www.example.com")
29+
}
30+
31+
@Test func fqdnWithNonWebScheme() async throws {
32+
#expect("ftp://www.example.com".buildBaseUrlFromDomain() == "ftp://www.example.com")
33+
}
34+
35+
@Test func fqdnNoScheme() async throws {
36+
#expect("www.example.com".buildBaseUrlFromDomain() == "https://www.example.com")
37+
}
38+
39+
@Test func fqdnAndPortWithScheme() async throws {
40+
#expect("http://www.example.com:8080".buildBaseUrlFromDomain() == "http://www.example.com:8080")
41+
#expect("https://www.example.com:443".buildBaseUrlFromDomain() == "https://www.example.com:443")
42+
}
43+
44+
@Test func fqdnAndPortWithNonWebScheme() async throws {
45+
#expect("ftp://www.example.com:21".buildBaseUrlFromDomain() == "ftp://www.example.com:21")
46+
}
47+
48+
@Test func fqdnAndPortNoScheme() async throws {
49+
#expect("www.example.com:8080".buildBaseUrlFromDomain() == "https://www.example.com:8080")
50+
}
51+
52+
@Test func hostnameNoScheme() async throws {
53+
#expect("example".buildBaseUrlFromDomain() == "https://example.openremote.app")
54+
}
55+
56+
@Test func ipAddressWithScheme () async throws {
57+
#expect("http://192.168.1.1".buildBaseUrlFromDomain() == "http://192.168.1.1")
58+
}
59+
60+
@Test func ipAddressWithNonWebScheme () async throws {
61+
#expect("ftp://192.168.1.1".buildBaseUrlFromDomain() == "ftp://192.168.1.1")
62+
}
63+
64+
@Test func ipAddressAndPortWithScheme () async throws {
65+
#expect("http://192.168.1.1:8080".buildBaseUrlFromDomain() == "http://192.168.1.1:8080")
66+
}
67+
68+
@Test func ipAddressAndPortWithNonWebScheme () async throws {
69+
#expect("ftp://192.168.1.1:25".buildBaseUrlFromDomain() == "ftp://192.168.1.1:25")
70+
}
71+
72+
@Test func ipAddressAndInvalidPortWithScheme () async throws {
73+
#expect("http://192.168.1.1:InvalidPort".buildBaseUrlFromDomain() == "http://192.168.1.1:InvalidPort")
74+
}
75+
76+
@Test func ipAddressNoScheme () async throws {
77+
#expect("192.168.1.1".buildBaseUrlFromDomain() == "https://192.168.1.1")
78+
}
79+
80+
@Test func ipAddressAndPortNoScheme () async throws {
81+
#expect("192.168.1.1:8080".buildBaseUrlFromDomain() == "https://192.168.1.1:8080")
82+
}
83+
84+
@Test func ipv6AddressWithScheme () async throws {
85+
#expect("http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]".buildBaseUrlFromDomain() == "http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]")
86+
}
87+
88+
@Test func ipv6AddressAndPortWithScheme () async throws {
89+
#expect("http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:8080".buildBaseUrlFromDomain() == "http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:8080")
90+
}
91+
92+
@Test func ipv6AddressNoScheme () async throws {
93+
#expect("2001:0db8:85a3:0000:0000:8a2e:0370:7334".buildBaseUrlFromDomain() == "https://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]")
94+
#expect("[2001:0db8:85a3:0000:0000:8a2e:0370:7334]".buildBaseUrlFromDomain() == "https://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]")
95+
}
96+
97+
@Test func ipv6AddressAndPortNoScheme () async throws {
98+
#expect("[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:8080".buildBaseUrlFromDomain() == "https://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:8080")
99+
}
100+
101+
@Test func ipv6CompressedAddressWithScheme () async throws {
102+
#expect("http://[2001:db8:85a3::8a2e:370:7334]".buildBaseUrlFromDomain() == "http://[2001:db8:85a3::8a2e:370:7334]")
103+
}
104+
105+
@Test func ipv6CompressedAddressAndPortWithScheme () async throws {
106+
#expect("http://[2001:db8:85a3::8a2e:370:7334]:8080".buildBaseUrlFromDomain() == "http://[2001:db8:85a3::8a2e:370:7334]:8080")
107+
}
108+
109+
@Test func ipv6CompressedAddressNoScheme () async throws {
110+
#expect("2001:db8:85a3::8a2e:370:7334".buildBaseUrlFromDomain() == "https://[2001:db8:85a3::8a2e:370:7334]")
111+
#expect("[2001:db8:85a3::8a2e:370:7334]".buildBaseUrlFromDomain() == "https://[2001:db8:85a3::8a2e:370:7334]")
112+
}
113+
114+
@Test func ipv6CompressedAddressAndPortNoScheme () async throws {
115+
#expect("[2001:db8:85a3::8a2e:370:7334]:8080".buildBaseUrlFromDomain() == "https://[2001:db8:85a3::8a2e:370:7334]:8080")
116+
}
117+
118+
}

Tests/URLTest.swift

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/*
2+
* Copyright 2017, OpenRemote Inc.
3+
*
4+
* See the CONTRIBUTORS.txt file in the distribution for a
5+
* full listing of individual contributors.
6+
*
7+
* This program is free software: you can redistribute it and/or modify
8+
* it under the terms of the GNU Affero General Public License as
9+
* published by the Free Software Foundation, either version 3 of the
10+
* License, or (at your option) any later version.
11+
*
12+
* This program is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
* GNU Affero General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU Affero General Public License
18+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
19+
*/
20+
21+
import Testing
22+
@testable import ORLib
23+
24+
/// Tests to understand how URL parsing works
25+
struct URLTest {
26+
27+
@Test func FQDNWithScheme() async throws {
28+
let url = URL(string: "https://www.example.com")
29+
#expect(url != nil)
30+
#expect(url!.host == "www.example.com")
31+
#expect(url!.scheme == "https")
32+
#expect(url!.port == nil)
33+
}
34+
35+
@Test func FQDNWithCustomScheme() async throws {
36+
let url = URL(string: "myscheme://www.example.com")
37+
#expect(url != nil)
38+
#expect(url!.host == "www.example.com")
39+
#expect(url!.scheme == "myscheme")
40+
#expect(url!.port == nil)
41+
}
42+
43+
@Test func FQDNWithSchemeAndPort() async throws {
44+
let url = URL(string: "https://www.example.com:1234")
45+
#expect(url != nil)
46+
#expect(url!.host == "www.example.com")
47+
#expect(url!.scheme == "https")
48+
#expect(url!.port == 1234)
49+
}
50+
51+
@Test func FQDNNoScheme() async throws {
52+
let url = URL(string: "www.example.com")
53+
#expect(url != nil)
54+
#expect(url!.host == nil)
55+
#expect(url!.scheme == nil)
56+
#expect(url!.port == nil)
57+
}
58+
59+
@Test func hostnameWithScheme() async throws {
60+
let url = URL(string: "http://example")
61+
#expect(url != nil)
62+
#expect(url!.host == "example")
63+
#expect(url!.scheme == "http")
64+
#expect(url!.port == nil)
65+
}
66+
67+
@Test func hostnameNoScheme() async throws {
68+
let url = URL(string: "example")
69+
#expect(url != nil)
70+
#expect(url!.host == nil)
71+
#expect(url!.scheme == nil)
72+
#expect(url!.port == nil)
73+
}
74+
75+
@Test func ipWithScheme() async throws {
76+
let url = URL(string: "http://192.168.1.1")
77+
#expect(url != nil)
78+
#expect(url!.host == "192.168.1.1")
79+
#expect(url!.scheme == "http")
80+
#expect(url!.port == nil)
81+
}
82+
83+
@Test func ipNoScheme() async throws {
84+
let url = URL(string: "192.168.1.1")
85+
#expect(url != nil)
86+
#expect(url!.host == nil)
87+
#expect(url!.scheme == nil)
88+
#expect(url!.port == nil)
89+
}
90+
91+
/// ! URL does not validate it's an IP address
92+
@Test func invalidIpNoScheme() async throws {
93+
let url = URL(string: "432.168.1.1")
94+
#expect(url != nil)
95+
#expect(url!.host == nil)
96+
#expect(url!.scheme == nil)
97+
#expect(url!.port == nil)
98+
}
99+
}

0 commit comments

Comments
 (0)