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

(#908) Update BINARY_SUPPORT to use Content-Encoding to identify if data is binary #1155

Merged
merged 22 commits into from
Nov 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f1881b6
:wrench: migrate https://github.com/zappa/Zappa/pull/971 to lastest m…
monkut Jul 28, 2022
19f74a9
:art: run black/isort
monkut Jul 28, 2022
d7fcee4
:recycle: refactor to allow for other binary ignore types based on mi…
monkut Jul 28, 2022
039cbfe
:art: run black/fix flake8
monkut Jul 28, 2022
bcdfbe4
:wrench: add EXCEPTION_HANDLER setting
monkut Jul 28, 2022
3f0d135
:bug: fix zappa_returndict["body"] assignment
monkut Jul 28, 2022
583cc4d
:pencil: add temp debug info
monkut Jul 28, 2022
20bd12f
:fire: delete unnecessary print statements
monkut Jul 28, 2022
2a6aacd
:recycle: Update comments and minor refactor for clarity
monkut Jul 28, 2022
153366d
:recycle: refactor for ease of testing and clarity
monkut Jul 29, 2022
4ddfaa5
:art: fix flake8
monkut Jul 29, 2022
0370119
:sparkles: add `additional_text_mimetypes` setting
monkut Aug 5, 2022
71c8aa3
:wrench: Expand default text mimetypes mentioned in https://github.co…
monkut Aug 12, 2022
48057fc
:art: run black/isort
monkut Aug 12, 2022
34e8755
Merge branch 'master' into feature/issue-908-update-binarysupport-han…
monkut Aug 21, 2022
7798bd5
Merge branch 'master' into feature/issue-908-update-binarysupport-han…
monkut Sep 1, 2022
cdb3337
Merge branch 'master' into feature/issue-908-update-binarysupport-han…
monkut Sep 27, 2022
a2370e9
Merge branch 'master' into feature/issue-908-update-binarysupport-han…
monkut Oct 22, 2022
58e221b
:art: run black/isort
monkut Oct 22, 2022
b1bde2a
Merge branch 'master' into feature/issue-908-update-binarysupport-han…
monkut Oct 22, 2022
63e8f57
Merge branch 'master' into feature/issue-908-update-binarysupport-han…
monkut Oct 25, 2022
15532f1
:art: remove unnecesasry comment (black now reformats code)
monkut Nov 7, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,7 @@ to change Zappa's behavior. Use these at your own risk!
```javascript
{
"dev": {
"additional_text_mimetypes": [], // allows you to provide additional mimetypes to be handled as text when binary_support is true.
"alb_enabled": false, // enable provisioning of application load balancing resources. If set to true, you _must_ fill out the alb_vpc_config option as well.
"alb_vpc_config": {
"CertificateArn": "your_acm_certificate_arn", // ACM certificate ARN for ALB
Expand Down
9 changes: 8 additions & 1 deletion test_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -120,5 +120,12 @@
"lambda_concurrency_enabled": {
"extends": "ttt888",
"lambda_concurrency": 6
}
},
"addtextmimetypes": {
"s3_bucket": "lmbda",
"app_function": "tests.test_app.hello_world",
"delete_local_zip": true,
"binary_support": true,
"additional_text_mimetypes": ["application/custommimetype"]
}
}
9 changes: 9 additions & 0 deletions tests/test_bad_additional_text_mimetypes_settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"nobinarysupport": {
"s3_bucket": "lmbda",
"app_function": "tests.test_app.hello_world",
"delete_local_zip": true,
"binary_support": false,
"additional_text_mimetypes": ["application/custommimetype"]
}
}
14 changes: 14 additions & 0 deletions tests/test_binary_support_additional_text_mimetypes_settings.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
API_STAGE = "dev"
APP_FUNCTION = "app"
APP_MODULE = "tests.test_wsgi_binary_support_app"
BINARY_SUPPORT = True
CONTEXT_HEADER_MAPPINGS = {}
DEBUG = "True"
DJANGO_SETTINGS = None
DOMAIN = "api.example.com"
ENVIRONMENT_VARIABLES = {}
LOG_LEVEL = "DEBUG"
PROJECT_NAME = "binary_support_settings"
COGNITO_TRIGGER_MAPPING = {}
EXCEPTION_HANDLER = None
ADDITIONAL_TEXT_MIMETYPES = ["application/vnd.oai.openapi"]
13 changes: 13 additions & 0 deletions tests/test_binary_support_settings.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
API_STAGE = "dev"
APP_FUNCTION = "app"
APP_MODULE = "tests.test_wsgi_binary_support_app"
BINARY_SUPPORT = True
CONTEXT_HEADER_MAPPINGS = {}
DEBUG = "True"
DJANGO_SETTINGS = None
DOMAIN = "api.example.com"
ENVIRONMENT_VARIABLES = {}
LOG_LEVEL = "DEBUG"
PROJECT_NAME = "binary_support_settings"
COGNITO_TRIGGER_MAPPING = {}
EXCEPTION_HANDLER = None
185 changes: 184 additions & 1 deletion tests/test_handler.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import sys
import unittest

from mock import Mock

from zappa.handler import LambdaHandler
from zappa.utilities import merge_headers

from .utils import is_base64


def no_args():
return
Expand Down Expand Up @@ -223,6 +224,188 @@ def test_exception_handler_on_web_request(self):
self.assertEqual(response["statusCode"], 500)
mocked_exception_handler.assert_called()

def test_wsgi_script_binary_support_with_content_encoding(self):
"""
Ensure that response body is base64 encoded when BINARY_SUPPORT is enabled and Content-Encoding header is present.
"""
lh = LambdaHandler("tests.test_binary_support_settings")

text_plain_event = {
"body": "",
"resource": "/{proxy+}",
"requestContext": {},
"queryStringParameters": {},
"headers": {
"Host": "1234567890.execute-api.us-east-1.amazonaws.com",
},
"pathParameters": {"proxy": "return/request/url"},
"httpMethod": "GET",
"stageVariables": {},
"path": "/content_encoding_header_json1",
}

# A likely scenario is that the application would be gzip compressing some json response. That's checked first.
response = lh.handler(text_plain_event, None)

self.assertEqual(response["statusCode"], 200)
self.assertIn("isBase64Encoded", response)
self.assertTrue(is_base64(response["body"]))

# We also verify that some unknown mimetype with a Content-Encoding also encodes to b64. This route serves
# bytes in the response.

text_arbitrary_event = {
**text_plain_event,
**{"path": "/content_encoding_header_textarbitrary1"},
}

response = lh.handler(text_arbitrary_event, None)

self.assertEqual(response["statusCode"], 200)
self.assertIn("isBase64Encoded", response)
self.assertTrue(is_base64(response["body"]))

# This route is similar to the above, but it serves its response as text and not bytes. That the response
# isn't bytes shouldn't matter because it still has a Content-Encoding header.

application_json_event = {
**text_plain_event,
**{"path": "/content_encoding_header_textarbitrary2"},
}

response = lh.handler(application_json_event, None)

self.assertEqual(response["statusCode"], 200)
self.assertIn("isBase64Encoded", response)
self.assertTrue(is_base64(response["body"]))

def test_wsgi_script_binary_support_without_content_encoding_edgecases(
self,
):
"""
Ensure zappa response bodies are NOT base64 encoded when BINARY_SUPPORT is enabled and the mimetype is "application/json" or starts with "text/".
"""

lh = LambdaHandler("tests.test_binary_support_settings")

text_plain_event = {
"body": "",
"resource": "/{proxy+}",
"requestContext": {},
"queryStringParameters": {},
"headers": {
"Host": "1234567890.execute-api.us-east-1.amazonaws.com",
},
"pathParameters": {"proxy": "return/request/url"},
"httpMethod": "GET",
"stageVariables": {},
"path": "/textplain_mimetype_response1",
}

for path in [
"/textplain_mimetype_response1", # text/plain mimetype should not be turned to base64
"/textarbitrary_mimetype_response1", # text/arbitrary mimetype should not be turned to base64
"/json_mimetype_response1", # application/json mimetype should not be turned to base64
]:
event = {**text_plain_event, "path": path}
response = lh.handler(event, None)

self.assertEqual(response["statusCode"], 200)
self.assertNotIn("isBase64Encoded", response)
self.assertFalse(is_base64(response["body"]))

def test_wsgi_script_binary_support_without_content_encoding(
self,
):
"""
Ensure zappa response bodies are base64 encoded when BINARY_SUPPORT is enabled and Content-Encoding is absent.
"""

lh = LambdaHandler("tests.test_binary_support_settings")

text_plain_event = {
"body": "",
"resource": "/{proxy+}",
"requestContext": {},
"queryStringParameters": {},
"headers": {
"Host": "1234567890.execute-api.us-east-1.amazonaws.com",
},
"pathParameters": {"proxy": "return/request/url"},
"httpMethod": "GET",
"stageVariables": {},
"path": "/textplain_mimetype_response1",
}

for path in [
"/arbitrarybinary_mimetype_response1",
"/arbitrarybinary_mimetype_response2",
]:
event = {**text_plain_event, "path": path}
response = lh.handler(event, None)

self.assertEqual(response["statusCode"], 200)
self.assertIn("isBase64Encoded", response)
self.assertTrue(is_base64(response["body"]))

def test_wsgi_script_binary_support_userdefined_additional_text_mimetypes__defined(
self,
):
"""
Ensure zappa response bodies are NOT base64 encoded when BINARY_SUPPORT is True, and additional_text_mimetypes are defined
"""
lh = LambdaHandler("tests.test_binary_support_additional_text_mimetypes_settings")
expected_additional_mimetypes = ["application/vnd.oai.openapi"]
self.assertEqual(lh.settings.ADDITIONAL_TEXT_MIMETYPES, expected_additional_mimetypes)

event = {
"body": "",
"resource": "/{proxy+}",
"requestContext": {},
"queryStringParameters": {},
"headers": {
"Host": "1234567890.execute-api.us-east-1.amazonaws.com",
},
"pathParameters": {"proxy": "return/request/url"},
"httpMethod": "GET",
"stageVariables": {},
"path": "/userdefined_additional_mimetype_response1",
}

response = lh.handler(event, None)

self.assertEqual(response["statusCode"], 200)
self.assertNotIn("isBase64Encoded", response)
self.assertFalse(is_base64(response["body"]))

def test_wsgi_script_binary_support_userdefined_additional_text_mimetypes__undefined(
self,
):
"""
Ensure zappa response bodies are base64 encoded when BINARY_SUPPORT is True and mimetype not defined in additional_text_mimetypes
"""
lh = LambdaHandler("tests.test_binary_support_settings")

event = {
"body": "",
"resource": "/{proxy+}",
"requestContext": {},
"queryStringParameters": {},
"headers": {
"Host": "1234567890.execute-api.us-east-1.amazonaws.com",
},
"pathParameters": {"proxy": "return/request/url"},
"httpMethod": "GET",
"stageVariables": {},
"path": "/userdefined_additional_mimetype_response1",
}

response = lh.handler(event, None)

self.assertEqual(response["statusCode"], 200)
self.assertIn("isBase64Encoded", response)
self.assertTrue(is_base64(response["body"]))

def test_wsgi_script_on_cognito_event_request(self):
"""
Ensure that requests sent by cognito behave sensibly
Expand Down
71 changes: 71 additions & 0 deletions tests/test_wsgi_binary_support_app.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
"""
This test application exists to confirm how Zappa handles WSGI application
_responses_ when Binary Support is enabled.
"""

import gzip
import json

from flask import Flask, Response

app = Flask(__name__)


@app.route("/textplain_mimetype_response1", methods=["GET"])
def text_mimetype_response_1():
return Response(response="OK", mimetype="text/plain")


@app.route("/textarbitrary_mimetype_response1", methods=["GET"])
def text_mimetype_response_2():
return Response(response="OK", mimetype="text/arbitary")


@app.route("/json_mimetype_response1", methods=["GET"])
def json_mimetype_response_1():
return Response(response=json.dumps({"some": "data"}), mimetype="application/json")


@app.route("/arbitrarybinary_mimetype_response1", methods=["GET"])
def arbitrary_mimetype_response_1():
return Response(response=b"some binary data", mimetype="arbitrary/binary_mimetype")


@app.route("/arbitrarybinary_mimetype_response2", methods=["GET"])
def arbitrary_mimetype_response_3():
return Response(response="doesnt_matter", mimetype="definitely_not_text")


@app.route("/content_encoding_header_json1", methods=["GET"])
def response_with_content_encoding_1():
return Response(
response=gzip.compress(json.dumps({"some": "data"}).encode()),
mimetype="application/json",
headers={"Content-Encoding": "gzip"},
)


@app.route("/content_encoding_header_textarbitrary1", methods=["GET"])
def response_with_content_encoding_2():
return Response(
response=b"OK",
mimetype="text/arbitrary",
headers={"Content-Encoding": "something_arbitrarily_binary"},
)


@app.route("/content_encoding_header_textarbitrary2", methods=["GET"])
def response_with_content_encoding_3():
return Response(
response="OK",
mimetype="text/arbitrary",
headers={"Content-Encoding": "with_content_type_but_not_bytes_response"},
)


@app.route("/userdefined_additional_mimetype_response1", methods=["GET"])
def response_with_userdefined_addtional_mimetype():
return Response(
response="OK",
mimetype="application/vnd.oai.openapi",
)
14 changes: 14 additions & 0 deletions tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1165,6 +1165,20 @@ def test_load_settings_toml(self):
zappa_cli.load_settings("tests/test_settings.toml")
self.assertEqual(False, zappa_cli.stage_config["touch"])

def test_load_settings_bad_additional_text_mimetypes(self):
zappa_cli = ZappaCLI()
zappa_cli.api_stage = "nobinarysupport"
with self.assertRaises(ClickException):
zappa_cli.load_settings("tests/test_bad_additional_text_mimetypes_settings.json")

def test_load_settings_additional_text_mimetypes(self):
zappa_cli = ZappaCLI()
zappa_cli.api_stage = "addtextmimetypes"
zappa_cli.load_settings("test_settings.json")
expected_additional_text_mimetypes = ["application/custommimetype"]
self.assertEqual(expected_additional_text_mimetypes, zappa_cli.stage_config["additional_text_mimetypes"])
self.assertEqual(True, zappa_cli.stage_config["binary_support"])

def test_settings_extension(self):
"""
Make sure Zappa uses settings in the proper order: JSON, TOML, YAML.
Expand Down
15 changes: 10 additions & 5 deletions tests/utils.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
import base64
import functools
import os
from collections import namedtuple
from contextlib import contextmanager
from io import IOBase as file

import boto3
import placebo
from mock import MagicMock, patch

try:
file
except NameError: # builtin 'file' was removed in Python 3
from io import IOBase as file

PLACEBO_DIR = os.path.join(os.path.dirname(__file__), "placebo")


Expand Down Expand Up @@ -75,6 +72,14 @@ def stub_open(*args, **kwargs):
yield mock_open, mock_file


def is_base64(test_string: str) -> bool:
# Taken from https://stackoverflow.com/a/45928164/3200002
try:
return base64.b64encode(base64.b64decode(test_string)).decode() == test_string
except Exception:
return False


def get_unsupported_sys_versioninfo() -> tuple:
"""Mock used to test the python unsupported version testcase"""
invalid_versioninfo = namedtuple("version_info", ["major", "minor", "micro", "releaselevel", "serial"])
Expand Down
Loading