From 578f6422b91ea074a082e7b4ee0ec4fe9e10ea22 Mon Sep 17 00:00:00 2001
From: PB Thiffault <pbthif@amazon.com>
Date: Thu, 26 Aug 2021 15:50:36 -0400
Subject: [PATCH 1/2] Add support for virtual hosted style S3 URLs in
 String.asS3ObjectIdentifier(). Extend String.asS3ObjectIdentifier() unit
 tests to validate a variety of valid S3 URL styles.

---
 .../String+asS3ObjectIdentifier.swift         |  88 +++++++--------
 Tests/S3ClientTests/S3ClientTests.swift       | 102 +++++++++++-------
 2 files changed, 107 insertions(+), 83 deletions(-)

diff --git a/Sources/S3Client/String+asS3ObjectIdentifier.swift b/Sources/S3Client/String+asS3ObjectIdentifier.swift
index 0b825543..b6db0fe4 100644
--- a/Sources/S3Client/String+asS3ObjectIdentifier.swift
+++ b/Sources/S3Client/String+asS3ObjectIdentifier.swift
@@ -13,10 +13,11 @@ public struct S3ObjectIdentifer: Equatable {
     internal static let s3Prefix = "s3://"
     internal static let httpsPrefix = "https://"
     internal static let httpPrefix = "http://"
-    
+    internal static let s3EndpointRegex = #"^https?:\/\/(.+\.)?s3[.-][a-z0-9-]+\."#
+
     public let bucketName: String
     public let keyPath: String
-    
+
     public init(bucketName: String,
                 keyPath: String) {
         self.bucketName = bucketName
@@ -28,10 +29,10 @@ public extension String {
     /**
      If possible creates an S3ObjectIdentifer instance from this string,
      seperating the bucket name and key path.
- 
+
      Will return nil if this string is not of the form-
        s3://bucketName/the/key/path
-     
+
      This would return S3ObjectIdentifer(bucketName: "bucketName",
                                          keyPath: "/the/key/path")
      */
@@ -39,45 +40,48 @@ public extension String {
         if self.starts(with: S3ObjectIdentifer.s3Prefix) {
             // get the url without the scheme - of the form {bucket}/{key+}
             let nonPrefixedUrl = self.dropFirst(S3ObjectIdentifer.s3Prefix.count)
-            
-            return asS3ObjectIdentifierFromNonPrefixedUrl(nonPrefixedUrl: nonPrefixedUrl)
-        } else if self.starts(with: S3ObjectIdentifer.httpsPrefix) {
-            // get the url without the scheme - of the form {host}/{bucket}/{key+}
-            let droppedPrefix = self.dropFirst(S3ObjectIdentifer.httpsPrefix.count)
-            
-            // get the index of the separator between the host and the bucket
-            guard let nextUrlSeparator = getIndexOfNextUrlSeparator(url: droppedPrefix) else {
-                return nil
-            }
-            
-            let bucketStartIndex = droppedPrefix.index(nextUrlSeparator,
-                                                               offsetBy: 1)
-            // get the url without the scheme or the host -
-            // of the form {bucket}/{key+}
-            let nonPrefixedUrl = droppedPrefix[bucketStartIndex...]
-            
-            return asS3ObjectIdentifierFromNonPrefixedUrl(nonPrefixedUrl: nonPrefixedUrl)
-        } else if self.starts(with: S3ObjectIdentifer.httpPrefix) {
-            // get the url without the scheme - of the form {host}/{bucket}/{key+}
-            let droppedPrefix = self.dropFirst(S3ObjectIdentifer.httpPrefix.count)
-            
-            // get the index of the separator between the host and the bucket
-            guard let nextUrlSeparator = getIndexOfNextUrlSeparator(url: droppedPrefix) else {
-                return nil
-            }
-            
-            let bucketStartIndex = droppedPrefix.index(nextUrlSeparator,
-                                                               offsetBy: 1)
-            // get the url without the scheme or the host -
-            // of the form {bucket}/{key+}
-            let nonPrefixedUrl = droppedPrefix[bucketStartIndex...]
-            
+
             return asS3ObjectIdentifierFromNonPrefixedUrl(nonPrefixedUrl: nonPrefixedUrl)
+        } else if self.starts(with: S3ObjectIdentifer.httpsPrefix) || self.starts(with: S3ObjectIdentifer.httpPrefix) {
+            return asS3ObjectIdentifierFromHttpOrHttps()
         }
-        
+
         return nil
     }
-    
+
+    /// Tries to parse the bucket and key names from an HTTP or HTTPS URL.
+    private func asS3ObjectIdentifierFromHttpOrHttps() -> S3ObjectIdentifer? {
+        guard let url = URL(string: self) else {
+            return nil
+        }
+
+        let urlPath = url.path.dropFirst()
+
+        guard let regex = try? NSRegularExpression(pattern: S3ObjectIdentifer.s3EndpointRegex, options: []) else {
+            return nil
+        }
+
+        let searchRange = NSRange(self.startIndex..<self.endIndex, in: self)
+        let match = regex.firstMatch(in: self, options: [], range: searchRange)
+        if let match = match,
+            match.numberOfRanges > 0,
+            let bucketRange = Range(match.range(at: 1), in: self),
+            !bucketRange.isEmpty,
+            self[bucketRange].count > 1 {
+            // If the capture group for the regex is not empty, the URL is the virtual hosted style, for example:
+            // https://bucket.s3.amazonaws.com/key
+            // The capture group is the bucket name (with trailing dot) and the URL path is the key name
+            let bucketName = String(self[bucketRange].dropLast())
+            let keyName = String(urlPath)
+            return S3ObjectIdentifer(bucketName: bucketName, keyPath: keyName)
+        }
+
+        // If the regex capture group is empty, the URL is in the path style, for example:
+        // https://s3.amazonaws.com/bucket/key
+        // Both the bucket and key names are in the URL path
+        return asS3ObjectIdentifierFromNonPrefixedUrl(nonPrefixedUrl: Substring(urlPath))
+    }
+
     private func getIndexOfNextUrlSeparator(url: Substring) -> String.Index? {
         #if swift(>=4.2)
         return url.firstIndex(of: "/")
@@ -85,18 +89,18 @@ public extension String {
         return url.index(of: "/")
         #endif
     }
-    
+
     /// Spilts a url of the form {bucket}/{key+} into a S3ObjectIdentifer if possible
     private func asS3ObjectIdentifierFromNonPrefixedUrl(nonPrefixedUrl: Substring) -> S3ObjectIdentifer? {
         guard let nextUrlSeparator = getIndexOfNextUrlSeparator(url: nonPrefixedUrl) else {
             return nil
         }
-        
+
         let bucketKeySeperatorIndex = nonPrefixedUrl.index(nextUrlSeparator,
                                                                offsetBy: 1)
         let bucketName = String(nonPrefixedUrl[..<nextUrlSeparator])
         let keyPath = String(nonPrefixedUrl[bucketKeySeperatorIndex...])
-        
+
         return S3ObjectIdentifer(bucketName: bucketName, keyPath: keyPath)
     }
 }
diff --git a/Tests/S3ClientTests/S3ClientTests.swift b/Tests/S3ClientTests/S3ClientTests.swift
index 5625c419..fadcf368 100644
--- a/Tests/S3ClientTests/S3ClientTests.swift
+++ b/Tests/S3ClientTests/S3ClientTests.swift
@@ -13,72 +13,92 @@ import AsyncHTTPClient
 import Logging
 
 class S3ClientTests: XCTestCase {
-    
+
     func testValidS3Uri() throws {
         let s3Uri = "s3://bucketName/the/key/path"
-        
+
         let identifier = s3Uri.asS3ObjectIdentifier()
-        
+
         let expected = S3ObjectIdentifer(bucketName: "bucketName",
                                          keyPath: "the/key/path")
-        
+
         XCTAssertEqual(expected, identifier)
     }
-    
+
     func testValidHttpsUri() throws {
-        let s3Uri = "https://host/bucketName/the/key/path"
-        
-        let identifier = s3Uri.asS3ObjectIdentifier()
-        
+        let s3Uris = [
+            "https://s3.amazonaws.com/bucketName/the/key/path",
+            "https://s3-abc.amazonaws.com/bucketName/the/key/path",
+            "https://s3.us-east-1.amazonaws.com/bucketName/the/key/path",
+            "https://s3-abc.us-east-1.amazonaws.com/bucketName/the/key/path",
+            "https://bucketName.s3.amazonaws.com/the/key/path",
+            "https://bucketName.s3-abc.amazonaws.com/the/key/path",
+            "https://bucketName.s3.us-east-1.amazonaws.com/the/key/path",
+            "https://bucketName.s3-abc.us-east-1.amazonaws.com/the/key/path",
+        ]
+
         let expected = S3ObjectIdentifer(bucketName: "bucketName",
                                          keyPath: "the/key/path")
-        
-        XCTAssertEqual(expected, identifier)
+
+        for s3Uri in s3Uris {
+            let identifier = s3Uri.asS3ObjectIdentifier()
+            XCTAssertEqual(expected, identifier)
+        }
     }
-    
+
     func testValidHttpUri() throws {
-        let s3Uri = "http://host/bucketName/the/key/path"
-        
-        let identifier = s3Uri.asS3ObjectIdentifier()
-        
+        let s3Uris = [
+            "http://s3.amazonaws.com/bucketName/the/key/path",
+            "http://s3-abc.amazonaws.com/bucketName/the/key/path",
+            "http://s3.us-east-1.amazonaws.com/bucketName/the/key/path",
+            "http://s3-abc.us-east-1.amazonaws.com/bucketName/the/key/path",
+            "http://bucketName.s3.amazonaws.com/the/key/path",
+            "http://bucketName.s3-abc.amazonaws.com/the/key/path",
+            "http://bucketName.s3.us-east-1.amazonaws.com/the/key/path",
+            "http://bucketName.s3-abc.us-east-1.amazonaws.com/the/key/path",
+        ]
+
         let expected = S3ObjectIdentifer(bucketName: "bucketName",
                                          keyPath: "the/key/path")
-        
-        XCTAssertEqual(expected, identifier)
+
+        for s3Uri in s3Uris {
+            let identifier = s3Uri.asS3ObjectIdentifier()
+            XCTAssertEqual(expected, identifier)
+        }
     }
-    
+
     func testInvalidS3UriPrefix() throws {
         let s3Uri = "ssh://bucketName/the/key/path"
-        
+
         let identifier = s3Uri.asS3ObjectIdentifier()
-        
+
         XCTAssertNil(identifier)
     }
-    
+
     func testS3UriNoSeparator() throws {
         let s3Uri = "s3://bucketName"
-        
+
         let identifier = s3Uri.asS3ObjectIdentifier()
-        
+
         XCTAssertNil(identifier)
     }
-    
+
     func testHttpsUriNoSeparatorForBucket() throws {
         let s3Uri = "https://bucketName"
-        
+
         let identifier = s3Uri.asS3ObjectIdentifier()
-        
+
         XCTAssertNil(identifier)
     }
-    
+
     func testHttpsUriNoSeparatorForKey() throws {
         let s3Uri = "https://host/bucketName"
-        
+
         let identifier = s3Uri.asS3ObjectIdentifier()
-        
+
         XCTAssertNil(identifier)
     }
-    
+
     func testAccessDeniedErrorDecode() throws {
         let message = "Access Denied"
         let errorResponse = """
@@ -88,7 +108,7 @@ class S3ClientTests: XCTestCase {
                 <Message>\(message)</Message>
             </Error>
             """
-        
+
         let response = HTTPClient.Response(host: "s3.us-west-2.amazonaws.com", status: .badRequest,
                                            version: HTTPVersion(major: 1, minor: 1),
                                            headers: HTTPHeaders(), body: nil)
@@ -100,16 +120,16 @@ class S3ClientTests: XCTestCase {
         let error = try clientDelegate.getResponseError(response: response,
                                                         responseComponents: components,
                                                         invocationReporting: invocationReporting)
-        
+
         guard case let S3Error.accessDenied(returnedMessage) = error.cause else {
             return XCTFail()
         }
-        
+
         guard message == returnedMessage else {
             return XCTFail()
         }
     }
-    
+
     func testAccessDeniedErrorDataAPIsDecode() throws {
         let message = "Access Denied"
         let errorResponse = """
@@ -119,7 +139,7 @@ class S3ClientTests: XCTestCase {
                 <Message>\(message)</Message>
             </Error>
             """
-        
+
         let response = HTTPClient.Response(host: "s3.us-west-2.amazonaws.com", status: .badRequest,
                                            version: HTTPVersion(major: 1, minor: 1),
                                            headers: HTTPHeaders(), body: nil)
@@ -131,16 +151,16 @@ class S3ClientTests: XCTestCase {
         let error = try clientDelegate.getResponseError(response: response,
                                                         responseComponents: components,
                                                         invocationReporting: invocationReporting)
-        
+
         guard case let S3Error.accessDenied(returnedMessage) = error.cause else {
             return XCTFail()
         }
-        
+
         guard message == returnedMessage else {
             return XCTFail()
         }
     }
-    
+
     func testKnownErrorDecode() throws {
         let message = "The specified key does not exist."
         let errorResponse = """
@@ -151,7 +171,7 @@ class S3ClientTests: XCTestCase {
                 <Key>myKey</Key>
             </Error>
             """
-        
+
         let response = HTTPClient.Response(host: "s3.us-west-2.amazonaws.com", status: .badRequest,
                                            version: HTTPVersion(major: 1, minor: 1),
                                            headers: HTTPHeaders(), body: nil)
@@ -163,7 +183,7 @@ class S3ClientTests: XCTestCase {
         let error = try clientDelegate.getResponseError(response: response,
                                                         responseComponents: components,
                                                         invocationReporting: invocationReporting)
-        
+
         guard case S3Error.noSuchKey = error.cause else {
             return XCTFail()
         }

From da243c561aaf424d47895ece2ef1c457cc1066ba Mon Sep 17 00:00:00 2001
From: PB Thiffault <pbthif@amazon.com>
Date: Thu, 26 Aug 2021 16:48:00 -0400
Subject: [PATCH 2/2] Undo whitespace changes

---
 .../String+asS3ObjectIdentifier.swift         | 30 ++++----
 Tests/S3ClientTests/S3ClientTests.swift       | 68 +++++++++----------
 2 files changed, 49 insertions(+), 49 deletions(-)

diff --git a/Sources/S3Client/String+asS3ObjectIdentifier.swift b/Sources/S3Client/String+asS3ObjectIdentifier.swift
index b6db0fe4..1524f2a4 100644
--- a/Sources/S3Client/String+asS3ObjectIdentifier.swift
+++ b/Sources/S3Client/String+asS3ObjectIdentifier.swift
@@ -14,10 +14,10 @@ public struct S3ObjectIdentifer: Equatable {
     internal static let httpsPrefix = "https://"
     internal static let httpPrefix = "http://"
     internal static let s3EndpointRegex = #"^https?:\/\/(.+\.)?s3[.-][a-z0-9-]+\."#
-
+    
     public let bucketName: String
     public let keyPath: String
-
+    
     public init(bucketName: String,
                 keyPath: String) {
         self.bucketName = bucketName
@@ -29,10 +29,10 @@ public extension String {
     /**
      If possible creates an S3ObjectIdentifer instance from this string,
      seperating the bucket name and key path.
-
+ 
      Will return nil if this string is not of the form-
        s3://bucketName/the/key/path
-
+     
      This would return S3ObjectIdentifer(bucketName: "bucketName",
                                          keyPath: "/the/key/path")
      */
@@ -40,27 +40,27 @@ public extension String {
         if self.starts(with: S3ObjectIdentifer.s3Prefix) {
             // get the url without the scheme - of the form {bucket}/{key+}
             let nonPrefixedUrl = self.dropFirst(S3ObjectIdentifer.s3Prefix.count)
-
+            
             return asS3ObjectIdentifierFromNonPrefixedUrl(nonPrefixedUrl: nonPrefixedUrl)
         } else if self.starts(with: S3ObjectIdentifer.httpsPrefix) || self.starts(with: S3ObjectIdentifer.httpPrefix) {
             return asS3ObjectIdentifierFromHttpOrHttps()
         }
-
+        
         return nil
     }
-
+    
     /// Tries to parse the bucket and key names from an HTTP or HTTPS URL.
     private func asS3ObjectIdentifierFromHttpOrHttps() -> S3ObjectIdentifer? {
         guard let url = URL(string: self) else {
             return nil
         }
-
+        
         let urlPath = url.path.dropFirst()
-
+        
         guard let regex = try? NSRegularExpression(pattern: S3ObjectIdentifer.s3EndpointRegex, options: []) else {
             return nil
         }
-
+        
         let searchRange = NSRange(self.startIndex..<self.endIndex, in: self)
         let match = regex.firstMatch(in: self, options: [], range: searchRange)
         if let match = match,
@@ -75,13 +75,13 @@ public extension String {
             let keyName = String(urlPath)
             return S3ObjectIdentifer(bucketName: bucketName, keyPath: keyName)
         }
-
+        
         // If the regex capture group is empty, the URL is in the path style, for example:
         // https://s3.amazonaws.com/bucket/key
         // Both the bucket and key names are in the URL path
         return asS3ObjectIdentifierFromNonPrefixedUrl(nonPrefixedUrl: Substring(urlPath))
     }
-
+    
     private func getIndexOfNextUrlSeparator(url: Substring) -> String.Index? {
         #if swift(>=4.2)
         return url.firstIndex(of: "/")
@@ -89,18 +89,18 @@ public extension String {
         return url.index(of: "/")
         #endif
     }
-
+    
     /// Spilts a url of the form {bucket}/{key+} into a S3ObjectIdentifer if possible
     private func asS3ObjectIdentifierFromNonPrefixedUrl(nonPrefixedUrl: Substring) -> S3ObjectIdentifer? {
         guard let nextUrlSeparator = getIndexOfNextUrlSeparator(url: nonPrefixedUrl) else {
             return nil
         }
-
+        
         let bucketKeySeperatorIndex = nonPrefixedUrl.index(nextUrlSeparator,
                                                                offsetBy: 1)
         let bucketName = String(nonPrefixedUrl[..<nextUrlSeparator])
         let keyPath = String(nonPrefixedUrl[bucketKeySeperatorIndex...])
-
+        
         return S3ObjectIdentifer(bucketName: bucketName, keyPath: keyPath)
     }
 }
diff --git a/Tests/S3ClientTests/S3ClientTests.swift b/Tests/S3ClientTests/S3ClientTests.swift
index fadcf368..1451daa4 100644
--- a/Tests/S3ClientTests/S3ClientTests.swift
+++ b/Tests/S3ClientTests/S3ClientTests.swift
@@ -13,18 +13,18 @@ import AsyncHTTPClient
 import Logging
 
 class S3ClientTests: XCTestCase {
-
+    
     func testValidS3Uri() throws {
         let s3Uri = "s3://bucketName/the/key/path"
-
+        
         let identifier = s3Uri.asS3ObjectIdentifier()
-
+        
         let expected = S3ObjectIdentifer(bucketName: "bucketName",
                                          keyPath: "the/key/path")
-
+        
         XCTAssertEqual(expected, identifier)
     }
-
+    
     func testValidHttpsUri() throws {
         let s3Uris = [
             "https://s3.amazonaws.com/bucketName/the/key/path",
@@ -36,16 +36,16 @@ class S3ClientTests: XCTestCase {
             "https://bucketName.s3.us-east-1.amazonaws.com/the/key/path",
             "https://bucketName.s3-abc.us-east-1.amazonaws.com/the/key/path",
         ]
-
+        
         let expected = S3ObjectIdentifer(bucketName: "bucketName",
                                          keyPath: "the/key/path")
-
+        
         for s3Uri in s3Uris {
             let identifier = s3Uri.asS3ObjectIdentifier()
             XCTAssertEqual(expected, identifier)
         }
     }
-
+    
     func testValidHttpUri() throws {
         let s3Uris = [
             "http://s3.amazonaws.com/bucketName/the/key/path",
@@ -57,48 +57,48 @@ class S3ClientTests: XCTestCase {
             "http://bucketName.s3.us-east-1.amazonaws.com/the/key/path",
             "http://bucketName.s3-abc.us-east-1.amazonaws.com/the/key/path",
         ]
-
+        
         let expected = S3ObjectIdentifer(bucketName: "bucketName",
                                          keyPath: "the/key/path")
-
-        for s3Uri in s3Uris {
+        
+    for s3Uri in s3Uris {
             let identifier = s3Uri.asS3ObjectIdentifier()
             XCTAssertEqual(expected, identifier)
         }
     }
-
+    
     func testInvalidS3UriPrefix() throws {
         let s3Uri = "ssh://bucketName/the/key/path"
-
+        
         let identifier = s3Uri.asS3ObjectIdentifier()
-
+        
         XCTAssertNil(identifier)
     }
-
+    
     func testS3UriNoSeparator() throws {
         let s3Uri = "s3://bucketName"
-
+        
         let identifier = s3Uri.asS3ObjectIdentifier()
-
+        
         XCTAssertNil(identifier)
     }
-
+    
     func testHttpsUriNoSeparatorForBucket() throws {
         let s3Uri = "https://bucketName"
-
+        
         let identifier = s3Uri.asS3ObjectIdentifier()
-
+        
         XCTAssertNil(identifier)
     }
-
+    
     func testHttpsUriNoSeparatorForKey() throws {
         let s3Uri = "https://host/bucketName"
-
+        
         let identifier = s3Uri.asS3ObjectIdentifier()
-
+        
         XCTAssertNil(identifier)
     }
-
+    
     func testAccessDeniedErrorDecode() throws {
         let message = "Access Denied"
         let errorResponse = """
@@ -108,7 +108,7 @@ class S3ClientTests: XCTestCase {
                 <Message>\(message)</Message>
             </Error>
             """
-
+        
         let response = HTTPClient.Response(host: "s3.us-west-2.amazonaws.com", status: .badRequest,
                                            version: HTTPVersion(major: 1, minor: 1),
                                            headers: HTTPHeaders(), body: nil)
@@ -120,16 +120,16 @@ class S3ClientTests: XCTestCase {
         let error = try clientDelegate.getResponseError(response: response,
                                                         responseComponents: components,
                                                         invocationReporting: invocationReporting)
-
+        
         guard case let S3Error.accessDenied(returnedMessage) = error.cause else {
             return XCTFail()
         }
-
+        
         guard message == returnedMessage else {
             return XCTFail()
         }
     }
-
+    
     func testAccessDeniedErrorDataAPIsDecode() throws {
         let message = "Access Denied"
         let errorResponse = """
@@ -139,7 +139,7 @@ class S3ClientTests: XCTestCase {
                 <Message>\(message)</Message>
             </Error>
             """
-
+        
         let response = HTTPClient.Response(host: "s3.us-west-2.amazonaws.com", status: .badRequest,
                                            version: HTTPVersion(major: 1, minor: 1),
                                            headers: HTTPHeaders(), body: nil)
@@ -151,16 +151,16 @@ class S3ClientTests: XCTestCase {
         let error = try clientDelegate.getResponseError(response: response,
                                                         responseComponents: components,
                                                         invocationReporting: invocationReporting)
-
+        
         guard case let S3Error.accessDenied(returnedMessage) = error.cause else {
             return XCTFail()
         }
-
+        
         guard message == returnedMessage else {
             return XCTFail()
         }
     }
-
+    
     func testKnownErrorDecode() throws {
         let message = "The specified key does not exist."
         let errorResponse = """
@@ -171,7 +171,7 @@ class S3ClientTests: XCTestCase {
                 <Key>myKey</Key>
             </Error>
             """
-
+        
         let response = HTTPClient.Response(host: "s3.us-west-2.amazonaws.com", status: .badRequest,
                                            version: HTTPVersion(major: 1, minor: 1),
                                            headers: HTTPHeaders(), body: nil)
@@ -183,7 +183,7 @@ class S3ClientTests: XCTestCase {
         let error = try clientDelegate.getResponseError(response: response,
                                                         responseComponents: components,
                                                         invocationReporting: invocationReporting)
-
+        
         guard case S3Error.noSuchKey = error.cause else {
             return XCTFail()
         }