From ff0add9e5f22c5a5e7e9123fcc34993bab0e9f63 Mon Sep 17 00:00:00 2001 From: Renzo Manganiello Date: Thu, 1 Apr 2021 13:46:55 -0300 Subject: [PATCH 1/4] Use importlib --- .gitignore | 3 ++ awslambdaric/bootstrap.py | 34 ++++---------- requirements/dev.txt | 2 +- tests/test_bootstrap.py | 98 ++++++++++++--------------------------- 4 files changed, 43 insertions(+), 94 deletions(-) diff --git a/.gitignore b/.gitignore index 165e345..116767e 100644 --- a/.gitignore +++ b/.gitignore @@ -143,3 +143,6 @@ dmypy.json # Cython debug symbols cython_debug/ + +# Test files generated +tmp*.py diff --git a/awslambdaric/bootstrap.py b/awslambdaric/bootstrap.py index 78e8393..d8053fe 100644 --- a/awslambdaric/bootstrap.py +++ b/awslambdaric/bootstrap.py @@ -2,23 +2,19 @@ Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved. """ +import importlib import json import logging import os import sys import time import traceback -import warnings from .lambda_context import LambdaContext from .lambda_runtime_client import LambdaRuntimeClient from .lambda_runtime_exception import FaultException from .lambda_runtime_marshaller import to_json -with warnings.catch_warnings(): - warnings.filterwarnings("ignore", category=DeprecationWarning) - import imp - ERROR_LOG_LINE_TERMINATE = "\r" ERROR_LOG_IDENT = "\u00a0" # NO-BREAK SPACE U+00A0 @@ -33,23 +29,14 @@ def _get_handler(handler): ) return make_fault_handler(fault) - file_handle, pathname, desc = None, None, None try: - # Recursively loading handler in nested directories - for segment in modname.split("."): - if pathname is not None: - pathname = [pathname] - file_handle, pathname, desc = imp.find_module(segment, pathname) - if file_handle is None: - module_type = desc[2] - if module_type == imp.C_BUILTIN: - fault = FaultException( - FaultException.BUILT_IN_MODULE_CONFLICT, - "Cannot use built-in module {} as a handler module".format(modname), - ) - request_handler = make_fault_handler(fault) - return request_handler - m = imp.load_module(modname, file_handle, pathname, desc) + if modname.split(".")[0] in sys.builtin_module_names: + fault = FaultException( + FaultException.BUILT_IN_MODULE_CONFLICT, + "Cannot use built-in module {} as a handler module".format(modname), + ) + return make_fault_handler(fault) + m = importlib.import_module(modname) except ImportError as e: fault = FaultException( FaultException.IMPORT_MODULE_ERROR, @@ -66,9 +53,6 @@ def _get_handler(handler): ) request_handler = make_fault_handler(fault) return request_handler - finally: - if file_handle is not None: - file_handle.close() try: request_handler = getattr(m, fname) @@ -402,7 +386,7 @@ def run(app_root, handler, lambda_runtime_api_addr): global _GLOBAL_AWS_REQUEST_ID request_handler = _get_handler(handler) - except Exception as e: + except Exception: error_result = build_fault_result(sys.exc_info(), None) log_error(error_result, log_sink) diff --git a/requirements/dev.txt b/requirements/dev.txt index 8c5add8..c432413 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -2,7 +2,7 @@ coverage>=4.4.0 flake8>=3.3.0 tox>=2.2.1 pytest-cov>=2.4.0 -pylint>=1.7.2,<2.0 +pylint>=1.7.2 black>=20.8b0 bandit>=1.6.2 diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py index 3d4775f..9663533 100644 --- a/tests/test_bootstrap.py +++ b/tests/test_bootstrap.py @@ -8,10 +8,9 @@ import tempfile import traceback import unittest -from imp import C_BUILTIN from io import StringIO from tempfile import NamedTemporaryFile -from unittest.mock import patch, Mock, MagicMock +from unittest.mock import MagicMock, Mock, patch import awslambdaric.bootstrap as bootstrap from awslambdaric.lambda_runtime_exception import FaultException @@ -350,7 +349,7 @@ def __init__(self, message): def test_handle_event_request_no_module(self): def unable_to_import_module(json_input, lambda_context): - import invalid_module + import invalid_module # noqa: F401 expected_response = { "errorType": "ModuleNotFoundError", @@ -381,8 +380,8 @@ def unable_to_import_module(json_input, lambda_context): def test_handle_event_request_fault_exception(self): def raise_exception_handler(json_input, lambda_context): try: - import invalid_module - except ImportError as e: + import invalid_module # noqa: F401 + except ImportError: raise FaultException( "FaultExceptionType", "Fault exception msg", @@ -429,8 +428,8 @@ def raise_exception_handler(json_input, lambda_context): def test_handle_event_request_fault_exception_logging(self, mock_stdout): def raise_exception_handler(json_input, lambda_context): try: - import invalid_module - except ImportError as e: + import invalid_module # noqa: F401 + except ImportError: raise bootstrap.FaultException( "FaultExceptionType", "Fault exception msg", @@ -469,8 +468,8 @@ def raise_exception_handler(json_input, lambda_context): def test_handle_event_request_fault_exception_logging_notrace(self, mock_stdout): def raise_exception_handler(json_input, lambda_context): try: - import invalid_module - except ImportError as e: + import invalid_module # noqa: F401 + except ImportError: raise bootstrap.FaultException( "FaultExceptionType", "Fault exception msg", None ) @@ -497,8 +496,8 @@ def test_handle_event_request_fault_exception_logging_nomessage_notrace( ): def raise_exception_handler(json_input, lambda_context): try: - import invalid_module - except ImportError as e: + import invalid_module # noqa: F401 + except ImportError: raise bootstrap.FaultException("FaultExceptionType", None, None) bootstrap.handle_event_request( @@ -523,8 +522,8 @@ def test_handle_event_request_fault_exception_logging_notype_notrace( ): def raise_exception_handler(json_input, lambda_context): try: - import invalid_module - except ImportError as e: + import invalid_module # noqa: F401 + except ImportError: raise bootstrap.FaultException(None, "Fault exception msg", None) bootstrap.handle_event_request( @@ -549,8 +548,8 @@ def test_handle_event_request_fault_exception_logging_notype_nomessage( ): def raise_exception_handler(json_input, lambda_context): try: - import invalid_module - except ImportError as e: + import invalid_module # noqa: F401 + except ImportError: raise bootstrap.FaultException( None, None, @@ -584,47 +583,6 @@ def raise_exception_handler(json_input, lambda_context): self.assertEqual(mock_stdout.getvalue(), error_logs) - @patch("sys.stdout", new_callable=StringIO) - @patch("imp.find_module") - @patch("imp.load_module") - def test_handle_event_request_fault_exception_logging_syntax_error( - self, mock_load_module, mock_find_module, mock_stdout - ): - - try: - eval("-") - except SyntaxError as e: - syntax_error = e - - mock_find_module.return_value = (None, None, ("", "", None)) - mock_load_module.side_effect = syntax_error - - response_handler = bootstrap._get_handler("a.b") - - bootstrap.handle_event_request( - self.lambda_runtime, - response_handler, - "invoke_id", - self.event_body, - "application/json", - {}, - {}, - "invoked_function_arn", - 0, - bootstrap.StandardLogSink(), - ) - - import sys - - sys.stderr.write(mock_stdout.getvalue()) - - error_logs = "[ERROR] Runtime.UserCodeSyntaxError: Syntax error in module 'a': unexpected EOF while parsing (, line 1)\r" - error_logs += "Traceback (most recent call last):\r" - error_logs += '  File "" Line 1\r' - error_logs += "    -\n" - - self.assertEqual(mock_stdout.getvalue(), error_logs) - class TestXrayFault(unittest.TestCase): def test_make_xray(self): @@ -775,11 +733,8 @@ def test_get_event_handler_missing_error(self): if os.path.exists(tmp_file.name): os.remove(tmp_file.name) - @patch("imp.find_module") - def test_get_event_handler_build_in_conflict(self, mock_find_module): - handler_name = "sys.hello" - mock_find_module.return_value = (None, None, ("", "", C_BUILTIN)) - response_handler = bootstrap._get_handler(handler_name) + def test_get_event_handler_build_in_conflict(self): + response_handler = bootstrap._get_handler("sys.hello") with self.assertRaises(FaultException) as cm: response_handler() returned_exception = cm.exception @@ -921,7 +876,10 @@ def test_log_error_indentation_standard_log_sink(self, mock_stdout): ) bootstrap.log_error(err_to_log, bootstrap.StandardLogSink()) - expected_logged_error = "[ERROR] ErrorType: Error message\rTraceback (most recent call last):\r\xa0\xa0line1 \r\xa0\xa0line2 \r\xa0\xa0\n" + expected_logged_error = ( + "[ERROR] ErrorType: Error message\rTraceback (most recent call last):" + "\r\xa0\xa0line1 \r\xa0\xa0line2 \r\xa0\xa0\n" + ) self.assertEqual(mock_stdout.getvalue(), expected_logged_error) def test_log_error_indentation_framed_log_sink(self): @@ -932,7 +890,10 @@ def test_log_error_indentation_framed_log_sink(self): ) bootstrap.log_error(err_to_log, log_sink) - expected_logged_error = "[ERROR] ErrorType: Error message\nTraceback (most recent call last):\n\xa0\xa0line1 \n\xa0\xa0line2 \n\xa0\xa0" + expected_logged_error = ( + "[ERROR] ErrorType: Error message\nTraceback (most recent call last):" + "\n\xa0\xa0line1 \n\xa0\xa0line2 \n\xa0\xa0" + ) with open(temp_file.name, "rb") as f: content = f.read() @@ -964,7 +925,10 @@ def test_log_error_empty_stacktrace_line_framed_log_sink(self): ) bootstrap.log_error(err_to_log, log_sink) - expected_logged_error = "[ERROR] ErrorType: Error message\nTraceback (most recent call last):\nline1\n\nline2" + expected_logged_error = ( + "[ERROR] ErrorType: Error message\nTraceback " + "(most recent call last):\nline1\n\nline2" + ) with open(temp_file.name, "rb") as f: content = f.read() @@ -1082,11 +1046,10 @@ def test_run(self, mock_runtime_client, mock_handle_event_request): MagicMock(), ] - with self.assertRaises(TypeError) as cm: + with self.assertRaises(TypeError): bootstrap.run( expected_app_root, expected_handler, expected_lambda_runtime_api_addr ) - returned_exception = cm.exception mock_handle_event_request.assert_called_once() @@ -1108,11 +1071,10 @@ class TestException(Exception): mock_sys.exit.side_effect = TestException("Boom!") - with self.assertRaises(TestException) as cm: + with self.assertRaises(TestException): bootstrap.run( expected_app_root, expected_handler, expected_lambda_runtime_api_addr ) - returned_exception = cm.exception mock_sys.exit.assert_called_once_with(1) From 1cda7e8f6440dda5f397fde3e9c144da47766ef1 Mon Sep 17 00:00:00 2001 From: Renzo Manganiello Date: Thu, 1 Apr 2021 14:39:23 -0300 Subject: [PATCH 2/4] Fix tests --- tests/test_bootstrap.py | 85 +++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 42 deletions(-) diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py index 9663533..2ca7b7d 100644 --- a/tests/test_bootstrap.py +++ b/tests/test_bootstrap.py @@ -2,6 +2,7 @@ Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. """ +import importlib import json import os import re @@ -688,50 +689,50 @@ def test_get_event_handler_import_error(self): ) def test_get_event_handler_syntax_error(self): - tmp_file = tempfile.NamedTemporaryFile(suffix=".py", dir=".", delete=False) - tmp_file.write( - b"def syntax_error()\n\tprint('syntax error, no colon after function')" - ) - tmp_file.close() - filename_w_ext = os.path.basename(tmp_file.name) - filename, _ = os.path.splitext(filename_w_ext) - handler_name = "{}.syntax_error".format(filename) - response_handler = bootstrap._get_handler(handler_name) - - with self.assertRaises(FaultException) as cm: - response_handler() - returned_exception = cm.exception - self.assertEqual( - self.FaultExceptionMatcher( - "Syntax error in", - "Runtime.UserCodeSyntaxError", - ".*File.*\\.py.*Line 1.*", - ), - returned_exception, - ) - if os.path.exists(tmp_file.name): - os.remove(tmp_file.name) + importlib.invalidate_caches() + with tempfile.NamedTemporaryFile(suffix=".py", dir=".", delete=False) as tmp_file: + tmp_file.write( + b"def syntax_error()\n\tprint('syntax error, no colon after function')" + ) + tmp_file.flush() + + filename_w_ext = os.path.basename(tmp_file.name) + filename, _ = os.path.splitext(filename_w_ext) + handler_name = "{}.syntax_error".format(filename) + response_handler = bootstrap._get_handler(handler_name) + + with self.assertRaises(FaultException) as cm: + response_handler() + returned_exception = cm.exception + self.assertEqual( + self.FaultExceptionMatcher( + "Syntax error in", + "Runtime.UserCodeSyntaxError", + ".*File.*\\.py.*Line 1.*", + ), + returned_exception, + ) def test_get_event_handler_missing_error(self): - tmp_file = tempfile.NamedTemporaryFile(suffix=".py", dir=".", delete=False) - tmp_file.write(b"def wrong_handler_name():\n\tprint('hello')") - tmp_file.close() - filename_w_ext = os.path.basename(tmp_file.name) - filename, _ = os.path.splitext(filename_w_ext) - handler_name = "{}.my_handler".format(filename) - response_handler = bootstrap._get_handler(handler_name) - with self.assertRaises(FaultException) as cm: - response_handler() - returned_exception = cm.exception - self.assertEqual( - self.FaultExceptionMatcher( - "Handler 'my_handler' missing on module '{}'".format(filename), - "Runtime.HandlerNotFound", - ), - returned_exception, - ) - if os.path.exists(tmp_file.name): - os.remove(tmp_file.name) + importlib.invalidate_caches() + with tempfile.NamedTemporaryFile(suffix=".py", dir=".", delete=False) as tmp_file: + tmp_file.write(b"def wrong_handler_name():\n\tprint('hello')") + tmp_file.flush() + + filename_w_ext = os.path.basename(tmp_file.name) + filename, _ = os.path.splitext(filename_w_ext) + handler_name = "{}.my_handler".format(filename) + response_handler = bootstrap._get_handler(handler_name) + with self.assertRaises(FaultException) as cm: + response_handler() + returned_exception = cm.exception + self.assertEqual( + self.FaultExceptionMatcher( + "Handler 'my_handler' missing on module '{}'".format(filename), + "Runtime.HandlerNotFound", + ), + returned_exception, + ) def test_get_event_handler_build_in_conflict(self): response_handler = bootstrap._get_handler("sys.hello") From 3efec2a4568712fb00bb08b3386ca3c3748c0481 Mon Sep 17 00:00:00 2001 From: Renzo Manganiello Date: Thu, 1 Apr 2021 14:43:18 -0300 Subject: [PATCH 3/4] Fix format --- tests/test_bootstrap.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py index 2ca7b7d..14ffa2b 100644 --- a/tests/test_bootstrap.py +++ b/tests/test_bootstrap.py @@ -690,7 +690,9 @@ def test_get_event_handler_import_error(self): def test_get_event_handler_syntax_error(self): importlib.invalidate_caches() - with tempfile.NamedTemporaryFile(suffix=".py", dir=".", delete=False) as tmp_file: + with tempfile.NamedTemporaryFile( + suffix=".py", dir=".", delete=False + ) as tmp_file: tmp_file.write( b"def syntax_error()\n\tprint('syntax error, no colon after function')" ) @@ -715,7 +717,9 @@ def test_get_event_handler_syntax_error(self): def test_get_event_handler_missing_error(self): importlib.invalidate_caches() - with tempfile.NamedTemporaryFile(suffix=".py", dir=".", delete=False) as tmp_file: + with tempfile.NamedTemporaryFile( + suffix=".py", dir=".", delete=False + ) as tmp_file: tmp_file.write(b"def wrong_handler_name():\n\tprint('hello')") tmp_file.flush() From 14fe54680208ecd2aad421ab51c4429381155048 Mon Sep 17 00:00:00 2001 From: Renzo Manganiello Date: Thu, 1 Apr 2021 15:01:04 -0300 Subject: [PATCH 4/4] Add removed test --- tests/test_bootstrap.py | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py index 14ffa2b..d28260b 100644 --- a/tests/test_bootstrap.py +++ b/tests/test_bootstrap.py @@ -584,6 +584,47 @@ def raise_exception_handler(json_input, lambda_context): self.assertEqual(mock_stdout.getvalue(), error_logs) + @patch("sys.stdout", new_callable=StringIO) + @patch("importlib.import_module") + def test_handle_event_request_fault_exception_logging_syntax_error( + self, mock_import_module, mock_stdout + ): + try: + eval("-") + except SyntaxError as e: + syntax_error = e + + mock_import_module.side_effect = syntax_error + + response_handler = bootstrap._get_handler("a.b") + + bootstrap.handle_event_request( + self.lambda_runtime, + response_handler, + "invoke_id", + self.event_body, + "application/json", + {}, + {}, + "invoked_function_arn", + 0, + bootstrap.StandardLogSink(), + ) + + import sys + + sys.stderr.write(mock_stdout.getvalue()) + + error_logs = ( + "[ERROR] Runtime.UserCodeSyntaxError: Syntax error in module 'a': " + "unexpected EOF while parsing (, line 1)\r" + ) + error_logs += "Traceback (most recent call last):\r" + error_logs += '  File "" Line 1\r' + error_logs += "    -\n" + + self.assertEqual(mock_stdout.getvalue(), error_logs) + class TestXrayFault(unittest.TestCase): def test_make_xray(self):