Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flask API passed urlencoded ampersand in query string works on local but not through API Gateway via Zappa #1227

Closed
atrojanowski opened this issue Mar 17, 2023 · 8 comments
Labels
bug Something isn't working next-release-candidate

Comments

@atrojanowski
Copy link

Context

Apologies if I've missed something simple. I'm trying to handle cases where some query parameters have an encoded ampersand (%26) in them. I can run the flask locally on my development machine and it works as expected but when I deploy via zappa to AWS using the API Gateway it seems the %26 is converted back to '&' and passed to the Lambda function so that when I try to use response.query_string or response.args the query parameter value is not passed correctly.

I have created a very simple flask example to show this behavior.

Expected Behavior

when a query string like ?test=M%26M is passed to the API gateway url, I expect

  • response.query_string to be b'test=M%26M'
  • response.args to be ImmutableMultiDict([('test', 'M&M')])
  • response.args.get('test') to be M&M

The expected behavior is the behavior I see when running my flask app via my local machine:

% python flaskdemo.py
 * Serving Flask app 'flaskdemo'
 * Debug mode: off
WARNING: This is a development server. Do not use it in a production deployment. Use a production WSGI server instead.
 * Running on http://127.0.0.1:5000
Press CTRL+C to quit
b'test=M%26M'
ImmutableMultiDict([('test', 'M&M')])
M&M
127.0.0.1 - - [17/Mar/2023 17:11:12] "POST /?test=M%26M HTTP/1.1" 200 -
b'test=M%26M'
ImmutableMultiDict([('test', 'M&M')])
M&M
127.0.0.1 - - [17/Mar/2023 17:11:16] "GET /?test=M%26M HTTP/1.1" 200 -

Actual Behavior

response.query_string is b'test=M&M' and response.args is { "M": "", "test": "M" }

Here are some cloudfront logs:

| 1679087710161 | b'test=M&M'                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                             |
| 1679087710161 | ImmutableMultiDict([('test', 'M'), ('M', '')])                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                          |
| 1679087710161 | M                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                       |
| 1679087710161 | [INFO] 2023-03-17T21:15:10.161Z cd12139e-ad29-46b8-bf3a-aec755a7f559 my_ip - - [17/Mar/2023:21:15:10 +0000] "GET /?test=M&M HTTP/1.1" 200 20 "" "PostmanRuntime/7.31.1" 0/0.9700000000000001                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                   |
| 1679087713691 | M                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                       |
| 1679087713691 | ImmutableMultiDict([('test', 'M'), ('M', '')])                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                          |
| 1679087713691 | b'test=M&M'                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                             |
| 1679087713691 | [INFO] 2023-03-17T21:15:13.689Z 3d350b64-c4f3-4adc-a6cb-5b0002239dc7 my_ip - - [17/Mar/2023:21:15:13 +0000] "POST /?test=M&M HTTP/1.1" 200 20 "" "PostmanRuntime/7.31.1" 0/0.861                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                               |

Possible Fix

My only thought is that there is some API Gateway (and by extension, Zappa) setting to stop this from happening.

Steps to Reproduce

  1. see flaskdemo.py, zappa_settings.json, and pip freeze in the Environment section
  2. deploy app with zappa deploy test
  3. send GET request to the API Gateway URL with this query string:
    https://xxxxxxxxx.execute-api.us-west-2.amazonaws.com/test/?test=M%26M

Your Environment

  • Zappa version used: 0.56.1
  • Operating System and Python version: Mac Ventura 13.2.1 & Python 3.8.16
  • The output of pip freeze: (I created a new virtual env and only installed flask and zappa)
% pip freeze
argcomplete==2.1.2
boto3==1.26.94
botocore==1.29.94
certifi==2022.12.7
cfn-flip==1.3.0
charset-normalizer==3.1.0
click==8.1.3
durationpy==0.5
Flask==2.2.3
hjson==3.1.0
idna==3.4
importlib-metadata==6.0.0
itsdangerous==2.1.2
Jinja2==3.1.2
jmespath==1.0.1
kappa==0.6.0
MarkupSafe==2.1.2
placebo==0.9.0
python-dateutil==2.8.2
python-slugify==8.0.1
PyYAML==6.0
requests==2.28.2
s3transfer==0.6.0
six==1.16.0
text-unidecode==1.3
toml==0.10.2
tqdm==4.65.0
troposphere==4.3.2
urllib3==1.26.15
Werkzeug==2.2.3
zappa==0.56.1
zipp==3.15.0
  • Link to your project (optional):
% more flaskdemo.py
from flask import Flask, jsonify, request

app = Flask(__name__)

@app.route('/', methods = ['GET', 'POST'])
def home():
    print(request.query_string)
    print(request.args)
    print(request.args.get('test'))
    return jsonify(request.args)

if __name__ == '__main__':
    app.run()
  • Your zappa_settings.json:
% more zappa_settings.json
{
  "test": {
    "apigateway_enabled": true,
    "app_function": "flaskdemo.app",
    "aws_region": "us-west-2",
    "project_name": "flaskdemo",
    "runtime": "python3.8",
    "s3_bucket": "<removed>"
  }
}
@atrojanowski
Copy link
Author

I believe that the url_unquote line in the Zappa codebase is causing the issue I'm seeing:

    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 ""
    query_string = urls.url_unquote(query_string)

I noticed that the event being passed to the Lambda function had the correct query string info:
'multiValueQueryStringParameters': {'test': ['M&M']}

so I did a quick python test:

from urllib.parse import urlencode
from werkzeug import urls

multiValueQueryStringParameters = {'test': ['M&M']}
query_string_original = urlencode(multiValueQueryStringParameters, doseq=True)
query_string_after = urls.url_unquote(query_string_original)

print(multiValueQueryStringParameters)
print(query_string_original)
print(query_string_after)

Results:

{'test': ['M&M']}
test=M%26M
test=M&M

AWS Results:

[1679331072390] {'query_string': b'test=M%26M', 'args': ImmutableMultiDict([('test', 'M&M')]), 'test': 'M&M'}
[1679331072391] [INFO] 2023-03-20T16:51:12.391Z 5848b702-5cf4-4032-8c2a-e4bf14efa1d9 my_ip - - [20/Mar/2023:16:51:12 +0000] "GET /?test=M%26M HTTP/1.1" 200 20 "" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/111.0.0.0 Safari/537.36" 0/1.0399999999999998

I'm not sure what the intent of that line is though (may cause other issues without that).

@dotarr
Copy link

dotarr commented Mar 20, 2023

I was about to create a similar ticket (it looks like we discovered and investigated in parallel), although mine was to do with '+' in url params, it's the same issue though, so I'll add my findings here, I tried ampersand and indeed it behaves the same way.

I also think the problematic line is the unquote. I cloned the Zappa repository to a local directory and pip installed with -e in my virtualenv so that I could edit the library and upload a modified version to allow for debug/modifications when making requests to the version of my API in AWS.

I modified to:

    if "multiValueQueryStringParameters" in event_info:
        query = event_info["multiValueQueryStringParameters"]
        query_string = urlencode(query, doseq=True) if query else ""
        logger.info(f"{query=} {query_string=}")
    else:
        query = event_info.get("queryStringParameters", {})
        query_string = urlencode(query) if query else ""
    query_string = urls.url_unquote(query_string)
    logger.info(f"{query_string=}")

And also added this at L590 of handler.py

logger.info(f"{environ['QUERY_STRING']=} {environ['lambda.event']['pathParameters']=} {environ['lambda.event']['queryStringParameters']=} {environ['lambda.event']['multiValueQueryStringParameters']=}")

So let's send https://myurl/endpoint?test=hello%2Bm%2Ete%26how%26are%26you

[2023-03-20 18:40:56,857] INFO wsgi::root:create_wsgi_request - query={'test': ['hello+m.te&how&are&you']} query_string='test=hello%2Bm.te%26how%26are%26you'
[2023-03-20 18:40:56,857] INFO wsgi::root:create_wsgi_request - query_string='test=hello+m.te&how&are&you'
[2023-03-20 18:40:56,857] INFO handler::root:handler - environ['QUERY_STRING']='test=hello+m.te&how&are&you' environ['lambda.event']['pathParameters']={'proxy': 'api/v1/status'} environ['lambda.event']['queryStringParameters']={'test': 'hello+m.te&how&are&you'} environ['lambda.event']['multiValueQueryStringParameters']={'test': ['hello+m.te&how&are&you']}

This confuses my API (it's returning the request parameters as a response), and breaks the ampersand values as expected, it also treats '+' as ' '.

  "params": {
    "are": "",
    "how": "",
    "test": "hello m.te",
    "you": ""
  }

If I comment out query_string = urls.url_unquote(query_string):

[2023-03-20 18:50:36,615] INFO wsgi::root:create_wsgi_request - query={'test': ['hello+m.te&how&are&you']} query_string='test=hello%2Bm.te%26how%26are%26you'
[2023-03-20 18:50:36,615] INFO wsgi::root:create_wsgi_request - query_string='test=hello%2Bm.te%26how%26are%26you'
[2023-03-20 18:50:36,615] INFO handler::root:handler - environ['QUERY_STRING']='test=hello%2Bm.te%26how%26are%26you' environ['lambda.event']['pathParameters']={'proxy': 'api/v1/status'} environ['lambda.event']['queryStringParameters']={'test': 'hello+m.te&how&are&you'} environ['lambda.event']['multiValueQueryStringParameters']={'test': ['hello+m.te&how&are&you']}

And the API returns:

  "params": {
    "test": "hello+m.te&how&are&youaaa"
  }

If I call the local version of my Flask API, it seems Werkzeug is pretty unfussy about whether URL's are encoded or contain raw values for most characters (e.g. escaping '!' to '%21') , but of course '+' and '&' carry special meaning in a URL which Werkzeug needs to strictly honour.

I don't know exactly what Werkzeug/Zappa expects (or what is in front of these when running the flask server in development mode), but after some playing with both local and lamda requests, it seems that it should be given the escaped version. I don't know if this will have wider implications for other wsgi receivers (e.g. Django)?

@dacot11
Copy link

dacot11 commented Mar 28, 2023

I see the same behavior, the query string is decoded when it gets to my app and therefore parameters that contain characters like '&' cannot get properly parsed.

Example

URL: http://myapp.com?query=Jane%26John

Expected query parameters

{"query": "Jane&John"}

Actual query parameters

{"query": "Jane", "John": ""}

Like @atrojanowski mentioned above it seems to be because of url_unquote

@tommie-lie
Copy link

Ran into the same issue (with a %-encoded + in my case). The bug was itnroduced in 62a53ed.

@monkut
Copy link
Collaborator

monkut commented Aug 16, 2023

From zappa's point of view, it is only preparing the value for the "QUERY_STRING" key for the environ created for wsgi.

In the current zappa implementation, with unquote, the "QUERY_STRING" value is assembled from the lambda event "queryStringParameters"/"multiValueQueryStringParameters".

So given the example:

URL: http://myapp.com/?query=Jane%26John&otherquery=B

The lambda event is expected to contain:

{
    "queryStringParameters": {
        "query": "Jane%26John",
        "otherquery": "B"
    }
}

And, as I understand, should be converted to the raw query string value for "QUERY_STRING" as:

query=Jane%26John&otherquery=B

However, as @atrojanowski mentioned, it was observed that the lambda event provides the unencoded string as the value in "queryStringParameters"/"multiValueQueryStringParameters". If that's assured, then it would be unexpected that we get an encoded value that would need to be "re-encoded" in order to preserve the "encoded" value.

Unfortunately, I couldn't find any clear documentation on the content of the apigateway event values.
https://docs.aws.amazon.com/lambda/latest/dg/services-apigateway.html#apigateway-example-event

In anycase, if assured that the "queryStringParameters"/"multiValueQueryStringParameters" are encoded, the suggestion in this issue appears to be correct, that we don't need the addtional query_string = unquote(query_string) call.

@monkut
Copy link
Collaborator

monkut commented Aug 16, 2023

@choich @jcunhafonte

I know this is an old zappa issue that you requested, but do you remember why you needed the QUERY_STRING to be unencoded? added in 62a53ed

We supported this Miserlou/Zappa#2134 / #879 in version 0.56.0, but this issue raised seems to suggest that it should not have been introduced.

It appears that this testcase describes the desire of introducing unquote():

62a53ed#diff-6337f0a34289544e437f2604c7c78f4455c4e211183b8bf450df853e0cff9efbR2735-R2746

Where {"a": "A,B", "b": "C#D"}, is expected to be passed to "QUERY_STRING" as, ""a=A,B&b=C#D".

Without unquote() "QUERY_STRING" will be, "a=A%2CB&b=C%23D".

Now, there is a "safe" parameter we can pass to urlencode to ignore ",#", but it would appear that it's correct that these are encoded. However, while the developer may need to perform unquote() on the resulting QUERY_STRING values, it seems that it's not wrong to have these values encoded.

@monkut
Copy link
Collaborator

monkut commented Aug 16, 2023

https://en.wikipedia.org/wiki/URL_encoding

When a character from the reserved set (a "reserved character") has a special meaning (a "reserved purpose") in a certain context, and a URI scheme says that it is necessary to use that character for some other purpose, then the character must be percent-encoded.

Since "#" and "," are in the reserved character set they should be percent encoded.
Assuming that values given to "queryStringParameters"/"multiValueQueryStringParameters" are the querystring values and not a value external to the querystring parameters, it appears that these values should be percent encoded.

@monkut
Copy link
Collaborator

monkut commented Mar 13, 2024

merged and released in 0.58.0

closing

@monkut monkut closed this as completed Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working next-release-candidate
Projects
None yet
Development

No branches or pull requests

5 participants