From f014e0e57a1a50d94c5cf6fcd725688425ddbc7c Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Wed, 8 May 2024 10:06:33 -0700 Subject: [PATCH 01/19] chore: update http requests, responses and endpoint with uri --- .../ClientRuntime/Networking/Endpoint.swift | 51 +++++++++++-------- .../Networking/Http/HttpUrlResponse.swift | 1 + .../Networking/Http/SdkHttpRequest.swift | 37 +++++--------- .../ClientRuntime/Networking/Http/URI.swift | 30 +++++++++++ .../ExpectedSdkHttpRequest.swift | 13 ++--- .../Http/HttpRequestTests.swift | 12 ++--- .../Http/SdkRequestBuilderTests.swift | 2 +- 7 files changed, 84 insertions(+), 62 deletions(-) create mode 100644 Sources/ClientRuntime/Networking/Http/URI.swift diff --git a/Sources/ClientRuntime/Networking/Endpoint.swift b/Sources/ClientRuntime/Networking/Endpoint.swift index dffe4b5f2..2ff6eb157 100644 --- a/Sources/ClientRuntime/Networking/Endpoint.swift +++ b/Sources/ClientRuntime/Networking/Endpoint.swift @@ -6,16 +6,17 @@ import Foundation public struct Endpoint: Hashable { - public let path: String - public let queryItems: [SDKURLQueryItem]? + public let uri: URI public let protocolType: ProtocolType? - public let host: String - public let port: Int16 - public let headers: Headers? + public var queryItems: [SDKURLQueryItem] { uri.query } + public var path: String { uri.path } + public var host: String { uri.host } + public var port: Int16 { uri.port } + public var headers: Headers public let properties: [String: AnyHashable] public init(urlString: String, - headers: Headers? = nil, + headers: Headers = Headers(), properties: [String: AnyHashable] = [:]) throws { guard let url = URL(string: urlString) else { throw ClientError.unknownError("invalid url \(urlString)") @@ -25,17 +26,19 @@ public struct Endpoint: Hashable { } public init(url: URL, - headers: Headers? = nil, + headers: Headers = Headers(), properties: [String: AnyHashable] = [:]) throws { guard let host = url.host else { throw ClientError.unknownError("invalid host \(String(describing: url.host))") } let protocolType = ProtocolType(rawValue: url.scheme ?? "") ?? .https - self.init(host: host, - path: url.path, - port: Int16(url.port ?? protocolType.port), - queryItems: url.toQueryItems(), + let uri = URI(scheme: protocolType.rawValue, + path: url.path, + host: host, + port: Int16(url.port ?? protocolType.port), + query: url.toQueryItems() ?? []) + self.init(uri: uri, protocolType: protocolType, headers: headers, properties: properties) @@ -45,13 +48,17 @@ public struct Endpoint: Hashable { path: String = "/", port: Int16 = 443, queryItems: [SDKURLQueryItem]? = nil, + headers: Headers = Headers(), + protocolType: ProtocolType = .https) { + let uri = URI(scheme: protocolType.rawValue, path: path, host: host, port: port, query: queryItems ?? []) + self.init(uri: uri, protocolType: protocolType, headers: headers) + } + + public init(uri: URI, protocolType: ProtocolType? = .https, - headers: Headers? = nil, + headers: Headers = Headers(), properties: [String: AnyHashable] = [:]) { - self.host = host - self.path = path - self.port = port - self.queryItems = queryItems + self.uri = uri self.protocolType = protocolType self.headers = headers self.properties = properties @@ -64,14 +71,16 @@ extension Endpoint { public var url: URL? { var components = URLComponents() components.scheme = protocolType?.rawValue - components.host = host.isEmpty ? nil : host // If host is empty, URL is invalid - components.percentEncodedPath = path - components.percentEncodedQuery = queryItemString + components.host = uri.host.isEmpty ? nil : uri.host // If host is empty, URL is invalid + components.percentEncodedPath = uri.path + components.percentEncodedQuery = query return (components.host == nil || components.scheme == nil) ? nil : components.url } - var queryItemString: String? { - guard let queryItems = queryItems else { return nil } + var query: String? { + if (queryItems.isEmpty) { + return nil + } return queryItems.map { queryItem in return [queryItem.name, queryItem.value].compactMap { $0 }.joined(separator: "=") }.joined(separator: "&") diff --git a/Sources/ClientRuntime/Networking/Http/HttpUrlResponse.swift b/Sources/ClientRuntime/Networking/Http/HttpUrlResponse.swift index c379e7e24..aebd2ce42 100644 --- a/Sources/ClientRuntime/Networking/Http/HttpUrlResponse.swift +++ b/Sources/ClientRuntime/Networking/Http/HttpUrlResponse.swift @@ -9,4 +9,5 @@ protocol HttpUrlResponse { var headers: Headers { get set } var body: ByteStream { get set} var statusCode: HttpStatusCode {get set} + var reason: String? {get set} } diff --git a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift index fd6adc82a..efab9c2ce 100644 --- a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift +++ b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift @@ -17,18 +17,14 @@ import struct Foundation.URLRequest // in the CRT engine so that is why it's a class public final class SdkHttpRequest: RequestMessage { public var body: ByteStream - public let endpoint: Endpoint + public var endpoint: Endpoint public let method: HttpMethodType - private var additionalHeaders: Headers = Headers() - public var headers: Headers { - var allHeaders = endpoint.headers ?? Headers() - allHeaders.addAll(headers: additionalHeaders) - return allHeaders - } - public var trailingHeaders: Headers = Headers() + public var destination: URI { endpoint.uri } + public var headers: Headers { endpoint.headers } public var path: String { endpoint.path } public var host: String { endpoint.host } public var queryItems: [SDKURLQueryItem]? { endpoint.queryItems } + public var trailingHeaders: Headers = Headers() public init(method: HttpMethodType, endpoint: Endpoint, @@ -55,11 +51,11 @@ public final class SdkHttpRequest: RequestMessage { } public func withHeader(name: String, value: String) { - self.additionalHeaders.add(name: name, value: value) + self.endpoint.headers.add(name: name, value: value) } public func withoutHeader(name: String) { - self.additionalHeaders.remove(name: name) + self.endpoint.headers.remove(name: name) } public func withBody(_ body: ByteStream) { @@ -92,7 +88,7 @@ extension SdkHttpRequest { public func toHttpRequest() throws -> HTTPRequest { let httpRequest = try HTTPRequest() httpRequest.method = method.rawValue - httpRequest.path = [endpoint.path, endpoint.queryItemString].compactMap { $0 }.joined(separator: "?") + httpRequest.path = [endpoint.path, endpoint.query].compactMap { $0 }.joined(separator: "?") httpRequest.addHeaders(headers: headers.toHttpHeaders()) httpRequest.body = isChunked ? nil : StreamableHttpBody(body: body) // body needs to be nil to use writeChunk() return httpRequest @@ -104,7 +100,7 @@ extension SdkHttpRequest { public func toHttp2Request() throws -> HTTPRequestBase { let httpRequest = try HTTPRequest() httpRequest.method = method.rawValue - httpRequest.path = [endpoint.path, endpoint.queryItemString].compactMap { $0 }.joined(separator: "?") + httpRequest.path = [endpoint.path, endpoint.query].compactMap { $0 }.joined(separator: "?") httpRequest.addHeaders(headers: headers.toHttpHeaders()) // Remove the "Transfer-Encoding" header if it exists since h2 does not support it @@ -198,7 +194,7 @@ public class SdkHttpRequestBuilder: RequestMessageBuilder { var host: String = "" var path: String = "/" var body: ByteStream = .noStream - var queryItems: [SDKURLQueryItem]? + var queryItems: [SDKURLQueryItem] = [] var port: Int16 = 443 var protocolType: ProtocolType = .https var trailingHeaders: Headers = Headers() @@ -267,8 +263,7 @@ public class SdkHttpRequestBuilder: RequestMessageBuilder { @discardableResult public func withQueryItems(_ value: [SDKURLQueryItem]) -> SdkHttpRequestBuilder { - self.queryItems = self.queryItems ?? [] - self.queryItems?.append(contentsOf: value) + self.queryItems.append(contentsOf: value) return self } @@ -290,15 +285,9 @@ public class SdkHttpRequestBuilder: RequestMessageBuilder { } public func build() -> SdkHttpRequest { - let endpoint = Endpoint(host: host, - path: path, - port: port, - queryItems: queryItems, - protocolType: protocolType, - headers: headers) - return SdkHttpRequest(method: methodType, - endpoint: endpoint, - body: body) + let uri = URI(scheme: protocolType.rawValue, path: path, host: host, port: port, query: queryItems) + let endpoint = Endpoint(uri: uri, protocolType: protocolType, headers: headers) + return SdkHttpRequest(method: methodType, endpoint: endpoint, body: body) } } diff --git a/Sources/ClientRuntime/Networking/Http/URI.swift b/Sources/ClientRuntime/Networking/Http/URI.swift new file mode 100644 index 000000000..69980bbfa --- /dev/null +++ b/Sources/ClientRuntime/Networking/Http/URI.swift @@ -0,0 +1,30 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +public struct URI: Hashable { + public let scheme: String + public let path: String + public let host: String + public let port: Int16 + public let query: [SDKURLQueryItem] + public let username: String? + public let password: String? + + public init(scheme: String, + path: String, + host: String, + port: Int16, + query: [SDKURLQueryItem], + username: String? = nil, + password: String? = nil) { + self.scheme = scheme + self.path = path + self.host = host + self.port = port + self.query = query + self.username = username + self.password = password + } +} diff --git a/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift b/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift index d8ba0506b..2f23ad7c9 100644 --- a/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift +++ b/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift @@ -12,7 +12,7 @@ public struct ExpectedSdkHttpRequest { public var headers: Headers? public var forbiddenHeaders: [String]? public var requiredHeaders: [String]? - public let queryItems: [SDKURLQueryItem]? + public var queryItems: [SDKURLQueryItem] { endpoint.queryItems } public let forbiddenQueryItems: [SDKURLQueryItem]? public let requiredQueryItems: [SDKURLQueryItem]? public let endpoint: Endpoint @@ -23,7 +23,6 @@ public struct ExpectedSdkHttpRequest { headers: Headers? = nil, forbiddenHeaders: [String]? = nil, requiredHeaders: [String]? = nil, - queryItems: [SDKURLQueryItem]? = nil, forbiddenQueryItems: [SDKURLQueryItem]? = nil, requiredQueryItems: [SDKURLQueryItem]? = nil, body: ByteStream = ByteStream.noStream) { @@ -32,7 +31,6 @@ public struct ExpectedSdkHttpRequest { self.headers = headers self.forbiddenHeaders = forbiddenHeaders self.requiredHeaders = requiredHeaders - self.queryItems = queryItems self.forbiddenQueryItems = forbiddenQueryItems self.requiredQueryItems = requiredQueryItems self.body = body @@ -139,12 +137,8 @@ public class ExpectedSdkHttpRequestBuilder { } public func build() -> ExpectedSdkHttpRequest { - let endpoint = Endpoint(host: host, - path: path, - port: port, - queryItems: queryItems, - protocolType: protocolType) - let queryItems = !queryItems.isEmpty ? queryItems : nil + let uri = URI(scheme: protocolType.rawValue, path: path, host: host, port: port, query: queryItems) + let endpoint = Endpoint(uri: uri, protocolType: protocolType) let forbiddenQueryItems = !forbiddenQueryItems.isEmpty ? forbiddenQueryItems : nil let requiredQueryItems = !requiredQueryItems.isEmpty ? requiredQueryItems : nil @@ -156,7 +150,6 @@ public class ExpectedSdkHttpRequestBuilder { headers: headers, forbiddenHeaders: forbiddenHeaders, requiredHeaders: requiredHeaders, - queryItems: queryItems, forbiddenQueryItems: forbiddenQueryItems, requiredQueryItems: requiredQueryItems, body: body) diff --git a/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift index 1b7d5a433..c91399658 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift @@ -118,17 +118,17 @@ class HttpRequestTests: NetworkingTestUtils { .withQueryItem(queryItem2) .withHeader(name: "Content-Length", value: "6") - XCTAssert(builder.queryItems?.count == 2) + XCTAssert(builder.queryItems.count == 2) let httpRequest = try builder.build().toHttpRequest() httpRequest.path = "/hello?foo=bar&quz=bar&signedthing=signed" let updatedRequest = builder.update(from: httpRequest, originalRequest: builder.build()) XCTAssert(updatedRequest.path == "/hello") - XCTAssert(updatedRequest.queryItems?.count == 3) - XCTAssert(updatedRequest.queryItems?.contains(queryItem1) ?? false) - XCTAssert(updatedRequest.queryItems?.contains(queryItem2) ?? false) - XCTAssert(updatedRequest.queryItems?.contains(SDKURLQueryItem(name: "signedthing", value: "signed")) ?? false) + XCTAssert(updatedRequest.queryItems.count == 3) + XCTAssert(updatedRequest.queryItems.contains(queryItem1) ?? false) + XCTAssert(updatedRequest.queryItems.contains(queryItem2) ?? false) + XCTAssert(updatedRequest.queryItems.contains(SDKURLQueryItem(name: "signedthing", value: "signed")) ?? false) } func testPathInInHttpRequestIsNotAltered() throws { @@ -143,7 +143,7 @@ class HttpRequestTests: NetworkingTestUtils { func testConversionToUrlRequestFailsWithInvalidEndpoint() { // Testing with an invalid endpoint where host is empty, // path is empty, and protocolType is nil. - let endpoint = Endpoint(host: "", path: "", protocolType: nil) + let endpoint = Endpoint(host: "", path: "") XCTAssertNil(endpoint.url, "An invalid endpoint should result in a nil URL.") } } diff --git a/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift index d6b18d1c3..60d7478c5 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift @@ -17,7 +17,7 @@ class SdkRequestBuilderTests: XCTestCase { crtRequest.path = pathToMatch let updatedRequest = SdkHttpRequestBuilder().update(from: crtRequest, originalRequest: originalRequest).build() - let updatedPath = [updatedRequest.endpoint.path, updatedRequest.endpoint.queryItemString].compactMap { $0 }.joined(separator: "?") + let updatedPath = [updatedRequest.endpoint.path, updatedRequest.endpoint.query].compactMap { $0 }.joined(separator: "?") XCTAssertEqual(pathToMatch, updatedPath) XCTAssertEqual(url, updatedRequest.endpoint.url?.absoluteString) } From 006a81fb175530cb0c00b3948b4c68cfe7c3e4f4 Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Mon, 13 May 2024 18:49:52 -0700 Subject: [PATCH 02/19] feat: add validations and builder to URI --- .../Endpoints/ServiceEndpointMetadata.swift | 2 +- .../ClientRuntime/Networking/Endpoint.swift | 61 ++++---- .../Networking/Http/CRT/CRTClientEngine.swift | 26 ++-- .../Networking/Http/Headers.swift | 4 + .../Middlewares/ContentTypeMiddleware.swift | 2 +- .../FlexibleChecksumsRequestMiddleware.swift | 2 +- .../Networking/Http/ProtocolType.swift | 2 + .../Networking/Http/SdkHttpRequest.swift | 69 +++++---- .../ClientRuntime/Networking/Http/URI.swift | 134 ++++++++++++++++-- .../URLSession/URLSessionHTTPClient.swift | 10 +- .../ExpectedSdkHttpRequest.swift | 12 +- .../RequestTestUtil/HttpRequestTestBase.swift | 4 +- Sources/WeatherSDK/EndpointResolver.swift | 4 +- .../InterceptorTests/InterceptorTests.swift | 6 +- .../NetworkingTests/EndpointTests.swift | 10 +- .../CRTClientEngineIntegrationTests.swift | 16 +-- .../Http/HttpRequestTests.swift | 8 +- .../Http/SdkRequestBuilderTests.swift | 8 +- .../NetworkingTests/NetworkingTestUtils.swift | 2 +- .../OrchestratorTests/OrchestratorTests.swift | 2 +- .../HttpRequestTestBaseTests.swift | 6 +- .../middleware/EndpointResolverMiddleware.kt | 4 +- 22 files changed, 266 insertions(+), 128 deletions(-) diff --git a/Sources/ClientRuntime/Endpoints/ServiceEndpointMetadata.swift b/Sources/ClientRuntime/Endpoints/ServiceEndpointMetadata.swift index 75b045d38..ce44a6fbb 100644 --- a/Sources/ClientRuntime/Endpoints/ServiceEndpointMetadata.swift +++ b/Sources/ClientRuntime/Endpoints/ServiceEndpointMetadata.swift @@ -52,7 +52,7 @@ extension ServiceEndpointMetadata { return SmithyEndpoint(endpoint: Endpoint(host: hostname, path: "/", - protocolType: ProtocolType(rawValue: transportProtocol)), + protocolType: ProtocolType(rawValue: transportProtocol)!), signingName: signingName) } diff --git a/Sources/ClientRuntime/Networking/Endpoint.swift b/Sources/ClientRuntime/Networking/Endpoint.swift index 2ff6eb157..9fd2f5176 100644 --- a/Sources/ClientRuntime/Networking/Endpoint.swift +++ b/Sources/ClientRuntime/Networking/Endpoint.swift @@ -7,18 +7,19 @@ import Foundation public struct Endpoint: Hashable { public let uri: URI - public let protocolType: ProtocolType? + public let headers: Headers + public var protocolType: ProtocolType? { uri.scheme } public var queryItems: [SDKURLQueryItem] { uri.query } public var path: String { uri.path } public var host: String { uri.host } public var port: Int16 { uri.port } - public var headers: Headers - public let properties: [String: AnyHashable] + public var url: URL? { uri.url } + private let properties: [String: AnyHashable] public init(urlString: String, headers: Headers = Headers(), properties: [String: AnyHashable] = [:]) throws { - guard let url = URL(string: urlString) else { + guard let url = URLComponents(string: urlString)?.url else { throw ClientError.unknownError("invalid url \(urlString)") } @@ -28,18 +29,22 @@ public struct Endpoint: Hashable { public init(url: URL, headers: Headers = Headers(), properties: [String: AnyHashable] = [:]) throws { + guard let host = url.host else { throw ClientError.unknownError("invalid host \(String(describing: url.host))") } let protocolType = ProtocolType(rawValue: url.scheme ?? "") ?? .https - let uri = URI(scheme: protocolType.rawValue, - path: url.path, - host: host, - port: Int16(url.port ?? protocolType.port), - query: url.toQueryItems() ?? []) + + let uri = URIBuilder() + .withScheme(protocolType) + .withPath(url.path) + .withHost(host) + .withPort(Int16(url.port ?? protocolType.port)) + .withQueryItems(url.toQueryItems() ?? []) + .build() + self.init(uri: uri, - protocolType: protocolType, headers: headers, properties: properties) } @@ -49,43 +54,29 @@ public struct Endpoint: Hashable { port: Int16 = 443, queryItems: [SDKURLQueryItem]? = nil, headers: Headers = Headers(), - protocolType: ProtocolType = .https) { - let uri = URI(scheme: protocolType.rawValue, path: path, host: host, port: port, query: queryItems ?? []) - self.init(uri: uri, protocolType: protocolType, headers: headers) + protocolType: ProtocolType? = .https) { + + let uri = URIBuilder() + .withScheme(protocolType) + .withPath(path) + .withHost(host) + .withPort(port) + .withQueryItems(queryItems ?? []) + .build() + + self.init(uri: uri, headers: headers) } public init(uri: URI, - protocolType: ProtocolType? = .https, headers: Headers = Headers(), properties: [String: AnyHashable] = [:]) { self.uri = uri - self.protocolType = protocolType self.headers = headers self.properties = properties } } extension Endpoint { - // We still have to keep 'url' as an optional, since we're - // dealing with dynamic components that could be invalid. - public var url: URL? { - var components = URLComponents() - components.scheme = protocolType?.rawValue - components.host = uri.host.isEmpty ? nil : uri.host // If host is empty, URL is invalid - components.percentEncodedPath = uri.path - components.percentEncodedQuery = query - return (components.host == nil || components.scheme == nil) ? nil : components.url - } - - var query: String? { - if (queryItems.isEmpty) { - return nil - } - return queryItems.map { queryItem in - return [queryItem.name, queryItem.value].compactMap { $0 }.joined(separator: "=") - }.joined(separator: "&") - } - /// Returns list of auth schemes /// This is an internal API and subject to change without notice /// - Returns: list of auth schemes if present diff --git a/Sources/ClientRuntime/Networking/Http/CRT/CRTClientEngine.swift b/Sources/ClientRuntime/Networking/Http/CRT/CRTClientEngine.swift index 90fe9e8c0..46ad2cac9 100644 --- a/Sources/ClientRuntime/Networking/Http/CRT/CRTClientEngine.swift +++ b/Sources/ClientRuntime/Networking/Http/CRT/CRTClientEngine.swift @@ -25,9 +25,9 @@ public class CRTClientEngine: HTTPClient { private let port: Int16 init(endpoint: Endpoint) { - self.protocolType = endpoint.protocolType - self.host = endpoint.host - self.port = endpoint.port + self.protocolType = endpoint.uri.scheme + self.host = endpoint.uri.host + self.port = endpoint.uri.port } } @@ -72,9 +72,9 @@ public class CRTClientEngine: HTTPClient { } private func createConnectionPool(endpoint: Endpoint) throws -> HTTPClientConnectionManager { - let tlsConnectionOptions = endpoint.protocolType == .https ? TLSConnectionOptions( + let tlsConnectionOptions = endpoint.uri.scheme == .https ? TLSConnectionOptions( context: self.crtTLSOptions?.resolveContext() ?? sharedDefaultIO.tlsContext, - serverName: endpoint.host + serverName: endpoint.uri.host ) : nil var socketOptions = SocketOptions(socketType: .stream) @@ -93,9 +93,9 @@ public class CRTClientEngine: HTTPClient { let options = HTTPClientConnectionOptions( clientBootstrap: sharedDefaultIO.clientBootstrap, - hostName: endpoint.host, + hostName: endpoint.uri.host, initialWindowSize: windowSize, - port: UInt32(endpoint.port), + port: UInt32(endpoint.uri.port), proxyOptions: nil, socketOptions: socketOptions, tlsOptions: tlsConnectionOptions, @@ -104,7 +104,7 @@ public class CRTClientEngine: HTTPClient { enableManualWindowManagement: false ) // not using backpressure yet logger.debug(""" - Creating connection pool for \(String(describing: endpoint.host)) \ + Creating connection pool for \(String(describing: endpoint.uri.host)) \ with max connections: \(maxConnectionsPerEndpoint) """) return try HTTPClientConnectionManager(options: options) @@ -122,20 +122,20 @@ public class CRTClientEngine: HTTPClient { let tlsConnectionOptions = TLSConnectionOptions( context: self.crtTLSOptions?.resolveContext() ?? sharedDefaultIO.tlsContext, alpnList: [ALPNProtocol.http2.rawValue], - serverName: endpoint.host + serverName: endpoint.uri.host ) let options = HTTP2StreamManagerOptions( clientBootstrap: sharedDefaultIO.clientBootstrap, - hostName: endpoint.host, - port: UInt32(endpoint.port), + hostName: endpoint.uri.host, + port: UInt32(endpoint.uri.port), maxConnections: maxConnectionsPerEndpoint, socketOptions: socketOptions, tlsOptions: tlsConnectionOptions, enableStreamManualWindowManagement: false ) logger.debug(""" - Creating connection pool for \(String(describing: endpoint.host)) \ + Creating connection pool for \(String(describing: endpoint.uri.host)) \ with max connections: \(maxConnectionsPerEndpoint) """) @@ -164,7 +164,7 @@ public class CRTClientEngine: HTTPClient { let connectionMgr = try await serialExecutor.getOrCreateConnectionPool(endpoint: request.endpoint) let connection = try await connectionMgr.acquireConnection() - self.logger.debug("Connection was acquired to: \(String(describing: request.endpoint.url?.absoluteString))") + self.logger.debug("Connection was acquired to: \(String(describing: request.destination.url?.absoluteString))") switch connection.httpVersion { case .version_1_1: self.logger.debug("Using HTTP/1.1 connection") diff --git a/Sources/ClientRuntime/Networking/Http/Headers.swift b/Sources/ClientRuntime/Networking/Http/Headers.swift index 315bbdd28..9e45afdf5 100644 --- a/Sources/ClientRuntime/Networking/Http/Headers.swift +++ b/Sources/ClientRuntime/Networking/Http/Headers.swift @@ -159,6 +159,10 @@ public struct Headers { return first + last } } + + public var isEmpty: Bool { + return self.headers.isEmpty + } } extension Headers: Equatable { diff --git a/Sources/ClientRuntime/Networking/Http/Middlewares/ContentTypeMiddleware.swift b/Sources/ClientRuntime/Networking/Http/Middlewares/ContentTypeMiddleware.swift index a0b97318a..34d8612aa 100644 --- a/Sources/ClientRuntime/Networking/Http/Middlewares/ContentTypeMiddleware.swift +++ b/Sources/ClientRuntime/Networking/Http/Middlewares/ContentTypeMiddleware.swift @@ -42,6 +42,6 @@ extension ContentTypeMiddleware: HttpInterceptor { ) async throws { let builder = context.getRequest().toBuilder() addHeaders(builder: builder) - context.updateRequest(updated: builder.build()) + context.updateRequest(updated: try builder.build()) } } diff --git a/Sources/ClientRuntime/Networking/Http/Middlewares/FlexibleChecksumsRequestMiddleware.swift b/Sources/ClientRuntime/Networking/Http/Middlewares/FlexibleChecksumsRequestMiddleware.swift index 929b5fe3e..ae9cb1791 100644 --- a/Sources/ClientRuntime/Networking/Http/Middlewares/FlexibleChecksumsRequestMiddleware.swift +++ b/Sources/ClientRuntime/Networking/Http/Middlewares/FlexibleChecksumsRequestMiddleware.swift @@ -109,6 +109,6 @@ extension FlexibleChecksumsRequestMiddleware: HttpInterceptor { ) async throws { let builder = context.getRequest().toBuilder() try await addHeaders(builder: builder, attributes: context.getAttributes()) - context.updateRequest(updated: builder.build()) + context.updateRequest(updated: try builder.build()) } } diff --git a/Sources/ClientRuntime/Networking/Http/ProtocolType.swift b/Sources/ClientRuntime/Networking/Http/ProtocolType.swift index a34be6f6a..190482acd 100644 --- a/Sources/ClientRuntime/Networking/Http/ProtocolType.swift +++ b/Sources/ClientRuntime/Networking/Http/ProtocolType.swift @@ -5,6 +5,8 @@ import Foundation +public typealias Scheme = ProtocolType + public enum ProtocolType: String, CaseIterable { case http case https diff --git a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift index efab9c2ce..8ddc328bd 100644 --- a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift +++ b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift @@ -17,20 +17,30 @@ import struct Foundation.URLRequest // in the CRT engine so that is why it's a class public final class SdkHttpRequest: RequestMessage { public var body: ByteStream - public var endpoint: Endpoint + public var destination: URI + public var headers: Headers public let method: HttpMethodType - public var destination: URI { endpoint.uri } - public var headers: Headers { endpoint.headers } - public var path: String { endpoint.path } - public var host: String { endpoint.host } - public var queryItems: [SDKURLQueryItem]? { endpoint.queryItems } + public var host: String { destination.host } + public var path: String { destination.path } + public var queryItems: [SDKURLQueryItem]? { destination.query } public var trailingHeaders: Headers = Headers() + public var endpoint: Endpoint { + return Endpoint(uri: self.destination, headers: self.headers) + } + + public convenience init(method: HttpMethodType, + endpoint: Endpoint, + body: ByteStream = ByteStream.noStream) { + self.init(method: method, uri: endpoint.uri, headers: endpoint.headers, body: body) + } public init(method: HttpMethodType, - endpoint: Endpoint, + uri: URI, + headers: Headers, body: ByteStream = ByteStream.noStream) { self.method = method - self.endpoint = endpoint + self.destination = uri + self.headers = headers self.body = body } @@ -40,22 +50,20 @@ public final class SdkHttpRequest: RequestMessage { .withMethod(self.method) .withHeaders(self.headers) .withTrailers(self.trailingHeaders) - .withPath(self.path) - .withHost(self.host) - .withPort(self.endpoint.port) - .withProtocol(self.endpoint.protocolType ?? .https) - if let qItems = self.queryItems { - builder.withQueryItems(qItems) - } + .withPath(self.destination.path) + .withHost(self.destination.host) + .withPort(self.destination.port) + .withProtocol(self.destination.scheme) + .withQueryItems(self.destination.query) return builder } public func withHeader(name: String, value: String) { - self.endpoint.headers.add(name: name, value: value) + self.headers.add(name: name, value: value) } public func withoutHeader(name: String) { - self.endpoint.headers.remove(name: name) + self.headers.remove(name: name) } public func withBody(_ body: ByteStream) { @@ -88,7 +96,8 @@ extension SdkHttpRequest { public func toHttpRequest() throws -> HTTPRequest { let httpRequest = try HTTPRequest() httpRequest.method = method.rawValue - httpRequest.path = [endpoint.path, endpoint.query].compactMap { $0 }.joined(separator: "?") + httpRequest.path = [self.destination.path, self.destination.queryString] + .compactMap { $0 }.joined(separator: "?") httpRequest.addHeaders(headers: headers.toHttpHeaders()) httpRequest.body = isChunked ? nil : StreamableHttpBody(body: body) // body needs to be nil to use writeChunk() return httpRequest @@ -100,7 +109,8 @@ extension SdkHttpRequest { public func toHttp2Request() throws -> HTTPRequestBase { let httpRequest = try HTTPRequest() httpRequest.method = method.rawValue - httpRequest.path = [endpoint.path, endpoint.query].compactMap { $0 }.joined(separator: "?") + httpRequest.path = [self.destination.path, self.destination.queryString] + .compactMap { $0 }.joined(separator: "?") httpRequest.addHeaders(headers: headers.toHttpHeaders()) // Remove the "Transfer-Encoding" header if it exists since h2 does not support it @@ -116,7 +126,7 @@ extension SdkHttpRequest { public extension URLRequest { init(sdkRequest: SdkHttpRequest) async throws { // Set URL - guard let url = sdkRequest.endpoint.url else { + guard let url = sdkRequest.destination.url else { throw ClientError.dataNotFound("Failed to construct URLRequest due to missing URL.") } self.init(url: url) @@ -151,9 +161,11 @@ extension SdkHttpRequest: CustomDebugStringConvertible, CustomStringConvertible public var description: String { let method = method.rawValue.uppercased() - let protocolType = endpoint.protocolType ?? ProtocolType.https - let query = String(describing: queryItems) - return "\(method) \(protocolType):\(endpoint.port) \n Path: \(endpoint.path) \n \(headers) \n \(query)" + let protocolType = self.destination.scheme + let query = self.destination.queryString ?? "" + let port = self.destination.port + return "\(method) \(protocolType):\(port) \n " + + "Path: \(endpoint.uri.path) \n Headers: \(headers) \n Query: \(query)" } } @@ -285,9 +297,14 @@ public class SdkHttpRequestBuilder: RequestMessageBuilder { } public func build() -> SdkHttpRequest { - let uri = URI(scheme: protocolType.rawValue, path: path, host: host, port: port, query: queryItems) - let endpoint = Endpoint(uri: uri, protocolType: protocolType, headers: headers) - return SdkHttpRequest(method: methodType, endpoint: endpoint, body: body) + let uri = URIBuilder() + .withScheme(protocolType) + .withPath(path) + .withHost(host) + .withPort(port) + .withQueryItems(queryItems) + .build() + return SdkHttpRequest(method: methodType, uri: uri, headers: headers, body: body) } } diff --git a/Sources/ClientRuntime/Networking/Http/URI.swift b/Sources/ClientRuntime/Networking/Http/URI.swift index 69980bbfa..0441bad96 100644 --- a/Sources/ClientRuntime/Networking/Http/URI.swift +++ b/Sources/ClientRuntime/Networking/Http/URI.swift @@ -3,22 +3,36 @@ * SPDX-License-Identifier: Apache-2.0. */ +import Foundation + public struct URI: Hashable { - public let scheme: String + public let scheme: Scheme public let path: String public let host: String public let port: Int16 public let query: [SDKURLQueryItem] public let username: String? public let password: String? + public var url: URL? { + self.toBuilder().getUrl() + } + public var queryString: String? { + if self.query.isEmpty { + return nil + } + + return self.query.map { queryItem in + return [queryItem.name, queryItem.value].compactMap { $0 }.joined(separator: "=") + }.joined(separator: "&") + } - public init(scheme: String, - path: String, - host: String, - port: Int16, - query: [SDKURLQueryItem], - username: String? = nil, - password: String? = nil) { + fileprivate init(scheme: Scheme, + path: String, + host: String, + port: Int16, + query: [SDKURLQueryItem], + username: String? = nil, + password: String? = nil) { self.scheme = scheme self.path = path self.host = host @@ -27,4 +41,108 @@ public struct URI: Hashable { self.username = username self.password = password } + + public func toBuilder() -> URIBuilder { + return URIBuilder() + .withScheme(self.scheme) + .withPath(self.path) + .withHost(self.host) + .withPort(self.port) + .withQueryItems(self.query) + } +} + +public class URIBuilder { + var urlComponents: URLComponents + var scheme: Scheme = Scheme.https + var path: String = "/" + var host: String = "" + var port: Int16 = Int16(Scheme.https.port) + var query: [SDKURLQueryItem] = [] + var username: String? + var password: String? + + required public init() { + self.urlComponents = URLComponents() + self.urlComponents.scheme = self.scheme.rawValue + self.urlComponents.path = self.path + self.urlComponents.host = self.host + } + + @discardableResult + public func withScheme(_ value: Scheme?) -> URIBuilder { + self.scheme = value ?? Scheme.https + self.urlComponents.scheme = self.scheme.rawValue + return self + } + + @discardableResult + public func withPath(_ value: String) -> URIBuilder { + self.path = value + if self.path.contains("%") { + self.urlComponents.percentEncodedPath = self.path + } else { + self.urlComponents.path = self.path + } + return self + } + + @discardableResult + public func withHost(_ value: String) -> URIBuilder { + self.host = value + self.urlComponents.host = self.host + return self + } + + @discardableResult + public func withPort(_ value: Int16) -> URIBuilder { + self.port = value + return self + } + + @discardableResult + public func withQueryItems(_ value: [SDKURLQueryItem]) -> URIBuilder { + self.query.append(contentsOf: value) + if !self.query.isEmpty { + self.urlComponents.percentEncodedQuery = self.query.map { queryItem in + return [queryItem.name, queryItem.value].compactMap { $0 }.joined(separator: "=") + }.joined(separator: "&") + } + return self + } + + @discardableResult + public func withQueryItem(_ value: SDKURLQueryItem) -> URIBuilder { + withQueryItems([value]) + } + + @discardableResult + public func withUsername(_ value: String) -> URIBuilder { + self.username = value + self.urlComponents.user = self.username + return self + } + + @discardableResult + public func withPassword(_ value: String) -> URIBuilder { + self.password = value + self.urlComponents.password = self.password + return self + } + + public func build() -> URI { + return URI(scheme: self.scheme, + path: self.path, + host: self.host, + port: self.port, + query: self.query, + username: self.username, + password: self.password) + } + + // We still have to keep 'url' as an optional, since we're + // dealing with dynamic components that could be invalid. + fileprivate func getUrl() -> URL? { + return self.path.isEmpty && self.host.isEmpty ? nil : self.urlComponents.url + } } diff --git a/Sources/ClientRuntime/Networking/Http/URLSession/URLSessionHTTPClient.swift b/Sources/ClientRuntime/Networking/Http/URLSession/URLSessionHTTPClient.swift index 433409b50..b3762a3fc 100644 --- a/Sources/ClientRuntime/Networking/Http/URLSession/URLSessionHTTPClient.swift +++ b/Sources/ClientRuntime/Networking/Http/URLSession/URLSessionHTTPClient.swift @@ -462,10 +462,10 @@ public final class URLSessionHTTPClient: HTTPClient { /// - Returns: A `URLRequest` ready to be transmitted by `URLSession` for this operation. private func makeURLRequest(from request: SdkHttpRequest, httpBodyStream: InputStream?) throws -> URLRequest { var components = URLComponents() - components.scheme = config.protocolType?.rawValue ?? request.endpoint.protocolType?.rawValue ?? "https" - components.host = request.endpoint.host + components.scheme = config.protocolType?.rawValue ?? request.destination.scheme.rawValue ?? "https" + components.host = request.endpoint.uri.host components.port = port(for: request) - components.percentEncodedPath = request.path + components.percentEncodedPath = request.destination.path if let queryItems = request.queryItems, !queryItems.isEmpty { components.percentEncodedQueryItems = queryItems.map { Foundation.URLQueryItem(name: $0.name, value: $0.value) @@ -484,12 +484,12 @@ public final class URLSessionHTTPClient: HTTPClient { } private func port(for request: SdkHttpRequest) -> Int? { - switch (request.endpoint.protocolType, request.endpoint.port) { + switch (request.destination.scheme, request.destination.port) { case (.https, 443), (.http, 80): // Don't set port explicitly if it's the default port for the scheme return nil default: - return Int(request.endpoint.port) + return Int(request.destination.port) } } } diff --git a/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift b/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift index 2f23ad7c9..9d7758304 100644 --- a/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift +++ b/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift @@ -12,7 +12,7 @@ public struct ExpectedSdkHttpRequest { public var headers: Headers? public var forbiddenHeaders: [String]? public var requiredHeaders: [String]? - public var queryItems: [SDKURLQueryItem] { endpoint.queryItems } + public var queryItems: [SDKURLQueryItem] { endpoint.uri.query } public let forbiddenQueryItems: [SDKURLQueryItem]? public let requiredQueryItems: [SDKURLQueryItem]? public let endpoint: Endpoint @@ -137,8 +137,14 @@ public class ExpectedSdkHttpRequestBuilder { } public func build() -> ExpectedSdkHttpRequest { - let uri = URI(scheme: protocolType.rawValue, path: path, host: host, port: port, query: queryItems) - let endpoint = Endpoint(uri: uri, protocolType: protocolType) + let uri = URIBuilder() + .withScheme(protocolType) + .withPath(path) + .withHost(host) + .withPort(port) + .withQueryItems(queryItems) + .build() + let endpoint = Endpoint(uri: uri, headers: headers) let forbiddenQueryItems = !forbiddenQueryItems.isEmpty ? forbiddenQueryItems : nil let requiredQueryItems = !requiredQueryItems.isEmpty ? requiredQueryItems : nil diff --git a/Sources/SmithyTestUtil/RequestTestUtil/HttpRequestTestBase.swift b/Sources/SmithyTestUtil/RequestTestUtil/HttpRequestTestBase.swift index 32fb6ed7e..c46f0f729 100644 --- a/Sources/SmithyTestUtil/RequestTestUtil/HttpRequestTestBase.swift +++ b/Sources/SmithyTestUtil/RequestTestUtil/HttpRequestTestBase.swift @@ -214,8 +214,8 @@ open class HttpRequestTestBase: XCTestCase { assertQueryItems(expected.queryItems, actual.queryItems, file: file, line: line) - XCTAssertEqual(expected.endpoint.path, actual.path, file: file, line: line) - XCTAssertEqual(expected.endpoint.host, actual.host, file: file, line: line) + XCTAssertEqual(expected.endpoint.uri.path, actual.destination.path, file: file, line: line) + XCTAssertEqual(expected.endpoint.uri.host, actual.destination.host, file: file, line: line) XCTAssertEqual(expected.method, actual.method, file: file, line: line) assertForbiddenQueryItems(expected.forbiddenQueryItems, actual.queryItems, file: file, line: line) diff --git a/Sources/WeatherSDK/EndpointResolver.swift b/Sources/WeatherSDK/EndpointResolver.swift index 6a3e397a0..ee1f5de61 100644 --- a/Sources/WeatherSDK/EndpointResolver.swift +++ b/Sources/WeatherSDK/EndpointResolver.swift @@ -132,8 +132,8 @@ extension EndpointResolverMiddleware: ApplyEndpoint { attributes.set(key: AttributeKeys.signingAlgorithm, value: AWSSigningAlgorithm(rawValue: signingAlgorithm)) } - if let headers = endpoint.headers { - builder.withHeaders(headers) + if !endpoint.headers.isEmpty { + builder.withHeaders(endpoint.headers) } return builder.withMethod(attributes.getMethod()) diff --git a/Tests/ClientRuntimeTests/InterceptorTests/InterceptorTests.swift b/Tests/ClientRuntimeTests/InterceptorTests/InterceptorTests.swift index e0d211b19..b435a6fd3 100644 --- a/Tests/ClientRuntimeTests/InterceptorTests/InterceptorTests.swift +++ b/Tests/ClientRuntimeTests/InterceptorTests/InterceptorTests.swift @@ -61,7 +61,7 @@ class InterceptorTests: XCTestCase { public func modifyBeforeTransmit(context: some MutableRequest) async throws { let builder = context.getRequest().toBuilder() builder.withHeader(name: headerName, value: headerValue) - context.updateRequest(updated: builder.build()) + context.updateRequest(updated: try builder.build()) } } @@ -84,7 +84,7 @@ class InterceptorTests: XCTestCase { let input: TestInput = try XCTUnwrap(context.getInput()) let builder = context.getRequest().toBuilder() builder.withHeader(name: "otherProperty", value: "\(input.otherProperty)") - context.updateRequest(updated: builder.build()) + context.updateRequest(updated: try builder.build()) } } @@ -106,7 +106,7 @@ class InterceptorTests: XCTestCase { for i in interceptors { try await i.modifyBeforeSerialization(context: interceptorContext) } - interceptorContext.updateRequest(updated: SdkHttpRequestBuilder().build()) + interceptorContext.updateRequest(updated: try SdkHttpRequestBuilder().build()) for i in interceptors { try await i.modifyBeforeTransmit(context: interceptorContext) } diff --git a/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift index b86a286a8..b618087ec 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift @@ -19,7 +19,7 @@ class EndpointTests: XCTestCase { SDKURLQueryItem(name: "ghi", value: "jkl"), SDKURLQueryItem(name: "mno", value: "pqr") ] - XCTAssertEqual(endpoint.queryItems, expectedQueryItems) + XCTAssertEqual(endpoint.uri.query, expectedQueryItems) } func test_hashableAndEquatable_hashesMatchWhenURLQueryItemsAreEqual() throws { @@ -30,23 +30,23 @@ class EndpointTests: XCTestCase { } func test_path_percentEncodedInput() throws { - let endpoint = Endpoint( + let endpoint = try Endpoint( host: "xctest.amazonaws.com", path: "/abc%2Bdef", protocolType: .https ) - let foundationURL = try XCTUnwrap(endpoint.url) + let foundationURL = try XCTUnwrap(endpoint.uri.url) let absoluteString = foundationURL.absoluteString XCTAssertEqual(absoluteString, "https://xctest.amazonaws.com/abc%2Bdef") } func test_path_unencodedInput() throws { - let endpoint = Endpoint( + let endpoint = try Endpoint( host: "xctest.amazonaws.com", path: "/abc+def", protocolType: .https ) - let foundationURL = try XCTUnwrap(endpoint.url) + let foundationURL = try XCTUnwrap(endpoint.uri.url) let absoluteString = foundationURL.absoluteString XCTAssertEqual(absoluteString, "https://xctest.amazonaws.com/abc+def") } diff --git a/Tests/ClientRuntimeTests/NetworkingTests/Http/CRTClientEngineIntegrationTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/Http/CRTClientEngineIntegrationTests.swift index 7b379568a..4a5cc1543 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/Http/CRTClientEngineIntegrationTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/Http/CRTClientEngineIntegrationTests.swift @@ -32,7 +32,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { var headers = Headers() headers.add(name: "Content-type", value: "application/json") headers.add(name: "Host", value: "httpbin.org") - let request = SdkHttpRequest(method: .get, endpoint: Endpoint(host: "httpbin.org", path: "/get", headers: headers)) + let request = SdkHttpRequest(method: .get, endpoint: try Endpoint(host: "httpbin.org", path: "/get", headers: headers)) let response = try await httpClient.send(request: request) XCTAssertNotNil(response) @@ -48,7 +48,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { let encoder = JSONEncoder() let encodedData = try encoder.encode(body) let request = SdkHttpRequest(method: .post, - endpoint: Endpoint(host: "httpbin.org", path: "/post", headers: headers), + endpoint: try Endpoint(host: "httpbin.org", path: "/post", headers: headers), body: ByteStream.data(encodedData)) let response = try await httpClient.send(request: request) XCTAssertNotNil(response) @@ -61,7 +61,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { headers.add(name: "Content-type", value: "application/json") headers.add(name: "Host", value: "httpbin.org") let request = SdkHttpRequest(method: .get, - endpoint: Endpoint(host: "httpbin.org", path: "/stream-bytes/1024", headers: headers), + endpoint: try Endpoint(host: "httpbin.org", path: "/stream-bytes/1024", headers: headers), body: ByteStream.stream(BufferedStream())) let response = try await httpClient.send(request: request) XCTAssertNotNil(response) @@ -75,7 +75,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { headers.add(name: "Host", value: "httpbin.org") let request = SdkHttpRequest(method: .get, - endpoint: Endpoint(host: "httpbin.org", path: "/stream-bytes/1024", headers: headers), + endpoint: try Endpoint(host: "httpbin.org", path: "/stream-bytes/1024", headers: headers), body: ByteStream.stream(BufferedStream())) let response = try await httpClient.send(request: request) XCTAssertNotNil(response) @@ -95,7 +95,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { headers.add(name: "Host", value: "httpbin.org") let request = SdkHttpRequest(method: .get, - endpoint: Endpoint(host: "httpbin.org", path: "/stream-bytes/1", headers: headers), + endpoint: try Endpoint(host: "httpbin.org", path: "/stream-bytes/1", headers: headers), body: ByteStream.stream(BufferedStream())) let response = try await httpClient.send(request: request) XCTAssertNotNil(response) @@ -115,7 +115,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { headers.add(name: "Host", value: "httpbin.org") let request = SdkHttpRequest(method: .get, - endpoint: Endpoint(host: "httpbin.org", path: "/stream-bytes/3000", headers: headers), + endpoint: try Endpoint(host: "httpbin.org", path: "/stream-bytes/3000", headers: headers), body: ByteStream.stream(BufferedStream())) let response = try await httpClient.send(request: request) XCTAssertNotNil(response) @@ -138,7 +138,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { let encodedData = try encoder.encode(body) let request = SdkHttpRequest(method: .post, - endpoint: Endpoint(host: "httpbin.org", path: "/post", headers: headers), + endpoint: try Endpoint(host: "httpbin.org", path: "/post", headers: headers), body: ByteStream.stream(BufferedStream(data: encodedData))) let response = try await httpClient.send(request: request) XCTAssertNotNil(response) @@ -156,7 +156,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { let request = SdkHttpRequest( method: .put, - endpoint: Endpoint(host: "nghttp2.org", path: "/httpbin/put", headers: headers), + endpoint: try Endpoint(host: "nghttp2.org", path: "/httpbin/put", headers: headers), body: .data(encodedData) ) diff --git a/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift index c91399658..f51c6dd6b 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift @@ -22,7 +22,7 @@ class HttpRequestTests: NetworkingTestUtils { func testSdkHttpRequestToHttpRequest() throws { let headers = Headers(["header-item-name": "header-item-value"]) - let endpoint = Endpoint(host: "host.com", path: "/", headers: headers) + let endpoint = try Endpoint(host: "host.com", path: "/", headers: headers) let httpBody = ByteStream.data(expectedMockRequestData) let mockHttpRequest = SdkHttpRequest(method: .get, endpoint: endpoint, body: httpBody) @@ -59,7 +59,7 @@ class HttpRequestTests: NetworkingTestUtils { func testSdkHttpRequestToURLRequest() async throws { let headers = Headers(["Testname-1": "testvalue-1", "Testname-2": "testvalue-2"]) - let endpoint = Endpoint(host: "host.com", path: "/", headers: headers) + let endpoint = try Endpoint(host: "host.com", path: "/", headers: headers) let httpBody = ByteStream.data(expectedMockRequestData) let mockHttpRequest = SdkHttpRequest(method: .get, endpoint: endpoint, body: httpBody) @@ -77,7 +77,7 @@ class HttpRequestTests: NetworkingTestUtils { XCTAssertTrue(headersFromRequest.contains { $0.key == "Testname-2" && $0.value == "testvalue-2" }) let expectedBody = try await httpBody.readData() XCTAssertEqual(urlRequest.httpBody, expectedBody) - XCTAssertEqual(urlRequest.url, endpoint.url) + XCTAssertEqual(urlRequest.url, endpoint.uri.url) XCTAssertEqual(urlRequest.httpMethod, mockHttpRequest.method.rawValue) } @@ -122,7 +122,7 @@ class HttpRequestTests: NetworkingTestUtils { let httpRequest = try builder.build().toHttpRequest() httpRequest.path = "/hello?foo=bar&quz=bar&signedthing=signed" - let updatedRequest = builder.update(from: httpRequest, originalRequest: builder.build()) + let updatedRequest = builder.update(from: httpRequest, originalRequest: try builder.build()) XCTAssert(updatedRequest.path == "/hello") XCTAssert(updatedRequest.queryItems.count == 3) diff --git a/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift index 60d7478c5..822515872 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift @@ -12,13 +12,13 @@ class SdkRequestBuilderTests: XCTestCase { let url = "https://stehlibstoragebucket144955-dev.s3.us-east-1.amazonaws.com/public/README.md?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAVIBY7SZR4C4MBGAW%2F20220225%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220225T034419Z&X-Amz-Expires=17999&X-Amz-SignedHeaders=host&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEEQaCXVzLWVhc3QtMSJHMEUCIEKUDdnCf1h3cZNdv10q6G24zLgn0r6th4m%2FXSS9TuR4AiEAwOwf2cG%2F1W%2FkAz1UMqFW9sZp7SY6j%2BmiicLy1dB8OXUqmgYInf%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FARABGgwzNjA4OTY3NjM0OTEiDH6CiNUJiwL4sNy9KCruBbdJnGSx88A%2Be0SBpKEpSurxunasaDsJb2ZJPqVhC%2FcKPr87PYcp5BrnkGumcYhUAEknVbHACLLUx2%2Fnzfacp13PHmclSsLe52qGwjFFVMz0m41PV3HiCoHgIc7vVIHnwNySaX9ZJbVowhNvI4V9eixVKhjjx7Tqku1bsOzlq9dJP15qz8FVNjlKGjZFqrMGTzpTgmS9dNPqphwCcxx306RLUd35SEvXjxnWcidOdddYQs9j4wu47DOM8ftveag6cDptJQN71dDIRHgFTisVshD78Rm9pycKf3g0QvBAGtrzhcxUcJtNgIWv%2B10hsEBURsEYommcjI8vT1yX2K8pLVOxgL%2FRWXndbAeIzAu5vmLm6RqkfGwkHQPQl7uII6YzL2Gku%2FMDilVFw9TBKIg6KDP9l2GzzVRQsvLMpFIp%2Bx3a1s4OVduJRFpDYCwEsfKhIoVkb610gBbFayPKjQVcZfULdq1r5DOZzpHVDoijnKHAxHtFgaFPP6KtG%2BmdKeix8gccdbsdgMokWKtJhisFo%2BzLn02oSSX4ITkZKzZcriGxQO4E1YUlYyBhjlCg1b74faQfWstk24PrkCfNXYcQ5oxgglIA0tBOdOfwGn2Je3MBEj2T8Yz0GS%2BZib3DKVWRzU0Xk9pwDXH3iaBn9Uld%2BNyw9gxdOCBVKtTILtdfsjw9lJSVOJomlJn1h8gH96PToBg9lOc4ms6aA2Z%2FoN%2F3UV%2Beo5%2FpB9xfMkBIeOg6vAI0VtjkUhH052UouEKU%2BVGSGuCSuVzZmIBJLWHZaQUJZ3hJCdGqM%2FoM4Ud51Cidcnr%2Bni%2Bgp4RAfg0gvX6Eb2e%2FNMqNd0Eg7ftmnPXzQR3ZVC1yyNFu2kBt8icKdMnkLZT8YO1Racd1QgrZ5DZJlU%2FS2eisLGSTMb9Dmaq8AGbxgGK55acIvsQLzvJhavFWbh%2FyNudQBSLC0c5BZGxQk2J%2BJx%2FlR5wR%2B0gXOfTg5EImMrLzh7gOo56i2Rhj2xRFSDDDnuGQBjqHArU56DsGZNeKwkVK87lVJAfiAsWmKaDlBNXcuhKl084syaZMiVUQa7dPupgeg%2BvVPv4dAwNULjuE3B7V5lSea4RIDOfGwTtlj4Ekn3t4PrlxHpEyMeuRgekNhpdcfL5pgRcKH4AKgqI7fCrghhH1qDMTpUkORRv6EwPGqM0LPm93XPVOjrG3EJ5ZOYLJM05bbrcalzN0mbSbsRrcphme7Z8zpD5jNeSWdBhWA04DQ7RZuJpqFnn40yKq7G8kgtLmOJGRs7CGTWT9on7EOpH3RdKGPXL06%2BgLo7r9%2Fdkb%2BHGJF6spjqbH0SN%2FySjnvgNL1NrGAy%2FQgPwfDw6oJ6TPNdH8dfM8tmdd&X-Amz-Signature=90b9f3353ad4f2596fdd166c00da8fb86d5d255bf3c8b296c5e5a7db25fd41aa" let pathToMatch = "/public/README.md?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAVIBY7SZR4C4MBGAW%2F20220225%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220225T034419Z&X-Amz-Expires=17999&X-Amz-SignedHeaders=host&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEEQaCXVzLWVhc3QtMSJHMEUCIEKUDdnCf1h3cZNdv10q6G24zLgn0r6th4m%2FXSS9TuR4AiEAwOwf2cG%2F1W%2FkAz1UMqFW9sZp7SY6j%2BmiicLy1dB8OXUqmgYInf%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FARABGgwzNjA4OTY3NjM0OTEiDH6CiNUJiwL4sNy9KCruBbdJnGSx88A%2Be0SBpKEpSurxunasaDsJb2ZJPqVhC%2FcKPr87PYcp5BrnkGumcYhUAEknVbHACLLUx2%2Fnzfacp13PHmclSsLe52qGwjFFVMz0m41PV3HiCoHgIc7vVIHnwNySaX9ZJbVowhNvI4V9eixVKhjjx7Tqku1bsOzlq9dJP15qz8FVNjlKGjZFqrMGTzpTgmS9dNPqphwCcxx306RLUd35SEvXjxnWcidOdddYQs9j4wu47DOM8ftveag6cDptJQN71dDIRHgFTisVshD78Rm9pycKf3g0QvBAGtrzhcxUcJtNgIWv%2B10hsEBURsEYommcjI8vT1yX2K8pLVOxgL%2FRWXndbAeIzAu5vmLm6RqkfGwkHQPQl7uII6YzL2Gku%2FMDilVFw9TBKIg6KDP9l2GzzVRQsvLMpFIp%2Bx3a1s4OVduJRFpDYCwEsfKhIoVkb610gBbFayPKjQVcZfULdq1r5DOZzpHVDoijnKHAxHtFgaFPP6KtG%2BmdKeix8gccdbsdgMokWKtJhisFo%2BzLn02oSSX4ITkZKzZcriGxQO4E1YUlYyBhjlCg1b74faQfWstk24PrkCfNXYcQ5oxgglIA0tBOdOfwGn2Je3MBEj2T8Yz0GS%2BZib3DKVWRzU0Xk9pwDXH3iaBn9Uld%2BNyw9gxdOCBVKtTILtdfsjw9lJSVOJomlJn1h8gH96PToBg9lOc4ms6aA2Z%2FoN%2F3UV%2Beo5%2FpB9xfMkBIeOg6vAI0VtjkUhH052UouEKU%2BVGSGuCSuVzZmIBJLWHZaQUJZ3hJCdGqM%2FoM4Ud51Cidcnr%2Bni%2Bgp4RAfg0gvX6Eb2e%2FNMqNd0Eg7ftmnPXzQR3ZVC1yyNFu2kBt8icKdMnkLZT8YO1Racd1QgrZ5DZJlU%2FS2eisLGSTMb9Dmaq8AGbxgGK55acIvsQLzvJhavFWbh%2FyNudQBSLC0c5BZGxQk2J%2BJx%2FlR5wR%2B0gXOfTg5EImMrLzh7gOo56i2Rhj2xRFSDDDnuGQBjqHArU56DsGZNeKwkVK87lVJAfiAsWmKaDlBNXcuhKl084syaZMiVUQa7dPupgeg%2BvVPv4dAwNULjuE3B7V5lSea4RIDOfGwTtlj4Ekn3t4PrlxHpEyMeuRgekNhpdcfL5pgRcKH4AKgqI7fCrghhH1qDMTpUkORRv6EwPGqM0LPm93XPVOjrG3EJ5ZOYLJM05bbrcalzN0mbSbsRrcphme7Z8zpD5jNeSWdBhWA04DQ7RZuJpqFnn40yKq7G8kgtLmOJGRs7CGTWT9on7EOpH3RdKGPXL06%2BgLo7r9%2Fdkb%2BHGJF6spjqbH0SN%2FySjnvgNL1NrGAy%2FQgPwfDw6oJ6TPNdH8dfM8tmdd&X-Amz-Signature=90b9f3353ad4f2596fdd166c00da8fb86d5d255bf3c8b296c5e5a7db25fd41aa" let queryItems = [SDKURLQueryItem(name: "Bucket", value: "stehlibstoragebucket144955-dev"), SDKURLQueryItem(name: "Key", value: "public%2FREADME.md")] - let originalRequest = SdkHttpRequest(method: .get, endpoint: Endpoint(host: "stehlibstoragebucket144955-dev.s3.us-east-1.amazonaws.com", path: "/", port: 80, queryItems: queryItems, protocolType: .https)) + let originalRequest = SdkHttpRequest(method: .get, endpoint: try Endpoint(host: "stehlibstoragebucket144955-dev.s3.us-east-1.amazonaws.com", path: "/", port: 80, queryItems: queryItems, protocolType: .https)) let crtRequest = try HTTPRequest() crtRequest.path = pathToMatch - let updatedRequest = SdkHttpRequestBuilder().update(from: crtRequest, originalRequest: originalRequest).build() - let updatedPath = [updatedRequest.endpoint.path, updatedRequest.endpoint.query].compactMap { $0 }.joined(separator: "?") + let updatedRequest = try SdkHttpRequestBuilder().update(from: crtRequest, originalRequest: originalRequest).build() + let updatedPath = [updatedRequest.destination.path, updatedRequest.destination.queryString].compactMap { $0 }.joined(separator: "?") XCTAssertEqual(pathToMatch, updatedPath) - XCTAssertEqual(url, updatedRequest.endpoint.url?.absoluteString) + XCTAssertEqual(url, updatedRequest.destination.url?.absoluteString) } } diff --git a/Tests/ClientRuntimeTests/NetworkingTests/NetworkingTestUtils.swift b/Tests/ClientRuntimeTests/NetworkingTests/NetworkingTestUtils.swift index 30fb4cf16..895a5c146 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/NetworkingTestUtils.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/NetworkingTestUtils.swift @@ -52,7 +52,7 @@ class NetworkingTestUtils: XCTestCase { let endpoint: Endpoint! queryItems.append(SDKURLQueryItem(name: "qualifier", value: "qualifier-value")) - endpoint = Endpoint(host: host, path: path, queryItems: queryItems, headers: headers) + endpoint = try? Endpoint(host: host, path: path, queryItems: queryItems, headers: headers) return endpoint } diff --git a/Tests/ClientRuntimeTests/OrchestratorTests/OrchestratorTests.swift b/Tests/ClientRuntimeTests/OrchestratorTests/OrchestratorTests.swift index 904791b27..7ec20c236 100644 --- a/Tests/ClientRuntimeTests/OrchestratorTests/OrchestratorTests.swift +++ b/Tests/ClientRuntimeTests/OrchestratorTests/OrchestratorTests.swift @@ -495,7 +495,7 @@ class OrchestratorTests: XCTestCase { let b = self.traceOrchestrator(trace: partitionIdTrace) // We can force a getPartitionId to fail by explicitly setting the host to '' b.interceptors.addModifyBeforeRetryLoop({ context in - context.updateRequest(updated: context.getRequest().toBuilder().withHost("").build()) + context.updateRequest(updated: try context.getRequest().toBuilder().withHost("").build()) }) return try await b.build().execute(input: TestInput(foo: "")) } diff --git a/Tests/SmithyTestUtilTests/RequestTestUtilTests/HttpRequestTestBaseTests.swift b/Tests/SmithyTestUtilTests/RequestTestUtilTests/HttpRequestTestBaseTests.swift index 18b593143..abc054a9f 100644 --- a/Tests/SmithyTestUtilTests/RequestTestUtilTests/HttpRequestTestBaseTests.swift +++ b/Tests/SmithyTestUtilTests/RequestTestUtilTests/HttpRequestTestBaseTests.swift @@ -164,7 +164,7 @@ class HttpRequestTestBaseTests: HttpRequestTestBase { // Mocks the code-generated unit test which includes testing for forbidden/required headers/queries func testSayHello() async throws { - let expected = buildExpectedHttpRequest(method: .post, + let expected = try buildExpectedHttpRequest(method: .post, path: "/", headers: ["Content-Type": "application/json", "RequiredHeader": "required header"], @@ -196,7 +196,7 @@ class HttpRequestTestBaseTests: HttpRequestTestBase { let forbiddenQueryParams = ["ForbiddenQuery"] for forbiddenQueryParam in forbiddenQueryParams { XCTAssertFalse( - self.queryItemExists(forbiddenQueryParam, in: actual.endpoint.queryItems), + self.queryItemExists(forbiddenQueryParam, in: actual.destination.query), "Forbidden Query:\(forbiddenQueryParam) exists in query items" ) } @@ -208,7 +208,7 @@ class HttpRequestTestBaseTests: HttpRequestTestBase { let requiredQueryParams = ["RequiredQuery"] for requiredQueryParam in requiredQueryParams { - XCTAssertTrue(self.queryItemExists(requiredQueryParam, in: actual.endpoint.queryItems), + XCTAssertTrue(self.queryItemExists(requiredQueryParam, in: actual.destination.query), "Required Query:\(requiredQueryParam) does not exist in query items") } diff --git a/smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/middleware/EndpointResolverMiddleware.kt b/smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/middleware/EndpointResolverMiddleware.kt index 7b8a754ef..c80bcef84 100644 --- a/smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/middleware/EndpointResolverMiddleware.kt +++ b/smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/middleware/EndpointResolverMiddleware.kt @@ -103,8 +103,8 @@ open class EndpointResolverMiddleware( attributes.set(key: AttributeKeys.signingAlgorithm, value: AWSSigningAlgorithm(rawValue: signingAlgorithm)) } - if let headers = endpoint.headers { - builder.withHeaders(headers) + if !endpoint.headers.isEmpty { + builder.withHeaders(endpoint.headers) } return builder.withMethod(attributes.getMethod()) From 52dcba4bcb704fd6a81e9695f9dd2928afca290d Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Tue, 14 May 2024 18:43:49 -0700 Subject: [PATCH 03/19] Remove `try`, update `uri.query` to `uri.queryItems` and add `destination` to `RequestMessage` --- Sources/ClientRuntime/Message/RequestMessage.swift | 3 +++ Sources/ClientRuntime/Networking/Endpoint.swift | 4 ++-- .../Networking/Http/Middlewares/ContentTypeMiddleware.swift | 2 +- .../Http/Middlewares/FlexibleChecksumsRequestMiddleware.swift | 2 +- Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift | 4 ++-- .../RequestTestUtil/ExpectedSdkHttpRequest.swift | 2 +- Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift | 2 +- .../RequestTestUtilTests/HttpRequestTestBaseTests.swift | 4 ++-- 8 files changed, 13 insertions(+), 10 deletions(-) diff --git a/Sources/ClientRuntime/Message/RequestMessage.swift b/Sources/ClientRuntime/Message/RequestMessage.swift index 94cbe7d7b..2000a1d7d 100644 --- a/Sources/ClientRuntime/Message/RequestMessage.swift +++ b/Sources/ClientRuntime/Message/RequestMessage.swift @@ -17,6 +17,9 @@ public protocol RequestMessage { /// The body of the request. var body: ByteStream { get } + // The uri of the request + var destination: URI { get } + /// - Returns: A new builder for this request message, with all properties copied. func toBuilder() -> RequestBuilderType } diff --git a/Sources/ClientRuntime/Networking/Endpoint.swift b/Sources/ClientRuntime/Networking/Endpoint.swift index 9fd2f5176..df81b08e1 100644 --- a/Sources/ClientRuntime/Networking/Endpoint.swift +++ b/Sources/ClientRuntime/Networking/Endpoint.swift @@ -9,7 +9,7 @@ public struct Endpoint: Hashable { public let uri: URI public let headers: Headers public var protocolType: ProtocolType? { uri.scheme } - public var queryItems: [SDKURLQueryItem] { uri.query } + public var queryItems: [SDKURLQueryItem] { uri.queryItems } public var path: String { uri.path } public var host: String { uri.host } public var port: Int16 { uri.port } @@ -57,7 +57,7 @@ public struct Endpoint: Hashable { protocolType: ProtocolType? = .https) { let uri = URIBuilder() - .withScheme(protocolType) + .withScheme(protocolType ?? .https) .withPath(path) .withHost(host) .withPort(port) diff --git a/Sources/ClientRuntime/Networking/Http/Middlewares/ContentTypeMiddleware.swift b/Sources/ClientRuntime/Networking/Http/Middlewares/ContentTypeMiddleware.swift index 34d8612aa..a0b97318a 100644 --- a/Sources/ClientRuntime/Networking/Http/Middlewares/ContentTypeMiddleware.swift +++ b/Sources/ClientRuntime/Networking/Http/Middlewares/ContentTypeMiddleware.swift @@ -42,6 +42,6 @@ extension ContentTypeMiddleware: HttpInterceptor { ) async throws { let builder = context.getRequest().toBuilder() addHeaders(builder: builder) - context.updateRequest(updated: try builder.build()) + context.updateRequest(updated: builder.build()) } } diff --git a/Sources/ClientRuntime/Networking/Http/Middlewares/FlexibleChecksumsRequestMiddleware.swift b/Sources/ClientRuntime/Networking/Http/Middlewares/FlexibleChecksumsRequestMiddleware.swift index ae9cb1791..929b5fe3e 100644 --- a/Sources/ClientRuntime/Networking/Http/Middlewares/FlexibleChecksumsRequestMiddleware.swift +++ b/Sources/ClientRuntime/Networking/Http/Middlewares/FlexibleChecksumsRequestMiddleware.swift @@ -109,6 +109,6 @@ extension FlexibleChecksumsRequestMiddleware: HttpInterceptor { ) async throws { let builder = context.getRequest().toBuilder() try await addHeaders(builder: builder, attributes: context.getAttributes()) - context.updateRequest(updated: try builder.build()) + context.updateRequest(updated: builder.build()) } } diff --git a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift index 8ddc328bd..a01d06b90 100644 --- a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift +++ b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift @@ -22,7 +22,7 @@ public final class SdkHttpRequest: RequestMessage { public let method: HttpMethodType public var host: String { destination.host } public var path: String { destination.path } - public var queryItems: [SDKURLQueryItem]? { destination.query } + public var queryItems: [SDKURLQueryItem]? { destination.queryItems } public var trailingHeaders: Headers = Headers() public var endpoint: Endpoint { return Endpoint(uri: self.destination, headers: self.headers) @@ -54,7 +54,7 @@ public final class SdkHttpRequest: RequestMessage { .withHost(self.destination.host) .withPort(self.destination.port) .withProtocol(self.destination.scheme) - .withQueryItems(self.destination.query) + .withQueryItems(self.destination.queryItems) return builder } diff --git a/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift b/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift index 9d7758304..ade90fd6f 100644 --- a/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift +++ b/Sources/SmithyTestUtil/RequestTestUtil/ExpectedSdkHttpRequest.swift @@ -12,7 +12,7 @@ public struct ExpectedSdkHttpRequest { public var headers: Headers? public var forbiddenHeaders: [String]? public var requiredHeaders: [String]? - public var queryItems: [SDKURLQueryItem] { endpoint.uri.query } + public var queryItems: [SDKURLQueryItem] { endpoint.uri.queryItems } public let forbiddenQueryItems: [SDKURLQueryItem]? public let requiredQueryItems: [SDKURLQueryItem]? public let endpoint: Endpoint diff --git a/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift index b618087ec..dca20f65d 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift @@ -19,7 +19,7 @@ class EndpointTests: XCTestCase { SDKURLQueryItem(name: "ghi", value: "jkl"), SDKURLQueryItem(name: "mno", value: "pqr") ] - XCTAssertEqual(endpoint.uri.query, expectedQueryItems) + XCTAssertEqual(endpoint.uri.queryItems, expectedQueryItems) } func test_hashableAndEquatable_hashesMatchWhenURLQueryItemsAreEqual() throws { diff --git a/Tests/SmithyTestUtilTests/RequestTestUtilTests/HttpRequestTestBaseTests.swift b/Tests/SmithyTestUtilTests/RequestTestUtilTests/HttpRequestTestBaseTests.swift index abc054a9f..3497edc42 100644 --- a/Tests/SmithyTestUtilTests/RequestTestUtilTests/HttpRequestTestBaseTests.swift +++ b/Tests/SmithyTestUtilTests/RequestTestUtilTests/HttpRequestTestBaseTests.swift @@ -196,7 +196,7 @@ class HttpRequestTestBaseTests: HttpRequestTestBase { let forbiddenQueryParams = ["ForbiddenQuery"] for forbiddenQueryParam in forbiddenQueryParams { XCTAssertFalse( - self.queryItemExists(forbiddenQueryParam, in: actual.destination.query), + self.queryItemExists(forbiddenQueryParam, in: actual.destination.queryItems), "Forbidden Query:\(forbiddenQueryParam) exists in query items" ) } @@ -208,7 +208,7 @@ class HttpRequestTestBaseTests: HttpRequestTestBase { let requiredQueryParams = ["RequiredQuery"] for requiredQueryParam in requiredQueryParams { - XCTAssertTrue(self.queryItemExists(requiredQueryParam, in: actual.destination.query), + XCTAssertTrue(self.queryItemExists(requiredQueryParam, in: actual.destination.queryItems), "Required Query:\(requiredQueryParam) does not exist in query items") } From 6b8842c12418436080a7c8c0548ac833da38be5f Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Tue, 14 May 2024 18:45:53 -0700 Subject: [PATCH 04/19] Update `URI.query` to `URI.queryItems` --- .../ClientRuntime/Networking/Http/URI.swift | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Sources/ClientRuntime/Networking/Http/URI.swift b/Sources/ClientRuntime/Networking/Http/URI.swift index 0441bad96..bdea07a78 100644 --- a/Sources/ClientRuntime/Networking/Http/URI.swift +++ b/Sources/ClientRuntime/Networking/Http/URI.swift @@ -10,18 +10,18 @@ public struct URI: Hashable { public let path: String public let host: String public let port: Int16 - public let query: [SDKURLQueryItem] + public let queryItems: [SDKURLQueryItem] public let username: String? public let password: String? public var url: URL? { self.toBuilder().getUrl() } public var queryString: String? { - if self.query.isEmpty { + if self.queryItems.isEmpty { return nil } - return self.query.map { queryItem in + return self.queryItems.map { queryItem in return [queryItem.name, queryItem.value].compactMap { $0 }.joined(separator: "=") }.joined(separator: "&") } @@ -30,14 +30,14 @@ public struct URI: Hashable { path: String, host: String, port: Int16, - query: [SDKURLQueryItem], + queryItems: [SDKURLQueryItem], username: String? = nil, password: String? = nil) { self.scheme = scheme self.path = path self.host = host self.port = port - self.query = query + self.queryItems = queryItems self.username = username self.password = password } @@ -48,7 +48,7 @@ public struct URI: Hashable { .withPath(self.path) .withHost(self.host) .withPort(self.port) - .withQueryItems(self.query) + .withQueryItems(self.queryItems) } } @@ -58,7 +58,7 @@ public class URIBuilder { var path: String = "/" var host: String = "" var port: Int16 = Int16(Scheme.https.port) - var query: [SDKURLQueryItem] = [] + var queryItems: [SDKURLQueryItem] = [] var username: String? var password: String? @@ -102,9 +102,9 @@ public class URIBuilder { @discardableResult public func withQueryItems(_ value: [SDKURLQueryItem]) -> URIBuilder { - self.query.append(contentsOf: value) - if !self.query.isEmpty { - self.urlComponents.percentEncodedQuery = self.query.map { queryItem in + self.queryItems.append(contentsOf: value) + if !self.queryItems.isEmpty { + self.urlComponents.percentEncodedQuery = self.queryItems.map { queryItem in return [queryItem.name, queryItem.value].compactMap { $0 }.joined(separator: "=") }.joined(separator: "&") } @@ -135,7 +135,7 @@ public class URIBuilder { path: self.path, host: self.host, port: self.port, - query: self.query, + queryItems: self.queryItems, username: self.username, password: self.password) } From e11c263669b493693fcaa6b7e4aa7b3d9923b0d8 Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Tue, 14 May 2024 18:54:41 -0700 Subject: [PATCH 05/19] Refactor URI and add docs --- .../ClientRuntime/Networking/Http/URI.swift | 122 +++++++++++------- 1 file changed, 75 insertions(+), 47 deletions(-) diff --git a/Sources/ClientRuntime/Networking/Http/URI.swift b/Sources/ClientRuntime/Networking/Http/URI.swift index bdea07a78..50a4ddc46 100644 --- a/Sources/ClientRuntime/Networking/Http/URI.swift +++ b/Sources/ClientRuntime/Networking/Http/URI.swift @@ -5,6 +5,7 @@ import Foundation +/// Universal Resource Identifier component used to construct target location for a Request. public struct URI: Hashable { public let scheme: Scheme public let path: String @@ -17,13 +18,7 @@ public struct URI: Hashable { self.toBuilder().getUrl() } public var queryString: String? { - if self.queryItems.isEmpty { - return nil - } - - return self.queryItems.map { queryItem in - return [queryItem.name, queryItem.value].compactMap { $0 }.joined(separator: "=") - }.joined(separator: "&") + return self.queryItems.queryString } fileprivate init(scheme: Scheme, @@ -49,48 +44,47 @@ public struct URI: Hashable { .withHost(self.host) .withPort(self.port) .withQueryItems(self.queryItems) + .withUsername(self.username) + .withPassword(self.password) } } -public class URIBuilder { +/// A builder class for URI +/// The builder performs validation to conform with RFC 3986 +public final class URIBuilder { var urlComponents: URLComponents - var scheme: Scheme = Scheme.https - var path: String = "/" - var host: String = "" - var port: Int16 = Int16(Scheme.https.port) - var queryItems: [SDKURLQueryItem] = [] - var username: String? - var password: String? - - required public init() { + var port: Int16 + var path: String + + public init() { + self.port = Int16(Scheme.https.port) + self.path = "/" self.urlComponents = URLComponents() - self.urlComponents.scheme = self.scheme.rawValue + self.urlComponents.scheme = Scheme.https.rawValue self.urlComponents.path = self.path - self.urlComponents.host = self.host + self.urlComponents.host = "" } @discardableResult - public func withScheme(_ value: Scheme?) -> URIBuilder { - self.scheme = value ?? Scheme.https - self.urlComponents.scheme = self.scheme.rawValue + public func withScheme(_ value: Scheme) -> URIBuilder { + self.urlComponents.scheme = value.rawValue return self } @discardableResult public func withPath(_ value: String) -> URIBuilder { - self.path = value - if self.path.contains("%") { - self.urlComponents.percentEncodedPath = self.path + if value.isPercentEncoded { + self.urlComponents.percentEncodedPath = value } else { - self.urlComponents.path = self.path + self.urlComponents.path = value } + self.path = value return self } @discardableResult public func withHost(_ value: String) -> URIBuilder { - self.host = value - self.urlComponents.host = self.host + self.urlComponents.host = value return self } @@ -102,47 +96,81 @@ public class URIBuilder { @discardableResult public func withQueryItems(_ value: [SDKURLQueryItem]) -> URIBuilder { - self.queryItems.append(contentsOf: value) - if !self.queryItems.isEmpty { - self.urlComponents.percentEncodedQuery = self.queryItems.map { queryItem in - return [queryItem.name, queryItem.value].compactMap { $0 }.joined(separator: "=") - }.joined(separator: "&") + if !value.isEmpty { + self.urlComponents.percentEncodedQueryItems = value.toURLQueryItems } return self } @discardableResult - public func withQueryItem(_ value: SDKURLQueryItem) -> URIBuilder { - withQueryItems([value]) + public func appendQueryItems(_ items: [SDKURLQueryItem]) -> URIBuilder { + guard !items.isEmpty else { + return self + } + var queryItems = self.urlComponents.percentEncodedQueryItems ?? [] + queryItems += items.toURLQueryItems + self.urlComponents.percentEncodedQueryItems = queryItems + return self + } + + @discardableResult + public func appendQueryItem(_ item: SDKURLQueryItem) -> URIBuilder { + self.appendQueryItems([item]) + return self } @discardableResult - public func withUsername(_ value: String) -> URIBuilder { - self.username = value - self.urlComponents.user = self.username + public func withUsername(_ value: String?) -> URIBuilder { + self.urlComponents.user = value return self } @discardableResult - public func withPassword(_ value: String) -> URIBuilder { - self.password = value - self.urlComponents.password = self.password + public func withPassword(_ value: String?) -> URIBuilder { + self.urlComponents.password = value return self } public func build() -> URI { - return URI(scheme: self.scheme, + return URI(scheme: Scheme(rawValue: self.urlComponents.scheme!)!, path: self.path, - host: self.host, + host: self.urlComponents.host!, port: self.port, - queryItems: self.queryItems, - username: self.username, - password: self.password) + queryItems: self.urlComponents.percentEncodedQueryItems?.map { item in + return SDKURLQueryItem(name: item.name, value: item.value) + } ?? [], + username: self.urlComponents.user, + password: self.urlComponents.password) } // We still have to keep 'url' as an optional, since we're // dealing with dynamic components that could be invalid. fileprivate func getUrl() -> URL? { - return self.path.isEmpty && self.host.isEmpty ? nil : self.urlComponents.url + let isInvalidHost = self.urlComponents.host?.isEmpty ?? false + return isInvalidHost && self.urlComponents.path.isEmpty ? nil : self.urlComponents.url + } +} + +extension String { + var isPercentEncoded: Bool { + let decoded = self.removingPercentEncoding + return decoded != nil && decoded != self + } +} + +extension Array where Element == SDKURLQueryItem { + public var queryString: String? { + if self.isEmpty { + return nil + } + return self.map { + return [$0.name, $0.value].compactMap { $0 }.joined(separator: "=") + }.joined(separator: "&") + } + + public var toURLQueryItems: [URLQueryItem] { + return self.map { + URLQueryItem(name: $0.name, value: $0.value) + } } } From 985036be2b1562efe6be5f1d2460577b4bc1f616 Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Tue, 14 May 2024 19:08:32 -0700 Subject: [PATCH 06/19] Set `SdkHttpRequest.destination` to `let` --- Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift index a01d06b90..926ee109c 100644 --- a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift +++ b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift @@ -17,7 +17,7 @@ import struct Foundation.URLRequest // in the CRT engine so that is why it's a class public final class SdkHttpRequest: RequestMessage { public var body: ByteStream - public var destination: URI + public let destination: URI public var headers: Headers public let method: HttpMethodType public var host: String { destination.host } From 96c5caea5015e9df7a729b17843756f08c18a599 Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Wed, 15 May 2024 12:10:40 -0700 Subject: [PATCH 07/19] Update `URIBuilder` to always return a percent encoded path in `URI` --- Sources/ClientRuntime/Networking/Http/URI.swift | 9 +++------ .../NetworkingTests/Http/HttpRequestTests.swift | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/Sources/ClientRuntime/Networking/Http/URI.swift b/Sources/ClientRuntime/Networking/Http/URI.swift index 50a4ddc46..65f4091f2 100644 --- a/Sources/ClientRuntime/Networking/Http/URI.swift +++ b/Sources/ClientRuntime/Networking/Http/URI.swift @@ -5,7 +5,7 @@ import Foundation -/// Universal Resource Identifier component used to construct target location for a Request. +/// A representation of the RFC 3986 Universal Resource Identifier public struct URI: Hashable { public let scheme: Scheme public let path: String @@ -54,14 +54,12 @@ public struct URI: Hashable { public final class URIBuilder { var urlComponents: URLComponents var port: Int16 - var path: String public init() { self.port = Int16(Scheme.https.port) - self.path = "/" self.urlComponents = URLComponents() + self.urlComponents.percentEncodedPath = "/" self.urlComponents.scheme = Scheme.https.rawValue - self.urlComponents.path = self.path self.urlComponents.host = "" } @@ -78,7 +76,6 @@ public final class URIBuilder { } else { self.urlComponents.path = value } - self.path = value return self } @@ -133,7 +130,7 @@ public final class URIBuilder { public func build() -> URI { return URI(scheme: Scheme(rawValue: self.urlComponents.scheme!)!, - path: self.path, + path: self.urlComponents.percentEncodedPath, host: self.urlComponents.host!, port: self.port, queryItems: self.urlComponents.percentEncodedQueryItems?.map { item in diff --git a/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift index f51c6dd6b..5a41dfd13 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift @@ -132,7 +132,7 @@ class HttpRequestTests: NetworkingTestUtils { } func testPathInInHttpRequestIsNotAltered() throws { - let path = "/space /colon:/dollar$/tilde~/dash-/underscore_/period." + let path = "/space%20/colon:/dollar$/tilde~/dash-/underscore_/period." let builder = SdkHttpRequestBuilder() .withHeader(name: "Host", value: "xctest.amazon.com") .withPath(path) From 786b3f8dc8cd64e04c6af89a9d99a0bc3c69f1d1 Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Wed, 15 May 2024 12:43:39 -0700 Subject: [PATCH 08/19] Update `URIBuilder` to add port to URLComponents and to be able to clear queryItems --- .../{Networking/Http => Message}/URI.swift | 11 ++++++----- .../NetworkingTests/EndpointTests.swift | 4 ++-- .../NetworkingTests/Http/SdkRequestBuilderTests.swift | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) rename Sources/ClientRuntime/{Networking/Http => Message}/URI.swift (95%) diff --git a/Sources/ClientRuntime/Networking/Http/URI.swift b/Sources/ClientRuntime/Message/URI.swift similarity index 95% rename from Sources/ClientRuntime/Networking/Http/URI.swift rename to Sources/ClientRuntime/Message/URI.swift index 65f4091f2..a75165c5d 100644 --- a/Sources/ClientRuntime/Networking/Http/URI.swift +++ b/Sources/ClientRuntime/Message/URI.swift @@ -53,11 +53,10 @@ public struct URI: Hashable { /// The builder performs validation to conform with RFC 3986 public final class URIBuilder { var urlComponents: URLComponents - var port: Int16 public init() { - self.port = Int16(Scheme.https.port) self.urlComponents = URLComponents() + self.urlComponents.port = Scheme.https.port self.urlComponents.percentEncodedPath = "/" self.urlComponents.scheme = Scheme.https.rawValue self.urlComponents.host = "" @@ -87,13 +86,15 @@ public final class URIBuilder { @discardableResult public func withPort(_ value: Int16) -> URIBuilder { - self.port = value + self.urlComponents.port = Int(value) return self } @discardableResult public func withQueryItems(_ value: [SDKURLQueryItem]) -> URIBuilder { - if !value.isEmpty { + if value.isEmpty { + self.urlComponents.percentEncodedQueryItems = nil + } else { self.urlComponents.percentEncodedQueryItems = value.toURLQueryItems } return self @@ -132,7 +133,7 @@ public final class URIBuilder { return URI(scheme: Scheme(rawValue: self.urlComponents.scheme!)!, path: self.urlComponents.percentEncodedPath, host: self.urlComponents.host!, - port: self.port, + port: Int16(self.urlComponents.port!), queryItems: self.urlComponents.percentEncodedQueryItems?.map { item in return SDKURLQueryItem(name: item.name, value: item.value) } ?? [], diff --git a/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift index dca20f65d..688220b55 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift @@ -37,7 +37,7 @@ class EndpointTests: XCTestCase { ) let foundationURL = try XCTUnwrap(endpoint.uri.url) let absoluteString = foundationURL.absoluteString - XCTAssertEqual(absoluteString, "https://xctest.amazonaws.com/abc%2Bdef") + XCTAssertEqual(absoluteString, "https://xctest.amazonaws.com:443/abc%2Bdef") } func test_path_unencodedInput() throws { @@ -48,6 +48,6 @@ class EndpointTests: XCTestCase { ) let foundationURL = try XCTUnwrap(endpoint.uri.url) let absoluteString = foundationURL.absoluteString - XCTAssertEqual(absoluteString, "https://xctest.amazonaws.com/abc+def") + XCTAssertEqual(absoluteString, "https://xctest.amazonaws.com:443/abc+def") } } diff --git a/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift index 822515872..5228177e2 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift @@ -9,7 +9,7 @@ import AwsCommonRuntimeKit class SdkRequestBuilderTests: XCTestCase { func testSdkRequestBuilderUpdate() throws { - let url = "https://stehlibstoragebucket144955-dev.s3.us-east-1.amazonaws.com/public/README.md?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAVIBY7SZR4C4MBGAW%2F20220225%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220225T034419Z&X-Amz-Expires=17999&X-Amz-SignedHeaders=host&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEEQaCXVzLWVhc3QtMSJHMEUCIEKUDdnCf1h3cZNdv10q6G24zLgn0r6th4m%2FXSS9TuR4AiEAwOwf2cG%2F1W%2FkAz1UMqFW9sZp7SY6j%2BmiicLy1dB8OXUqmgYInf%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FARABGgwzNjA4OTY3NjM0OTEiDH6CiNUJiwL4sNy9KCruBbdJnGSx88A%2Be0SBpKEpSurxunasaDsJb2ZJPqVhC%2FcKPr87PYcp5BrnkGumcYhUAEknVbHACLLUx2%2Fnzfacp13PHmclSsLe52qGwjFFVMz0m41PV3HiCoHgIc7vVIHnwNySaX9ZJbVowhNvI4V9eixVKhjjx7Tqku1bsOzlq9dJP15qz8FVNjlKGjZFqrMGTzpTgmS9dNPqphwCcxx306RLUd35SEvXjxnWcidOdddYQs9j4wu47DOM8ftveag6cDptJQN71dDIRHgFTisVshD78Rm9pycKf3g0QvBAGtrzhcxUcJtNgIWv%2B10hsEBURsEYommcjI8vT1yX2K8pLVOxgL%2FRWXndbAeIzAu5vmLm6RqkfGwkHQPQl7uII6YzL2Gku%2FMDilVFw9TBKIg6KDP9l2GzzVRQsvLMpFIp%2Bx3a1s4OVduJRFpDYCwEsfKhIoVkb610gBbFayPKjQVcZfULdq1r5DOZzpHVDoijnKHAxHtFgaFPP6KtG%2BmdKeix8gccdbsdgMokWKtJhisFo%2BzLn02oSSX4ITkZKzZcriGxQO4E1YUlYyBhjlCg1b74faQfWstk24PrkCfNXYcQ5oxgglIA0tBOdOfwGn2Je3MBEj2T8Yz0GS%2BZib3DKVWRzU0Xk9pwDXH3iaBn9Uld%2BNyw9gxdOCBVKtTILtdfsjw9lJSVOJomlJn1h8gH96PToBg9lOc4ms6aA2Z%2FoN%2F3UV%2Beo5%2FpB9xfMkBIeOg6vAI0VtjkUhH052UouEKU%2BVGSGuCSuVzZmIBJLWHZaQUJZ3hJCdGqM%2FoM4Ud51Cidcnr%2Bni%2Bgp4RAfg0gvX6Eb2e%2FNMqNd0Eg7ftmnPXzQR3ZVC1yyNFu2kBt8icKdMnkLZT8YO1Racd1QgrZ5DZJlU%2FS2eisLGSTMb9Dmaq8AGbxgGK55acIvsQLzvJhavFWbh%2FyNudQBSLC0c5BZGxQk2J%2BJx%2FlR5wR%2B0gXOfTg5EImMrLzh7gOo56i2Rhj2xRFSDDDnuGQBjqHArU56DsGZNeKwkVK87lVJAfiAsWmKaDlBNXcuhKl084syaZMiVUQa7dPupgeg%2BvVPv4dAwNULjuE3B7V5lSea4RIDOfGwTtlj4Ekn3t4PrlxHpEyMeuRgekNhpdcfL5pgRcKH4AKgqI7fCrghhH1qDMTpUkORRv6EwPGqM0LPm93XPVOjrG3EJ5ZOYLJM05bbrcalzN0mbSbsRrcphme7Z8zpD5jNeSWdBhWA04DQ7RZuJpqFnn40yKq7G8kgtLmOJGRs7CGTWT9on7EOpH3RdKGPXL06%2BgLo7r9%2Fdkb%2BHGJF6spjqbH0SN%2FySjnvgNL1NrGAy%2FQgPwfDw6oJ6TPNdH8dfM8tmdd&X-Amz-Signature=90b9f3353ad4f2596fdd166c00da8fb86d5d255bf3c8b296c5e5a7db25fd41aa" + let url = "https://stehlibstoragebucket144955-dev.s3.us-east-1.amazonaws.com:443/public/README.md?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAVIBY7SZR4C4MBGAW%2F20220225%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220225T034419Z&X-Amz-Expires=17999&X-Amz-SignedHeaders=host&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEEQaCXVzLWVhc3QtMSJHMEUCIEKUDdnCf1h3cZNdv10q6G24zLgn0r6th4m%2FXSS9TuR4AiEAwOwf2cG%2F1W%2FkAz1UMqFW9sZp7SY6j%2BmiicLy1dB8OXUqmgYInf%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FARABGgwzNjA4OTY3NjM0OTEiDH6CiNUJiwL4sNy9KCruBbdJnGSx88A%2Be0SBpKEpSurxunasaDsJb2ZJPqVhC%2FcKPr87PYcp5BrnkGumcYhUAEknVbHACLLUx2%2Fnzfacp13PHmclSsLe52qGwjFFVMz0m41PV3HiCoHgIc7vVIHnwNySaX9ZJbVowhNvI4V9eixVKhjjx7Tqku1bsOzlq9dJP15qz8FVNjlKGjZFqrMGTzpTgmS9dNPqphwCcxx306RLUd35SEvXjxnWcidOdddYQs9j4wu47DOM8ftveag6cDptJQN71dDIRHgFTisVshD78Rm9pycKf3g0QvBAGtrzhcxUcJtNgIWv%2B10hsEBURsEYommcjI8vT1yX2K8pLVOxgL%2FRWXndbAeIzAu5vmLm6RqkfGwkHQPQl7uII6YzL2Gku%2FMDilVFw9TBKIg6KDP9l2GzzVRQsvLMpFIp%2Bx3a1s4OVduJRFpDYCwEsfKhIoVkb610gBbFayPKjQVcZfULdq1r5DOZzpHVDoijnKHAxHtFgaFPP6KtG%2BmdKeix8gccdbsdgMokWKtJhisFo%2BzLn02oSSX4ITkZKzZcriGxQO4E1YUlYyBhjlCg1b74faQfWstk24PrkCfNXYcQ5oxgglIA0tBOdOfwGn2Je3MBEj2T8Yz0GS%2BZib3DKVWRzU0Xk9pwDXH3iaBn9Uld%2BNyw9gxdOCBVKtTILtdfsjw9lJSVOJomlJn1h8gH96PToBg9lOc4ms6aA2Z%2FoN%2F3UV%2Beo5%2FpB9xfMkBIeOg6vAI0VtjkUhH052UouEKU%2BVGSGuCSuVzZmIBJLWHZaQUJZ3hJCdGqM%2FoM4Ud51Cidcnr%2Bni%2Bgp4RAfg0gvX6Eb2e%2FNMqNd0Eg7ftmnPXzQR3ZVC1yyNFu2kBt8icKdMnkLZT8YO1Racd1QgrZ5DZJlU%2FS2eisLGSTMb9Dmaq8AGbxgGK55acIvsQLzvJhavFWbh%2FyNudQBSLC0c5BZGxQk2J%2BJx%2FlR5wR%2B0gXOfTg5EImMrLzh7gOo56i2Rhj2xRFSDDDnuGQBjqHArU56DsGZNeKwkVK87lVJAfiAsWmKaDlBNXcuhKl084syaZMiVUQa7dPupgeg%2BvVPv4dAwNULjuE3B7V5lSea4RIDOfGwTtlj4Ekn3t4PrlxHpEyMeuRgekNhpdcfL5pgRcKH4AKgqI7fCrghhH1qDMTpUkORRv6EwPGqM0LPm93XPVOjrG3EJ5ZOYLJM05bbrcalzN0mbSbsRrcphme7Z8zpD5jNeSWdBhWA04DQ7RZuJpqFnn40yKq7G8kgtLmOJGRs7CGTWT9on7EOpH3RdKGPXL06%2BgLo7r9%2Fdkb%2BHGJF6spjqbH0SN%2FySjnvgNL1NrGAy%2FQgPwfDw6oJ6TPNdH8dfM8tmdd&X-Amz-Signature=90b9f3353ad4f2596fdd166c00da8fb86d5d255bf3c8b296c5e5a7db25fd41aa" let pathToMatch = "/public/README.md?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAVIBY7SZR4C4MBGAW%2F20220225%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220225T034419Z&X-Amz-Expires=17999&X-Amz-SignedHeaders=host&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEEQaCXVzLWVhc3QtMSJHMEUCIEKUDdnCf1h3cZNdv10q6G24zLgn0r6th4m%2FXSS9TuR4AiEAwOwf2cG%2F1W%2FkAz1UMqFW9sZp7SY6j%2BmiicLy1dB8OXUqmgYInf%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FARABGgwzNjA4OTY3NjM0OTEiDH6CiNUJiwL4sNy9KCruBbdJnGSx88A%2Be0SBpKEpSurxunasaDsJb2ZJPqVhC%2FcKPr87PYcp5BrnkGumcYhUAEknVbHACLLUx2%2Fnzfacp13PHmclSsLe52qGwjFFVMz0m41PV3HiCoHgIc7vVIHnwNySaX9ZJbVowhNvI4V9eixVKhjjx7Tqku1bsOzlq9dJP15qz8FVNjlKGjZFqrMGTzpTgmS9dNPqphwCcxx306RLUd35SEvXjxnWcidOdddYQs9j4wu47DOM8ftveag6cDptJQN71dDIRHgFTisVshD78Rm9pycKf3g0QvBAGtrzhcxUcJtNgIWv%2B10hsEBURsEYommcjI8vT1yX2K8pLVOxgL%2FRWXndbAeIzAu5vmLm6RqkfGwkHQPQl7uII6YzL2Gku%2FMDilVFw9TBKIg6KDP9l2GzzVRQsvLMpFIp%2Bx3a1s4OVduJRFpDYCwEsfKhIoVkb610gBbFayPKjQVcZfULdq1r5DOZzpHVDoijnKHAxHtFgaFPP6KtG%2BmdKeix8gccdbsdgMokWKtJhisFo%2BzLn02oSSX4ITkZKzZcriGxQO4E1YUlYyBhjlCg1b74faQfWstk24PrkCfNXYcQ5oxgglIA0tBOdOfwGn2Je3MBEj2T8Yz0GS%2BZib3DKVWRzU0Xk9pwDXH3iaBn9Uld%2BNyw9gxdOCBVKtTILtdfsjw9lJSVOJomlJn1h8gH96PToBg9lOc4ms6aA2Z%2FoN%2F3UV%2Beo5%2FpB9xfMkBIeOg6vAI0VtjkUhH052UouEKU%2BVGSGuCSuVzZmIBJLWHZaQUJZ3hJCdGqM%2FoM4Ud51Cidcnr%2Bni%2Bgp4RAfg0gvX6Eb2e%2FNMqNd0Eg7ftmnPXzQR3ZVC1yyNFu2kBt8icKdMnkLZT8YO1Racd1QgrZ5DZJlU%2FS2eisLGSTMb9Dmaq8AGbxgGK55acIvsQLzvJhavFWbh%2FyNudQBSLC0c5BZGxQk2J%2BJx%2FlR5wR%2B0gXOfTg5EImMrLzh7gOo56i2Rhj2xRFSDDDnuGQBjqHArU56DsGZNeKwkVK87lVJAfiAsWmKaDlBNXcuhKl084syaZMiVUQa7dPupgeg%2BvVPv4dAwNULjuE3B7V5lSea4RIDOfGwTtlj4Ekn3t4PrlxHpEyMeuRgekNhpdcfL5pgRcKH4AKgqI7fCrghhH1qDMTpUkORRv6EwPGqM0LPm93XPVOjrG3EJ5ZOYLJM05bbrcalzN0mbSbsRrcphme7Z8zpD5jNeSWdBhWA04DQ7RZuJpqFnn40yKq7G8kgtLmOJGRs7CGTWT9on7EOpH3RdKGPXL06%2BgLo7r9%2Fdkb%2BHGJF6spjqbH0SN%2FySjnvgNL1NrGAy%2FQgPwfDw6oJ6TPNdH8dfM8tmdd&X-Amz-Signature=90b9f3353ad4f2596fdd166c00da8fb86d5d255bf3c8b296c5e5a7db25fd41aa" let queryItems = [SDKURLQueryItem(name: "Bucket", value: "stehlibstoragebucket144955-dev"), SDKURLQueryItem(name: "Key", value: "public%2FREADME.md")] let originalRequest = SdkHttpRequest(method: .get, endpoint: try Endpoint(host: "stehlibstoragebucket144955-dev.s3.us-east-1.amazonaws.com", path: "/", port: 80, queryItems: queryItems, protocolType: .https)) From bf58e53d6d2bc48ad5fd9ff82da76b7b4ee3ca84 Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Wed, 15 May 2024 16:40:19 -0700 Subject: [PATCH 09/19] Update port to be nullable in SdkHttpRequest and Endpoint --- Sources/ClientRuntime/Message/URI.swift | 46 +++++++++++-------- .../ClientRuntime/Networking/Endpoint.swift | 6 +-- .../Networking/Http/CRT/CRTClientEngine.swift | 6 +-- .../Networking/Http/SdkHttpRequest.swift | 4 +- .../URLSession/URLSessionHTTPClient.swift | 4 +- .../URL+Extension.swift | 2 +- .../Http/SdkRequestBuilderTests.swift | 2 +- 7 files changed, 38 insertions(+), 32 deletions(-) diff --git a/Sources/ClientRuntime/Message/URI.swift b/Sources/ClientRuntime/Message/URI.swift index a75165c5d..eb3a43f6d 100644 --- a/Sources/ClientRuntime/Message/URI.swift +++ b/Sources/ClientRuntime/Message/URI.swift @@ -10,7 +10,10 @@ public struct URI: Hashable { public let scheme: Scheme public let path: String public let host: String - public let port: Int16 + public let port: Int16? + public var defaultPort: Int16 { + Int16(scheme.port) + } public let queryItems: [SDKURLQueryItem] public let username: String? public let password: String? @@ -18,13 +21,13 @@ public struct URI: Hashable { self.toBuilder().getUrl() } public var queryString: String? { - return self.queryItems.queryString + self.queryItems.queryString } fileprivate init(scheme: Scheme, path: String, host: String, - port: Int16, + port: Int16?, queryItems: [SDKURLQueryItem], username: String? = nil, password: String? = nil) { @@ -56,7 +59,6 @@ public final class URIBuilder { public init() { self.urlComponents = URLComponents() - self.urlComponents.port = Scheme.https.port self.urlComponents.percentEncodedPath = "/" self.urlComponents.scheme = Scheme.https.rawValue self.urlComponents.host = "" @@ -85,18 +87,20 @@ public final class URIBuilder { } @discardableResult - public func withPort(_ value: Int16) -> URIBuilder { - self.urlComponents.port = Int(value) + public func withPort(_ value: Int16?) -> URIBuilder { + self.urlComponents.port = value.map { Int($0) } + return self + } + + @discardableResult + public func withPort(_ value: Int?) -> URIBuilder { + self.urlComponents.port = value return self } @discardableResult public func withQueryItems(_ value: [SDKURLQueryItem]) -> URIBuilder { - if value.isEmpty { - self.urlComponents.percentEncodedQueryItems = nil - } else { - self.urlComponents.percentEncodedQueryItems = value.toURLQueryItems - } + self.urlComponents.percentEncodedQueryItems = value.isEmpty ? nil : value.toURLQueryItems return self } @@ -133,9 +137,9 @@ public final class URIBuilder { return URI(scheme: Scheme(rawValue: self.urlComponents.scheme!)!, path: self.urlComponents.percentEncodedPath, host: self.urlComponents.host!, - port: Int16(self.urlComponents.port!), - queryItems: self.urlComponents.percentEncodedQueryItems?.map { item in - return SDKURLQueryItem(name: item.name, value: item.value) + port: self.urlComponents.port.map { Int16($0) }, + queryItems: self.urlComponents.percentEncodedQueryItems?.map { + SDKURLQueryItem(name: $0.name, value: $0.value) } ?? [], username: self.urlComponents.user, password: self.urlComponents.password) @@ -161,14 +165,16 @@ extension Array where Element == SDKURLQueryItem { if self.isEmpty { return nil } - return self.map { - return [$0.name, $0.value].compactMap { $0 }.joined(separator: "=") - }.joined(separator: "&") + return self.map { [$0.name, $0.value].compactMap { $0 }.joined(separator: "=") }.joined(separator: "&") } public var toURLQueryItems: [URLQueryItem] { - return self.map { - URLQueryItem(name: $0.name, value: $0.value) - } + return self.map { URLQueryItem(name: $0.name, value: $0.value) } + } +} + +extension Array where Element == URLQueryItem { + public var toSDKURLQueryItems: [SDKURLQueryItem] { + return self.map { SDKURLQueryItem(name: $0.name, value: $0.value) } } } diff --git a/Sources/ClientRuntime/Networking/Endpoint.swift b/Sources/ClientRuntime/Networking/Endpoint.swift index df81b08e1..8d4d81d13 100644 --- a/Sources/ClientRuntime/Networking/Endpoint.swift +++ b/Sources/ClientRuntime/Networking/Endpoint.swift @@ -12,7 +12,7 @@ public struct Endpoint: Hashable { public var queryItems: [SDKURLQueryItem] { uri.queryItems } public var path: String { uri.path } public var host: String { uri.host } - public var port: Int16 { uri.port } + public var port: Int16? { uri.port } public var url: URL? { uri.url } private let properties: [String: AnyHashable] @@ -40,8 +40,8 @@ public struct Endpoint: Hashable { .withScheme(protocolType) .withPath(url.path) .withHost(host) - .withPort(Int16(url.port ?? protocolType.port)) - .withQueryItems(url.toQueryItems() ?? []) + .withPort(url.port) + .withQueryItems(url.getQueryItems() ?? []) .build() self.init(uri: uri, diff --git a/Sources/ClientRuntime/Networking/Http/CRT/CRTClientEngine.swift b/Sources/ClientRuntime/Networking/Http/CRT/CRTClientEngine.swift index 46ad2cac9..fba6a2b5b 100644 --- a/Sources/ClientRuntime/Networking/Http/CRT/CRTClientEngine.swift +++ b/Sources/ClientRuntime/Networking/Http/CRT/CRTClientEngine.swift @@ -27,7 +27,7 @@ public class CRTClientEngine: HTTPClient { init(endpoint: Endpoint) { self.protocolType = endpoint.uri.scheme self.host = endpoint.uri.host - self.port = endpoint.uri.port + self.port = endpoint.uri.port ?? endpoint.uri.defaultPort } } @@ -95,7 +95,7 @@ public class CRTClientEngine: HTTPClient { clientBootstrap: sharedDefaultIO.clientBootstrap, hostName: endpoint.uri.host, initialWindowSize: windowSize, - port: UInt32(endpoint.uri.port), + port: UInt32(endpoint.uri.port ?? endpoint.uri.defaultPort), proxyOptions: nil, socketOptions: socketOptions, tlsOptions: tlsConnectionOptions, @@ -128,7 +128,7 @@ public class CRTClientEngine: HTTPClient { let options = HTTP2StreamManagerOptions( clientBootstrap: sharedDefaultIO.clientBootstrap, hostName: endpoint.uri.host, - port: UInt32(endpoint.uri.port), + port: UInt32(endpoint.uri.port ?? endpoint.uri.defaultPort), maxConnections: maxConnectionsPerEndpoint, socketOptions: socketOptions, tlsOptions: tlsConnectionOptions, diff --git a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift index 926ee109c..b56461672 100644 --- a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift +++ b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift @@ -207,7 +207,7 @@ public class SdkHttpRequestBuilder: RequestMessageBuilder { var path: String = "/" var body: ByteStream = .noStream var queryItems: [SDKURLQueryItem] = [] - var port: Int16 = 443 + var port: Int16? var protocolType: ProtocolType = .https var trailingHeaders: Headers = Headers() @@ -285,7 +285,7 @@ public class SdkHttpRequestBuilder: RequestMessageBuilder { } @discardableResult - public func withPort(_ value: Int16) -> SdkHttpRequestBuilder { + public func withPort(_ value: Int16?) -> SdkHttpRequestBuilder { self.port = value return self } diff --git a/Sources/ClientRuntime/Networking/Http/URLSession/URLSessionHTTPClient.swift b/Sources/ClientRuntime/Networking/Http/URLSession/URLSessionHTTPClient.swift index b3762a3fc..ef5817aca 100644 --- a/Sources/ClientRuntime/Networking/Http/URLSession/URLSessionHTTPClient.swift +++ b/Sources/ClientRuntime/Networking/Http/URLSession/URLSessionHTTPClient.swift @@ -462,7 +462,7 @@ public final class URLSessionHTTPClient: HTTPClient { /// - Returns: A `URLRequest` ready to be transmitted by `URLSession` for this operation. private func makeURLRequest(from request: SdkHttpRequest, httpBodyStream: InputStream?) throws -> URLRequest { var components = URLComponents() - components.scheme = config.protocolType?.rawValue ?? request.destination.scheme.rawValue ?? "https" + components.scheme = config.protocolType?.rawValue ?? request.destination.scheme.rawValue components.host = request.endpoint.uri.host components.port = port(for: request) components.percentEncodedPath = request.destination.path @@ -489,7 +489,7 @@ public final class URLSessionHTTPClient: HTTPClient { // Don't set port explicitly if it's the default port for the scheme return nil default: - return Int(request.destination.port) + return request.destination.port.map { Int($0) } } } } diff --git a/Sources/ClientRuntime/PrimitiveTypeExtensions/URL+Extension.swift b/Sources/ClientRuntime/PrimitiveTypeExtensions/URL+Extension.swift index 791ff1fe1..095435642 100644 --- a/Sources/ClientRuntime/PrimitiveTypeExtensions/URL+Extension.swift +++ b/Sources/ClientRuntime/PrimitiveTypeExtensions/URL+Extension.swift @@ -9,7 +9,7 @@ public typealias URL = Foundation.URL extension URL { - func toQueryItems() -> [SDKURLQueryItem]? { + func getQueryItems() -> [SDKURLQueryItem]? { URLComponents(url: self, resolvingAgainstBaseURL: false)? .queryItems? .map { SDKURLQueryItem(name: $0.name, value: $0.value) } diff --git a/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift index 5228177e2..822515872 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift @@ -9,7 +9,7 @@ import AwsCommonRuntimeKit class SdkRequestBuilderTests: XCTestCase { func testSdkRequestBuilderUpdate() throws { - let url = "https://stehlibstoragebucket144955-dev.s3.us-east-1.amazonaws.com:443/public/README.md?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAVIBY7SZR4C4MBGAW%2F20220225%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220225T034419Z&X-Amz-Expires=17999&X-Amz-SignedHeaders=host&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEEQaCXVzLWVhc3QtMSJHMEUCIEKUDdnCf1h3cZNdv10q6G24zLgn0r6th4m%2FXSS9TuR4AiEAwOwf2cG%2F1W%2FkAz1UMqFW9sZp7SY6j%2BmiicLy1dB8OXUqmgYInf%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FARABGgwzNjA4OTY3NjM0OTEiDH6CiNUJiwL4sNy9KCruBbdJnGSx88A%2Be0SBpKEpSurxunasaDsJb2ZJPqVhC%2FcKPr87PYcp5BrnkGumcYhUAEknVbHACLLUx2%2Fnzfacp13PHmclSsLe52qGwjFFVMz0m41PV3HiCoHgIc7vVIHnwNySaX9ZJbVowhNvI4V9eixVKhjjx7Tqku1bsOzlq9dJP15qz8FVNjlKGjZFqrMGTzpTgmS9dNPqphwCcxx306RLUd35SEvXjxnWcidOdddYQs9j4wu47DOM8ftveag6cDptJQN71dDIRHgFTisVshD78Rm9pycKf3g0QvBAGtrzhcxUcJtNgIWv%2B10hsEBURsEYommcjI8vT1yX2K8pLVOxgL%2FRWXndbAeIzAu5vmLm6RqkfGwkHQPQl7uII6YzL2Gku%2FMDilVFw9TBKIg6KDP9l2GzzVRQsvLMpFIp%2Bx3a1s4OVduJRFpDYCwEsfKhIoVkb610gBbFayPKjQVcZfULdq1r5DOZzpHVDoijnKHAxHtFgaFPP6KtG%2BmdKeix8gccdbsdgMokWKtJhisFo%2BzLn02oSSX4ITkZKzZcriGxQO4E1YUlYyBhjlCg1b74faQfWstk24PrkCfNXYcQ5oxgglIA0tBOdOfwGn2Je3MBEj2T8Yz0GS%2BZib3DKVWRzU0Xk9pwDXH3iaBn9Uld%2BNyw9gxdOCBVKtTILtdfsjw9lJSVOJomlJn1h8gH96PToBg9lOc4ms6aA2Z%2FoN%2F3UV%2Beo5%2FpB9xfMkBIeOg6vAI0VtjkUhH052UouEKU%2BVGSGuCSuVzZmIBJLWHZaQUJZ3hJCdGqM%2FoM4Ud51Cidcnr%2Bni%2Bgp4RAfg0gvX6Eb2e%2FNMqNd0Eg7ftmnPXzQR3ZVC1yyNFu2kBt8icKdMnkLZT8YO1Racd1QgrZ5DZJlU%2FS2eisLGSTMb9Dmaq8AGbxgGK55acIvsQLzvJhavFWbh%2FyNudQBSLC0c5BZGxQk2J%2BJx%2FlR5wR%2B0gXOfTg5EImMrLzh7gOo56i2Rhj2xRFSDDDnuGQBjqHArU56DsGZNeKwkVK87lVJAfiAsWmKaDlBNXcuhKl084syaZMiVUQa7dPupgeg%2BvVPv4dAwNULjuE3B7V5lSea4RIDOfGwTtlj4Ekn3t4PrlxHpEyMeuRgekNhpdcfL5pgRcKH4AKgqI7fCrghhH1qDMTpUkORRv6EwPGqM0LPm93XPVOjrG3EJ5ZOYLJM05bbrcalzN0mbSbsRrcphme7Z8zpD5jNeSWdBhWA04DQ7RZuJpqFnn40yKq7G8kgtLmOJGRs7CGTWT9on7EOpH3RdKGPXL06%2BgLo7r9%2Fdkb%2BHGJF6spjqbH0SN%2FySjnvgNL1NrGAy%2FQgPwfDw6oJ6TPNdH8dfM8tmdd&X-Amz-Signature=90b9f3353ad4f2596fdd166c00da8fb86d5d255bf3c8b296c5e5a7db25fd41aa" + let url = "https://stehlibstoragebucket144955-dev.s3.us-east-1.amazonaws.com/public/README.md?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAVIBY7SZR4C4MBGAW%2F20220225%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220225T034419Z&X-Amz-Expires=17999&X-Amz-SignedHeaders=host&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEEQaCXVzLWVhc3QtMSJHMEUCIEKUDdnCf1h3cZNdv10q6G24zLgn0r6th4m%2FXSS9TuR4AiEAwOwf2cG%2F1W%2FkAz1UMqFW9sZp7SY6j%2BmiicLy1dB8OXUqmgYInf%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FARABGgwzNjA4OTY3NjM0OTEiDH6CiNUJiwL4sNy9KCruBbdJnGSx88A%2Be0SBpKEpSurxunasaDsJb2ZJPqVhC%2FcKPr87PYcp5BrnkGumcYhUAEknVbHACLLUx2%2Fnzfacp13PHmclSsLe52qGwjFFVMz0m41PV3HiCoHgIc7vVIHnwNySaX9ZJbVowhNvI4V9eixVKhjjx7Tqku1bsOzlq9dJP15qz8FVNjlKGjZFqrMGTzpTgmS9dNPqphwCcxx306RLUd35SEvXjxnWcidOdddYQs9j4wu47DOM8ftveag6cDptJQN71dDIRHgFTisVshD78Rm9pycKf3g0QvBAGtrzhcxUcJtNgIWv%2B10hsEBURsEYommcjI8vT1yX2K8pLVOxgL%2FRWXndbAeIzAu5vmLm6RqkfGwkHQPQl7uII6YzL2Gku%2FMDilVFw9TBKIg6KDP9l2GzzVRQsvLMpFIp%2Bx3a1s4OVduJRFpDYCwEsfKhIoVkb610gBbFayPKjQVcZfULdq1r5DOZzpHVDoijnKHAxHtFgaFPP6KtG%2BmdKeix8gccdbsdgMokWKtJhisFo%2BzLn02oSSX4ITkZKzZcriGxQO4E1YUlYyBhjlCg1b74faQfWstk24PrkCfNXYcQ5oxgglIA0tBOdOfwGn2Je3MBEj2T8Yz0GS%2BZib3DKVWRzU0Xk9pwDXH3iaBn9Uld%2BNyw9gxdOCBVKtTILtdfsjw9lJSVOJomlJn1h8gH96PToBg9lOc4ms6aA2Z%2FoN%2F3UV%2Beo5%2FpB9xfMkBIeOg6vAI0VtjkUhH052UouEKU%2BVGSGuCSuVzZmIBJLWHZaQUJZ3hJCdGqM%2FoM4Ud51Cidcnr%2Bni%2Bgp4RAfg0gvX6Eb2e%2FNMqNd0Eg7ftmnPXzQR3ZVC1yyNFu2kBt8icKdMnkLZT8YO1Racd1QgrZ5DZJlU%2FS2eisLGSTMb9Dmaq8AGbxgGK55acIvsQLzvJhavFWbh%2FyNudQBSLC0c5BZGxQk2J%2BJx%2FlR5wR%2B0gXOfTg5EImMrLzh7gOo56i2Rhj2xRFSDDDnuGQBjqHArU56DsGZNeKwkVK87lVJAfiAsWmKaDlBNXcuhKl084syaZMiVUQa7dPupgeg%2BvVPv4dAwNULjuE3B7V5lSea4RIDOfGwTtlj4Ekn3t4PrlxHpEyMeuRgekNhpdcfL5pgRcKH4AKgqI7fCrghhH1qDMTpUkORRv6EwPGqM0LPm93XPVOjrG3EJ5ZOYLJM05bbrcalzN0mbSbsRrcphme7Z8zpD5jNeSWdBhWA04DQ7RZuJpqFnn40yKq7G8kgtLmOJGRs7CGTWT9on7EOpH3RdKGPXL06%2BgLo7r9%2Fdkb%2BHGJF6spjqbH0SN%2FySjnvgNL1NrGAy%2FQgPwfDw6oJ6TPNdH8dfM8tmdd&X-Amz-Signature=90b9f3353ad4f2596fdd166c00da8fb86d5d255bf3c8b296c5e5a7db25fd41aa" let pathToMatch = "/public/README.md?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAVIBY7SZR4C4MBGAW%2F20220225%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220225T034419Z&X-Amz-Expires=17999&X-Amz-SignedHeaders=host&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEEQaCXVzLWVhc3QtMSJHMEUCIEKUDdnCf1h3cZNdv10q6G24zLgn0r6th4m%2FXSS9TuR4AiEAwOwf2cG%2F1W%2FkAz1UMqFW9sZp7SY6j%2BmiicLy1dB8OXUqmgYInf%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FARABGgwzNjA4OTY3NjM0OTEiDH6CiNUJiwL4sNy9KCruBbdJnGSx88A%2Be0SBpKEpSurxunasaDsJb2ZJPqVhC%2FcKPr87PYcp5BrnkGumcYhUAEknVbHACLLUx2%2Fnzfacp13PHmclSsLe52qGwjFFVMz0m41PV3HiCoHgIc7vVIHnwNySaX9ZJbVowhNvI4V9eixVKhjjx7Tqku1bsOzlq9dJP15qz8FVNjlKGjZFqrMGTzpTgmS9dNPqphwCcxx306RLUd35SEvXjxnWcidOdddYQs9j4wu47DOM8ftveag6cDptJQN71dDIRHgFTisVshD78Rm9pycKf3g0QvBAGtrzhcxUcJtNgIWv%2B10hsEBURsEYommcjI8vT1yX2K8pLVOxgL%2FRWXndbAeIzAu5vmLm6RqkfGwkHQPQl7uII6YzL2Gku%2FMDilVFw9TBKIg6KDP9l2GzzVRQsvLMpFIp%2Bx3a1s4OVduJRFpDYCwEsfKhIoVkb610gBbFayPKjQVcZfULdq1r5DOZzpHVDoijnKHAxHtFgaFPP6KtG%2BmdKeix8gccdbsdgMokWKtJhisFo%2BzLn02oSSX4ITkZKzZcriGxQO4E1YUlYyBhjlCg1b74faQfWstk24PrkCfNXYcQ5oxgglIA0tBOdOfwGn2Je3MBEj2T8Yz0GS%2BZib3DKVWRzU0Xk9pwDXH3iaBn9Uld%2BNyw9gxdOCBVKtTILtdfsjw9lJSVOJomlJn1h8gH96PToBg9lOc4ms6aA2Z%2FoN%2F3UV%2Beo5%2FpB9xfMkBIeOg6vAI0VtjkUhH052UouEKU%2BVGSGuCSuVzZmIBJLWHZaQUJZ3hJCdGqM%2FoM4Ud51Cidcnr%2Bni%2Bgp4RAfg0gvX6Eb2e%2FNMqNd0Eg7ftmnPXzQR3ZVC1yyNFu2kBt8icKdMnkLZT8YO1Racd1QgrZ5DZJlU%2FS2eisLGSTMb9Dmaq8AGbxgGK55acIvsQLzvJhavFWbh%2FyNudQBSLC0c5BZGxQk2J%2BJx%2FlR5wR%2B0gXOfTg5EImMrLzh7gOo56i2Rhj2xRFSDDDnuGQBjqHArU56DsGZNeKwkVK87lVJAfiAsWmKaDlBNXcuhKl084syaZMiVUQa7dPupgeg%2BvVPv4dAwNULjuE3B7V5lSea4RIDOfGwTtlj4Ekn3t4PrlxHpEyMeuRgekNhpdcfL5pgRcKH4AKgqI7fCrghhH1qDMTpUkORRv6EwPGqM0LPm93XPVOjrG3EJ5ZOYLJM05bbrcalzN0mbSbsRrcphme7Z8zpD5jNeSWdBhWA04DQ7RZuJpqFnn40yKq7G8kgtLmOJGRs7CGTWT9on7EOpH3RdKGPXL06%2BgLo7r9%2Fdkb%2BHGJF6spjqbH0SN%2FySjnvgNL1NrGAy%2FQgPwfDw6oJ6TPNdH8dfM8tmdd&X-Amz-Signature=90b9f3353ad4f2596fdd166c00da8fb86d5d255bf3c8b296c5e5a7db25fd41aa" let queryItems = [SDKURLQueryItem(name: "Bucket", value: "stehlibstoragebucket144955-dev"), SDKURLQueryItem(name: "Key", value: "public%2FREADME.md")] let originalRequest = SdkHttpRequest(method: .get, endpoint: try Endpoint(host: "stehlibstoragebucket144955-dev.s3.us-east-1.amazonaws.com", path: "/", port: 80, queryItems: queryItems, protocolType: .https)) From 6c59920925886fd3e6cbd32fee6127883886e1df Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Fri, 17 May 2024 08:45:04 -0700 Subject: [PATCH 10/19] Add URITests --- .../MessageTests/URITests.swift | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 Tests/ClientRuntimeTests/MessageTests/URITests.swift diff --git a/Tests/ClientRuntimeTests/MessageTests/URITests.swift b/Tests/ClientRuntimeTests/MessageTests/URITests.swift new file mode 100644 index 000000000..db4a6d430 --- /dev/null +++ b/Tests/ClientRuntimeTests/MessageTests/URITests.swift @@ -0,0 +1,72 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +import Foundation +import XCTest +@testable import ClientRuntime + +class URITests: XCTestCase { + let url = URL(string: "https://xctest.amazonaws.com?abc=def&ghi=jkl&mno=pqr")! + + func test_queryItems_setsQueryItemsFromURLInOrder() throws { + let uri = URIBuilder() + .withScheme(Scheme(rawValue: url.scheme!)!) + .withPath(url.path) + .withHost(url.host!) + .withPort(url.port) + .withQueryItems(url.getQueryItems()!) + .build() + + let expectedQueryItems = [ + SDKURLQueryItem(name: "abc", value: "def"), + SDKURLQueryItem(name: "ghi", value: "jkl"), + SDKURLQueryItem(name: "mno", value: "pqr") + ] + XCTAssertEqual(uri.queryItems, expectedQueryItems) + XCTAssertEqual(uri.queryString, "abc=def&ghi=jkl&mno=pqr") + } + + func test_hashableAndEquatable_hashesMatch() throws { + let uri1 = URIBuilder() + .withScheme(Scheme(rawValue: url.scheme!)!) + .withPath(url.path) + .withHost(url.host!) + .withPort(url.port) + .withQueryItems(url.getQueryItems()!) + .build() + let uri2 = URIBuilder() + .withScheme(Scheme(rawValue: url.scheme!)!) + .withPath(url.path) + .withHost(url.host!) + .withPort(url.port) + .withQueryItems(url.getQueryItems()!) + .build() + XCTAssertEqual(uri1, uri2) + XCTAssertEqual(uri1.hashValue, uri2.hashValue) + } + + func test_path_percentEncodedInput() throws { + let uri = URIBuilder() + .withScheme(Scheme(rawValue: url.scheme!)!) + .withPath(url.path) + .withHost(url.host!) + .withPort(443) + .withQueryItems(url.getQueryItems()!) + .build() + XCTAssertEqual(uri.url?.absoluteString, "https://xctest.amazonaws.com:443?abc=def&ghi=jkl&mno=pqr") + } + + func test_path_unencodedInput() throws { + let uri = URIBuilder() + .withScheme(.https) + .withPath("/abc+def") + .withHost("xctest.amazonaws.com") + .withPort(443) + .build() + XCTAssertEqual(uri.url?.absoluteString, "https://xctest.amazonaws.com:443/abc+def") + } +} From 9f3fc17bc96109cfec19a17249bfdea45ecfa771 Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Fri, 17 May 2024 09:32:21 -0700 Subject: [PATCH 11/19] Remove `try` --- .../InterceptorTests/InterceptorTests.swift | 6 +++--- .../NetworkingTests/EndpointTests.swift | 4 ++-- .../Http/CRTClientEngineIntegrationTests.swift | 16 ++++++++-------- .../NetworkingTests/Http/HttpRequestTests.swift | 6 +++--- .../Http/SdkRequestBuilderTests.swift | 2 +- .../OrchestratorTests/OrchestratorTests.swift | 2 +- .../HttpRequestTestBaseTests.swift | 2 +- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Tests/ClientRuntimeTests/InterceptorTests/InterceptorTests.swift b/Tests/ClientRuntimeTests/InterceptorTests/InterceptorTests.swift index b435a6fd3..e0d211b19 100644 --- a/Tests/ClientRuntimeTests/InterceptorTests/InterceptorTests.swift +++ b/Tests/ClientRuntimeTests/InterceptorTests/InterceptorTests.swift @@ -61,7 +61,7 @@ class InterceptorTests: XCTestCase { public func modifyBeforeTransmit(context: some MutableRequest) async throws { let builder = context.getRequest().toBuilder() builder.withHeader(name: headerName, value: headerValue) - context.updateRequest(updated: try builder.build()) + context.updateRequest(updated: builder.build()) } } @@ -84,7 +84,7 @@ class InterceptorTests: XCTestCase { let input: TestInput = try XCTUnwrap(context.getInput()) let builder = context.getRequest().toBuilder() builder.withHeader(name: "otherProperty", value: "\(input.otherProperty)") - context.updateRequest(updated: try builder.build()) + context.updateRequest(updated: builder.build()) } } @@ -106,7 +106,7 @@ class InterceptorTests: XCTestCase { for i in interceptors { try await i.modifyBeforeSerialization(context: interceptorContext) } - interceptorContext.updateRequest(updated: try SdkHttpRequestBuilder().build()) + interceptorContext.updateRequest(updated: SdkHttpRequestBuilder().build()) for i in interceptors { try await i.modifyBeforeTransmit(context: interceptorContext) } diff --git a/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift index 688220b55..e30ac80f4 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/EndpointTests.swift @@ -30,7 +30,7 @@ class EndpointTests: XCTestCase { } func test_path_percentEncodedInput() throws { - let endpoint = try Endpoint( + let endpoint = Endpoint( host: "xctest.amazonaws.com", path: "/abc%2Bdef", protocolType: .https @@ -41,7 +41,7 @@ class EndpointTests: XCTestCase { } func test_path_unencodedInput() throws { - let endpoint = try Endpoint( + let endpoint = Endpoint( host: "xctest.amazonaws.com", path: "/abc+def", protocolType: .https diff --git a/Tests/ClientRuntimeTests/NetworkingTests/Http/CRTClientEngineIntegrationTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/Http/CRTClientEngineIntegrationTests.swift index 4a5cc1543..7b379568a 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/Http/CRTClientEngineIntegrationTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/Http/CRTClientEngineIntegrationTests.swift @@ -32,7 +32,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { var headers = Headers() headers.add(name: "Content-type", value: "application/json") headers.add(name: "Host", value: "httpbin.org") - let request = SdkHttpRequest(method: .get, endpoint: try Endpoint(host: "httpbin.org", path: "/get", headers: headers)) + let request = SdkHttpRequest(method: .get, endpoint: Endpoint(host: "httpbin.org", path: "/get", headers: headers)) let response = try await httpClient.send(request: request) XCTAssertNotNil(response) @@ -48,7 +48,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { let encoder = JSONEncoder() let encodedData = try encoder.encode(body) let request = SdkHttpRequest(method: .post, - endpoint: try Endpoint(host: "httpbin.org", path: "/post", headers: headers), + endpoint: Endpoint(host: "httpbin.org", path: "/post", headers: headers), body: ByteStream.data(encodedData)) let response = try await httpClient.send(request: request) XCTAssertNotNil(response) @@ -61,7 +61,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { headers.add(name: "Content-type", value: "application/json") headers.add(name: "Host", value: "httpbin.org") let request = SdkHttpRequest(method: .get, - endpoint: try Endpoint(host: "httpbin.org", path: "/stream-bytes/1024", headers: headers), + endpoint: Endpoint(host: "httpbin.org", path: "/stream-bytes/1024", headers: headers), body: ByteStream.stream(BufferedStream())) let response = try await httpClient.send(request: request) XCTAssertNotNil(response) @@ -75,7 +75,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { headers.add(name: "Host", value: "httpbin.org") let request = SdkHttpRequest(method: .get, - endpoint: try Endpoint(host: "httpbin.org", path: "/stream-bytes/1024", headers: headers), + endpoint: Endpoint(host: "httpbin.org", path: "/stream-bytes/1024", headers: headers), body: ByteStream.stream(BufferedStream())) let response = try await httpClient.send(request: request) XCTAssertNotNil(response) @@ -95,7 +95,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { headers.add(name: "Host", value: "httpbin.org") let request = SdkHttpRequest(method: .get, - endpoint: try Endpoint(host: "httpbin.org", path: "/stream-bytes/1", headers: headers), + endpoint: Endpoint(host: "httpbin.org", path: "/stream-bytes/1", headers: headers), body: ByteStream.stream(BufferedStream())) let response = try await httpClient.send(request: request) XCTAssertNotNil(response) @@ -115,7 +115,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { headers.add(name: "Host", value: "httpbin.org") let request = SdkHttpRequest(method: .get, - endpoint: try Endpoint(host: "httpbin.org", path: "/stream-bytes/3000", headers: headers), + endpoint: Endpoint(host: "httpbin.org", path: "/stream-bytes/3000", headers: headers), body: ByteStream.stream(BufferedStream())) let response = try await httpClient.send(request: request) XCTAssertNotNil(response) @@ -138,7 +138,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { let encodedData = try encoder.encode(body) let request = SdkHttpRequest(method: .post, - endpoint: try Endpoint(host: "httpbin.org", path: "/post", headers: headers), + endpoint: Endpoint(host: "httpbin.org", path: "/post", headers: headers), body: ByteStream.stream(BufferedStream(data: encodedData))) let response = try await httpClient.send(request: request) XCTAssertNotNil(response) @@ -156,7 +156,7 @@ class CRTClientEngineIntegrationTests: NetworkingTestUtils { let request = SdkHttpRequest( method: .put, - endpoint: try Endpoint(host: "nghttp2.org", path: "/httpbin/put", headers: headers), + endpoint: Endpoint(host: "nghttp2.org", path: "/httpbin/put", headers: headers), body: .data(encodedData) ) diff --git a/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift index 5a41dfd13..2a6ad166a 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift @@ -22,7 +22,7 @@ class HttpRequestTests: NetworkingTestUtils { func testSdkHttpRequestToHttpRequest() throws { let headers = Headers(["header-item-name": "header-item-value"]) - let endpoint = try Endpoint(host: "host.com", path: "/", headers: headers) + let endpoint = Endpoint(host: "host.com", path: "/", headers: headers) let httpBody = ByteStream.data(expectedMockRequestData) let mockHttpRequest = SdkHttpRequest(method: .get, endpoint: endpoint, body: httpBody) @@ -59,7 +59,7 @@ class HttpRequestTests: NetworkingTestUtils { func testSdkHttpRequestToURLRequest() async throws { let headers = Headers(["Testname-1": "testvalue-1", "Testname-2": "testvalue-2"]) - let endpoint = try Endpoint(host: "host.com", path: "/", headers: headers) + let endpoint = Endpoint(host: "host.com", path: "/", headers: headers) let httpBody = ByteStream.data(expectedMockRequestData) let mockHttpRequest = SdkHttpRequest(method: .get, endpoint: endpoint, body: httpBody) @@ -122,7 +122,7 @@ class HttpRequestTests: NetworkingTestUtils { let httpRequest = try builder.build().toHttpRequest() httpRequest.path = "/hello?foo=bar&quz=bar&signedthing=signed" - let updatedRequest = builder.update(from: httpRequest, originalRequest: try builder.build()) + let updatedRequest = builder.update(from: httpRequest, originalRequest: builder.build()) XCTAssert(updatedRequest.path == "/hello") XCTAssert(updatedRequest.queryItems.count == 3) diff --git a/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift index 822515872..da0ac7b88 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift @@ -16,7 +16,7 @@ class SdkRequestBuilderTests: XCTestCase { let crtRequest = try HTTPRequest() crtRequest.path = pathToMatch - let updatedRequest = try SdkHttpRequestBuilder().update(from: crtRequest, originalRequest: originalRequest).build() + let updatedRequest = SdkHttpRequestBuilder().update(from: crtRequest, originalRequest: originalRequest).build() let updatedPath = [updatedRequest.destination.path, updatedRequest.destination.queryString].compactMap { $0 }.joined(separator: "?") XCTAssertEqual(pathToMatch, updatedPath) XCTAssertEqual(url, updatedRequest.destination.url?.absoluteString) diff --git a/Tests/ClientRuntimeTests/OrchestratorTests/OrchestratorTests.swift b/Tests/ClientRuntimeTests/OrchestratorTests/OrchestratorTests.swift index 7ec20c236..904791b27 100644 --- a/Tests/ClientRuntimeTests/OrchestratorTests/OrchestratorTests.swift +++ b/Tests/ClientRuntimeTests/OrchestratorTests/OrchestratorTests.swift @@ -495,7 +495,7 @@ class OrchestratorTests: XCTestCase { let b = self.traceOrchestrator(trace: partitionIdTrace) // We can force a getPartitionId to fail by explicitly setting the host to '' b.interceptors.addModifyBeforeRetryLoop({ context in - context.updateRequest(updated: try context.getRequest().toBuilder().withHost("").build()) + context.updateRequest(updated: context.getRequest().toBuilder().withHost("").build()) }) return try await b.build().execute(input: TestInput(foo: "")) } diff --git a/Tests/SmithyTestUtilTests/RequestTestUtilTests/HttpRequestTestBaseTests.swift b/Tests/SmithyTestUtilTests/RequestTestUtilTests/HttpRequestTestBaseTests.swift index 3497edc42..b5971b20f 100644 --- a/Tests/SmithyTestUtilTests/RequestTestUtilTests/HttpRequestTestBaseTests.swift +++ b/Tests/SmithyTestUtilTests/RequestTestUtilTests/HttpRequestTestBaseTests.swift @@ -164,7 +164,7 @@ class HttpRequestTestBaseTests: HttpRequestTestBase { // Mocks the code-generated unit test which includes testing for forbidden/required headers/queries func testSayHello() async throws { - let expected = try buildExpectedHttpRequest(method: .post, + let expected = buildExpectedHttpRequest(method: .post, path: "/", headers: ["Content-Type": "application/json", "RequiredHeader": "required header"], From 33f0b9aaa9875cc18dccaf30ff26b38fbffc51da Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Fri, 17 May 2024 09:37:32 -0700 Subject: [PATCH 12/19] Update query item related vars to func --- Sources/ClientRuntime/Message/URI.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/ClientRuntime/Message/URI.swift b/Sources/ClientRuntime/Message/URI.swift index eb3a43f6d..9aa01c760 100644 --- a/Sources/ClientRuntime/Message/URI.swift +++ b/Sources/ClientRuntime/Message/URI.swift @@ -100,7 +100,7 @@ public final class URIBuilder { @discardableResult public func withQueryItems(_ value: [SDKURLQueryItem]) -> URIBuilder { - self.urlComponents.percentEncodedQueryItems = value.isEmpty ? nil : value.toURLQueryItems + self.urlComponents.percentEncodedQueryItems = value.isEmpty ? nil : value.toURLQueryItems() return self } @@ -110,7 +110,7 @@ public final class URIBuilder { return self } var queryItems = self.urlComponents.percentEncodedQueryItems ?? [] - queryItems += items.toURLQueryItems + queryItems += items.toURLQueryItems() self.urlComponents.percentEncodedQueryItems = queryItems return self } @@ -168,13 +168,13 @@ extension Array where Element == SDKURLQueryItem { return self.map { [$0.name, $0.value].compactMap { $0 }.joined(separator: "=") }.joined(separator: "&") } - public var toURLQueryItems: [URLQueryItem] { + public func toURLQueryItems() -> [URLQueryItem] { return self.map { URLQueryItem(name: $0.name, value: $0.value) } } } extension Array where Element == URLQueryItem { - public var toSDKURLQueryItems: [SDKURLQueryItem] { + public func toSDKURLQueryItems() -> [SDKURLQueryItem] { return self.map { SDKURLQueryItem(name: $0.name, value: $0.value) } } } From e86d31b2badfaba172afa1626eb3ac98209d5501 Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Fri, 17 May 2024 09:42:31 -0700 Subject: [PATCH 13/19] Add fragment --- Sources/ClientRuntime/Message/URI.swift | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/Sources/ClientRuntime/Message/URI.swift b/Sources/ClientRuntime/Message/URI.swift index 9aa01c760..f58e174cd 100644 --- a/Sources/ClientRuntime/Message/URI.swift +++ b/Sources/ClientRuntime/Message/URI.swift @@ -17,6 +17,7 @@ public struct URI: Hashable { public let queryItems: [SDKURLQueryItem] public let username: String? public let password: String? + public let fragment: String? public var url: URL? { self.toBuilder().getUrl() } @@ -30,7 +31,8 @@ public struct URI: Hashable { port: Int16?, queryItems: [SDKURLQueryItem], username: String? = nil, - password: String? = nil) { + password: String? = nil, + fragment: String? = nil) { self.scheme = scheme self.path = path self.host = host @@ -38,6 +40,7 @@ public struct URI: Hashable { self.queryItems = queryItems self.username = username self.password = password + self.fragment = fragment } public func toBuilder() -> URIBuilder { @@ -49,6 +52,7 @@ public struct URI: Hashable { .withQueryItems(self.queryItems) .withUsername(self.username) .withPassword(self.password) + .withFragment(self.fragment) } } @@ -133,6 +137,18 @@ public final class URIBuilder { return self } + @discardableResult + public func withFragment(_ value: String?) -> URIBuilder { + if let fragment = value { + if fragment.isPercentEncoded { + self.urlComponents.percentEncodedFragment = fragment + } else { + self.urlComponents.fragment = fragment + } + } + return self + } + public func build() -> URI { return URI(scheme: Scheme(rawValue: self.urlComponents.scheme!)!, path: self.urlComponents.percentEncodedPath, @@ -142,7 +158,8 @@ public final class URIBuilder { SDKURLQueryItem(name: $0.name, value: $0.value) } ?? [], username: self.urlComponents.user, - password: self.urlComponents.password) + password: self.urlComponents.password, + fragment: self.urlComponents.fragment) } // We still have to keep 'url' as an optional, since we're From ff32e8d2c09deccfca717b6101e4c7ed14d25e6c Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Fri, 17 May 2024 14:27:53 -0700 Subject: [PATCH 14/19] Address swiftlint warnings in PR --- Sources/ClientRuntime/Message/URI.swift | 2 +- .../NetworkingTests/Http/HttpRequestTests.swift | 6 +++--- .../NetworkingTests/Http/SdkRequestBuilderTests.swift | 2 +- .../NetworkingTests/NetworkingTestUtils.swift | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/ClientRuntime/Message/URI.swift b/Sources/ClientRuntime/Message/URI.swift index f58e174cd..84e94beb4 100644 --- a/Sources/ClientRuntime/Message/URI.swift +++ b/Sources/ClientRuntime/Message/URI.swift @@ -5,7 +5,7 @@ import Foundation -/// A representation of the RFC 3986 Universal Resource Identifier +/// A representation of the RFC 3986 Uniform Resource Identifier public struct URI: Hashable { public let scheme: Scheme public let path: String diff --git a/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift index 2a6ad166a..1f3f5ee20 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/Http/HttpRequestTests.swift @@ -126,9 +126,9 @@ class HttpRequestTests: NetworkingTestUtils { XCTAssert(updatedRequest.path == "/hello") XCTAssert(updatedRequest.queryItems.count == 3) - XCTAssert(updatedRequest.queryItems.contains(queryItem1) ?? false) - XCTAssert(updatedRequest.queryItems.contains(queryItem2) ?? false) - XCTAssert(updatedRequest.queryItems.contains(SDKURLQueryItem(name: "signedthing", value: "signed")) ?? false) + XCTAssert(updatedRequest.queryItems.contains(queryItem1)) + XCTAssert(updatedRequest.queryItems.contains(queryItem2)) + XCTAssert(updatedRequest.queryItems.contains(SDKURLQueryItem(name: "signedthing", value: "signed"))) } func testPathInInHttpRequestIsNotAltered() throws { diff --git a/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift index da0ac7b88..8c6c96b17 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/Http/SdkRequestBuilderTests.swift @@ -12,7 +12,7 @@ class SdkRequestBuilderTests: XCTestCase { let url = "https://stehlibstoragebucket144955-dev.s3.us-east-1.amazonaws.com/public/README.md?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAVIBY7SZR4C4MBGAW%2F20220225%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220225T034419Z&X-Amz-Expires=17999&X-Amz-SignedHeaders=host&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEEQaCXVzLWVhc3QtMSJHMEUCIEKUDdnCf1h3cZNdv10q6G24zLgn0r6th4m%2FXSS9TuR4AiEAwOwf2cG%2F1W%2FkAz1UMqFW9sZp7SY6j%2BmiicLy1dB8OXUqmgYInf%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FARABGgwzNjA4OTY3NjM0OTEiDH6CiNUJiwL4sNy9KCruBbdJnGSx88A%2Be0SBpKEpSurxunasaDsJb2ZJPqVhC%2FcKPr87PYcp5BrnkGumcYhUAEknVbHACLLUx2%2Fnzfacp13PHmclSsLe52qGwjFFVMz0m41PV3HiCoHgIc7vVIHnwNySaX9ZJbVowhNvI4V9eixVKhjjx7Tqku1bsOzlq9dJP15qz8FVNjlKGjZFqrMGTzpTgmS9dNPqphwCcxx306RLUd35SEvXjxnWcidOdddYQs9j4wu47DOM8ftveag6cDptJQN71dDIRHgFTisVshD78Rm9pycKf3g0QvBAGtrzhcxUcJtNgIWv%2B10hsEBURsEYommcjI8vT1yX2K8pLVOxgL%2FRWXndbAeIzAu5vmLm6RqkfGwkHQPQl7uII6YzL2Gku%2FMDilVFw9TBKIg6KDP9l2GzzVRQsvLMpFIp%2Bx3a1s4OVduJRFpDYCwEsfKhIoVkb610gBbFayPKjQVcZfULdq1r5DOZzpHVDoijnKHAxHtFgaFPP6KtG%2BmdKeix8gccdbsdgMokWKtJhisFo%2BzLn02oSSX4ITkZKzZcriGxQO4E1YUlYyBhjlCg1b74faQfWstk24PrkCfNXYcQ5oxgglIA0tBOdOfwGn2Je3MBEj2T8Yz0GS%2BZib3DKVWRzU0Xk9pwDXH3iaBn9Uld%2BNyw9gxdOCBVKtTILtdfsjw9lJSVOJomlJn1h8gH96PToBg9lOc4ms6aA2Z%2FoN%2F3UV%2Beo5%2FpB9xfMkBIeOg6vAI0VtjkUhH052UouEKU%2BVGSGuCSuVzZmIBJLWHZaQUJZ3hJCdGqM%2FoM4Ud51Cidcnr%2Bni%2Bgp4RAfg0gvX6Eb2e%2FNMqNd0Eg7ftmnPXzQR3ZVC1yyNFu2kBt8icKdMnkLZT8YO1Racd1QgrZ5DZJlU%2FS2eisLGSTMb9Dmaq8AGbxgGK55acIvsQLzvJhavFWbh%2FyNudQBSLC0c5BZGxQk2J%2BJx%2FlR5wR%2B0gXOfTg5EImMrLzh7gOo56i2Rhj2xRFSDDDnuGQBjqHArU56DsGZNeKwkVK87lVJAfiAsWmKaDlBNXcuhKl084syaZMiVUQa7dPupgeg%2BvVPv4dAwNULjuE3B7V5lSea4RIDOfGwTtlj4Ekn3t4PrlxHpEyMeuRgekNhpdcfL5pgRcKH4AKgqI7fCrghhH1qDMTpUkORRv6EwPGqM0LPm93XPVOjrG3EJ5ZOYLJM05bbrcalzN0mbSbsRrcphme7Z8zpD5jNeSWdBhWA04DQ7RZuJpqFnn40yKq7G8kgtLmOJGRs7CGTWT9on7EOpH3RdKGPXL06%2BgLo7r9%2Fdkb%2BHGJF6spjqbH0SN%2FySjnvgNL1NrGAy%2FQgPwfDw6oJ6TPNdH8dfM8tmdd&X-Amz-Signature=90b9f3353ad4f2596fdd166c00da8fb86d5d255bf3c8b296c5e5a7db25fd41aa" let pathToMatch = "/public/README.md?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAVIBY7SZR4C4MBGAW%2F20220225%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220225T034419Z&X-Amz-Expires=17999&X-Amz-SignedHeaders=host&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEEQaCXVzLWVhc3QtMSJHMEUCIEKUDdnCf1h3cZNdv10q6G24zLgn0r6th4m%2FXSS9TuR4AiEAwOwf2cG%2F1W%2FkAz1UMqFW9sZp7SY6j%2BmiicLy1dB8OXUqmgYInf%2F%2F%2F%2F%2F%2F%2F%2F%2F%2FARABGgwzNjA4OTY3NjM0OTEiDH6CiNUJiwL4sNy9KCruBbdJnGSx88A%2Be0SBpKEpSurxunasaDsJb2ZJPqVhC%2FcKPr87PYcp5BrnkGumcYhUAEknVbHACLLUx2%2Fnzfacp13PHmclSsLe52qGwjFFVMz0m41PV3HiCoHgIc7vVIHnwNySaX9ZJbVowhNvI4V9eixVKhjjx7Tqku1bsOzlq9dJP15qz8FVNjlKGjZFqrMGTzpTgmS9dNPqphwCcxx306RLUd35SEvXjxnWcidOdddYQs9j4wu47DOM8ftveag6cDptJQN71dDIRHgFTisVshD78Rm9pycKf3g0QvBAGtrzhcxUcJtNgIWv%2B10hsEBURsEYommcjI8vT1yX2K8pLVOxgL%2FRWXndbAeIzAu5vmLm6RqkfGwkHQPQl7uII6YzL2Gku%2FMDilVFw9TBKIg6KDP9l2GzzVRQsvLMpFIp%2Bx3a1s4OVduJRFpDYCwEsfKhIoVkb610gBbFayPKjQVcZfULdq1r5DOZzpHVDoijnKHAxHtFgaFPP6KtG%2BmdKeix8gccdbsdgMokWKtJhisFo%2BzLn02oSSX4ITkZKzZcriGxQO4E1YUlYyBhjlCg1b74faQfWstk24PrkCfNXYcQ5oxgglIA0tBOdOfwGn2Je3MBEj2T8Yz0GS%2BZib3DKVWRzU0Xk9pwDXH3iaBn9Uld%2BNyw9gxdOCBVKtTILtdfsjw9lJSVOJomlJn1h8gH96PToBg9lOc4ms6aA2Z%2FoN%2F3UV%2Beo5%2FpB9xfMkBIeOg6vAI0VtjkUhH052UouEKU%2BVGSGuCSuVzZmIBJLWHZaQUJZ3hJCdGqM%2FoM4Ud51Cidcnr%2Bni%2Bgp4RAfg0gvX6Eb2e%2FNMqNd0Eg7ftmnPXzQR3ZVC1yyNFu2kBt8icKdMnkLZT8YO1Racd1QgrZ5DZJlU%2FS2eisLGSTMb9Dmaq8AGbxgGK55acIvsQLzvJhavFWbh%2FyNudQBSLC0c5BZGxQk2J%2BJx%2FlR5wR%2B0gXOfTg5EImMrLzh7gOo56i2Rhj2xRFSDDDnuGQBjqHArU56DsGZNeKwkVK87lVJAfiAsWmKaDlBNXcuhKl084syaZMiVUQa7dPupgeg%2BvVPv4dAwNULjuE3B7V5lSea4RIDOfGwTtlj4Ekn3t4PrlxHpEyMeuRgekNhpdcfL5pgRcKH4AKgqI7fCrghhH1qDMTpUkORRv6EwPGqM0LPm93XPVOjrG3EJ5ZOYLJM05bbrcalzN0mbSbsRrcphme7Z8zpD5jNeSWdBhWA04DQ7RZuJpqFnn40yKq7G8kgtLmOJGRs7CGTWT9on7EOpH3RdKGPXL06%2BgLo7r9%2Fdkb%2BHGJF6spjqbH0SN%2FySjnvgNL1NrGAy%2FQgPwfDw6oJ6TPNdH8dfM8tmdd&X-Amz-Signature=90b9f3353ad4f2596fdd166c00da8fb86d5d255bf3c8b296c5e5a7db25fd41aa" let queryItems = [SDKURLQueryItem(name: "Bucket", value: "stehlibstoragebucket144955-dev"), SDKURLQueryItem(name: "Key", value: "public%2FREADME.md")] - let originalRequest = SdkHttpRequest(method: .get, endpoint: try Endpoint(host: "stehlibstoragebucket144955-dev.s3.us-east-1.amazonaws.com", path: "/", port: 80, queryItems: queryItems, protocolType: .https)) + let originalRequest = SdkHttpRequest(method: .get, endpoint: Endpoint(host: "stehlibstoragebucket144955-dev.s3.us-east-1.amazonaws.com", path: "/", port: 80, queryItems: queryItems, protocolType: .https)) let crtRequest = try HTTPRequest() crtRequest.path = pathToMatch diff --git a/Tests/ClientRuntimeTests/NetworkingTests/NetworkingTestUtils.swift b/Tests/ClientRuntimeTests/NetworkingTests/NetworkingTestUtils.swift index 895a5c146..30fb4cf16 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/NetworkingTestUtils.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/NetworkingTestUtils.swift @@ -52,7 +52,7 @@ class NetworkingTestUtils: XCTestCase { let endpoint: Endpoint! queryItems.append(SDKURLQueryItem(name: "qualifier", value: "qualifier-value")) - endpoint = try? Endpoint(host: host, path: path, queryItems: queryItems, headers: headers) + endpoint = Endpoint(host: host, path: path, queryItems: queryItems, headers: headers) return endpoint } From 3de76d3575a28f36beabf2cb2cbd78b1ad4bc925 Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Tue, 21 May 2024 09:07:29 -0700 Subject: [PATCH 15/19] Add more URI tests --- Sources/ClientRuntime/Message/URI.swift | 4 +++- .../MessageTests/URITests.swift | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/Sources/ClientRuntime/Message/URI.swift b/Sources/ClientRuntime/Message/URI.swift index 84e94beb4..3e2c44d1a 100644 --- a/Sources/ClientRuntime/Message/URI.swift +++ b/Sources/ClientRuntime/Message/URI.swift @@ -6,6 +6,7 @@ import Foundation /// A representation of the RFC 3986 Uniform Resource Identifier +/// Note: URIBuilder returns an URI instance with all components percent encoded public struct URI: Hashable { public let scheme: Scheme public let path: String @@ -58,6 +59,7 @@ public struct URI: Hashable { /// A builder class for URI /// The builder performs validation to conform with RFC 3986 +/// Note: URIBuilder returns an URI instance with all components percent encoded public final class URIBuilder { var urlComponents: URLComponents @@ -159,7 +161,7 @@ public final class URIBuilder { } ?? [], username: self.urlComponents.user, password: self.urlComponents.password, - fragment: self.urlComponents.fragment) + fragment: self.urlComponents.percentEncodedFragment) } // We still have to keep 'url' as an optional, since we're diff --git a/Tests/ClientRuntimeTests/MessageTests/URITests.swift b/Tests/ClientRuntimeTests/MessageTests/URITests.swift index db4a6d430..c03bd75b9 100644 --- a/Tests/ClientRuntimeTests/MessageTests/URITests.swift +++ b/Tests/ClientRuntimeTests/MessageTests/URITests.swift @@ -69,4 +69,26 @@ class URITests: XCTestCase { .build() XCTAssertEqual(uri.url?.absoluteString, "https://xctest.amazonaws.com:443/abc+def") } + + func test_modifyURI() throws { + var uri = URIBuilder() + .withScheme(Scheme(rawValue: url.scheme!)!) + .withPath(url.path) + .withHost(url.host!) + .withPort(url.port) + .withQueryItems(url.getQueryItems()!) + .build() + + uri = uri.toBuilder() + .withPath("/x%2Dy%2Dz") + .withHost("xctest2.amazonaws.com") + .appendQueryItem(SDKURLQueryItem(name: "test", value: "1%2B2")) + .withFragment("fragment%21") + .withUsername("dan") + .withPassword("008") + .build() + + XCTAssertEqual(uri.url?.absoluteString, + "https://dan:008@xctest2.amazonaws.com/x%2Dy%2Dz?abc=def&ghi=jkl&mno=pqr&test=1%2B2#fragment%21") + } } From d2f6d2327c3ec7589ec169d452b4cf65b76acd7e Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Wed, 22 May 2024 12:47:07 -0700 Subject: [PATCH 16/19] Add isPercentEncoded check to username, password and host in URI --- Sources/ClientRuntime/Message/URI.swift | 28 +++++++++++++++---- .../MessageTests/URITests.swift | 15 ++++++---- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/Sources/ClientRuntime/Message/URI.swift b/Sources/ClientRuntime/Message/URI.swift index 3e2c44d1a..d435b8cc7 100644 --- a/Sources/ClientRuntime/Message/URI.swift +++ b/Sources/ClientRuntime/Message/URI.swift @@ -88,7 +88,11 @@ public final class URIBuilder { @discardableResult public func withHost(_ value: String) -> URIBuilder { - self.urlComponents.host = value + if value.isPercentEncoded { + self.urlComponents.percentEncodedHost = value + } else { + self.urlComponents.host = value + } return self } @@ -129,13 +133,25 @@ public final class URIBuilder { @discardableResult public func withUsername(_ value: String?) -> URIBuilder { - self.urlComponents.user = value + if let username = value { + if username.isPercentEncoded { + self.urlComponents.percentEncodedUser = username + } else { + self.urlComponents.user = username + } + } return self } @discardableResult public func withPassword(_ value: String?) -> URIBuilder { - self.urlComponents.password = value + if let password = value { + if password.isPercentEncoded { + self.urlComponents.percentEncodedPassword = password + } else { + self.urlComponents.password = password + } + } return self } @@ -154,13 +170,13 @@ public final class URIBuilder { public func build() -> URI { return URI(scheme: Scheme(rawValue: self.urlComponents.scheme!)!, path: self.urlComponents.percentEncodedPath, - host: self.urlComponents.host!, + host: self.urlComponents.percentEncodedHost!, port: self.urlComponents.port.map { Int16($0) }, queryItems: self.urlComponents.percentEncodedQueryItems?.map { SDKURLQueryItem(name: $0.name, value: $0.value) } ?? [], - username: self.urlComponents.user, - password: self.urlComponents.password, + username: self.urlComponents.percentEncodedUser, + password: self.urlComponents.percentEncodedPassword, fragment: self.urlComponents.percentEncodedFragment) } diff --git a/Tests/ClientRuntimeTests/MessageTests/URITests.swift b/Tests/ClientRuntimeTests/MessageTests/URITests.swift index c03bd75b9..a49cfc73b 100644 --- a/Tests/ClientRuntimeTests/MessageTests/URITests.swift +++ b/Tests/ClientRuntimeTests/MessageTests/URITests.swift @@ -81,14 +81,19 @@ class URITests: XCTestCase { uri = uri.toBuilder() .withPath("/x%2Dy%2Dz") - .withHost("xctest2.amazonaws.com") + .withHost("%2Bxctest2.com") .appendQueryItem(SDKURLQueryItem(name: "test", value: "1%2B2")) .withFragment("fragment%21") - .withUsername("dan") - .withPassword("008") + .withUsername("dan%21") + .withPassword("%24008") .build() - XCTAssertEqual(uri.url?.absoluteString, - "https://dan:008@xctest2.amazonaws.com/x%2Dy%2Dz?abc=def&ghi=jkl&mno=pqr&test=1%2B2#fragment%21") + #if os(Linux) + XCTAssertEqual(uri.url?.absoluteString, + "https://dan%21:%24008@%2Bxctest2.com/x%2Dy%2Dz?abc=def&ghi=jkl&mno=pqr&test=1%2B2#fragment%21") + #else + XCTAssertEqual(uri.url?.absoluteString, + "https://dan%21:%24008@+xctest2.com/x%2Dy%2Dz?abc=def&ghi=jkl&mno=pqr&test=1%2B2#fragment%21") + #endif } } From b2525b6bb0ba08d05d731bad20fc6b546352b536 Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Wed, 22 May 2024 14:29:11 -0700 Subject: [PATCH 17/19] Set urlComponents.percentEncodedHost if os is not linux --- Sources/ClientRuntime/Message/URI.swift | 10 +++++++++- Tests/ClientRuntimeTests/MessageTests/URITests.swift | 9 ++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/Sources/ClientRuntime/Message/URI.swift b/Sources/ClientRuntime/Message/URI.swift index d435b8cc7..365631f38 100644 --- a/Sources/ClientRuntime/Message/URI.swift +++ b/Sources/ClientRuntime/Message/URI.swift @@ -89,7 +89,15 @@ public final class URIBuilder { @discardableResult public func withHost(_ value: String) -> URIBuilder { if value.isPercentEncoded { - self.urlComponents.percentEncodedHost = value + // URLComponents.percentEncodedHost follows RFC 3986 + // and returns a decoded value if it is set with a percent encoded value + // However on Linux platform, it returns a percent encoded value. + // To ensure consistent behaviour, we will decode it ourselves on Linux platform + if currentOS == .linux { + self.urlComponents.host = value.removingPercentEncoding! + } else { + self.urlComponents.percentEncodedHost = value + } } else { self.urlComponents.host = value } diff --git a/Tests/ClientRuntimeTests/MessageTests/URITests.swift b/Tests/ClientRuntimeTests/MessageTests/URITests.swift index a49cfc73b..bc0689559 100644 --- a/Tests/ClientRuntimeTests/MessageTests/URITests.swift +++ b/Tests/ClientRuntimeTests/MessageTests/URITests.swift @@ -88,12 +88,7 @@ class URITests: XCTestCase { .withPassword("%24008") .build() - #if os(Linux) - XCTAssertEqual(uri.url?.absoluteString, - "https://dan%21:%24008@%2Bxctest2.com/x%2Dy%2Dz?abc=def&ghi=jkl&mno=pqr&test=1%2B2#fragment%21") - #else - XCTAssertEqual(uri.url?.absoluteString, - "https://dan%21:%24008@+xctest2.com/x%2Dy%2Dz?abc=def&ghi=jkl&mno=pqr&test=1%2B2#fragment%21") - #endif + XCTAssertEqual(uri.url?.absoluteString, + "https://dan%21:%24008@+xctest2.com/x%2Dy%2Dz?abc=def&ghi=jkl&mno=pqr&test=1%2B2#fragment%21") } } From 1e2b745d5a8c08a30acdac4015dcf85ff32cd86f Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Thu, 23 May 2024 16:08:03 -0700 Subject: [PATCH 18/19] Add URITests test cases with encoded and unencoded reserved characters --- Sources/ClientRuntime/Message/URI.swift | 62 ++++++++++- .../MessageTests/URITests.swift | 105 ++++++++++++++++++ 2 files changed, 161 insertions(+), 6 deletions(-) diff --git a/Sources/ClientRuntime/Message/URI.swift b/Sources/ClientRuntime/Message/URI.swift index 365631f38..5c4da9c0c 100644 --- a/Sources/ClientRuntime/Message/URI.swift +++ b/Sources/ClientRuntime/Message/URI.swift @@ -76,12 +76,29 @@ public final class URIBuilder { return self } + /// According to https://developer.apple.com/documentation/foundation/nsurlcomponents/1408161-percentencodedpath + /// "Although an unencoded semicolon is a valid character in a percent-encoded path, + /// for compatibility with the NSURL class, you should always percent-encode it." + /// + /// URI also always return a percent-encoded path. + /// If an percent-encoded path is provided, we will replace the semicolon with %3B in the path. + /// If an unencoded path is provided, we should percent-encode the path including semicolon. @discardableResult public func withPath(_ value: String) -> URIBuilder { if value.isPercentEncoded { - self.urlComponents.percentEncodedPath = value + if value.contains(";") { + let encodedPath = value.replacingOccurrences( + of: ";", with: "%3B", options: NSString.CompareOptions.literal, range: nil) + self.urlComponents.percentEncodedPath = encodedPath + } else { + self.urlComponents.percentEncodedPath = value + } } else { - self.urlComponents.path = value + if value.contains(";") { + self.urlComponents.percentEncodedPath = value.percentEncodePathIncludingSemicolon() + } else { + self.urlComponents.path = value + } } return self } @@ -118,7 +135,14 @@ public final class URIBuilder { @discardableResult public func withQueryItems(_ value: [SDKURLQueryItem]) -> URIBuilder { - self.urlComponents.percentEncodedQueryItems = value.isEmpty ? nil : value.toURLQueryItems() + if value.isEmpty { + return self + } + if value.containsPercentEncode() { + self.urlComponents.percentEncodedQueryItems = value.toURLQueryItems() + } else { + self.urlComponents.queryItems = value.toURLQueryItems() + } return self } @@ -129,7 +153,13 @@ public final class URIBuilder { } var queryItems = self.urlComponents.percentEncodedQueryItems ?? [] queryItems += items.toURLQueryItems() - self.urlComponents.percentEncodedQueryItems = queryItems + + if queryItems.containsPercentEncode() { + self.urlComponents.percentEncodedQueryItems = queryItems + } else { + self.urlComponents.queryItems = queryItems + } + return self } @@ -201,6 +231,18 @@ extension String { let decoded = self.removingPercentEncoding return decoded != nil && decoded != self } + + public func percentEncodePathIncludingSemicolon() -> String { + let allowed = + // swiftlint:disable:next force_cast + (CharacterSet.urlPathAllowed as NSCharacterSet).mutableCopy() as! NSMutableCharacterSet + allowed.removeCharacters(in: ";") + return self.addingPercentEncoding(withAllowedCharacters: allowed as CharacterSet)! + } + + public func percentEncodeQuery() -> String { + return self.addingPercentEncoding(withAllowedCharacters: CharacterSet.urlQueryAllowed as CharacterSet)! + } } extension Array where Element == SDKURLQueryItem { @@ -214,10 +256,18 @@ extension Array where Element == SDKURLQueryItem { public func toURLQueryItems() -> [URLQueryItem] { return self.map { URLQueryItem(name: $0.name, value: $0.value) } } + + public func containsPercentEncode() -> Bool { + return self.contains { item in + return item.name.isPercentEncoded || (item.value?.isPercentEncoded ?? false) + } + } } extension Array where Element == URLQueryItem { - public func toSDKURLQueryItems() -> [SDKURLQueryItem] { - return self.map { SDKURLQueryItem(name: $0.name, value: $0.value) } + public func containsPercentEncode() -> Bool { + return self.contains { item in + return item.name.isPercentEncoded || (item.value?.isPercentEncoded ?? false) + } } } diff --git a/Tests/ClientRuntimeTests/MessageTests/URITests.swift b/Tests/ClientRuntimeTests/MessageTests/URITests.swift index bc0689559..db3c14da8 100644 --- a/Tests/ClientRuntimeTests/MessageTests/URITests.swift +++ b/Tests/ClientRuntimeTests/MessageTests/URITests.swift @@ -12,6 +12,10 @@ import XCTest class URITests: XCTestCase { let url = URL(string: "https://xctest.amazonaws.com?abc=def&ghi=jkl&mno=pqr")! + let unencodedReservedCharacters: String = "!$&'()*+,;=" + + let encodedReservedCharacters: String = "%21%24%26%27%28%29%2A%2B%2C%3B%3D" + func test_queryItems_setsQueryItemsFromURLInOrder() throws { let uri = URIBuilder() .withScheme(Scheme(rawValue: url.scheme!)!) @@ -91,4 +95,105 @@ class URITests: XCTestCase { XCTAssertEqual(uri.url?.absoluteString, "https://dan%21:%24008@+xctest2.com/x%2Dy%2Dz?abc=def&ghi=jkl&mno=pqr&test=1%2B2#fragment%21") } + + func test_host_unencodedReservedCharacters() throws { + let uri = URIBuilder() + .withScheme(.https) + .withHost("xctest.\(unencodedReservedCharacters).com") + .withPath("/") + .build() + XCTAssertEqual(uri.url?.absoluteString, "https://xctest.!$&\'()*+,;=.com/") + } + + func test_host_encodedReservedCharacters() throws { + let uri = URIBuilder() + .withScheme(.https) + .withHost("xctest.\(encodedReservedCharacters).com") + .withPath("/") + .build() + XCTAssertEqual(uri.url?.absoluteString, "https://xctest.!$&\'()*+,;=.com/") + } + + func test_host_encodedAndUnencodedReservedCharacters() throws { + let uri = URIBuilder() + .withScheme(.https) + .withHost("xctest.\(unencodedReservedCharacters)\(encodedReservedCharacters).com") + .withPath("/") + .build() + XCTAssertEqual(uri.url?.absoluteString, "https://xctest.!$&\'()*+,;=!$&\'()*+,;=.com/") + } + + func test_path_unencodedReservedCharacters() throws { + let uri = URIBuilder() + .withScheme(.https) + .withHost("xctest.com") + .withPath("/:@\(unencodedReservedCharacters)") + .build() + XCTAssertEqual(uri.url?.absoluteString, "https://xctest.com/:@!$&\'()*+,%3B=") + } + + func test_path_encodedReservedCharacters() throws { + let uri = URIBuilder() + .withScheme(.https) + .withHost("xctest.com") + .withPath("/\(encodedReservedCharacters)") + .build() + XCTAssertEqual(uri.url?.absoluteString, "https://xctest.com/%21%24%26%27%28%29%2A%2B%2C%3B%3D") + } + + func test_path_encodedAndUnencodedReservedCharacters() throws { + let uri = URIBuilder() + .withScheme(.https) + .withHost("xctest.com") + .withPath("/:@\(unencodedReservedCharacters)\(encodedReservedCharacters)") + .build() + XCTAssertEqual(uri.url?.absoluteString, "https://xctest.com/:@!$&\'()*+,%3B=%21%24%26%27%28%29%2A%2B%2C%3B%3D") + } + + func test_query_unencodedReservedCharacters() throws { + let uri = URIBuilder() + .withScheme(.https) + .withHost("xctest.com") + .withPath("/") + .withQueryItems([ + SDKURLQueryItem( + name: "key:@\(unencodedReservedCharacters))", + value: "value:@\(unencodedReservedCharacters)" + ), + ]) + .build() + XCTAssertEqual(uri.url?.absoluteString, "https://xctest.com/?key:@!$%26\'()*+,;%3D)=value:@!$%26\'()*+,;%3D") + } + + func test_query_encodedReservedCharacters() throws { + let uri = URIBuilder() + .withScheme(.https) + .withHost("xctest.com") + .withPath("/") + .withQueryItems([ + SDKURLQueryItem( + name: "key:@\(encodedReservedCharacters))", + value: "value:@\(encodedReservedCharacters)" + ), + ]) + .build() + XCTAssertEqual(uri.url?.absoluteString, + "https://xctest.com/?key:@%21%24%26%27%28%29%2A%2B%2C%3B%3D)=value:@%21%24%26%27%28%29%2A%2B%2C%3B%3D") + } + + func test_query_unencodedAndEncodedReservedCharacters() throws { + let uri = URIBuilder() + .withScheme(.https) + .withHost("xctest.com") + .withPath("/") + .withQueryItems([ + SDKURLQueryItem( + name: "key:@\(encodedReservedCharacters))", + value: "value:@\(unencodedReservedCharacters)" + ), + ]) + .build() + XCTAssertEqual(uri.url?.absoluteString, + "https://xctest.com/?key:@%21%24%26%27%28%29%2A%2B%2C%3B%3D)=value:@!$&\'()*+,;=") + } } From 7dfc184f31cf4f53649ec670df8d68ac3a88284d Mon Sep 17 00:00:00 2001 From: Andrew Foss Date: Tue, 28 May 2024 15:08:01 -0700 Subject: [PATCH 19/19] Address swiftlint warnings --- Sources/ClientRuntime/Message/URI.swift | 3 ++- Sources/ClientRuntime/Networking/Http/HttpResponse.swift | 9 +++++++-- .../ClientRuntime/Networking/Http/SdkHttpRequest.swift | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Sources/ClientRuntime/Message/URI.swift b/Sources/ClientRuntime/Message/URI.swift index 5c4da9c0c..7cb89a7e7 100644 --- a/Sources/ClientRuntime/Message/URI.swift +++ b/Sources/ClientRuntime/Message/URI.swift @@ -234,8 +234,9 @@ extension String { public func percentEncodePathIncludingSemicolon() -> String { let allowed = - // swiftlint:disable:next force_cast + // swiftlint:disable force_cast (CharacterSet.urlPathAllowed as NSCharacterSet).mutableCopy() as! NSMutableCharacterSet + // swiftlint:enable force_cast allowed.removeCharacters(in: ";") return self.addingPercentEncoding(withAllowedCharacters: allowed as CharacterSet)! } diff --git a/Sources/ClientRuntime/Networking/Http/HttpResponse.swift b/Sources/ClientRuntime/Networking/Http/HttpResponse.swift index 672792d75..873f6fde0 100644 --- a/Sources/ClientRuntime/Networking/Http/HttpResponse.swift +++ b/Sources/ClientRuntime/Networking/Http/HttpResponse.swift @@ -13,6 +13,7 @@ public class HttpResponse: HttpUrlResponse, ResponseMessage { public var headers: Headers public var body: ByteStream + public var reason: String? private var _statusCode: HttpStatusCode private let statusCodeQueue = DispatchQueue(label: "statusCodeSerialQueue") @@ -29,13 +30,17 @@ public class HttpResponse: HttpUrlResponse, ResponseMessage { } } - public init(headers: Headers = .init(), statusCode: HttpStatusCode = .processing, body: ByteStream = .noStream) { + public init( + headers: Headers = .init(), + statusCode: HttpStatusCode = .processing, + body: ByteStream = .noStream, + reason: String? = nil) { self.headers = headers self._statusCode = statusCode self.body = body } - public init(headers: Headers = .init(), body: ByteStream, statusCode: HttpStatusCode) { + public init(headers: Headers = .init(), body: ByteStream, statusCode: HttpStatusCode, reason: String? = nil) { self.body = body self._statusCode = statusCode self.headers = headers diff --git a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift index b56461672..76847af03 100644 --- a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift +++ b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift @@ -163,7 +163,7 @@ extension SdkHttpRequest: CustomDebugStringConvertible, CustomStringConvertible let method = method.rawValue.uppercased() let protocolType = self.destination.scheme let query = self.destination.queryString ?? "" - let port = self.destination.port + let port = self.destination.port.map { String($0) } ?? "" return "\(method) \(protocolType):\(port) \n " + "Path: \(endpoint.uri.path) \n Headers: \(headers) \n Query: \(query)" }