From f0d817c6985541888c364901a404fd946f9a356d Mon Sep 17 00:00:00 2001 From: Benjamin Simon Date: Thu, 25 Jul 2024 21:47:30 +0200 Subject: [PATCH 1/6] add test --- .../apigateway/test_apigateway_lambda.py | 13 + .../test_apigateway_lambda.snapshot.json | 308 ++++++++++++++++-- .../test_apigateway_lambda.validation.json | 2 +- 3 files changed, 286 insertions(+), 37 deletions(-) diff --git a/tests/aws/services/apigateway/test_apigateway_lambda.py b/tests/aws/services/apigateway/test_apigateway_lambda.py index 37751aaa75018..f7e4d2eafdba8 100644 --- a/tests/aws/services/apigateway/test_apigateway_lambda.py +++ b/tests/aws/services/apigateway/test_apigateway_lambda.py @@ -181,6 +181,19 @@ def invoke_api(url): response_trailing_slash = retry(invoke_api, sleep=2, retries=10, url=f"{invocation_url}/") snapshot.match("invocation-payload-with-trailing-slash", response_trailing_slash.json()) + # invoke rest api with double slash in proxy param + response_double_slash = retry( + invoke_api, sleep=2, retries=10, url=f"{invocation_url}//test-path" + ) + snapshot.match("invocation-payload-with-double-slash", response_double_slash.json()) + + # invoke rest api with prepended slash + slash_between_stage_and_path = invocation_url.replace("/test-path", "//test-path") + response_prepend_slash = retry( + invoke_api, sleep=2, retries=10, url=slash_between_stage_and_path + ) + snapshot.match("invocation-payload-with-prepended-slash", response_prepend_slash.json()) + response_no_trailing_slash = retry( invoke_api, sleep=2, retries=10, url=f"{invocation_url}?urlparam=test" ) diff --git a/tests/aws/services/apigateway/test_apigateway_lambda.snapshot.json b/tests/aws/services/apigateway/test_apigateway_lambda.snapshot.json index 8af3f800b0cf3..69341de38ef8b 100644 --- a/tests/aws/services/apigateway/test_apigateway_lambda.snapshot.json +++ b/tests/aws/services/apigateway/test_apigateway_lambda.snapshot.json @@ -1,6 +1,6 @@ { "tests/aws/services/apigateway/test_apigateway_lambda.py::test_lambda_aws_proxy_integration": { - "recorded-date": "22-07-2024, 22:41:52", + "recorded-date": "25-07-2024, 19:31:32", "recorded-content": { "invocation-payload-without-trailing-slash": { "body": null, @@ -238,7 +238,7 @@ "resource": "/{proxy+}", "stageVariables": null }, - "invocation-payload-without-trailing-slash-and-query-params": { + "invocation-payload-with-double-slash": { "body": null, "headers": { "Accept": "*/*", @@ -316,6 +316,242 @@ "aValUE" ] }, + "multiValueQueryStringParameters": null, + "path": "/test-path//test-path", + "pathParameters": { + "proxy": "test-path//test-path" + }, + "queryStringParameters": null, + "requestContext": { + "accountId": "111111111111", + "apiId": "", + "deploymentId": "", + "domainName": "", + "domainPrefix": "", + "extendedRequestId": "", + "httpMethod": "GET", + "identity": { + "accessKey": null, + "accountId": null, + "caller": null, + "cognitoAuthenticationProvider": null, + "cognitoAuthenticationType": null, + "cognitoIdentityId": null, + "cognitoIdentityPoolId": null, + "principalOrgId": null, + "sourceIp": "", + "user": null, + "userAgent": "python-requests/testing", + "userArn": null + }, + "path": "/test/test-path//test-path", + "protocol": "HTTP/1.1", + "requestId": "", + "requestTime": "", + "requestTimeEpoch": "", + "resourceId": "", + "resourcePath": "/{proxy+}", + "stage": "test" + }, + "resource": "/{proxy+}", + "stageVariables": null + }, + "invocation-payload-with-prepended-slash": { + "body": null, + "headers": { + "Accept": "*/*", + "Accept-Encoding": "gzip, deflate", + "Authorization": "random-value", + "CloudFront-Forwarded-Proto": "https", + "CloudFront-Is-Desktop-Viewer": "true", + "CloudFront-Is-Mobile-Viewer": "false", + "CloudFront-Is-SmartTV-Viewer": "false", + "CloudFront-Is-Tablet-Viewer": "false", + "CloudFront-Viewer-ASN": "", + "CloudFront-Viewer-Country": "", + "Host": "", + "User-Agent": "python-requests/testing", + "Via": "", + "X-Amz-Cf-Id": "", + "X-Amzn-Trace-Id": "", + "X-Forwarded-For": "", + "X-Forwarded-Port": "", + "X-Forwarded-Proto": "", + "tEsT-HEADeR": "aValUE" + }, + "httpMethod": "GET", + "isBase64Encoded": false, + "multiValueHeaders": { + "Accept": [ + "*/*" + ], + "Accept-Encoding": [ + "gzip, deflate" + ], + "Authorization": [ + "random-value" + ], + "CloudFront-Forwarded-Proto": [ + "https" + ], + "CloudFront-Is-Desktop-Viewer": [ + "true" + ], + "CloudFront-Is-Mobile-Viewer": [ + "false" + ], + "CloudFront-Is-SmartTV-Viewer": [ + "false" + ], + "CloudFront-Is-Tablet-Viewer": [ + "false" + ], + "CloudFront-Viewer-ASN": [ + "" + ], + "CloudFront-Viewer-Country": [ + "" + ], + "Host": [ + "" + ], + "User-Agent": [ + "python-requests/testing" + ], + "Via": [ + "" + ], + "X-Amz-Cf-Id": [ + "" + ], + "X-Amzn-Trace-Id": [ + "" + ], + "X-Forwarded-For": "", + "X-Forwarded-Port": "", + "X-Forwarded-Proto": "", + "tEsT-HEADeR": [ + "aValUE" + ] + }, + "multiValueQueryStringParameters": null, + "path": "/test-path", + "pathParameters": { + "proxy": "test-path" + }, + "queryStringParameters": null, + "requestContext": { + "accountId": "111111111111", + "apiId": "", + "deploymentId": "", + "domainName": "", + "domainPrefix": "", + "extendedRequestId": "", + "httpMethod": "GET", + "identity": { + "accessKey": null, + "accountId": null, + "caller": null, + "cognitoAuthenticationProvider": null, + "cognitoAuthenticationType": null, + "cognitoIdentityId": null, + "cognitoIdentityPoolId": null, + "principalOrgId": null, + "sourceIp": "", + "user": null, + "userAgent": "python-requests/testing", + "userArn": null + }, + "path": "/test//test-path", + "protocol": "HTTP/1.1", + "requestId": "", + "requestTime": "", + "requestTimeEpoch": "", + "resourceId": "", + "resourcePath": "/{proxy+}", + "stage": "test" + }, + "resource": "/{proxy+}", + "stageVariables": null + }, + "invocation-payload-without-trailing-slash-and-query-params": { + "body": null, + "headers": { + "Accept": "*/*", + "Accept-Encoding": "gzip, deflate", + "Authorization": "random-value", + "CloudFront-Forwarded-Proto": "https", + "CloudFront-Is-Desktop-Viewer": "true", + "CloudFront-Is-Mobile-Viewer": "false", + "CloudFront-Is-SmartTV-Viewer": "false", + "CloudFront-Is-Tablet-Viewer": "false", + "CloudFront-Viewer-ASN": "", + "CloudFront-Viewer-Country": "", + "Host": "", + "User-Agent": "python-requests/testing", + "Via": "", + "X-Amz-Cf-Id": "", + "X-Amzn-Trace-Id": "", + "X-Forwarded-For": "", + "X-Forwarded-Port": "", + "X-Forwarded-Proto": "", + "tEsT-HEADeR": "aValUE" + }, + "httpMethod": "GET", + "isBase64Encoded": false, + "multiValueHeaders": { + "Accept": [ + "*/*" + ], + "Accept-Encoding": [ + "gzip, deflate" + ], + "Authorization": [ + "random-value" + ], + "CloudFront-Forwarded-Proto": [ + "https" + ], + "CloudFront-Is-Desktop-Viewer": [ + "true" + ], + "CloudFront-Is-Mobile-Viewer": [ + "false" + ], + "CloudFront-Is-SmartTV-Viewer": [ + "false" + ], + "CloudFront-Is-Tablet-Viewer": [ + "false" + ], + "CloudFront-Viewer-ASN": [ + "" + ], + "CloudFront-Viewer-Country": [ + "" + ], + "Host": [ + "" + ], + "User-Agent": [ + "python-requests/testing" + ], + "Via": [ + "" + ], + "X-Amz-Cf-Id": [ + "" + ], + "X-Amzn-Trace-Id": [ + "" + ], + "X-Forwarded-For": "", + "X-Forwarded-Port": "", + "X-Forwarded-Proto": "", + "tEsT-HEADeR": [ + "aValUE" + ] + }, "multiValueQueryStringParameters": { "urlparam": [ "test" @@ -334,7 +570,7 @@ "deploymentId": "", "domainName": "", "domainPrefix": "", - "extendedRequestId": "", + "extendedRequestId": "", "httpMethod": "GET", "identity": { "accessKey": null, @@ -352,7 +588,7 @@ }, "path": "/test/test-path", "protocol": "HTTP/1.1", - "requestId": "", + "requestId": "", "requestTime": "", "requestTimeEpoch": "", "resourceId": "", @@ -377,9 +613,9 @@ "CloudFront-Viewer-Country": "", "Host": "", "User-Agent": "python-requests/testing", - "Via": "", - "X-Amz-Cf-Id": "", - "X-Amzn-Trace-Id": "", + "Via": "", + "X-Amz-Cf-Id": "", + "X-Amzn-Trace-Id": "", "X-Forwarded-For": "", "X-Forwarded-Port": "", "X-Forwarded-Proto": "", @@ -425,13 +661,13 @@ "python-requests/testing" ], "Via": [ - "" + "" ], "X-Amz-Cf-Id": [ - "" + "" ], "X-Amzn-Trace-Id": [ - "" + "" ], "X-Forwarded-For": "", "X-Forwarded-Port": "", @@ -458,7 +694,7 @@ "deploymentId": "", "domainName": "", "domainPrefix": "", - "extendedRequestId": "", + "extendedRequestId": "", "httpMethod": "GET", "identity": { "accessKey": null, @@ -476,7 +712,7 @@ }, "path": "/test/test-path/", "protocol": "HTTP/1.1", - "requestId": "", + "requestId": "", "requestTime": "", "requestTimeEpoch": "", "resourceId": "", @@ -501,9 +737,9 @@ "CloudFront-Viewer-Country": "", "Host": "", "User-Agent": "python-requests/testing", - "Via": "", - "X-Amz-Cf-Id": "", - "X-Amzn-Trace-Id": "", + "Via": "", + "X-Amz-Cf-Id": "", + "X-Amzn-Trace-Id": "", "X-Forwarded-For": "", "X-Forwarded-Port": "", "X-Forwarded-Proto": "", @@ -549,13 +785,13 @@ "python-requests/testing" ], "Via": [ - "" + "" ], "X-Amz-Cf-Id": [ - "" + "" ], "X-Amzn-Trace-Id": [ - "" + "" ], "X-Forwarded-For": "", "X-Forwarded-Port": "", @@ -576,7 +812,7 @@ "deploymentId": "", "domainName": "", "domainPrefix": "", - "extendedRequestId": "", + "extendedRequestId": "", "httpMethod": "GET", "identity": { "accessKey": null, @@ -594,7 +830,7 @@ }, "path": "/test/test-path/api/user/test%2Balias@gmail.com/plus/test+alias@gmail.com", "protocol": "HTTP/1.1", - "requestId": "", + "requestId": "", "requestTime": "", "requestTimeEpoch": "", "resourceId": "", @@ -619,9 +855,9 @@ "CloudFront-Viewer-Country": "", "Host": "", "User-Agent": "python-requests/testing", - "Via": "", - "X-Amz-Cf-Id": "", - "X-Amzn-Trace-Id": "", + "Via": "", + "X-Amz-Cf-Id": "", + "X-Amzn-Trace-Id": "", "X-Forwarded-For": "", "X-Forwarded-Port": "", "X-Forwarded-Proto": "", @@ -667,13 +903,13 @@ "python-requests/testing" ], "Via": [ - "" + "" ], "X-Amz-Cf-Id": [ - "" + "" ], "X-Amzn-Trace-Id": [ - "" + "" ], "X-Forwarded-For": "", "X-Forwarded-Port": "", @@ -720,7 +956,7 @@ "deploymentId": "", "domainName": "", "domainPrefix": "", - "extendedRequestId": "", + "extendedRequestId": "", "httpMethod": "GET", "identity": { "accessKey": null, @@ -738,7 +974,7 @@ }, "path": "/test/test-path/api", "protocol": "HTTP/1.1", - "requestId": "", + "requestId": "", "requestTime": "", "requestTimeEpoch": "", "resourceId": "", @@ -766,9 +1002,9 @@ "Content-Type": "application/json;charset=utf-8", "Host": "", "User-Agent": "python-requests/testing", - "Via": "", - "X-Amz-Cf-Id": "", - "X-Amzn-Trace-Id": "", + "Via": "", + "X-Amz-Cf-Id": "", + "X-Amzn-Trace-Id": "", "X-Forwarded-For": "", "X-Forwarded-Port": "", "X-Forwarded-Proto": "" @@ -816,13 +1052,13 @@ "python-requests/testing" ], "Via": [ - "" + "" ], "X-Amz-Cf-Id": [ - "" + "" ], "X-Amzn-Trace-Id": [ - "" + "" ], "X-Forwarded-For": "", "X-Forwarded-Port": "", @@ -853,7 +1089,7 @@ "deploymentId": "", "domainName": "", "domainPrefix": "", - "extendedRequestId": "", + "extendedRequestId": "", "httpMethod": "POST", "identity": { "accessKey": null, @@ -871,7 +1107,7 @@ }, "path": "/test/test-path", "protocol": "HTTP/1.1", - "requestId": "", + "requestId": "", "requestTime": "", "requestTimeEpoch": "", "resourceId": "", diff --git a/tests/aws/services/apigateway/test_apigateway_lambda.validation.json b/tests/aws/services/apigateway/test_apigateway_lambda.validation.json index 1bfd60ef179b1..558a50302ec50 100644 --- a/tests/aws/services/apigateway/test_apigateway_lambda.validation.json +++ b/tests/aws/services/apigateway/test_apigateway_lambda.validation.json @@ -9,7 +9,7 @@ "last_validated_date": "2023-05-31T21:09:38+00:00" }, "tests/aws/services/apigateway/test_apigateway_lambda.py::test_lambda_aws_proxy_integration": { - "last_validated_date": "2024-07-22T22:41:52+00:00" + "last_validated_date": "2024-07-25T19:31:32+00:00" }, "tests/aws/services/apigateway/test_apigateway_lambda.py::test_lambda_aws_proxy_integration_non_post_method": { "last_validated_date": "2024-07-10T15:43:36+00:00" From aeabbc1888acae2c4e059e471bee81ecf5f8f01d Mon Sep 17 00:00:00 2001 From: Benjamin Simon Date: Thu, 25 Jul 2024 22:43:14 +0200 Subject: [PATCH 2/6] add GreedyPathConverter for EdgeRouter --- .../localstack/aws/protocol/op_router.py | 2 +- .../localstack/aws/protocol/routing.py | 16 +----- localstack-core/localstack/services/edge.py | 20 +++++++- tests/unit/aws/protocol/test_op_router.py | 3 +- tests/unit/http_/test_router.py | 51 +++++++++++++++++++ 5 files changed, 74 insertions(+), 18 deletions(-) diff --git a/localstack-core/localstack/aws/protocol/op_router.py b/localstack-core/localstack/aws/protocol/op_router.py index 126d417d94742..e45e2984edbf3 100644 --- a/localstack-core/localstack/aws/protocol/op_router.py +++ b/localstack-core/localstack/aws/protocol/op_router.py @@ -8,7 +8,6 @@ from werkzeug.routing import Map, MapAdapter from localstack.aws.protocol.routing import ( - GreedyPathConverter, StrictMethodRule, path_param_regex, post_process_arg_name, @@ -16,6 +15,7 @@ ) from localstack.http import Request from localstack.http.request import get_raw_path +from localstack.services.edge import GreedyPathConverter class _HttpOperation(NamedTuple): diff --git a/localstack-core/localstack/aws/protocol/routing.py b/localstack-core/localstack/aws/protocol/routing.py index 62df3dca73fcb..f793bd051ec27 100644 --- a/localstack-core/localstack/aws/protocol/routing.py +++ b/localstack-core/localstack/aws/protocol/routing.py @@ -1,7 +1,7 @@ import re from typing import AnyStr -from werkzeug.routing import PathConverter, Rule +from werkzeug.routing import Rule # Regex to find path parameters in requestUris of AWS service specs (f.e. /{param1}/{param2+}) path_param_regex = re.compile(r"({.+?})") @@ -12,20 +12,6 @@ _rule_replacement_table = str.maketrans(_rule_replacements) -class GreedyPathConverter(PathConverter): - """ - This converter makes sure that the path ``/mybucket//mykey`` can be matched to the pattern - ``/`` and will result in `Key` being `/mykey`. - """ - - regex = ".*?" - - part_isolating = False - """From the werkzeug docs: If a custom converter can match a forward slash, /, it should have the - attribute part_isolating set to False. This will ensure that rules using the custom converter are - correctly matched.""" - - class StrictMethodRule(Rule): """ Small extension to Werkzeug's Rule class which reverts unwanted assumptions made by Werkzeug. diff --git a/localstack-core/localstack/services/edge.py b/localstack-core/localstack/services/edge.py index 495525867383d..ec900a27cb952 100644 --- a/localstack-core/localstack/services/edge.py +++ b/localstack-core/localstack/services/edge.py @@ -5,6 +5,8 @@ import sys from typing import List, Optional, TypeVar +from werkzeug.routing import PathConverter + from localstack import config, constants from localstack.config import HostAndPort from localstack.constants import ( @@ -23,7 +25,23 @@ LOG = logging.getLogger(__name__) -ROUTER: Router[Handler] = Router(dispatcher=handler_dispatcher()) +class GreedyPathConverter(PathConverter): + """ + This converter makes sure that the path ``/mybucket//mykey`` can be matched to the pattern + ``/`` and will result in `Key` being `/mykey`. + """ + + regex = ".*?" + + part_isolating = False + """From the werkzeug docs: If a custom converter can match a forward slash, /, it should have the + attribute part_isolating set to False. This will ensure that rules using the custom converter are + correctly matched.""" + + +ROUTER: Router[Handler] = Router( + dispatcher=handler_dispatcher(), converters={"greedy_path": GreedyPathConverter} +) """This special Router is part of the edge proxy. Use the router to inject custom handlers that are handled before the actual AWS service call is made.""" diff --git a/tests/unit/aws/protocol/test_op_router.py b/tests/unit/aws/protocol/test_op_router.py index d010ba83df515..8afed21d413a2 100644 --- a/tests/unit/aws/protocol/test_op_router.py +++ b/tests/unit/aws/protocol/test_op_router.py @@ -2,9 +2,10 @@ from werkzeug.exceptions import NotFound from werkzeug.routing import Map, Rule -from localstack.aws.protocol.op_router import GreedyPathConverter, RestServiceOperationRouter +from localstack.aws.protocol.op_router import RestServiceOperationRouter from localstack.aws.spec import list_services, load_service from localstack.http import Request +from localstack.services.edge import GreedyPathConverter def _collect_services(): diff --git a/tests/unit/http_/test_router.py b/tests/unit/http_/test_router.py index e3c132b9004f3..1df42ffae0223 100644 --- a/tests/unit/http_/test_router.py +++ b/tests/unit/http_/test_router.py @@ -9,6 +9,7 @@ from localstack.http import Request, Response, Router from localstack.http.router import E, RequestArguments, RuleAdapter, WithHost, route +from localstack.services.edge import GreedyPathConverter from localstack.utils.common import get_free_tcp_port @@ -288,6 +289,56 @@ def test_path_converter_and_greedy_regex_in_host(self): "port": None, } + def test_greedy_path_converter(self): + router = Router(converters={"greedy_path": GreedyPathConverter}) + router.add(path="/", endpoint=echo_params_json) + + assert router.dispatch(Request(path="/my")).json == {"path": "my"} + assert router.dispatch(Request(path="/my/")).json == {"path": "my/"} + assert router.dispatch(Request(path="/my//path")).json == {"path": "my//path"} + assert router.dispatch(Request(path="/my//path/")).json == {"path": "my//path/"} + assert router.dispatch(Request(path="/my/path foobar")).json == {"path": "my/path foobar"} + assert router.dispatch(Request(path="//foobar")).json == {"path": "foobar"} + assert router.dispatch(Request(path="//foobar/")).json == {"path": "foobar/"} + + def test_greedy_path_converter_with_args(self): + router = Router(converters={"greedy_path": GreedyPathConverter}) + router.add(path="/with-args//", endpoint=echo_params_json) + + assert router.dispatch(Request(path="/with-args/123456/my")).json == { + "some_id": "123456", + "path": "my", + } + + # werkzeug no longer removes trailing slashes in matches + assert router.dispatch(Request(path="/with-args/123456/my/")).json == { + "some_id": "123456", + "path": "my/", + } + + # works with sub paths + assert router.dispatch(Request(path="/with-args/123456/my/path")).json == { + "some_id": "123456", + "path": "my/path", + } + + # no sub path with no trailing slash raises 404 + with pytest.raises(NotFound): + router.dispatch(Request(path="/with-args/123456")) + + # greedy path accepts empty sub path if there's a trailing slash + assert router.dispatch(Request(path="/with-args/123456/")).json == { + "some_id": "123456", + "path": "", + } + + # with the GreedyPath converter, we no longer redirect and accept the request + # in order the retrieve the double slash between parameter, we might need to use the RAW_URI + assert router.dispatch(Request(path="/with-args/123456//my/test//")).json == { + "some_id": "123456", + "path": "/my/test//", + } + def test_remove_rule(self): router = Router() From aa0f950e10e0ca5467eb5cc904a21d67fa265af4 Mon Sep 17 00:00:00 2001 From: Benjamin Simon Date: Thu, 25 Jul 2024 22:43:30 +0200 Subject: [PATCH 3/6] fix double slash for APIGW NG --- .../next_gen/execute_api/handlers/parse.py | 3 +- .../execute_api/handlers/resource_router.py | 2 +- .../apigateway/next_gen/execute_api/router.py | 4 +- .../apigateway/test_apigateway_lambda.py | 7 + .../test_apigateway_lambda.snapshot.json | 190 ++++++++++++++---- .../test_apigateway_lambda.validation.json | 2 +- 6 files changed, 166 insertions(+), 42 deletions(-) diff --git a/localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/parse.py b/localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/parse.py index 311ac20069d87..020ee10ca8ce5 100644 --- a/localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/parse.py +++ b/localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/parse.py @@ -144,8 +144,7 @@ def create_context_variables(context: RestApiInvocationContext) -> ContextVariab userAgent=invocation_request["headers"].get("User-Agent"), userArn=None, ), - # TODO: check if we need the raw path? with forward slashes - path=f"/{context.stage}{invocation_request['path']}", + path=f"/{context.stage}{invocation_request['raw_path']}", protocol="HTTP/1.1", requestId=long_uid(), requestTime=timestamp(time=now, format=REQUEST_TIME_DATE_FORMAT), diff --git a/localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/resource_router.py b/localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/resource_router.py index f5b3064db8cf7..9f11a38a22dc3 100644 --- a/localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/resource_router.py +++ b/localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/resource_router.py @@ -8,13 +8,13 @@ from localstack.aws.api.apigateway import Resource from localstack.aws.protocol.routing import ( - GreedyPathConverter, path_param_regex, post_process_arg_name, transform_path_params_to_rule_vars, ) from localstack.http import Response from localstack.services.apigateway.models import RestApiDeployment +from localstack.services.edge import GreedyPathConverter from ..api import RestApiGatewayHandler, RestApiGatewayHandlerChain from ..context import RestApiInvocationContext diff --git a/localstack-core/localstack/services/apigateway/next_gen/execute_api/router.py b/localstack-core/localstack/services/apigateway/next_gen/execute_api/router.py index 4965e4eda66df..7eb7476d56c37 100644 --- a/localstack-core/localstack/services/apigateway/next_gen/execute_api/router.py +++ b/localstack-core/localstack/services/apigateway/next_gen/execute_api/router.py @@ -107,7 +107,7 @@ def register_routes(self) -> None: strict_slashes=False, ), self.router.add( - path="//", + path="//", host=host_pattern, endpoint=self.handler, strict_slashes=True, @@ -119,7 +119,7 @@ def register_routes(self) -> None: defaults={"path": ""}, ), self.router.add( - path="/restapis///_user_request_/", + path="/restapis///_user_request_/", endpoint=self.handler, strict_slashes=True, ), diff --git a/tests/aws/services/apigateway/test_apigateway_lambda.py b/tests/aws/services/apigateway/test_apigateway_lambda.py index f7e4d2eafdba8..f2bb3df57fb00 100644 --- a/tests/aws/services/apigateway/test_apigateway_lambda.py +++ b/tests/aws/services/apigateway/test_apigateway_lambda.py @@ -187,6 +187,13 @@ def invoke_api(url): ) snapshot.match("invocation-payload-with-double-slash", response_double_slash.json()) + # invoke rest api with prepended slash to the stage (///) + double_slash_before_stage = invocation_url.replace(f"/{stage_name}/", f"//{stage_name}/") + response_prepend_slash = retry(invoke_api, sleep=2, retries=10, url=double_slash_before_stage) + snapshot.match( + "invocation-payload-with-prepended-slash-to-stage", response_prepend_slash.json() + ) + # invoke rest api with prepended slash slash_between_stage_and_path = invocation_url.replace("/test-path", "//test-path") response_prepend_slash = retry( diff --git a/tests/aws/services/apigateway/test_apigateway_lambda.snapshot.json b/tests/aws/services/apigateway/test_apigateway_lambda.snapshot.json index 69341de38ef8b..96817abc33fb8 100644 --- a/tests/aws/services/apigateway/test_apigateway_lambda.snapshot.json +++ b/tests/aws/services/apigateway/test_apigateway_lambda.snapshot.json @@ -1,6 +1,6 @@ { "tests/aws/services/apigateway/test_apigateway_lambda.py::test_lambda_aws_proxy_integration": { - "recorded-date": "25-07-2024, 19:31:32", + "recorded-date": "25-07-2024, 20:18:13", "recorded-content": { "invocation-payload-without-trailing-slash": { "body": null, @@ -356,7 +356,7 @@ "resource": "/{proxy+}", "stageVariables": null }, - "invocation-payload-with-prepended-slash": { + "invocation-payload-with-prepended-slash-to-stage": { "body": null, "headers": { "Accept": "*/*", @@ -371,7 +371,7 @@ "CloudFront-Viewer-Country": "", "Host": "", "User-Agent": "python-requests/testing", - "Via": "", + "Via": "", "X-Amz-Cf-Id": "", "X-Amzn-Trace-Id": "", "X-Forwarded-For": "", @@ -419,7 +419,7 @@ "python-requests/testing" ], "Via": [ - "" + "" ], "X-Amz-Cf-Id": [ "" @@ -462,7 +462,7 @@ "userAgent": "python-requests/testing", "userArn": null }, - "path": "/test//test-path", + "path": "/test/test-path", "protocol": "HTTP/1.1", "requestId": "", "requestTime": "", @@ -474,7 +474,7 @@ "resource": "/{proxy+}", "stageVariables": null }, - "invocation-payload-without-trailing-slash-and-query-params": { + "invocation-payload-with-prepended-slash": { "body": null, "headers": { "Accept": "*/*", @@ -489,7 +489,7 @@ "CloudFront-Viewer-Country": "", "Host": "", "User-Agent": "python-requests/testing", - "Via": "", + "Via": "", "X-Amz-Cf-Id": "", "X-Amzn-Trace-Id": "", "X-Forwarded-For": "", @@ -537,7 +537,7 @@ "python-requests/testing" ], "Via": [ - "" + "" ], "X-Amz-Cf-Id": [ "" @@ -552,6 +552,124 @@ "aValUE" ] }, + "multiValueQueryStringParameters": null, + "path": "/test-path", + "pathParameters": { + "proxy": "test-path" + }, + "queryStringParameters": null, + "requestContext": { + "accountId": "111111111111", + "apiId": "", + "deploymentId": "", + "domainName": "", + "domainPrefix": "", + "extendedRequestId": "", + "httpMethod": "GET", + "identity": { + "accessKey": null, + "accountId": null, + "caller": null, + "cognitoAuthenticationProvider": null, + "cognitoAuthenticationType": null, + "cognitoIdentityId": null, + "cognitoIdentityPoolId": null, + "principalOrgId": null, + "sourceIp": "", + "user": null, + "userAgent": "python-requests/testing", + "userArn": null + }, + "path": "/test//test-path", + "protocol": "HTTP/1.1", + "requestId": "", + "requestTime": "", + "requestTimeEpoch": "", + "resourceId": "", + "resourcePath": "/{proxy+}", + "stage": "test" + }, + "resource": "/{proxy+}", + "stageVariables": null + }, + "invocation-payload-without-trailing-slash-and-query-params": { + "body": null, + "headers": { + "Accept": "*/*", + "Accept-Encoding": "gzip, deflate", + "Authorization": "random-value", + "CloudFront-Forwarded-Proto": "https", + "CloudFront-Is-Desktop-Viewer": "true", + "CloudFront-Is-Mobile-Viewer": "false", + "CloudFront-Is-SmartTV-Viewer": "false", + "CloudFront-Is-Tablet-Viewer": "false", + "CloudFront-Viewer-ASN": "", + "CloudFront-Viewer-Country": "", + "Host": "", + "User-Agent": "python-requests/testing", + "Via": "", + "X-Amz-Cf-Id": "", + "X-Amzn-Trace-Id": "", + "X-Forwarded-For": "", + "X-Forwarded-Port": "", + "X-Forwarded-Proto": "", + "tEsT-HEADeR": "aValUE" + }, + "httpMethod": "GET", + "isBase64Encoded": false, + "multiValueHeaders": { + "Accept": [ + "*/*" + ], + "Accept-Encoding": [ + "gzip, deflate" + ], + "Authorization": [ + "random-value" + ], + "CloudFront-Forwarded-Proto": [ + "https" + ], + "CloudFront-Is-Desktop-Viewer": [ + "true" + ], + "CloudFront-Is-Mobile-Viewer": [ + "false" + ], + "CloudFront-Is-SmartTV-Viewer": [ + "false" + ], + "CloudFront-Is-Tablet-Viewer": [ + "false" + ], + "CloudFront-Viewer-ASN": [ + "" + ], + "CloudFront-Viewer-Country": [ + "" + ], + "Host": [ + "" + ], + "User-Agent": [ + "python-requests/testing" + ], + "Via": [ + "" + ], + "X-Amz-Cf-Id": [ + "" + ], + "X-Amzn-Trace-Id": [ + "" + ], + "X-Forwarded-For": "", + "X-Forwarded-Port": "", + "X-Forwarded-Proto": "", + "tEsT-HEADeR": [ + "aValUE" + ] + }, "multiValueQueryStringParameters": { "urlparam": [ "test" @@ -570,7 +688,7 @@ "deploymentId": "", "domainName": "", "domainPrefix": "", - "extendedRequestId": "", + "extendedRequestId": "", "httpMethod": "GET", "identity": { "accessKey": null, @@ -588,7 +706,7 @@ }, "path": "/test/test-path", "protocol": "HTTP/1.1", - "requestId": "", + "requestId": "", "requestTime": "", "requestTimeEpoch": "", "resourceId": "", @@ -614,8 +732,8 @@ "Host": "", "User-Agent": "python-requests/testing", "Via": "", - "X-Amz-Cf-Id": "", - "X-Amzn-Trace-Id": "", + "X-Amz-Cf-Id": "", + "X-Amzn-Trace-Id": "", "X-Forwarded-For": "", "X-Forwarded-Port": "", "X-Forwarded-Proto": "", @@ -664,10 +782,10 @@ "" ], "X-Amz-Cf-Id": [ - "" + "" ], "X-Amzn-Trace-Id": [ - "" + "" ], "X-Forwarded-For": "", "X-Forwarded-Port": "", @@ -694,7 +812,7 @@ "deploymentId": "", "domainName": "", "domainPrefix": "", - "extendedRequestId": "", + "extendedRequestId": "", "httpMethod": "GET", "identity": { "accessKey": null, @@ -712,7 +830,7 @@ }, "path": "/test/test-path/", "protocol": "HTTP/1.1", - "requestId": "", + "requestId": "", "requestTime": "", "requestTimeEpoch": "", "resourceId": "", @@ -738,8 +856,8 @@ "Host": "", "User-Agent": "python-requests/testing", "Via": "", - "X-Amz-Cf-Id": "", - "X-Amzn-Trace-Id": "", + "X-Amz-Cf-Id": "", + "X-Amzn-Trace-Id": "", "X-Forwarded-For": "", "X-Forwarded-Port": "", "X-Forwarded-Proto": "", @@ -788,10 +906,10 @@ "" ], "X-Amz-Cf-Id": [ - "" + "" ], "X-Amzn-Trace-Id": [ - "" + "" ], "X-Forwarded-For": "", "X-Forwarded-Port": "", @@ -812,7 +930,7 @@ "deploymentId": "", "domainName": "", "domainPrefix": "", - "extendedRequestId": "", + "extendedRequestId": "", "httpMethod": "GET", "identity": { "accessKey": null, @@ -830,7 +948,7 @@ }, "path": "/test/test-path/api/user/test%2Balias@gmail.com/plus/test+alias@gmail.com", "protocol": "HTTP/1.1", - "requestId": "", + "requestId": "", "requestTime": "", "requestTimeEpoch": "", "resourceId": "", @@ -856,8 +974,8 @@ "Host": "", "User-Agent": "python-requests/testing", "Via": "", - "X-Amz-Cf-Id": "", - "X-Amzn-Trace-Id": "", + "X-Amz-Cf-Id": "", + "X-Amzn-Trace-Id": "", "X-Forwarded-For": "", "X-Forwarded-Port": "", "X-Forwarded-Proto": "", @@ -906,10 +1024,10 @@ "" ], "X-Amz-Cf-Id": [ - "" + "" ], "X-Amzn-Trace-Id": [ - "" + "" ], "X-Forwarded-For": "", "X-Forwarded-Port": "", @@ -956,7 +1074,7 @@ "deploymentId": "", "domainName": "", "domainPrefix": "", - "extendedRequestId": "", + "extendedRequestId": "", "httpMethod": "GET", "identity": { "accessKey": null, @@ -974,7 +1092,7 @@ }, "path": "/test/test-path/api", "protocol": "HTTP/1.1", - "requestId": "", + "requestId": "", "requestTime": "", "requestTimeEpoch": "", "resourceId": "", @@ -1002,9 +1120,9 @@ "Content-Type": "application/json;charset=utf-8", "Host": "", "User-Agent": "python-requests/testing", - "Via": "", - "X-Amz-Cf-Id": "", - "X-Amzn-Trace-Id": "", + "Via": "", + "X-Amz-Cf-Id": "", + "X-Amzn-Trace-Id": "", "X-Forwarded-For": "", "X-Forwarded-Port": "", "X-Forwarded-Proto": "" @@ -1052,13 +1170,13 @@ "python-requests/testing" ], "Via": [ - "" + "" ], "X-Amz-Cf-Id": [ - "" + "" ], "X-Amzn-Trace-Id": [ - "" + "" ], "X-Forwarded-For": "", "X-Forwarded-Port": "", @@ -1089,7 +1207,7 @@ "deploymentId": "", "domainName": "", "domainPrefix": "", - "extendedRequestId": "", + "extendedRequestId": "", "httpMethod": "POST", "identity": { "accessKey": null, @@ -1107,7 +1225,7 @@ }, "path": "/test/test-path", "protocol": "HTTP/1.1", - "requestId": "", + "requestId": "", "requestTime": "", "requestTimeEpoch": "", "resourceId": "", diff --git a/tests/aws/services/apigateway/test_apigateway_lambda.validation.json b/tests/aws/services/apigateway/test_apigateway_lambda.validation.json index 558a50302ec50..f2b75c8644081 100644 --- a/tests/aws/services/apigateway/test_apigateway_lambda.validation.json +++ b/tests/aws/services/apigateway/test_apigateway_lambda.validation.json @@ -9,7 +9,7 @@ "last_validated_date": "2023-05-31T21:09:38+00:00" }, "tests/aws/services/apigateway/test_apigateway_lambda.py::test_lambda_aws_proxy_integration": { - "last_validated_date": "2024-07-25T19:31:32+00:00" + "last_validated_date": "2024-07-25T20:18:13+00:00" }, "tests/aws/services/apigateway/test_apigateway_lambda.py::test_lambda_aws_proxy_integration_non_post_method": { "last_validated_date": "2024-07-10T15:43:36+00:00" From 4ec49928be9e8fbaa365821fd550cad74bb6a100 Mon Sep 17 00:00:00 2001 From: Benjamin Simon Date: Thu, 25 Jul 2024 23:15:26 +0200 Subject: [PATCH 4/6] fix double slash for legacy --- localstack-core/localstack/services/apigateway/helpers.py | 3 ++- localstack-core/localstack/services/apigateway/integration.py | 2 +- localstack-core/localstack/services/apigateway/router_asf.py | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/localstack-core/localstack/services/apigateway/helpers.py b/localstack-core/localstack/services/apigateway/helpers.py index db03da7f606e0..d1b1e6c8ae707 100644 --- a/localstack-core/localstack/services/apigateway/helpers.py +++ b/localstack-core/localstack/services/apigateway/helpers.py @@ -702,7 +702,8 @@ def extract_path_params(path: str, extracted_path: str) -> Dict[str, str]: def extract_query_string_params(path: str) -> Tuple[str, Dict[str, str]]: parsed_path = urlparse.urlparse(path) - path = parsed_path.path + if not path.startswith("//"): + path = parsed_path.path parsed_query_string_params = urlparse.parse_qs(parsed_path.query) query_string_params = {} diff --git a/localstack-core/localstack/services/apigateway/integration.py b/localstack-core/localstack/services/apigateway/integration.py index bd68d37ef8cd2..0c0868058d292 100644 --- a/localstack-core/localstack/services/apigateway/integration.py +++ b/localstack-core/localstack/services/apigateway/integration.py @@ -293,7 +293,7 @@ def construct_invocation_event( headers = canonicalize_headers(headers) return { - "path": path, + "path": "/" + path.lstrip("/"), "headers": headers, "multiValueHeaders": multi_value_dict_for_list(headers), "body": data, diff --git a/localstack-core/localstack/services/apigateway/router_asf.py b/localstack-core/localstack/services/apigateway/router_asf.py index 44725eca593c3..4b3fa97a579f5 100644 --- a/localstack-core/localstack/services/apigateway/router_asf.py +++ b/localstack-core/localstack/services/apigateway/router_asf.py @@ -129,7 +129,7 @@ def register_routes(self) -> None: strict_slashes=False, ) self.router.add( - "//", + "//", host=host_pattern, endpoint=self.invoke_rest_api, strict_slashes=True, @@ -142,7 +142,7 @@ def register_routes(self) -> None: defaults={"path": ""}, ) self.router.add( - "/restapis///_user_request_/", + "/restapis///_user_request_/", endpoint=self.invoke_rest_api, strict_slashes=True, ) From 2d908613576c1735d154a68d1842c33533684193 Mon Sep 17 00:00:00 2001 From: Benjamin Simon Date: Fri, 26 Jul 2024 01:26:02 +0200 Subject: [PATCH 5/6] fix bad test --- tests/unit/test_apigateway.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/unit/test_apigateway.py b/tests/unit/test_apigateway.py index 08a14339d872f..e6f3812c543bd 100644 --- a/tests/unit/test_apigateway.py +++ b/tests/unit/test_apigateway.py @@ -859,17 +859,18 @@ def test_create_invocation_headers(): class TestApigatewayEvents: + # TODO: remove this tests, assertion are wrong def test_construct_invocation_event(self): tt = [ { "method": "GET", - "path": "http://localhost.localstack.cloud", + "path": "/test/path", "headers": {}, "data": None, "query_string_params": None, "is_base64_encoded": False, "expected": { - "path": "http://localhost.localstack.cloud", + "path": "/test/path", "headers": {}, "multiValueHeaders": {}, "body": None, @@ -881,13 +882,13 @@ def test_construct_invocation_event(self): }, { "method": "GET", - "path": "http://localhost.localstack.cloud", + "path": "/test/path", "headers": {}, "data": None, "query_string_params": {}, "is_base64_encoded": False, "expected": { - "path": "http://localhost.localstack.cloud", + "path": "/test/path", "headers": {}, "multiValueHeaders": {}, "body": None, @@ -899,13 +900,13 @@ def test_construct_invocation_event(self): }, { "method": "GET", - "path": "http://localhost.localstack.cloud", + "path": "/test/path", "headers": {}, "data": None, "query_string_params": {"foo": "bar"}, "is_base64_encoded": False, "expected": { - "path": "http://localhost.localstack.cloud", + "path": "/test/path", "headers": {}, "multiValueHeaders": {}, "body": None, @@ -917,13 +918,13 @@ def test_construct_invocation_event(self): }, { "method": "GET", - "path": "http://localhost.localstack.cloud?baz=qux", + "path": "/test/path?baz=qux", "headers": {}, "data": None, "query_string_params": {"foo": "bar"}, "is_base64_encoded": False, "expected": { - "path": "http://localhost.localstack.cloud?baz=qux", + "path": "/test/path?baz=qux", "headers": {}, "multiValueHeaders": {}, "body": None, From 767c0950cfa312a6e46e64ede9c4d2a96fc323d6 Mon Sep 17 00:00:00 2001 From: Benjamin Simon Date: Mon, 29 Jul 2024 11:20:16 +0200 Subject: [PATCH 6/6] move GreedyPathConverter --- .../localstack/aws/protocol/op_router.py | 2 +- localstack-core/localstack/http/router.py | 17 +++++++++++++++++ .../execute_api/handlers/resource_router.py | 2 +- localstack-core/localstack/services/edge.py | 17 +---------------- tests/unit/aws/protocol/test_op_router.py | 2 +- tests/unit/http_/test_router.py | 10 ++++++++-- 6 files changed, 29 insertions(+), 21 deletions(-) diff --git a/localstack-core/localstack/aws/protocol/op_router.py b/localstack-core/localstack/aws/protocol/op_router.py index e45e2984edbf3..f4c5f1019aa02 100644 --- a/localstack-core/localstack/aws/protocol/op_router.py +++ b/localstack-core/localstack/aws/protocol/op_router.py @@ -15,7 +15,7 @@ ) from localstack.http import Request from localstack.http.request import get_raw_path -from localstack.services.edge import GreedyPathConverter +from localstack.http.router import GreedyPathConverter class _HttpOperation(NamedTuple): diff --git a/localstack-core/localstack/http/router.py b/localstack-core/localstack/http/router.py index 319e88c0e7ced..da3bcdfe043c0 100644 --- a/localstack-core/localstack/http/router.py +++ b/localstack-core/localstack/http/router.py @@ -14,12 +14,28 @@ route, ) from rolo.routing.router import Dispatcher, call_endpoint +from werkzeug.routing import PathConverter HTTP_METHODS = ("GET", "POST", "PUT", "PATCH", "DELETE", "HEAD", "OPTIONS", "TRACE") E = TypeVar("E") RequestArguments = Mapping[str, Any] + +class GreedyPathConverter(PathConverter): + """ + This converter makes sure that the path ``/mybucket//mykey`` can be matched to the pattern + ``/`` and will result in `Key` being `/mykey`. + """ + + regex = ".*?" + + part_isolating = False + """From the werkzeug docs: If a custom converter can match a forward slash, /, it should have the + attribute part_isolating set to False. This will ensure that rules using the custom converter are + correctly matched.""" + + __all__ = [ "RequestArguments", "HTTP_METHODS", @@ -32,4 +48,5 @@ "RuleAdapter", "WithHost", "RuleGroup", + "GreedyPathConverter", ] diff --git a/localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/resource_router.py b/localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/resource_router.py index 9f11a38a22dc3..c957e24fb00bd 100644 --- a/localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/resource_router.py +++ b/localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/resource_router.py @@ -13,8 +13,8 @@ transform_path_params_to_rule_vars, ) from localstack.http import Response +from localstack.http.router import GreedyPathConverter from localstack.services.apigateway.models import RestApiDeployment -from localstack.services.edge import GreedyPathConverter from ..api import RestApiGatewayHandler, RestApiGatewayHandlerChain from ..context import RestApiInvocationContext diff --git a/localstack-core/localstack/services/edge.py b/localstack-core/localstack/services/edge.py index ec900a27cb952..5c3ede66b65e5 100644 --- a/localstack-core/localstack/services/edge.py +++ b/localstack-core/localstack/services/edge.py @@ -5,8 +5,6 @@ import sys from typing import List, Optional, TypeVar -from werkzeug.routing import PathConverter - from localstack import config, constants from localstack.config import HostAndPort from localstack.constants import ( @@ -14,6 +12,7 @@ ) from localstack.http import Router from localstack.http.dispatcher import Handler, handler_dispatcher +from localstack.http.router import GreedyPathConverter from localstack.utils.collections import split_list_by from localstack.utils.net import get_free_tcp_port from localstack.utils.run import is_root, run @@ -25,20 +24,6 @@ LOG = logging.getLogger(__name__) -class GreedyPathConverter(PathConverter): - """ - This converter makes sure that the path ``/mybucket//mykey`` can be matched to the pattern - ``/`` and will result in `Key` being `/mykey`. - """ - - regex = ".*?" - - part_isolating = False - """From the werkzeug docs: If a custom converter can match a forward slash, /, it should have the - attribute part_isolating set to False. This will ensure that rules using the custom converter are - correctly matched.""" - - ROUTER: Router[Handler] = Router( dispatcher=handler_dispatcher(), converters={"greedy_path": GreedyPathConverter} ) diff --git a/tests/unit/aws/protocol/test_op_router.py b/tests/unit/aws/protocol/test_op_router.py index 8afed21d413a2..e42db6fcc512c 100644 --- a/tests/unit/aws/protocol/test_op_router.py +++ b/tests/unit/aws/protocol/test_op_router.py @@ -5,7 +5,7 @@ from localstack.aws.protocol.op_router import RestServiceOperationRouter from localstack.aws.spec import list_services, load_service from localstack.http import Request -from localstack.services.edge import GreedyPathConverter +from localstack.http.router import GreedyPathConverter def _collect_services(): diff --git a/tests/unit/http_/test_router.py b/tests/unit/http_/test_router.py index 1df42ffae0223..149cdba1ff005 100644 --- a/tests/unit/http_/test_router.py +++ b/tests/unit/http_/test_router.py @@ -8,8 +8,14 @@ from werkzeug.routing import RequestRedirect, Submount from localstack.http import Request, Response, Router -from localstack.http.router import E, RequestArguments, RuleAdapter, WithHost, route -from localstack.services.edge import GreedyPathConverter +from localstack.http.router import ( + E, + GreedyPathConverter, + RequestArguments, + RuleAdapter, + WithHost, + route, +) from localstack.utils.common import get_free_tcp_port