From d4be7408980c1f29b871fd50dc5daa8310aee2b6 Mon Sep 17 00:00:00 2001 From: walmsles <2704782+walmsles@users.noreply.github.com> Date: Mon, 17 Oct 2022 16:30:28 +1100 Subject: [PATCH 01/12] feat(router): add feature and e2e tests for trailing slash on API calls --- .../e2e/event_handler/handlers/alb_handler.py | 7 +- .../handlers/api_gateway_http_handler.py | 7 +- .../handlers/api_gateway_rest_handler.py | 7 +- .../handlers/lambda_function_url_handler.py | 7 +- tests/e2e/event_handler/infrastructure.py | 8 ++ .../test_paths_ending_with_slash.py | 95 +++++++++++++++++++ tests/events/albEventPathTrailingSlash.json | 28 ++++++ ...apiGatewayProxyEventPathTrailingSlash.json | 80 ++++++++++++++++ ...iGatewayProxyV2EventpathTrailingSlash.json | 69 ++++++++++++++ ...mbdaFunctionUrlEventPathTrailingSlash.json | 52 ++++++++++ .../event_handler/test_api_gateway.py | 80 ++++++++++++++++ .../event_handler/test_lambda_function_url.py | 19 ++++ 12 files changed, 455 insertions(+), 4 deletions(-) create mode 100644 tests/e2e/event_handler/test_paths_ending_with_slash.py create mode 100644 tests/events/albEventPathTrailingSlash.json create mode 100644 tests/events/apiGatewayProxyEventPathTrailingSlash.json create mode 100644 tests/events/apiGatewayProxyV2EventpathTrailingSlash.json create mode 100644 tests/events/lambdaFunctionUrlEventPathTrailingSlash.json diff --git a/tests/e2e/event_handler/handlers/alb_handler.py b/tests/e2e/event_handler/handlers/alb_handler.py index 0e386c82c51..2ab11a87fb2 100644 --- a/tests/e2e/event_handler/handlers/alb_handler.py +++ b/tests/e2e/event_handler/handlers/alb_handler.py @@ -4,7 +4,7 @@ @app.post("/todos") -def hello(): +def todos(): payload = app.current_event.json_body body = payload.get("body", "Hello World") @@ -22,5 +22,10 @@ def hello(): ) +@app.get("/hello") +def hello(): + return Response(status_code=200, content_type=content_types.TEXT_PLAIN, body="Hello World") + + def lambda_handler(event, context): return app.resolve(event, context) diff --git a/tests/e2e/event_handler/handlers/api_gateway_http_handler.py b/tests/e2e/event_handler/handlers/api_gateway_http_handler.py index 9edacc1c807..98f8fc421fb 100644 --- a/tests/e2e/event_handler/handlers/api_gateway_http_handler.py +++ b/tests/e2e/event_handler/handlers/api_gateway_http_handler.py @@ -8,7 +8,7 @@ @app.post("/todos") -def hello(): +def todos(): payload = app.current_event.json_body body = payload.get("body", "Hello World") @@ -26,5 +26,10 @@ def hello(): ) +@app.get("/hello") +def hello(): + return Response(status_code=200, content_type=content_types.TEXT_PLAIN, body="Hello World") + + def lambda_handler(event, context): return app.resolve(event, context) diff --git a/tests/e2e/event_handler/handlers/api_gateway_rest_handler.py b/tests/e2e/event_handler/handlers/api_gateway_rest_handler.py index 3127634e995..3eb564ebf64 100644 --- a/tests/e2e/event_handler/handlers/api_gateway_rest_handler.py +++ b/tests/e2e/event_handler/handlers/api_gateway_rest_handler.py @@ -8,7 +8,7 @@ @app.post("/todos") -def hello(): +def todos(): payload = app.current_event.json_body body = payload.get("body", "Hello World") @@ -26,5 +26,10 @@ def hello(): ) +@app.get("/hello") +def hello(): + return Response(status_code=200, content_type=content_types.TEXT_PLAIN, body="Hello World") + + def lambda_handler(event, context): return app.resolve(event, context) diff --git a/tests/e2e/event_handler/handlers/lambda_function_url_handler.py b/tests/e2e/event_handler/handlers/lambda_function_url_handler.py index 884704893ae..a787f4dd330 100644 --- a/tests/e2e/event_handler/handlers/lambda_function_url_handler.py +++ b/tests/e2e/event_handler/handlers/lambda_function_url_handler.py @@ -8,7 +8,7 @@ @app.post("/todos") -def hello(): +def todos(): payload = app.current_event.json_body body = payload.get("body", "Hello World") @@ -26,5 +26,10 @@ def hello(): ) +@app.get("/hello") +def hello(): + return Response(status_code=200, content_type=content_types.TEXT_PLAIN, body="Hello World") + + def lambda_handler(event, context): return app.resolve(event, context) diff --git a/tests/e2e/event_handler/infrastructure.py b/tests/e2e/event_handler/infrastructure.py index da456038a25..7774fc773a6 100644 --- a/tests/e2e/event_handler/infrastructure.py +++ b/tests/e2e/event_handler/infrastructure.py @@ -64,6 +64,11 @@ def _create_api_gateway_http(self, function: Function): integration=apigwv2integrations.HttpLambdaIntegration("TodosIntegration", function), ) + apigw.add_routes( + path="/hello", + methods=[apigwv2.HttpMethod.GET], + integration=apigwv2integrations.HttpLambdaIntegration("HelloIntegration", function), + ) CfnOutput(self.stack, "APIGatewayHTTPUrl", value=(apigw.url or "")) def _create_api_gateway_rest(self, function: Function): @@ -72,6 +77,9 @@ def _create_api_gateway_rest(self, function: Function): todos = apigw.root.add_resource("todos") todos.add_method("POST", apigwv1.LambdaIntegration(function, proxy=True)) + hello = apigw.root.add_resource("hello") + hello.add_method("GET", apigwv1.LambdaIntegration(function, proxy=True)) + CfnOutput(self.stack, "APIGatewayRestUrl", value=apigw.url) def _create_lambda_function_url(self, function: Function): diff --git a/tests/e2e/event_handler/test_paths_ending_with_slash.py b/tests/e2e/event_handler/test_paths_ending_with_slash.py new file mode 100644 index 00000000000..94f40a5dca0 --- /dev/null +++ b/tests/e2e/event_handler/test_paths_ending_with_slash.py @@ -0,0 +1,95 @@ +import pytest +from requests import HTTPError, Request + +from tests.e2e.utils import data_fetcher + + +@pytest.fixture +def alb_basic_listener_endpoint(infrastructure: dict) -> str: + dns_name = infrastructure.get("ALBDnsName") + port = infrastructure.get("ALBBasicListenerPort", "") + return f"http://{dns_name}:{port}" + + +@pytest.fixture +def alb_multi_value_header_listener_endpoint(infrastructure: dict) -> str: + dns_name = infrastructure.get("ALBDnsName") + port = infrastructure.get("ALBMultiValueHeaderListenerPort", "") + return f"http://{dns_name}:{port}" + + +@pytest.fixture +def apigw_rest_endpoint(infrastructure: dict) -> str: + return infrastructure.get("APIGatewayRestUrl", "") + + +@pytest.fixture +def apigw_http_endpoint(infrastructure: dict) -> str: + return infrastructure.get("APIGatewayHTTPUrl", "") + + +@pytest.fixture +def lambda_function_url_endpoint(infrastructure: dict) -> str: + return infrastructure.get("LambdaFunctionUrl", "") + + +def test_api_gateway_rest_trailing_slash(apigw_rest_endpoint): + # GIVEN + url = f"{apigw_rest_endpoint}hello/" + body = "Hello World" + status_code = 200 + + # WHEN + response = data_fetcher.get_http_response( + Request( + method="GET", + url=url, + ) + ) + + # THEN + assert response.status_code == status_code + # response.content is a binary string, needs to be decoded to compare with the real string + assert response.content.decode("ascii") == body + + +def test_api_gateway_http_trailing_slash(apigw_http_endpoint): + # GIVEN the URL for the API ends in a trailing slash API gateway should return a 404 + url = f"{apigw_http_endpoint}hello/" + + # WHEN + with pytest.raises(HTTPError): + data_fetcher.get_http_response( + Request( + method="GET", + url=url, + ) + ) + + +def test_lambda_function_url_trailing_slash(lambda_function_url_endpoint): + # GIVEN the URL for the API ends in a trailing slash it should behave as if there was not one + url = f"{lambda_function_url_endpoint}hello/" # the function url endpoint already has the trailing / + + # WHEN + with pytest.raises(HTTPError): + data_fetcher.get_http_response( + Request( + method="GET", + url=url, + ) + ) + + +def test_alb_url_trailing_slash(alb_multi_value_header_listener_endpoint): + # GIVEN url has a trailing slash - it should behave as if there was not one + url = f"{alb_multi_value_header_listener_endpoint}/hello/" + + # WHEN + with pytest.raises(HTTPError): + data_fetcher.get_http_response( + Request( + method="GET", + url=url, + ) + ) diff --git a/tests/events/albEventPathTrailingSlash.json b/tests/events/albEventPathTrailingSlash.json new file mode 100644 index 00000000000..c517a3f6b04 --- /dev/null +++ b/tests/events/albEventPathTrailingSlash.json @@ -0,0 +1,28 @@ +{ + "requestContext": { + "elb": { + "targetGroupArn": "arn:aws:elasticloadbalancing:us-east-2:123456789012:targetgroup/lambda-279XGJDqGZ5rsrHC2Fjr/49e9d65c45c6791a" + } + }, + "httpMethod": "GET", + "path": "/lambda/", + "queryStringParameters": { + "query": "1234ABCD" + }, + "headers": { + "accept": "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8", + "accept-encoding": "gzip", + "accept-language": "en-US,en;q=0.9", + "connection": "keep-alive", + "host": "lambda-alb-123578498.us-east-2.elb.amazonaws.com", + "upgrade-insecure-requests": "1", + "user-agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.98 Safari/537.36", + "x-amzn-trace-id": "Root=1-5c536348-3d683b8b04734faae651f476", + "x-forwarded-for": "72.12.164.125", + "x-forwarded-port": "80", + "x-forwarded-proto": "http", + "x-imforwards": "20" + }, + "body": "Test", + "isBase64Encoded": false + } \ No newline at end of file diff --git a/tests/events/apiGatewayProxyEventPathTrailingSlash.json b/tests/events/apiGatewayProxyEventPathTrailingSlash.json new file mode 100644 index 00000000000..8a321d96c8c --- /dev/null +++ b/tests/events/apiGatewayProxyEventPathTrailingSlash.json @@ -0,0 +1,80 @@ +{ + "version": "1.0", + "resource": "/my/path", + "path": "/my/path/", + "httpMethod": "GET", + "headers": { + "Header1": "value1", + "Header2": "value2" + }, + "multiValueHeaders": { + "Header1": [ + "value1" + ], + "Header2": [ + "value1", + "value2" + ] + }, + "queryStringParameters": { + "parameter1": "value1", + "parameter2": "value" + }, + "multiValueQueryStringParameters": { + "parameter1": [ + "value1", + "value2" + ], + "parameter2": [ + "value" + ] + }, + "requestContext": { + "accountId": "123456789012", + "apiId": "id", + "authorizer": { + "claims": null, + "scopes": null + }, + "domainName": "id.execute-api.us-east-1.amazonaws.com", + "domainPrefix": "id", + "extendedRequestId": "request-id", + "httpMethod": "GET", + "identity": { + "accessKey": null, + "accountId": null, + "caller": null, + "cognitoAuthenticationProvider": null, + "cognitoAuthenticationType": null, + "cognitoIdentityId": null, + "cognitoIdentityPoolId": null, + "principalOrgId": null, + "sourceIp": "192.168.0.1/32", + "user": null, + "userAgent": "user-agent", + "userArn": null, + "clientCert": { + "clientCertPem": "CERT_CONTENT", + "subjectDN": "www.example.com", + "issuerDN": "Example issuer", + "serialNumber": "a1:a1:a1:a1:a1:a1:a1:a1:a1:a1:a1:a1:a1:a1:a1:a1", + "validity": { + "notBefore": "May 28 12:30:02 2019 GMT", + "notAfter": "Aug 5 09:36:04 2021 GMT" + } + } + }, + "path": "/my/path", + "protocol": "HTTP/1.1", + "requestId": "id=", + "requestTime": "04/Mar/2020:19:15:17 +0000", + "requestTimeEpoch": 1583349317135, + "resourceId": null, + "resourcePath": "/my/path", + "stage": "$default" + }, + "pathParameters": null, + "stageVariables": null, + "body": "Hello from Lambda!", + "isBase64Encoded": true + } \ No newline at end of file diff --git a/tests/events/apiGatewayProxyV2EventpathTrailingSlash.json b/tests/events/apiGatewayProxyV2EventpathTrailingSlash.json new file mode 100644 index 00000000000..dfb0d98f2e1 --- /dev/null +++ b/tests/events/apiGatewayProxyV2EventpathTrailingSlash.json @@ -0,0 +1,69 @@ +{ + "version": "2.0", + "routeKey": "$default", + "rawPath": "/my/path/", + "rawQueryString": "parameter1=value1¶meter1=value2¶meter2=value", + "cookies": [ + "cookie1", + "cookie2" + ], + "headers": { + "Header1": "value1", + "Header2": "value1,value2" + }, + "queryStringParameters": { + "parameter1": "value1,value2", + "parameter2": "value" + }, + "requestContext": { + "accountId": "123456789012", + "apiId": "api-id", + "authentication": { + "clientCert": { + "clientCertPem": "CERT_CONTENT", + "subjectDN": "www.example.com", + "issuerDN": "Example issuer", + "serialNumber": "a1:a1:a1:a1:a1:a1:a1:a1:a1:a1:a1:a1:a1:a1:a1:a1", + "validity": { + "notBefore": "May 28 12:30:02 2019 GMT", + "notAfter": "Aug 5 09:36:04 2021 GMT" + } + } + }, + "authorizer": { + "jwt": { + "claims": { + "claim1": "value1", + "claim2": "value2" + }, + "scopes": [ + "scope1", + "scope2" + ] + } + }, + "domainName": "id.execute-api.us-east-1.amazonaws.com", + "domainPrefix": "id", + "http": { + "method": "POST", + "path": "/my/path", + "protocol": "HTTP/1.1", + "sourceIp": "192.168.0.1/32", + "userAgent": "agent" + }, + "requestId": "id", + "routeKey": "$default", + "stage": "$default", + "time": "12/Mar/2020:19:03:58 +0000", + "timeEpoch": 1583348638390 + }, + "body": "{\"message\": \"hello world\", \"username\": \"tom\"}", + "pathParameters": { + "parameter1": "value1" + }, + "isBase64Encoded": false, + "stageVariables": { + "stageVariable1": "value1", + "stageVariable2": "value2" + } + } \ No newline at end of file diff --git a/tests/events/lambdaFunctionUrlEventPathTrailingSlash.json b/tests/events/lambdaFunctionUrlEventPathTrailingSlash.json new file mode 100644 index 00000000000..b1f82265187 --- /dev/null +++ b/tests/events/lambdaFunctionUrlEventPathTrailingSlash.json @@ -0,0 +1,52 @@ +{ + "version": "2.0", + "routeKey": "$default", + "rawPath": "/my/path/", + "rawQueryString": "parameter1=value1¶meter1=value2¶meter2=value", + "cookies": [ + "cookie1", + "cookie2" + ], + "headers": { + "header1": "value1", + "header2": "value1,value2" + }, + "queryStringParameters": { + "parameter1": "value1,value2", + "parameter2": "value" + }, + "requestContext": { + "accountId": "123456789012", + "apiId": "", + "authentication": null, + "authorizer": { + "iam": { + "accessKey": "AKIA...", + "accountId": "111122223333", + "callerId": "AIDA...", + "cognitoIdentity": null, + "principalOrgId": null, + "userArn": "arn:aws:iam::111122223333:user/example-user", + "userId": "AIDA..." + } + }, + "domainName": ".lambda-url.us-west-2.on.aws", + "domainPrefix": "", + "http": { + "method": "POST", + "path": "/my/path", + "protocol": "HTTP/1.1", + "sourceIp": "123.123.123.123", + "userAgent": "agent" + }, + "requestId": "id", + "routeKey": "$default", + "stage": "$default", + "time": "12/Mar/2020:19:03:58 +0000", + "timeEpoch": 1583348638390 + }, + "body": "Hello from client!", + "pathParameters": null, + "isBase64Encoded": false, + "stageVariables": null + } \ No newline at end of file diff --git a/tests/functional/event_handler/test_api_gateway.py b/tests/functional/event_handler/test_api_gateway.py index 8491754b65e..ffccde799d3 100644 --- a/tests/functional/event_handler/test_api_gateway.py +++ b/tests/functional/event_handler/test_api_gateway.py @@ -53,6 +53,7 @@ def read_media(file_name: str) -> bytes: LOAD_GW_EVENT = load_event("apiGatewayProxyEvent.json") +LOAD_GW_EVENT_TRAILING_SLASH = load_event("apiGatewayProxyEventPathTrailingSlash.json") def test_alb_event(): @@ -76,6 +77,27 @@ def foo(): assert result["body"] == "foo" +def test_alb_event_path_trailing_slash(json_dump): + # GIVEN an Application Load Balancer proxy type event + app = ALBResolver() + + @app.get("/lambda") + def foo(): + assert isinstance(app.current_event, ALBEvent) + assert app.lambda_context == {} + assert app.current_event.request_context.elb_target_group_arn is not None + return Response(200, content_types.TEXT_HTML, "foo") + + # WHEN calling the event handler using path with trailing "/" + result = app(load_event("albEventPathTrailingSlash.json"), {}) + + # THEN + assert result["statusCode"] == 404 + assert result["headers"]["Content-Type"] == content_types.APPLICATION_JSON + expected = {"statusCode": 404, "message": "Not found"} + assert result["body"] == json_dump(expected) + + def test_api_gateway_v1(): # GIVEN a Http API V1 proxy type event app = APIGatewayRestResolver() @@ -96,6 +118,26 @@ def get_lambda() -> Response: assert result["multiValueHeaders"]["Content-Type"] == [content_types.APPLICATION_JSON] +def test_api_gateway_v1_path_trailing_slash(): + # GIVEN a Http API V1 proxy type event + app = APIGatewayRestResolver() + + @app.get("/my/path") + def get_lambda() -> Response: + assert isinstance(app.current_event, APIGatewayProxyEvent) + assert app.lambda_context == {} + assert app.current_event.request_context.domain_name == "id.execute-api.us-east-1.amazonaws.com" + return Response(200, content_types.APPLICATION_JSON, json.dumps({"foo": "value"})) + + # WHEN calling the event handler + result = app(LOAD_GW_EVENT_TRAILING_SLASH, {}) + + # THEN process event correctly + # AND set the current_event type as APIGatewayProxyEvent + assert result["statusCode"] == 200 + assert result["multiValueHeaders"]["Content-Type"] == [content_types.APPLICATION_JSON] + + def test_api_gateway_v1_cookies(): # GIVEN a Http API V1 proxy type event app = APIGatewayRestResolver() @@ -134,6 +176,24 @@ def get_lambda() -> Response: assert result["body"] == "foo" +def test_api_gateway_event_path_trailing_slash(json_dump): + # GIVEN a Rest API Gateway proxy type event + app = ApiGatewayResolver(proxy_type=ProxyEventType.APIGatewayProxyEvent) + + @app.get("/my/path") + def get_lambda() -> Response: + assert isinstance(app.current_event, APIGatewayProxyEvent) + return Response(200, content_types.TEXT_HTML, "foo") + + # WHEN calling the event handler + result = app(LOAD_GW_EVENT_TRAILING_SLASH, {}) + # THEN + assert result["statusCode"] == 404 + assert result["multiValueHeaders"]["Content-Type"] == [content_types.APPLICATION_JSON] + expected = {"statusCode": 404, "message": "Not found"} + assert result["body"] == json_dump(expected) + + def test_api_gateway_v2(): # GIVEN a Http API V2 proxy type event app = APIGatewayHttpResolver() @@ -156,6 +216,26 @@ def my_path() -> Response: assert result["body"] == "tom" +def test_api_gateway_v2_http_path_trailing_slash(json_dump): + # GIVEN a Http API V2 proxy type event + app = APIGatewayHttpResolver() + + @app.post("/my/path") + def my_path() -> Response: + assert isinstance(app.current_event, APIGatewayProxyEventV2) + post_data = app.current_event.json_body + return Response(200, content_types.TEXT_PLAIN, post_data["username"]) + + # WHEN calling the event handler + result = app(load_event("apiGatewayProxyV2EventPathTrailingSlash.json"), {}) + + # THEN expect a 404 response + assert result["statusCode"] == 404 + assert result["headers"]["Content-Type"] == content_types.APPLICATION_JSON + expected = {"statusCode": 404, "message": "Not found"} + assert result["body"] == json_dump(expected) + + def test_api_gateway_v2_cookies(): # GIVEN a Http API V2 proxy type event app = APIGatewayHttpResolver() diff --git a/tests/functional/event_handler/test_lambda_function_url.py b/tests/functional/event_handler/test_lambda_function_url.py index aacbc94129b..4474619b5b8 100644 --- a/tests/functional/event_handler/test_lambda_function_url.py +++ b/tests/functional/event_handler/test_lambda_function_url.py @@ -30,6 +30,25 @@ def foo(): assert result["body"] == "foo" +def test_lambda_function_url_event_path_trailing_slash(): + # GIVEN a Lambda Function Url type event + app = LambdaFunctionUrlResolver() + + @app.post("/my/path") + def foo(): + assert isinstance(app.current_event, LambdaFunctionUrlEvent) + assert app.lambda_context == {} + assert app.current_event.request_context.stage is not None + return Response(200, content_types.TEXT_HTML, "foo") + + # WHEN calling the event handler with an event with a trailing slash + result = app(load_event("lambdaFunctionUrlEventPathTrailingSlash.json"), {}) + + # THEN return a 404 error + assert result["statusCode"] == 404 + assert result["headers"]["Content-Type"] == content_types.APPLICATION_JSON + + def test_lambda_function_url_event_with_cookies(): # GIVEN a Lambda Function Url type event app = LambdaFunctionUrlResolver() From 50c7becfc1cace8bc6997ace2e126f8262b9ece4 Mon Sep 17 00:00:00 2001 From: walmsles <2704782+walmsles@users.noreply.github.com> Date: Mon, 17 Oct 2022 16:57:32 +1100 Subject: [PATCH 02/12] feat(router): add routing of url paths ending in slashes correctly for APIGatewayRestRsolver --- .../event_handler/api_gateway.py | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/aws_lambda_powertools/event_handler/api_gateway.py b/aws_lambda_powertools/event_handler/api_gateway.py index 4dbf753d22c..de557faa956 100644 --- a/aws_lambda_powertools/event_handler/api_gateway.py +++ b/aws_lambda_powertools/event_handler/api_gateway.py @@ -46,6 +46,7 @@ # API GW/ALB decode non-safe URI chars; we must support them too _UNSAFE_URI = "%<> \[\]{}|^" # noqa: W605 _NAMED_GROUP_BOUNDARY_PATTERN = rf"(?P\1[{_SAFE_URI}{_UNSAFE_URI}\\w]+)" +_ROUTE_REGEX = "^{}$" class ProxyEventType(Enum): @@ -561,8 +562,8 @@ def _has_debug(debug: Optional[bool] = None) -> bool: return powertools_dev_is_set() - @staticmethod - def _compile_regex(rule: str): + @classmethod + def _compile_regex(cls, rule: str, base_regex: Optional[str] = _ROUTE_REGEX): """Precompile regex pattern Logic @@ -592,7 +593,7 @@ def _compile_regex(rule: str): NOTE: See #520 for context """ rule_regex: str = re.sub(_DYNAMIC_ROUTE_PATTERN, _NAMED_GROUP_BOUNDARY_PATTERN, rule) - return re.compile("^{}$".format(rule_regex)) + return re.compile(base_regex.format(rule_regex)) def _to_proxy_event(self, event: Dict) -> BaseProxyEvent: """Convert the event dict to the corresponding data class""" @@ -819,6 +820,24 @@ def __init__( """Amazon API Gateway REST and HTTP API v1 payload resolver""" super().__init__(ProxyEventType.APIGatewayProxyEvent, cors, debug, serializer, strip_prefixes) + # override route to ignore trailing "/" in routes for REST API + def route( + self, + rule: str, + method: Union[str, Union[List[str], Tuple[str]]], + cors: Optional[bool] = None, + compress: bool = False, + cache_control: Optional[str] = None, + ): + # remove trailing "/" character from route rule for correct routing behaviour + return super().route(rule.rstrip("/"), method, cors, compress, cache_control) + + # Override _compile_regex to exclude traling slashes for route resolution + @classmethod + def _compile_regex(cls, rule: str, base_regex: Optional[str] = _ROUTE_REGEX): + + return super()._compile_regex(rule, "^{}/*$") + class APIGatewayHttpResolver(ApiGatewayResolver): current_event: APIGatewayProxyEventV2 From 2e1e972a08a52792e3d4a03f8658dfcb3bda5418 Mon Sep 17 00:00:00 2001 From: walmsles <2704782+walmsles@users.noreply.github.com> Date: Tue, 18 Oct 2022 19:30:06 +1100 Subject: [PATCH 03/12] feat(router): add routing of url paths ending in slashes correctly for APIGatewayRestRsolver --- aws_lambda_powertools/event_handler/api_gateway.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/aws_lambda_powertools/event_handler/api_gateway.py b/aws_lambda_powertools/event_handler/api_gateway.py index de557faa956..437ea8dc1a7 100644 --- a/aws_lambda_powertools/event_handler/api_gateway.py +++ b/aws_lambda_powertools/event_handler/api_gateway.py @@ -563,7 +563,7 @@ def _has_debug(debug: Optional[bool] = None) -> bool: return powertools_dev_is_set() @classmethod - def _compile_regex(cls, rule: str, base_regex: Optional[str] = _ROUTE_REGEX): + def _compile_regex(cls, rule: str, base_regex: str = _ROUTE_REGEX): """Precompile regex pattern Logic @@ -832,9 +832,9 @@ def route( # remove trailing "/" character from route rule for correct routing behaviour return super().route(rule.rstrip("/"), method, cors, compress, cache_control) - # Override _compile_regex to exclude traling slashes for route resolution + # Override _compile_regex to exclude trailing slashes for route resolution @classmethod - def _compile_regex(cls, rule: str, base_regex: Optional[str] = _ROUTE_REGEX): + def _compile_regex(cls, rule: str, base_regex: str = _ROUTE_REGEX): return super()._compile_regex(rule, "^{}/*$") From 011dc22d698f797e237424b409012899c3fdbf09 Mon Sep 17 00:00:00 2001 From: walmsles <2704782+walmsles@users.noreply.github.com> Date: Tue, 18 Oct 2022 19:45:13 +1100 Subject: [PATCH 04/12] feat(router): change _compile_regex back to staticmethod not a required change - can call super static method in python. --- aws_lambda_powertools/event_handler/api_gateway.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/aws_lambda_powertools/event_handler/api_gateway.py b/aws_lambda_powertools/event_handler/api_gateway.py index 437ea8dc1a7..82f64faa712 100644 --- a/aws_lambda_powertools/event_handler/api_gateway.py +++ b/aws_lambda_powertools/event_handler/api_gateway.py @@ -562,8 +562,8 @@ def _has_debug(debug: Optional[bool] = None) -> bool: return powertools_dev_is_set() - @classmethod - def _compile_regex(cls, rule: str, base_regex: str = _ROUTE_REGEX): + @staticmethod + def _compile_regex(rule: str, base_regex: str = _ROUTE_REGEX): """Precompile regex pattern Logic @@ -833,10 +833,10 @@ def route( return super().route(rule.rstrip("/"), method, cors, compress, cache_control) # Override _compile_regex to exclude trailing slashes for route resolution - @classmethod - def _compile_regex(cls, rule: str, base_regex: str = _ROUTE_REGEX): + @staticmethod + def _compile_regex(rule: str, base_regex: str = _ROUTE_REGEX): - return super()._compile_regex(rule, "^{}/*$") + return super(APIGatewayRestResolver, APIGatewayRestResolver)._compile_regex(rule, "^{}/*$") class APIGatewayHttpResolver(ApiGatewayResolver): From 1969f992b7050e6c8a58a0c5561b8acaea167440 Mon Sep 17 00:00:00 2001 From: walmsles <2704782+walmsles@users.noreply.github.com> Date: Tue, 18 Oct 2022 20:09:22 +1100 Subject: [PATCH 05/12] feat(router): move event file to fix misplaced case in test filename --- ...ingSlash.json => apiGatewayProxyV2EventPathTrailingSlash.json} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/events/{apiGatewayProxyV2EventpathTrailingSlash.json => apiGatewayProxyV2EventPathTrailingSlash.json} (100%) diff --git a/tests/events/apiGatewayProxyV2EventpathTrailingSlash.json b/tests/events/apiGatewayProxyV2EventPathTrailingSlash.json similarity index 100% rename from tests/events/apiGatewayProxyV2EventpathTrailingSlash.json rename to tests/events/apiGatewayProxyV2EventPathTrailingSlash.json From 73a29253571546de99ce0d2db2d426c5fdc874c8 Mon Sep 17 00:00:00 2001 From: walmsles <2704782+walmsles@users.noreply.github.com> Date: Tue, 18 Oct 2022 23:06:57 +1100 Subject: [PATCH 06/12] Update aws_lambda_powertools/event_handler/api_gateway.py Co-authored-by: Heitor Lessa --- aws_lambda_powertools/event_handler/api_gateway.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws_lambda_powertools/event_handler/api_gateway.py b/aws_lambda_powertools/event_handler/api_gateway.py index 82f64faa712..38aaaf096db 100644 --- a/aws_lambda_powertools/event_handler/api_gateway.py +++ b/aws_lambda_powertools/event_handler/api_gateway.py @@ -829,7 +829,7 @@ def route( compress: bool = False, cache_control: Optional[str] = None, ): - # remove trailing "/" character from route rule for correct routing behaviour + # NOTE: see #1552 for more context. return super().route(rule.rstrip("/"), method, cors, compress, cache_control) # Override _compile_regex to exclude trailing slashes for route resolution From 6d61851a771f0331c0472022e94ab0df6f1b6b68 Mon Sep 17 00:00:00 2001 From: walmsles <2704782+walmsles@users.noreply.github.com> Date: Tue, 18 Oct 2022 23:09:01 +1100 Subject: [PATCH 07/12] Update tests/functional/event_handler/test_api_gateway.py Remove unnecessary assertions. Co-authored-by: Heitor Lessa --- tests/functional/event_handler/test_api_gateway.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/functional/event_handler/test_api_gateway.py b/tests/functional/event_handler/test_api_gateway.py index ffccde799d3..b9bc391ade6 100644 --- a/tests/functional/event_handler/test_api_gateway.py +++ b/tests/functional/event_handler/test_api_gateway.py @@ -124,9 +124,6 @@ def test_api_gateway_v1_path_trailing_slash(): @app.get("/my/path") def get_lambda() -> Response: - assert isinstance(app.current_event, APIGatewayProxyEvent) - assert app.lambda_context == {} - assert app.current_event.request_context.domain_name == "id.execute-api.us-east-1.amazonaws.com" return Response(200, content_types.APPLICATION_JSON, json.dumps({"foo": "value"})) # WHEN calling the event handler From 798a350adb21b98176dba00d07f65d30952da211 Mon Sep 17 00:00:00 2001 From: walmsles <2704782+walmsles@users.noreply.github.com> Date: Tue, 18 Oct 2022 23:09:58 +1100 Subject: [PATCH 08/12] Update tests/functional/event_handler/test_api_gateway.py Remove unnecessary assertions Co-authored-by: Heitor Lessa --- tests/functional/event_handler/test_api_gateway.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/event_handler/test_api_gateway.py b/tests/functional/event_handler/test_api_gateway.py index b9bc391ade6..a78d3747d28 100644 --- a/tests/functional/event_handler/test_api_gateway.py +++ b/tests/functional/event_handler/test_api_gateway.py @@ -219,7 +219,6 @@ def test_api_gateway_v2_http_path_trailing_slash(json_dump): @app.post("/my/path") def my_path() -> Response: - assert isinstance(app.current_event, APIGatewayProxyEventV2) post_data = app.current_event.json_body return Response(200, content_types.TEXT_PLAIN, post_data["username"]) From fb1a0858789cf80ca576956276f316ac2c20d361 Mon Sep 17 00:00:00 2001 From: walmsles <2704782+walmsles@users.noreply.github.com> Date: Tue, 18 Oct 2022 23:10:32 +1100 Subject: [PATCH 09/12] Update tests/functional/event_handler/test_lambda_function_url.py remove unecessary assertions Co-authored-by: Heitor Lessa --- tests/functional/event_handler/test_lambda_function_url.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/functional/event_handler/test_lambda_function_url.py b/tests/functional/event_handler/test_lambda_function_url.py index 4474619b5b8..41baed68a7c 100644 --- a/tests/functional/event_handler/test_lambda_function_url.py +++ b/tests/functional/event_handler/test_lambda_function_url.py @@ -36,9 +36,6 @@ def test_lambda_function_url_event_path_trailing_slash(): @app.post("/my/path") def foo(): - assert isinstance(app.current_event, LambdaFunctionUrlEvent) - assert app.lambda_context == {} - assert app.current_event.request_context.stage is not None return Response(200, content_types.TEXT_HTML, "foo") # WHEN calling the event handler with an event with a trailing slash From bfec2efa805903e06449c31912367caf0839c562 Mon Sep 17 00:00:00 2001 From: walmsles <2704782+walmsles@users.noreply.github.com> Date: Tue, 18 Oct 2022 23:24:42 +1100 Subject: [PATCH 10/12] feat(router): simplify assertion for response text --- tests/e2e/event_handler/test_paths_ending_with_slash.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/event_handler/test_paths_ending_with_slash.py b/tests/e2e/event_handler/test_paths_ending_with_slash.py index 94f40a5dca0..44a9c84f633 100644 --- a/tests/e2e/event_handler/test_paths_ending_with_slash.py +++ b/tests/e2e/event_handler/test_paths_ending_with_slash.py @@ -50,7 +50,7 @@ def test_api_gateway_rest_trailing_slash(apigw_rest_endpoint): # THEN assert response.status_code == status_code # response.content is a binary string, needs to be decoded to compare with the real string - assert response.content.decode("ascii") == body + assert response.text == body def test_api_gateway_http_trailing_slash(apigw_http_endpoint): From e9b8836be2a7a20a423d957a468b6f1fd0f6c276 Mon Sep 17 00:00:00 2001 From: walmsles <2704782+walmsles@users.noreply.github.com> Date: Wed, 19 Oct 2022 00:23:04 +1100 Subject: [PATCH 11/12] feat(router): Removed complexity around test setup and simplified testing for trailing slash in E2E tests for all API types and expected behaviours. --- .../e2e/event_handler/handlers/alb_handler.py | 8 ++-- .../handlers/api_gateway_http_handler.py | 8 ++-- .../handlers/api_gateway_rest_handler.py | 8 ++-- .../handlers/lambda_function_url_handler.py | 8 ++-- tests/e2e/event_handler/infrastructure.py | 8 ---- .../test_paths_ending_with_slash.py | 38 ++++++++++--------- 6 files changed, 33 insertions(+), 45 deletions(-) diff --git a/tests/e2e/event_handler/handlers/alb_handler.py b/tests/e2e/event_handler/handlers/alb_handler.py index 2ab11a87fb2..26746284aee 100644 --- a/tests/e2e/event_handler/handlers/alb_handler.py +++ b/tests/e2e/event_handler/handlers/alb_handler.py @@ -2,6 +2,9 @@ app = ALBResolver() +# The reason we use post is that whoever is writing tests can easily assert on the +# content being sent (body, headers, cookies, content-type) to reduce cognitive load. + @app.post("/todos") def todos(): @@ -22,10 +25,5 @@ def todos(): ) -@app.get("/hello") -def hello(): - return Response(status_code=200, content_type=content_types.TEXT_PLAIN, body="Hello World") - - def lambda_handler(event, context): return app.resolve(event, context) diff --git a/tests/e2e/event_handler/handlers/api_gateway_http_handler.py b/tests/e2e/event_handler/handlers/api_gateway_http_handler.py index 98f8fc421fb..1012af7b3fb 100644 --- a/tests/e2e/event_handler/handlers/api_gateway_http_handler.py +++ b/tests/e2e/event_handler/handlers/api_gateway_http_handler.py @@ -6,6 +6,9 @@ app = APIGatewayHttpResolver() +# The reason we use post is that whoever is writing tests can easily assert on the +# content being sent (body, headers, cookies, content-type) to reduce cognitive load. + @app.post("/todos") def todos(): @@ -26,10 +29,5 @@ def todos(): ) -@app.get("/hello") -def hello(): - return Response(status_code=200, content_type=content_types.TEXT_PLAIN, body="Hello World") - - def lambda_handler(event, context): return app.resolve(event, context) diff --git a/tests/e2e/event_handler/handlers/api_gateway_rest_handler.py b/tests/e2e/event_handler/handlers/api_gateway_rest_handler.py index 3eb564ebf64..d52e2728cab 100644 --- a/tests/e2e/event_handler/handlers/api_gateway_rest_handler.py +++ b/tests/e2e/event_handler/handlers/api_gateway_rest_handler.py @@ -6,6 +6,9 @@ app = APIGatewayRestResolver() +# The reason we use post is that whoever is writing tests can easily assert on the +# content being sent (body, headers, cookies, content-type) to reduce cognitive load. + @app.post("/todos") def todos(): @@ -26,10 +29,5 @@ def todos(): ) -@app.get("/hello") -def hello(): - return Response(status_code=200, content_type=content_types.TEXT_PLAIN, body="Hello World") - - def lambda_handler(event, context): return app.resolve(event, context) diff --git a/tests/e2e/event_handler/handlers/lambda_function_url_handler.py b/tests/e2e/event_handler/handlers/lambda_function_url_handler.py index a787f4dd330..f90037afc75 100644 --- a/tests/e2e/event_handler/handlers/lambda_function_url_handler.py +++ b/tests/e2e/event_handler/handlers/lambda_function_url_handler.py @@ -6,6 +6,9 @@ app = LambdaFunctionUrlResolver() +# The reason we use post is that whoever is writing tests can easily assert on the +# content being sent (body, headers, cookies, content-type) to reduce cognitive load. + @app.post("/todos") def todos(): @@ -26,10 +29,5 @@ def todos(): ) -@app.get("/hello") -def hello(): - return Response(status_code=200, content_type=content_types.TEXT_PLAIN, body="Hello World") - - def lambda_handler(event, context): return app.resolve(event, context) diff --git a/tests/e2e/event_handler/infrastructure.py b/tests/e2e/event_handler/infrastructure.py index 7774fc773a6..da456038a25 100644 --- a/tests/e2e/event_handler/infrastructure.py +++ b/tests/e2e/event_handler/infrastructure.py @@ -64,11 +64,6 @@ def _create_api_gateway_http(self, function: Function): integration=apigwv2integrations.HttpLambdaIntegration("TodosIntegration", function), ) - apigw.add_routes( - path="/hello", - methods=[apigwv2.HttpMethod.GET], - integration=apigwv2integrations.HttpLambdaIntegration("HelloIntegration", function), - ) CfnOutput(self.stack, "APIGatewayHTTPUrl", value=(apigw.url or "")) def _create_api_gateway_rest(self, function: Function): @@ -77,9 +72,6 @@ def _create_api_gateway_rest(self, function: Function): todos = apigw.root.add_resource("todos") todos.add_method("POST", apigwv1.LambdaIntegration(function, proxy=True)) - hello = apigw.root.add_resource("hello") - hello.add_method("GET", apigwv1.LambdaIntegration(function, proxy=True)) - CfnOutput(self.stack, "APIGatewayRestUrl", value=apigw.url) def _create_lambda_function_url(self, function: Function): diff --git a/tests/e2e/event_handler/test_paths_ending_with_slash.py b/tests/e2e/event_handler/test_paths_ending_with_slash.py index 44a9c84f633..4c1461d6fc5 100644 --- a/tests/e2e/event_handler/test_paths_ending_with_slash.py +++ b/tests/e2e/event_handler/test_paths_ending_with_slash.py @@ -34,62 +34,66 @@ def lambda_function_url_endpoint(infrastructure: dict) -> str: def test_api_gateway_rest_trailing_slash(apigw_rest_endpoint): - # GIVEN - url = f"{apigw_rest_endpoint}hello/" + # GIVEN API URL ends in a trailing slash + url = f"{apigw_rest_endpoint}todos/" body = "Hello World" - status_code = 200 # WHEN response = data_fetcher.get_http_response( Request( - method="GET", + method="POST", url=url, + json={"body": body}, ) ) - # THEN - assert response.status_code == status_code - # response.content is a binary string, needs to be decoded to compare with the real string - assert response.text == body + # THEN expect a HTTP 200 response + assert response.status_code == 200 def test_api_gateway_http_trailing_slash(apigw_http_endpoint): # GIVEN the URL for the API ends in a trailing slash API gateway should return a 404 - url = f"{apigw_http_endpoint}hello/" + url = f"{apigw_http_endpoint}todos/" + body = "Hello World" - # WHEN + # WHEN calling an invalid URL (with trailing slash) expect HTTPError exception from data_fetcher with pytest.raises(HTTPError): data_fetcher.get_http_response( Request( - method="GET", + method="POST", url=url, + json={"body": body}, ) ) def test_lambda_function_url_trailing_slash(lambda_function_url_endpoint): # GIVEN the URL for the API ends in a trailing slash it should behave as if there was not one - url = f"{lambda_function_url_endpoint}hello/" # the function url endpoint already has the trailing / + url = f"{lambda_function_url_endpoint}todos/" # the function url endpoint already has the trailing / + body = "Hello World" - # WHEN + # WHEN calling an invalid URL (with trailing slash) expect HTTPError exception from data_fetcher with pytest.raises(HTTPError): data_fetcher.get_http_response( Request( - method="GET", + method="POST", url=url, + json={"body": body}, ) ) def test_alb_url_trailing_slash(alb_multi_value_header_listener_endpoint): # GIVEN url has a trailing slash - it should behave as if there was not one - url = f"{alb_multi_value_header_listener_endpoint}/hello/" + url = f"{alb_multi_value_header_listener_endpoint}/todos/" + body = "Hello World" - # WHEN + # WHEN calling an invalid URL (with trailing slash) expect HTTPError exception from data_fetcher with pytest.raises(HTTPError): data_fetcher.get_http_response( Request( - method="GET", + method="POST", url=url, + json={"body": body}, ) ) From ce588d350673f26885ff120f2d1eeb018f7ac3a7 Mon Sep 17 00:00:00 2001 From: heitorlessa Date: Wed, 19 Oct 2022 08:51:48 +0200 Subject: [PATCH 12/12] docs(apigateway): document trailing slash in routes for API GW REST v1 --- docs/core/event_handler/api_gateway.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/core/event_handler/api_gateway.md b/docs/core/event_handler/api_gateway.md index 8ee07890b48..ec6116403e3 100644 --- a/docs/core/event_handler/api_gateway.md +++ b/docs/core/event_handler/api_gateway.md @@ -42,10 +42,10 @@ Before you decorate your functions to handle a given path and HTTP method(s), yo A resolver will handle request resolution, including [one or more routers](#split-routes-with-router), and give you access to the current event via typed properties. -For resolvers, we provide: `APIGatewayRestResolver`, `APIGatewayHttpResolver`, `ALBResolver`, and `LambdaFunctionUrlResolver` . +For resolvers, we provide: `APIGatewayRestResolver`, `APIGatewayHttpResolver`, `ALBResolver`, and `LambdaFunctionUrlResolver`. From here on, we will default to `APIGatewayRestResolver` across examples. -???+ info - We will use `APIGatewayRestResolver` as the default across examples. +???+ info "Auto-serialization" + We serialize `Dict` responses as JSON, trim whitespace for compact responses, and set content-type to `application/json`. #### API Gateway REST API @@ -53,8 +53,8 @@ When using Amazon API Gateway REST API to front your Lambda functions, you can u Here's an example on how we can handle the `/todos` path. -???+ info - We automatically serialize `Dict` responses as JSON, trim whitespace for compact responses, and set content-type to `application/json`. +???+ info "Trailing slash in routes" + For `APIGatewayRestResolver`, we seamless handle routes with a trailing slash (`/todos/`). === "getting_started_rest_api_resolver.py"