From 625fc51e876415e842806fd6d0784b12b938b5e8 Mon Sep 17 00:00:00 2001 From: heitorlessa Date: Fri, 23 Jul 2021 20:18:56 +0200 Subject: [PATCH 1/4] fix(apigw): route regression for non-word chars 1st take --- .../event_handler/api_gateway.py | 2 +- .../functional/event_handler/test_api_gateway.py | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/aws_lambda_powertools/event_handler/api_gateway.py b/aws_lambda_powertools/event_handler/api_gateway.py index 6948646a360..dacadd6f952 100644 --- a/aws_lambda_powertools/event_handler/api_gateway.py +++ b/aws_lambda_powertools/event_handler/api_gateway.py @@ -21,7 +21,7 @@ logger = logging.getLogger(__name__) _DYNAMIC_ROUTE_PATTERN = r"(<\w+>)" -_NAMED_GROUP_BOUNDARY_PATTERN = r"(?P\1\\w+\\b)" +_NAMED_GROUP_BOUNDARY_PATTERN = r"(?P\1[-%@.:\\w]+)" class ProxyEventType(Enum): diff --git a/tests/functional/event_handler/test_api_gateway.py b/tests/functional/event_handler/test_api_gateway.py index 1c7c53f7187..d098b4d8697 100644 --- a/tests/functional/event_handler/test_api_gateway.py +++ b/tests/functional/event_handler/test_api_gateway.py @@ -701,3 +701,19 @@ def get_network_account(account_id: str, network_id: str): event["resource"] = "/accounts/{account_id}/source_networks/{network_id}" event["path"] = "/accounts/nested_account/source_networks/network" app.resolve(event, {}) + + +def test_non_word_chars_route(): + # GIVEN + app = ApiGatewayResolver() + event = deepcopy(LOAD_GW_EVENT) + + # WHEN + @app.get("/accounts/") + def get_account(account_id: str): + assert account_id == "12345" + + # THEN + event["resource"] = "/accounts/{account_id}" + event["path"] = "/accounts/12345" + app.resolve(event, None) From 9e58198dcea81141f6b02f932db34b8947e51c9f Mon Sep 17 00:00:00 2001 From: heitorlessa Date: Fri, 23 Jul 2021 20:36:03 +0200 Subject: [PATCH 2/4] fix(apigw): add all safe URI chars RFC3986 --- aws_lambda_powertools/event_handler/api_gateway.py | 3 ++- tests/functional/event_handler/test_api_gateway.py | 11 +++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/aws_lambda_powertools/event_handler/api_gateway.py b/aws_lambda_powertools/event_handler/api_gateway.py index dacadd6f952..d4f0add826d 100644 --- a/aws_lambda_powertools/event_handler/api_gateway.py +++ b/aws_lambda_powertools/event_handler/api_gateway.py @@ -21,7 +21,8 @@ logger = logging.getLogger(__name__) _DYNAMIC_ROUTE_PATTERN = r"(<\w+>)" -_NAMED_GROUP_BOUNDARY_PATTERN = r"(?P\1[-%@.:\\w]+)" +# Safe URI chars https://www.ietf.org/rfc/rfc3986.txt +_NAMED_GROUP_BOUNDARY_PATTERN = r"(?P\1[-._~()'!*:@,;\\w]+)" class ProxyEventType(Enum): diff --git a/tests/functional/event_handler/test_api_gateway.py b/tests/functional/event_handler/test_api_gateway.py index d098b4d8697..131d98560b6 100644 --- a/tests/functional/event_handler/test_api_gateway.py +++ b/tests/functional/event_handler/test_api_gateway.py @@ -703,7 +703,8 @@ def get_network_account(account_id: str, network_id: str): app.resolve(event, {}) -def test_non_word_chars_route(): +@pytest.mark.parametrize("req", [123456789, "user@example.com", "", "-._~'!*:@,;"]) +def test_non_word_chars_route(req): # GIVEN app = ApiGatewayResolver() event = deepcopy(LOAD_GW_EVENT) @@ -711,9 +712,11 @@ def test_non_word_chars_route(): # WHEN @app.get("/accounts/") def get_account(account_id: str): - assert account_id == "12345" + assert account_id == f"{req}" # THEN event["resource"] = "/accounts/{account_id}" - event["path"] = "/accounts/12345" - app.resolve(event, None) + event["path"] = f"/accounts/{req}" + + ret = app.resolve(event, None) + assert ret["statusCode"] == 200 From db20c9b2f62bffe41f561ba605ee0040960d97d0 Mon Sep 17 00:00:00 2001 From: heitorlessa Date: Fri, 23 Jul 2021 20:39:39 +0200 Subject: [PATCH 3/4] fix(apigw): add all safe URI chars RFC3986 pt2 --- aws_lambda_powertools/event_handler/api_gateway.py | 2 +- tests/functional/event_handler/test_api_gateway.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/aws_lambda_powertools/event_handler/api_gateway.py b/aws_lambda_powertools/event_handler/api_gateway.py index d4f0add826d..f779526afa1 100644 --- a/aws_lambda_powertools/event_handler/api_gateway.py +++ b/aws_lambda_powertools/event_handler/api_gateway.py @@ -22,7 +22,7 @@ _DYNAMIC_ROUTE_PATTERN = r"(<\w+>)" # Safe URI chars https://www.ietf.org/rfc/rfc3986.txt -_NAMED_GROUP_BOUNDARY_PATTERN = r"(?P\1[-._~()'!*:@,;\\w]+)" +_NAMED_GROUP_BOUNDARY_PATTERN = r"(?P\1[-._~()'!*:@%#,;\\w]+)" class ProxyEventType(Enum): diff --git a/tests/functional/event_handler/test_api_gateway.py b/tests/functional/event_handler/test_api_gateway.py index 131d98560b6..e99feed8b04 100644 --- a/tests/functional/event_handler/test_api_gateway.py +++ b/tests/functional/event_handler/test_api_gateway.py @@ -703,7 +703,7 @@ def get_network_account(account_id: str, network_id: str): app.resolve(event, {}) -@pytest.mark.parametrize("req", [123456789, "user@example.com", "", "-._~'!*:@,;"]) +@pytest.mark.parametrize("req", [123456789, "user@example.com", "-._~'!*:@,;%#"]) def test_non_word_chars_route(req): # GIVEN app = ApiGatewayResolver() From 4dd0149054e3ae2550cd3acfdc598ad451782aeb Mon Sep 17 00:00:00 2001 From: heitorlessa Date: Fri, 23 Jul 2021 21:22:06 +0200 Subject: [PATCH 4/4] fix(apigw): add unsafe URI chars as APIGW ALB decode them --- aws_lambda_powertools/event_handler/api_gateway.py | 7 +++++-- tests/functional/event_handler/test_api_gateway.py | 10 +++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/aws_lambda_powertools/event_handler/api_gateway.py b/aws_lambda_powertools/event_handler/api_gateway.py index f779526afa1..44d3f2b07de 100644 --- a/aws_lambda_powertools/event_handler/api_gateway.py +++ b/aws_lambda_powertools/event_handler/api_gateway.py @@ -21,8 +21,11 @@ logger = logging.getLogger(__name__) _DYNAMIC_ROUTE_PATTERN = r"(<\w+>)" -# Safe URI chars https://www.ietf.org/rfc/rfc3986.txt -_NAMED_GROUP_BOUNDARY_PATTERN = r"(?P\1[-._~()'!*:@%#,;\\w]+)" +_SAFE_URI = "-._~()'!*:@,;" # https://www.ietf.org/rfc/rfc3986.txt +# API GW/ALB decode non-safe URI chars; we must support them too +_UNSAFE_URI = "%<>\[\]{}|^" # noqa: W605 + +_NAMED_GROUP_BOUNDARY_PATTERN = fr"(?P\1[{_SAFE_URI}{_UNSAFE_URI}\\w]+)" class ProxyEventType(Enum): diff --git a/tests/functional/event_handler/test_api_gateway.py b/tests/functional/event_handler/test_api_gateway.py index e99feed8b04..f16086ba634 100644 --- a/tests/functional/event_handler/test_api_gateway.py +++ b/tests/functional/event_handler/test_api_gateway.py @@ -703,7 +703,15 @@ def get_network_account(account_id: str, network_id: str): app.resolve(event, {}) -@pytest.mark.parametrize("req", [123456789, "user@example.com", "-._~'!*:@,;%#"]) +@pytest.mark.parametrize( + "req", + [ + pytest.param(123456789, id="num"), + pytest.param("user@example.com", id="email"), + pytest.param("-._~'!*:@,;()", id="safe-rfc3986"), + pytest.param("%<>[]{}|^", id="unsafe-rfc3986"), + ], +) def test_non_word_chars_route(req): # GIVEN app = ApiGatewayResolver()