Skip to content

Commit 0df2a96

Browse files
diegocstndiegocstn
authored
feat(api): fix querystring encoding according to AWS SigV4 (#1068)
* fix(api): fix querystring encoding according to AWS SigV4 fix #977 * fix(api): refactor RESTOperationRequestUtils tests, remove params reordering * fix(api): add integration tests * fix(api): encode individual components * fix(api): address PR comments * fix(api): throw if invalid query params * fix(api): throw APIError if invalid query params * fix(api): error message rewording * chore: add more tests
1 parent 56bf172 commit 0df2a96

File tree

6 files changed

+249
-7
lines changed

6 files changed

+249
-7
lines changed

‎AmplifyPlugins/API/APICategoryPlugin.xcodeproj/project.pbxproj‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@
135135
6B8D479D25801DF700E841CB /* AmplifyReachability.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B8D479C25801DF700E841CB /* AmplifyReachability.swift */; };
136136
6BD4620625380EA200906831 /* OIDCAuthProviderWrapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6BD4620525380EA200906831 /* OIDCAuthProviderWrapper.swift */; };
137137
6BD462082538102500906831 /* AuthTokenProviderWrapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6BD462072538102500906831 /* AuthTokenProviderWrapper.swift */; };
138+
76148E0925D896EA007F3F21 /* URLComponents+sigv4Encoding.swift in Sources */ = {isa = PBXBuildFile; fileRef = 76148E0825D896EA007F3F21 /* URLComponents+sigv4Encoding.swift */; };
138139
7632AD8A252E1E10009B5BC9 /* AppSyncJSONValue+toJSONValue.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7632AD89252E1E10009B5BC9 /* AppSyncJSONValue+toJSONValue.swift */; };
139140
9B13EA5E48896E8B38883633 /* Pods_HostApp.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 930DD773E0FB4047393CA2AD /* Pods_HostApp.framework */; };
140141
A04815BCD5F9181C8AEDEF43 /* Pods_AWSAPICategoryPlugin.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 881AB4B98B48235DEC7754C2 /* Pods_AWSAPICategoryPlugin.framework */; };
@@ -471,6 +472,7 @@
471472
6BD462072538102500906831 /* AuthTokenProviderWrapper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AuthTokenProviderWrapper.swift; sourceTree = "<group>"; };
472473
6DD6386039136045F18D44AC /* Pods_HostApp_AWSAPICategoryPluginTestCommon_RESTWithUserPoolIntegrationTests.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_HostApp_AWSAPICategoryPluginTestCommon_RESTWithUserPoolIntegrationTests.framework; sourceTree = BUILT_PRODUCTS_DIR; };
473474
74EDB7008F5342ED4B38C9CA /* Pods_HostApp_AWSAPICategoryPluginIntegrationTests.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_HostApp_AWSAPICategoryPluginIntegrationTests.framework; sourceTree = BUILT_PRODUCTS_DIR; };
475+
76148E0825D896EA007F3F21 /* URLComponents+sigv4Encoding.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "URLComponents+sigv4Encoding.swift"; sourceTree = "<group>"; };
474476
7632AD89252E1E10009B5BC9 /* AppSyncJSONValue+toJSONValue.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "AppSyncJSONValue+toJSONValue.swift"; sourceTree = "<group>"; };
475477
77792DD821FC754D857FC63C /* Pods-HostApp-AWSAPICategoryPluginTestCommon-GraphQLWithIAMIntegrationTests.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-HostApp-AWSAPICategoryPluginTestCommon-GraphQLWithIAMIntegrationTests.release.xcconfig"; path = "Target Support Files/Pods-HostApp-AWSAPICategoryPluginTestCommon-GraphQLWithIAMIntegrationTests/Pods-HostApp-AWSAPICategoryPluginTestCommon-GraphQLWithIAMIntegrationTests.release.xcconfig"; sourceTree = "<group>"; };
476478
7866FCFB5807C2D20219CEBE /* Pods-HostApp-AWSAPICategoryPluginTestCommon-RESTWithIAMIntegrationTests.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-HostApp-AWSAPICategoryPluginTestCommon-RESTWithIAMIntegrationTests.release.xcconfig"; path = "Target Support Files/Pods-HostApp-AWSAPICategoryPluginTestCommon-RESTWithIAMIntegrationTests/Pods-HostApp-AWSAPICategoryPluginTestCommon-RESTWithIAMIntegrationTests.release.xcconfig"; sourceTree = "<group>"; };
@@ -963,6 +965,7 @@
963965
isa = PBXGroup;
964966
children = (
965967
21D7A0C5237B54D90057D00D /* AWSAppSyncGraphQLResponse.swift */,
968+
76148E0825D896EA007F3F21 /* URLComponents+sigv4Encoding.swift */,
966969
);
967970
path = Internal;
968971
sourceTree = "<group>";
@@ -2398,6 +2401,7 @@
23982401
6B382B452538E53700906593 /* AWSAPIPlugin+APIAuthProviderFactoryBehavior.swift in Sources */,
23992402
6B33897023AABF1800561E5B /* NetworkReachability.swift in Sources */,
24002403
21D7A0FE237B54D90057D00D /* URLSessionDataTaskBehavior.swift in Sources */,
2404+
76148E0925D896EA007F3F21 /* URLComponents+sigv4Encoding.swift in Sources */,
24012405
21A4F94C25A7ACE600E1047D /* GraphQLRequest+toListQuery.swift in Sources */,
24022406
216449482587E92B00C548A5 /* GraphQLResponseDecoder.swift in Sources */,
24032407
21D7A0E5237B54D90057D00D /* AWSAPICategoryPluginConfiguration.swift in Sources */,

‎AmplifyPlugins/API/AWSAPICategoryPlugin/Support/Internal/URLComponents+sigv4Encoding.swift‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
//
2+
// Copyright Amazon.com Inc. or its affiliates.
3+
// All Rights Reserved.
4+
//
5+
// SPDX-License-Identifier: Apache-2.0
6+
//
7+
8+
import Foundation
9+
import Amplify
10+
11+
/// Per AWS reference guide https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html
12+
/// URL querystring should be encoded according to the following rules:
13+
/// - percent-encode with %XY (X and Y are hexadecimal characters) all characters
14+
/// but any of the __unreserved characters__ defined by `RFC3986` (A-Za-z0-9-_.~)
15+
/// - encode spaces with %20
16+
/// - double encode any equals in params values
17+
///
18+
/// `URLComponents` encodes queryItems values by strictly following `RFC 3986`.
19+
extension URLComponents {
20+
static var sigV4UnreservedCharacters: CharacterSet = {
21+
var sigV4UnreservedCharacters = CharacterSet(charactersIn: "A" ... "Z")
22+
sigV4UnreservedCharacters = sigV4UnreservedCharacters.union(CharacterSet(charactersIn: "a" ... "z"))
23+
sigV4UnreservedCharacters = sigV4UnreservedCharacters.union(CharacterSet(charactersIn: "0" ... "9"))
24+
sigV4UnreservedCharacters = sigV4UnreservedCharacters.union(CharacterSet(charactersIn: "-_~."))
25+
return sigV4UnreservedCharacters
26+
}()
27+
28+
private func encodeQueryParamItemBySigV4Rules(_ value: String) -> String? {
29+
// removingPercentEncoding returns `nil` if called on a value
30+
// that hasn't been prior encoded
31+
let unencoded = value.removingPercentEncoding ?? value
32+
33+
return unencoded.addingPercentEncoding(
34+
withAllowedCharacters: Self.sigV4UnreservedCharacters)
35+
}
36+
37+
/// Encodes query items per SigV4 rules, no-op for already encoded items.
38+
mutating func encodeQueryItemsPerSigV4Rules(_ queryItems: [String: String]?) throws {
39+
guard let queryItems = queryItems else {
40+
return
41+
}
42+
percentEncodedQuery = try queryItems.map { name, value in
43+
guard let encodedName = encodeQueryParamItemBySigV4Rules(name),
44+
let encodedValue = encodeQueryParamItemBySigV4Rules(value) else {
45+
throw APIError.invalidURL(
46+
"Invalid query parameter.",
47+
"""
48+
Review your Amplify.API call to make sure you are passing \
49+
valid UTF-8 query parameters in your request.
50+
The value passed was '\(name)=\(value)'
51+
""")
52+
}
53+
54+
return [encodedName, encodedValue].joined(separator: "=")
55+
}.joined(separator: "&")
56+
}
57+
}

‎AmplifyPlugins/API/AWSAPICategoryPlugin/Support/Utils/RESTOperationRequestUtils.swift‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,7 @@ final class RESTOperationRequestUtils {
2929
components.path.append(path)
3030
}
3131

32-
if let queryParameters = queryParameters {
33-
components.queryItems = queryParameters.map { (name, value) -> URLQueryItem in
34-
URLQueryItem(name: name, value: value)
35-
}
36-
}
32+
try components.encodeQueryItemsPerSigV4Rules(queryParameters)
3733

3834
guard let url = components.url else {
3935
throw APIError.invalidURL(

‎AmplifyPlugins/API/AWSAPICategoryPluginIntegrationTests/REST/RESTWithIAMIntegrationTests/RESTWithIAMIntegrationTests.swift‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,49 @@ class RESTWithIAMIntegrationTests: XCTestCase {
8181
wait(for: [failedInvoked], timeout: TestCommonConstants.networkTimeout)
8282
}
8383

84+
func testGetAPIWithQueryParamsSuccess() {
85+
let completeInvoked = expectation(description: "request completed")
86+
let request = RESTRequest(path: "/items",
87+
queryParameters: [
88+
"user": "[email protected]",
89+
"created": "2021-06-18T09:00:00Z"
90+
])
91+
_ = Amplify.API.get(request: request) { event in
92+
switch event {
93+
case .success(let data):
94+
let result = String(decoding: data, as: UTF8.self)
95+
print(result)
96+
completeInvoked.fulfill()
97+
case .failure(let error):
98+
XCTFail("Unexpected .failed event: \(error)")
99+
}
100+
}
101+
102+
wait(for: [completeInvoked], timeout: TestCommonConstants.networkTimeout)
103+
}
104+
105+
func testGetAPIWithEncodedQueryParamsSuccess() {
106+
let completeInvoked = expectation(description: "request completed")
107+
let request = RESTRequest(path: "/items",
108+
queryParameters: [
109+
"user": "hello%40email.com",
110+
"created": "2021-06-18T09%3A00%3A00Z"
111+
])
112+
_ = Amplify.API.get(request: request) { event in
113+
switch event {
114+
case .success(let data):
115+
let result = String(decoding: data, as: UTF8.self)
116+
print(result)
117+
completeInvoked.fulfill()
118+
case .failure(let error):
119+
XCTFail("Unexpected .failed event: \(error)")
120+
}
121+
}
122+
123+
wait(for: [completeInvoked], timeout: TestCommonConstants.networkTimeout)
124+
}
125+
126+
84127
func testPutAPISuccess() {
85128
let completeInvoked = expectation(description: "request completed")
86129
let request = RESTRequest(path: "/items")

‎AmplifyPlugins/API/AWSAPICategoryPluginIntegrationTests/REST/RESTWithUserPoolIntegrationTests/RESTWithUserPoolIntegrationTests.swift‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,50 @@ class RESTWithUserPoolIntegrationTests: XCTestCase {
6565
wait(for: [completeInvoked], timeout: TestCommonConstants.networkTimeout)
6666
}
6767

68+
func testGetAPIWithQueryParamsSuccess() {
69+
signIn(username: user1.username, password: user1.password)
70+
let completeInvoked = expectation(description: "request completed")
71+
let request = RESTRequest(path: "/items",
72+
queryParameters: [
73+
"user": "[email protected]",
74+
"created": "2021-06-18T09:00:00Z"
75+
])
76+
_ = Amplify.API.get(request: request) { event in
77+
switch event {
78+
case .success(let data):
79+
let result = String(decoding: data, as: UTF8.self)
80+
print(result)
81+
completeInvoked.fulfill()
82+
case .failure(let error):
83+
XCTFail("Unexpected .failed event: \(error)")
84+
}
85+
}
86+
87+
wait(for: [completeInvoked], timeout: TestCommonConstants.networkTimeout)
88+
}
89+
90+
func testGetAPIWithEncodedQueryParamsSuccess() {
91+
signIn(username: user1.username, password: user1.password)
92+
let completeInvoked = expectation(description: "request completed")
93+
let request = RESTRequest(path: "/items",
94+
queryParameters: [
95+
"user": "hello%40email.com",
96+
"created": "2021-06-18T09%3A00%3A00Z"
97+
])
98+
_ = Amplify.API.get(request: request) { event in
99+
switch event {
100+
case .success(let data):
101+
let result = String(decoding: data, as: UTF8.self)
102+
print(result)
103+
completeInvoked.fulfill()
104+
case .failure(let error):
105+
XCTFail("Unexpected .failed event: \(error)")
106+
}
107+
}
108+
109+
wait(for: [completeInvoked], timeout: TestCommonConstants.networkTimeout)
110+
}
111+
68112
func testGetAPIFailedWithSignedOutError() {
69113
let failedInvoked = expectation(description: "request failed")
70114
let request = RESTRequest(path: "/items")

‎AmplifyPlugins/API/AWSAPICategoryPluginTests/Support/Utils/RESTRequestUtilsTests.swift‎

Lines changed: 100 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,107 @@
66
//
77

88
import XCTest
9+
@testable import Amplify
10+
@testable import AWSAPICategoryPlugin
911

1012
class RESTRequestUtilsTests: XCTestCase {
11-
func testClassMustNotBeEmptyOrSwiftFormatWillCrash() {
12-
//TODO implement code
13+
private struct ConstructURLTestCase {
14+
let baseURL: URL
15+
let path: String?
16+
let queryParameters: [String: String]?
17+
let expectedParameters: [String: String]?
18+
19+
init(_ url: String,
20+
_ path: String?,
21+
_ params: [String: String]?,
22+
expectedParameters: [String: String]?) {
23+
self.baseURL = URL(string: url)!
24+
self.path = path
25+
self.queryParameters = params
26+
self.expectedParameters = expectedParameters
27+
}
28+
}
29+
30+
private func assertQueryParameters(testCase: Int, withURL url: URL, expected: [String: String]?) throws {
31+
var queryParams: [String: String] = [:]
32+
url.query?.split(separator: "&").forEach {
33+
let components = $0.split(separator: "=")
34+
if let name = components.first, let value = components.last {
35+
queryParams[String(name)] = String(value)
36+
}
37+
}
38+
39+
guard let expected = expected else {
40+
XCTAssertTrue(queryParams.isEmpty,
41+
"Test \(testCase): Unexpected query items found \(queryParams)")
42+
return
43+
}
44+
XCTAssertEqual(queryParams, expected, "Test \(testCase): query params mismatch")
45+
}
46+
47+
func testConstructURL() throws {
48+
let baseURL = "https://aws.amazon.com"
49+
let path = "/projects"
50+
let testCases: [ConstructURLTestCase] = [
51+
ConstructURLTestCase(baseURL,
52+
path,
53+
["author": "[email protected]"],
54+
expectedParameters: ["author": "john%40email.com"]),
55+
ConstructURLTestCase(baseURL,
56+
path,
57+
["created": "2021-06-18T09:00:00Z"],
58+
expectedParameters: ["created": "2021-06-18T09%3A00%3A00Z"]),
59+
ConstructURLTestCase(baseURL,
60+
path,
61+
[
62+
"created": "2021-06-18T09:00:00Z",
63+
"param1": "query!",
64+
"param2": "!*';:@&=+$,/?%#[]()\\"],
65+
expectedParameters: [
66+
"created": "2021-06-18T09%3A00%3A00Z",
67+
"param1": "query%21",
68+
"param2": "%21%2A%27%3B%3A%40%26%3D%2B%24%2C%2F%3F%25%23%5B%5D%28%29%5C"]),
69+
ConstructURLTestCase(baseURL,
70+
path,
71+
nil,
72+
expectedParameters: nil),
73+
ConstructURLTestCase(baseURL, nil, nil, expectedParameters: nil),
74+
ConstructURLTestCase(baseURL,
75+
path,
76+
["author": "john%40email.com"],
77+
expectedParameters: ["author": "john%40email.com"])
78+
]
79+
for (index, test) in testCases.enumerated() {
80+
let resultURL = try RESTOperationRequestUtils.constructURL(
81+
for: test.baseURL,
82+
with: test.path,
83+
with: test.queryParameters)
84+
try assertQueryParameters(testCase: index, withURL: resultURL, expected: test.expectedParameters)
85+
}
86+
}
87+
88+
func testConstructURLRequest() throws {
89+
let baseURL = URL(string: "https://aws.amazon.com")!
90+
let url = try RESTOperationRequestUtils.constructURL(for: baseURL, with: "/projects", with: nil)
91+
let urlRequest = RESTOperationRequestUtils.constructURLRequest(with: url,
92+
operationType: .get,
93+
headers: nil,
94+
requestPayload: nil)
95+
96+
XCTAssertEqual(urlRequest.httpMethod, RESTOperationType.get.rawValue)
97+
98+
// a REST operation request should always have at least a "content-type" header
99+
XCTAssertFalse(urlRequest.allHTTPHeaderFields!.isEmpty)
100+
}
101+
102+
func testConstructURLRequestFailsWithInvalidQueryParams() throws {
103+
let baseURL = URL(string: "https://aws.amazon.com")!
104+
let paramValue = String(
105+
bytes: [0xd8, 0x00] as [UInt8],
106+
encoding: String.Encoding.utf16BigEndian)!
107+
let invalidQueryParams: [String: String] = ["param": paramValue]
108+
XCTAssertThrowsError(try RESTOperationRequestUtils.constructURL(for: baseURL,
109+
with: "/projects",
110+
with: invalidQueryParams))
13111
}
14112
}

0 commit comments

Comments
 (0)