Repository: adamchainz/apig-wsgi

Issue:
QueryStringParameters encoding issue
Hi,

I suspect an issue while providing queryParameters.

When performing a query to my API using <API_ROOT>/logs?start_time=2020-10-12T14:00:00%2B02:00&end_time=2020-10-12T15:00:00%2B02:00

I get my query string values in Flask endpoint as:
start_time = "2020-10-12T14:00:00 02:00"
end_time = "2020-10-12T15:00:00 02:00"

and it should be :
start_time = "2020-10-12T14:00:00+02:00"
end_time = "2020-10-12T15:00:00+02:00"

Note: The "+" has been removed

I think this is because API Gateway is providing queryStringParameters decoded and your are not encoding them back to store them in environ["QUERY_STRING"].
If we look to what other WSGI adapters (like Zappa) are doing:
```
       if 'multiValueQueryStringParameters' in event_info:
            query = event_info['multiValueQueryStringParameters']
            query_string = urlencode(query, doseq=True) if query else ''
        else:
            query = event_info.get('queryStringParameters', {})
            query_string = urlencode(query) if query else ''
```

Hope it helps.

Discussion:

Patch:
diff --git a/HISTORY.rst b/HISTORY.rst
index 724abe9..f4bc806 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -3,6 +3,9 @@ History
 =======
 
 * Support Python 3.9.
+* Fix query string parameter encoding so that symbols are correctly re-encoded
+  for WSGI, for API Gateway format version 1
+  (`Issue #186 <https://github.com/adamchainz/apig-wsgi/pull/186>`__).
 
 2.9.0 (2020-10-12)
 ------------------
@@ -11,7 +14,8 @@ History
 * Always send ``isBase64Encoded`` in responses, as per the AWS documentation.
 * Support `format version
   2 <https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-lambda.html>`,
-  which was introduced by API Gateway for “HTTP API's”.
+  which was introduced by API Gateway for “HTTP API's”
+  (`Issue #124 <https://github.com/adamchainz/apig-wsgi/pull/124>`__)..
 * ``binary_support`` now defaults to ``None``, which means that it will
   automatically enable binary support for format version 2 events.
 
diff --git a/src/apig_wsgi.py b/src/apig_wsgi.py
index a261c38..424d008 100644
--- a/src/apig_wsgi.py
+++ b/src/apig_wsgi.py
@@ -2,6 +2,7 @@ import sys
 from base64 import b64decode, b64encode
 from collections import defaultdict
 from io import BytesIO
+from urllib.parse import urlencode
 
 __all__ = ("make_lambda_handler",)
 
@@ -87,14 +88,13 @@ def get_environ_v1(event, context, binary_support):
 
     # Multi-value query strings need explicit activation on ALB
     if "multiValueQueryStringParameters" in event:
-        # may be None when testing on console
-        multi_params = event["multiValueQueryStringParameters"] or {}
+        environ["QUERY_STRING"] = urlencode(
+            # may be None when testing on console
+            event["multiValueQueryStringParameters"] or (),
+            doseq=True,
+        )
     else:
-        single_params = event.get("queryStringParameters") or {}
-        multi_params = {key: [value] for key, value in single_params.items()}
-    environ["QUERY_STRING"] = "&".join(
-        "{}={}".format(key, val) for (key, vals) in multi_params.items() for val in vals
-    )
+        environ["QUERY_STRING"] = urlencode(event.get("queryStringParameters") or ())
 
     # Multi-value headers need explicit activation on ALB
     if "multiValueHeaders" in event:


Test patch:
diff --git a/tests/test_apig_wsgi.py b/tests/test_apig_wsgi.py
index 6eca17c..2c98ab4 100644
--- a/tests/test_apig_wsgi.py
+++ b/tests/test_apig_wsgi.py
@@ -328,19 +328,19 @@ class TestV1Events:
 
         assert simple_app.environ["QUERY_STRING"] == "foo=bar"
 
-    def test_querystring_encoding_value(self, simple_app):
-        event = make_v1_event(qs_params={"foo": ["a%20bar"]})
+    def test_querystring_encoding_plus_value(self, simple_app):
+        event = make_v1_event(qs_params={"a": ["b+c"]}, qs_params_multi=False)
 
         simple_app.handler(event, None)
 
-        assert simple_app.environ["QUERY_STRING"] == "foo=a%20bar"
+        assert simple_app.environ["QUERY_STRING"] == "a=b%2Bc"
 
-    def test_querystring_encoding_key(self, simple_app):
-        event = make_v1_event(qs_params={"a%20foo": ["bar"]})
+    def test_querystring_encoding_plus_key(self, simple_app):
+        event = make_v1_event(qs_params={"a+b": ["c"]}, qs_params_multi=False)
 
         simple_app.handler(event, None)
 
-        assert simple_app.environ["QUERY_STRING"] == "a%20foo=bar"
+        assert simple_app.environ["QUERY_STRING"] == "a%2Bb=c"
 
     def test_querystring_multi(self, simple_app):
         event = make_v1_event(qs_params={"foo": ["bar", "baz"]})
@@ -349,6 +349,20 @@ class TestV1Events:
 
         assert simple_app.environ["QUERY_STRING"] == "foo=bar&foo=baz"
 
+    def test_querystring_multi_encoding_plus_value(self, simple_app):
+        event = make_v1_event(qs_params={"a": ["b+c", "d"]})
+
+        simple_app.handler(event, None)
+
+        assert simple_app.environ["QUERY_STRING"] == "a=b%2Bc&a=d"
+
+    def test_querystring_multi_encoding_plus_key(self, simple_app):
+        event = make_v1_event(qs_params={"a+b": ["c"]})
+
+        simple_app.handler(event, None)
+
+        assert simple_app.environ["QUERY_STRING"] == "a%2Bb=c"
+
     def test_plain_header(self, simple_app):
         event = make_v1_event(headers={"Test-Header": ["foobar"]})