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

monkut · web-flow · commit bad24ddfa1d8 · 2022-11-11T16:46:00.000+09:00
* 🔧 migrate #971 to lastest master * 🎨 run black/isort * ♻️ refactor to allow for other binary ignore types based on mimetype. (currently openapi schema can't be passed as text. * 🎨 run black/fix flake8 * 🔧 add EXCEPTION_HANDLER setting * 🐛 fix zappa_returndict["body"] assignment * 📝 add temp debug info * 🔥 delete unnecessary print statements * ♻️ Update comments and minor refactor for clarity * ♻️ refactor for ease of testing and clarity * 🎨 fix flake8 * ✨ add `additional_text_mimetypes` setting ✅ add testcases for additional_text_mimetypes handling * 🔧 Expand default text mimetypes mentioned in #1023 ♻️ define "DEFAULT_TEXT_MIMETYPES" and move to utilities.py * 🎨 run black/isort * 🎨 run black/isort * 🎨 remove unnecesasry comment (black now reformats code) 🎨 change commented lines to docstring for test app
diff --git a/README.md b/README.md
@@ -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
diff --git a/test_settings.json b/test_settings.json
@@ -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"]
+    }
 }
diff --git a/tests/test_bad_additional_text_mimetypes_settings.json b/tests/test_bad_additional_text_mimetypes_settings.json
@@ -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"]
+    }
+}
diff --git a/tests/test_binary_support_additional_text_mimetypes_settings.py b/tests/test_binary_support_additional_text_mimetypes_settings.py
@@ -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"]
diff --git a/tests/test_binary_support_settings.py b/tests/test_binary_support_settings.py
@@ -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
diff --git a/tests/test_handler.py b/tests/test_handler.py
@@ -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
@@ -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
diff --git a/tests/test_wsgi_binary_support_app.py b/tests/test_wsgi_binary_support_app.py
@@ -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",
+    )
diff --git a/tests/tests.py b/tests/tests.py
@@ -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.
diff --git a/tests/utils.py b/tests/utils.py
@@ -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")
 
 
@@ -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"])
diff --git a/zappa/cli.py b/zappa/cli.py
diff --git a/zappa/handler.py b/zappa/handler.py
diff --git a/zappa/utilities.py b/zappa/utilities.py

Original file line number	Diff line number	Diff line change
`@@ -854,6 +854,7 @@ to change Zappa's behavior. Use these at your own risk!`
`854`	`854`	```javascript
`855`	`855`	`{`
`856`	`856`	`"dev": {`
	`857`	`+ "additional_text_mimetypes": [], // allows you to provide additional mimetypes to be handled as text when binary_support is true.`
`857`	`858`	`"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.`
`858`	`859`	`"alb_vpc_config": {`
`859`	`860`	`"CertificateArn": "your_acm_certificate_arn", // ACM certificate ARN for ALB`