fix: url-encode OAuth2 query parameters in Android, Apple and Unity

mock-server/app/http.php

@@ -615,6 +615,31 @@
     ->action(function () {
     });
 
+// Query values must arrive intact: a raw `&`, `+` or `#` splits, decodes to a space or truncates them.
+App::get('/v1/mock/tests/general/query')
+    ->desc('Query Param')
+    ->groups(['mock'])
+    ->label('scope', 'public')
+    ->label('sdk.auth', [APP_AUTH_TYPE_SESSION, APP_AUTH_TYPE_KEY, APP_AUTH_TYPE_JWT])
+    ->label('sdk.namespace', 'general')
+    ->label('sdk.method', 'getQuery')
+    ->label('sdk.description', 'Mock a request with query values that need encoding.')
+    ->label('sdk.response.code', Response::STATUS_CODE_OK)
+    ->label('sdk.response.type', Response::CONTENT_TYPE_JSON)
+    ->label('sdk.response.model', Response::MODEL_MOCK)
+    ->label('sdk.mock', true)
+    ->param('value', '', new Text(100), 'Query value.')
+    ->param('values', [], new ArrayList(new Text(100), APP_LIMIT_ARRAY_PARAMS_SIZE), 'Query values.')
+    ->action(function (string $value, array $values) {
+        if ($value !== 'a&b+c#d=e%f') {
+            throw new Exception(Exception::GENERAL_MOCK, 'Query value was not encoded: ' . $value);
+        }
+
+        if ($values !== ['a&b', 'c+d#e']) {
+            throw new Exception(Exception::GENERAL_MOCK, 'Query values were not encoded: ' . \implode(',', $values));
+        }
+    });
+
 // Shared nullable-path e2e contract. Null is serialized literally by targets
 // that previously could not compile nullable string interpolation.
 foreach (['0', 'null'] as $id) {

templates/android/library/src/main/java/io/package/services/Service.kt.twig

@@ -116,11 +116,11 @@ class {{ service.name | caseUcfirst }}(client: Client) : Service(client) {
                 }
                 is List<*> -> {
                     (it.value as List<*>).forEach { v ->
-                        apiQuery.add("${it.key}[]=${v.toString()}")
+                        apiQuery.add("${Uri.encode(it.key)}[]=${Uri.encode(v.toString())}")
                     }
                 }
                 else -> {
-                    apiQuery.add("${it.key}=${it.value.toString()}")
+                    apiQuery.add("${Uri.encode(it.key)}=${Uri.encode(it.value.toString())}")
                 }
             }
         }

templates/apple/Sources/Client.swift.twig

@@ -268,43 +268,34 @@ open class Client {
     /// @return String
     ///
     open func parametersToQueryString(params: [String: Any?]) -> String {
-        var output: String = ""
-
-        func appendWhenNotLast(
-            _ index: Int, ofTotal count: Int, outerIndex: Int? = nil, outerCount: Int? = nil
-        ) {
-            if index != count - 1
-                || (outerIndex != nil
-                    && outerCount != nil
-                    && index == count - 1
-                    && outerIndex! != outerCount! - 1)
-            {
-                output += "&"
-            }
+        // RFC 3986 unreserved characters only, so `&`, `=`, `+` and `#` inside a value are escaped
+        let allowed = CharacterSet(
+            charactersIn: "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~"
+        )
+
+        func encode(_ value: Any) -> String {
+            return "\(value)".addingPercentEncoding(withAllowedCharacters: allowed) ?? ""
         }
 
-        for (parameterIndex, element) in params.enumerated() {
-            switch element.value {
+        var output: [String] = []
+
+        for (key, value) in params {
+            switch value {
             case nil:
-                break
-            case is [Any?]:
-                let list = element.value as! [Any?]
-                for (nestedIndex, item) in list.enumerated() {
-                    output += "\(element.key)[]=\(item!)"
-                    appendWhenNotLast(
-                        nestedIndex, ofTotal: list.count, outerIndex: parameterIndex,
-                        outerCount: params.count)
+                continue
+            case let list as [Any?]:
+                for item in list {
+                    guard let item = item else {
+                        continue
+                    }
+                    output.append("\(encode("\(key)[]"))=\(encode(item))")
                 }
-                appendWhenNotLast(parameterIndex, ofTotal: params.count)
             default:
-                output += "\(element.key)=\(element.value!)"
-                appendWhenNotLast(parameterIndex, ofTotal: params.count)
+                output.append("\(encode(key))=\(encode(value!))")
             }
         }
 
-        return output.addingPercentEncoding(
-            withAllowedCharacters: .urlHostAllowed
-        )?.replacingOccurrences(of: "+", with: "%2B") ?? ""  // since urlHostAllowed doesn't include +
+        return output.joined(separator: "&")
     }
 
     {%~ for clientMethodName, clientMethods in clientMethods %}

templates/dotnet/Package/Extensions/Extensions.cs.twig

@@ -75,16 +75,16 @@ namespace {{ spec.info.title | caseUcfirst }}.Extensions
                     case IList list:
                         foreach (var item in list)
                         {
-                            query.Add($"{kvp.Key}[]={item}");
+                            query.Add($"{Uri.EscapeDataString(kvp.Key)}[]={Uri.EscapeDataString(item?.ToString() ?? string.Empty)}");
                         }
                         break;
                     default:
-                        query.Add($"{kvp.Key}={kvp.Value.ToString()}");
+                        query.Add($"{Uri.EscapeDataString(kvp.Key)}={Uri.EscapeDataString(kvp.Value.ToString() ?? string.Empty)}");
                         break;
                 }
             }
 
-            return Uri.EscapeUriString(string.Join("&", query));
+            return string.Join("&", query);
         }
 
         public static Dictionary<string, string> FromQueryString(this string queryString)

templates/swift/Sources/Client.swift.twig

@@ -171,43 +171,34 @@ open class Client {
     /// @return String
     ///
     open func parametersToQueryString(params: [String: Any?]) -> String {
-        var output: String = ""
-
-        func appendWhenNotLast(
-            _ index: Int, ofTotal count: Int, outerIndex: Int? = nil, outerCount: Int? = nil
-        ) {
-            if index != count - 1
-                || (outerIndex != nil
-                    && outerCount != nil
-                    && index == count - 1
-                    && outerIndex! != outerCount! - 1)
-            {
-                output += "&"
-            }
+        // RFC 3986 unreserved characters only, so `&`, `=`, `+` and `#` inside a value are escaped
+        let allowed = CharacterSet(
+            charactersIn: "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~"
+        )
+
+        func encode(_ value: Any) -> String {
+            return "\(value)".addingPercentEncoding(withAllowedCharacters: allowed) ?? ""
         }
 
-        for (parameterIndex, element) in params.enumerated() {
-            switch element.value {
+        var output: [String] = []
+
+        for (key, value) in params {
+            switch value {
             case nil:
-                break
-            case is [Any?]:
-                let list = element.value as! [Any?]
-                for (nestedIndex, item) in list.enumerated() {
-                    output += "\(element.key)[]=\(item!)"
-                    appendWhenNotLast(
-                        nestedIndex, ofTotal: list.count, outerIndex: parameterIndex,
-                        outerCount: params.count)
+                continue
+            case let list as [Any?]:
+                for item in list {
+                    guard let item = item else {
+                        continue
+                    }
+                    output.append("\(encode("\(key)[]"))=\(encode(item))")
                 }
-                appendWhenNotLast(parameterIndex, ofTotal: params.count)
             default:
-                output += "\(element.key)=\(element.value!)"
-                appendWhenNotLast(parameterIndex, ofTotal: params.count)
+                output.append("\(encode(key))=\(encode(value!))")
             }
         }
 
-        return output.addingPercentEncoding(
-            withAllowedCharacters: .urlHostAllowed
-        )?.replacingOccurrences(of: "+", with: "%2B") ?? ""  // since urlHostAllowed doesn't include +
+        return output.joined(separator: "&")
     }
 
     {%~ for clientMethodName, clientMethods in clientMethods %}

tests/e2e/AppleSwift61Test.php

@@ -39,6 +39,7 @@ final class AppleSwift61Test extends Base
         ...Base::GENERAL_RESPONSES,
         ...Base::PATH_VALIDATION_RESPONSES,
         ...Base::NULL_PATH_RESPONSE,
+        ...Base::QUERY_ENCODING_RESPONSES,
         ...Base::TEXT_RESPONSES,
         ...Base::TEXT_UPLOAD_RESPONSES,
         ...Base::OPTIONAL_ATTACHMENT_RESPONSES,

tests/e2e/Base.php

@@ -68,6 +68,11 @@ abstract class Base extends TestCase
         'GET:/v1/mock/tests/general/path-validation/null/0:passed',
     ];
 
+    // Query values containing `&`, `+` and `#` reach the server unchanged.
+    protected const QUERY_ENCODING_RESPONSES = [
+        'GET:/v1/mock/tests/general/query:passed',
+    ];
+
     protected const OAUTH_RESPONSES = [
         'https://localhost?code=abcdef&state=123456',
     ];

tests/e2e/DotNet60Test.php

@@ -38,6 +38,7 @@ final class DotNet60Test extends Base
         ...Base::BAR_RESPONSES,
         ...Base::GENERAL_RESPONSES,
         ...Base::PATH_VALIDATION_RESPONSES,
+        ...Base::QUERY_ENCODING_RESPONSES,
         ...Base::TEXT_RESPONSES,
         ...Base::TEXT_UPLOAD_RESPONSES,
         ...Base::OPTIONAL_ATTACHMENT_RESPONSES,

tests/e2e/DotNet80Test.php

@@ -38,6 +38,7 @@ final class DotNet80Test extends Base
         ...Base::BAR_RESPONSES,
         ...Base::GENERAL_RESPONSES,
         ...Base::PATH_VALIDATION_RESPONSES,
+        ...Base::QUERY_ENCODING_RESPONSES,
         ...Base::TEXT_RESPONSES,
         ...Base::TEXT_UPLOAD_RESPONSES,
         ...Base::OPTIONAL_ATTACHMENT_RESPONSES,

tests/e2e/DotNet90Test.php

@@ -38,6 +38,7 @@ final class DotNet90Test extends Base
         ...Base::BAR_RESPONSES,
         ...Base::GENERAL_RESPONSES,
         ...Base::PATH_VALIDATION_RESPONSES,
+        ...Base::QUERY_ENCODING_RESPONSES,
         ...Base::TEXT_RESPONSES,
         ...Base::TEXT_UPLOAD_RESPONSES,
         ...Base::OPTIONAL_ATTACHMENT_RESPONSES,

tests/e2e/Swift61Test.php

@@ -39,6 +39,7 @@ final class Swift61Test extends Base
         ...Base::GENERAL_RESPONSES,
         ...Base::PATH_VALIDATION_RESPONSES,
         ...Base::NULL_PATH_RESPONSE,
+        ...Base::QUERY_ENCODING_RESPONSES,
         ...Base::TEXT_RESPONSES,
         ...Base::TEXT_UPLOAD_RESPONSES,
         ...Base::OPTIONAL_ATTACHMENT_RESPONSES,

tests/e2e/Unity2021Test.php

@@ -94,6 +94,7 @@ public function tearDown(): void
         ...Base::BAR_RESPONSES,
         ...Base::GENERAL_RESPONSES,
         ...Base::PATH_VALIDATION_RESPONSES,
+        ...Base::QUERY_ENCODING_RESPONSES,
         ...Base::TEXT_RESPONSES,
         ...Base::TEXT_UPLOAD_RESPONSES,
         ...Base::OPTIONAL_ATTACHMENT_RESPONSES,

tests/e2e/languages/apple/Tests.swift

@@ -149,6 +149,7 @@ class Tests: XCTestCase {
         }
         print(try await general.validatePath(plain: "0", id: "0").result)
         print(try await general.validatePath(plain: "0", id: nil).result)
+        print(try await general.getQuery(value: "a&b+c#d=e%f", values: ["a&b", "c+d#e"]).result)
 
         let zone: String = try await Plaintext(client).getZone()
         print(zone)

tests/e2e/languages/dotnet/Tests.cs

@@ -85,6 +85,7 @@ public async Task Test1()
                 }
             }
             TestContext.WriteLine((await general.ValidatePath("0", "0")).Result);
+            TestContext.WriteLine((await general.GetQuery("a&b+c#d=e%f", new List<string>() { "a&b", "c+d#e" })).Result);
 
             string zone = await new Plaintext(client).GetZone();
             TestContext.WriteLine(zone);

tests/e2e/languages/swift/Tests.swift

@@ -90,6 +90,7 @@ class Tests: XCTestCase {
         }
         print(try await general.validatePath(plain: "0", id: "0").result)
         print(try await general.validatePath(plain: "0", id: nil).result)
+        print(try await general.getQuery(value: "a&b+c#d=e%f", values: ["a&b", "c+d#e"]).result)
 
         let zone: String = try await Plaintext(client).getZone()
         print(zone)

tests/e2e/languages/unity/Tests.cs

@@ -171,6 +171,7 @@ private async Task RunAsyncTest()
                 }
             }
             LogResult((await general.ValidatePath("0", "0")).Result);
+            LogResult((await general.GetQuery("a&b+c#d=e%f", new List<string>() { "a&b", "c+d#e" })).Result);
 
             string zone = await new Plaintext(client).GetZone();
             LogResult(zone);

tests/resources/spec-openapi3.json

@@ -2152,6 +2152,74 @@
         ]
       }
     },
+    "\/mock\/tests\/general\/query": {
+      "get": {
+        "summary": "Query Param",
+        "operationId": "generalGetQuery",
+        "tags": [
+          "general"
+        ],
+        "description": "",
+        "x-appwrite": {
+          "demo": "general\/get-query.md",
+          "rate-limit": 0,
+          "rate-time": 3600,
+          "rate-key": "url:{url},ip:{ip}",
+          "scope": "public",
+          "platforms": [
+            "client",
+            "server"
+          ],
+          "packaging": false,
+          "auth": {
+            "Project": []
+          }
+        },
+        "security": [
+          {
+            "Project": [],
+            "Key": [],
+            "JWT": []
+          }
+        ],
+        "responses": {
+          "200": {
+            "description": "Mock",
+            "content": {
+              "application\/json": {
+                "schema": {
+                  "$ref": "#\/components\/schemas\/mock"
+                }
+              }
+            }
+          }
+        },
+        "parameters": [
+          {
+            "name": "value",
+            "description": "Query value.",
+            "required": true,
+            "in": "query",
+            "schema": {
+              "type": "string",
+              "example": "a&b"
+            }
+          },
+          {
+            "name": "values",
+            "description": "Query values.",
+            "required": true,
+            "in": "query",
+            "schema": {
+              "type": "array",
+              "items": {
+                "type": "string"
+              }
+            }
+          }
+        ]
+      }
+    },
     "\/mock\/tests\/general\/path\/{pathId}": {
       "get": {
         "summary": "Path Param",