diff --git a/Sources/ClientRuntime/Networking/Endpoint.swift b/Sources/ClientRuntime/Networking/Endpoint.swift index 691d51400..f51cf8dd5 100644 --- a/Sources/ClientRuntime/Networking/Endpoint.swift +++ b/Sources/ClientRuntime/Networking/Endpoint.swift @@ -11,7 +11,6 @@ public struct Endpoint: Hashable { public let protocolType: ProtocolType? public let host: String public let port: Int16 - public let headers: Headers? public let properties: [String: AnyHashable] @@ -67,18 +66,13 @@ extension Endpoint { components.host = host components.percentEncodedPath = path components.percentEncodedQuery = queryItemString - return components.url } var queryItemString: String? { guard let queryItems = queryItems else { return nil } return queryItems.map { queryItem in - if let value = queryItem.value { - return "\(queryItem.name)=\(value)" - } else { - return queryItem.name - } + return [queryItem.name, queryItem.value].compactMap { $0 }.joined(separator: "=") }.joined(separator: "&") } } diff --git a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift index c8b75d661..83de78b85 100644 --- a/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift +++ b/Sources/ClientRuntime/Networking/Http/SdkHttpRequest.swift @@ -29,11 +29,10 @@ public class SdkHttpRequest { extension SdkHttpRequest { - public func toHttpRequest(escaping: Bool = false) throws -> HTTPRequest { + public func toHttpRequest() throws -> HTTPRequest { let httpRequest = try HTTPRequest() httpRequest.method = method.rawValue - let encodedPath = escaping ? endpoint.path.urlPercentEncodedForPath : endpoint.path - httpRequest.path = [encodedPath, endpoint.queryItemString].compactMap { $0 }.joined(separator: "?") + httpRequest.path = [endpoint.path, endpoint.queryItemString].compactMap { $0 }.joined(separator: "?") httpRequest.addHeaders(headers: headers.toHttpHeaders()) httpRequest.body = StreamableHttpBody(body: body) return httpRequest @@ -42,11 +41,10 @@ extension SdkHttpRequest { /// Convert the SDK request to a CRT HTTPRequestBase /// CRT converts the HTTPRequestBase to HTTP2Request internally if the protocol is HTTP/2 /// - Returns: the CRT request - public func toHttp2Request(escaping: Bool = false) throws -> HTTPRequestBase { + public func toHttp2Request() throws -> HTTPRequestBase { let httpRequest = try HTTPRequest() httpRequest.method = method.rawValue - let encodedPath = escaping ? endpoint.path.urlPercentEncodedForPath : endpoint.path - httpRequest.path = [encodedPath, endpoint.queryItemString].compactMap { $0 }.joined(separator: "?") + httpRequest.path = [endpoint.path, endpoint.queryItemString].compactMap { $0 }.joined(separator: "?") httpRequest.addHeaders(headers: headers.toHttpHeaders()) // HTTP2Request used with manual writes hence we need to set the body to nil diff --git a/Sources/ClientRuntime/PrimitiveTypeExtensions/String+Extensions.swift b/Sources/ClientRuntime/PrimitiveTypeExtensions/String+Extensions.swift index ffbc29d46..149569672 100644 --- a/Sources/ClientRuntime/PrimitiveTypeExtensions/String+Extensions.swift +++ b/Sources/ClientRuntime/PrimitiveTypeExtensions/String+Extensions.swift @@ -100,8 +100,8 @@ extension String { } extension String { - public func urlPercentEncoding() -> String { - self.urlPercentEncodedForQuery + public func urlPercentEncoding(encodeForwardSlash: Bool = true) -> String { + encodeForwardSlash ? urlPercentEncodedForQuery : urlPercentEncodedForPath } } diff --git a/Tests/ClientRuntimeTests/ClientRuntimeTests/MiddlewareTests/AttributesTests.swift b/Tests/ClientRuntimeTests/ClientRuntimeTests/MiddlewareTests/AttributesTests.swift index 6746b6765..8f5c8ce6a 100644 --- a/Tests/ClientRuntimeTests/ClientRuntimeTests/MiddlewareTests/AttributesTests.swift +++ b/Tests/ClientRuntimeTests/ClientRuntimeTests/MiddlewareTests/AttributesTests.swift @@ -21,7 +21,7 @@ class AttributesTests: XCTestCase { } func test_contains_doesNotContainUnsetKey() { - var subject = Attributes() + let subject = Attributes() XCTAssertFalse(subject.contains(key: intKey)) } diff --git a/Tests/ClientRuntimeTests/NetworkingTests/HttpRequestTests.swift b/Tests/ClientRuntimeTests/NetworkingTests/HttpRequestTests.swift index bb5259c94..1873557e1 100644 --- a/Tests/ClientRuntimeTests/NetworkingTests/HttpRequestTests.swift +++ b/Tests/ClientRuntimeTests/NetworkingTests/HttpRequestTests.swift @@ -15,7 +15,7 @@ class HttpRequestTests: NetworkingTestUtils { } func testSdkHttpRequestToHttpRequest() throws { - var headers = Headers(["header-item-name": "header-item-value"]) + let headers = Headers(["header-item-name": "header-item-value"]) let endpoint = Endpoint(host: "host.com", path: "/", headers: headers) let httpBody = HttpBody.data(expectedMockRequestData) @@ -90,16 +90,7 @@ class HttpRequestTests: NetworkingTestUtils { XCTAssert(updatedRequest.queryItems?.contains(ClientRuntime.URLQueryItem(name: "signedthing", value: "signed")) ?? false) } - func testPathInInHttpRequestIsEscapedPerRFC3986() throws { - let builder = SdkHttpRequestBuilder() - .withHeader(name: "Host", value: "xctest.amazon.com") - .withPath("/space /colon:/dollar$/tilde~/dash-/underscore_/period.") - let httpRequest = try builder.build().toHttpRequest(escaping: true) - let escapedPath = "/space%20/colon%3A/dollar%24/tilde~/dash-/underscore_/period." - XCTAssertEqual(httpRequest.path, escapedPath) - } - - func testPathInInHttpRequestIsNotEscapedPerRFC3986WhenNotDesired() throws { + func testPathInInHttpRequestIsNotAltered() throws { let path = "/space /colon:/dollar$/tilde~/dash-/underscore_/period." let builder = SdkHttpRequestBuilder() .withHeader(name: "Host", value: "xctest.amazon.com") diff --git a/smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/middlewares/providers/HttpUrlPathProvider.kt b/smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/middlewares/providers/HttpUrlPathProvider.kt index fc527d99f..c19ac05c6 100644 --- a/smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/middlewares/providers/HttpUrlPathProvider.kt +++ b/smithy-swift-codegen/src/main/kotlin/software/amazon/smithy/swift/codegen/integration/middlewares/providers/HttpUrlPathProvider.kt @@ -83,14 +83,14 @@ class HttpUrlPathProvider( ) } ShapeType.STRING -> { - val percentEncoded = if (!it.isGreedyLabel) ".urlPercentEncoding()" else "" + val percentEncoded = urlEncoding(it.isGreedyLabel) val enumRawValueSuffix = targetShape.getTrait(EnumTrait::class.java).map { ".rawValue" }.orElse("") "$labelMemberName$enumRawValueSuffix$percentEncoded" } ShapeType.FLOAT, ShapeType.DOUBLE -> "$labelMemberName.encoded()" ShapeType.ENUM -> { - val percentEncoded = if (!it.isGreedyLabel) ".urlPercentEncoding()" else "" + val percentEncoded = urlEncoding(it.isGreedyLabel) "$labelMemberName.rawValue$percentEncoded" } else -> labelMemberName @@ -115,4 +115,20 @@ class HttpUrlPathProvider( val uri = resolvedURIComponents.joinToString(separator = "/", prefix = "/", postfix = "") writer.write("return \"\$L\"", uri) } + + /** + * Provides the appropriate Swift method call for URL encoding a String path component. + * + * For non-greedy labels, forward-slash is encoded because the label must fill in + * exactly one path component. + * + * For greedy labels, forward-slash is not encoded because it is expected that the + * label contents will include multiple path components. + * + * Swift .urlPercentEncoding() method encodes forward slashes by default. + */ + private fun urlEncoding(greedy: Boolean): String { + val options = "encodeForwardSlash: false".takeIf { greedy } ?: "" + return ".urlPercentEncoding($options)" + } }