From fa7ff9600cb7a764fbcf1bfbb70ace072ca21f16 Mon Sep 17 00:00:00 2001 From: stealthycoin Date: Tue, 31 Oct 2017 09:45:48 -0700 Subject: [PATCH] Fix handling of trailing slash to match API Gateway API Gateway will not match a URI against a route that has a capture group as the last path component if that capture group would be filled with an empty string. Example: With the route: /resource/{name} /resource/bob matches /resource/ does not match Previously local mode would match both URIs to that route, setting the name parameter to an empty string. closes #582 This change also prvents `chalice local` from running if there is a route that ends with a / since `chalice deploy` will not let you deploy such routes. --- CHANGELOG.rst | 7 +++++++ chalice/cli/__init__.py | 5 +++++ chalice/local.py | 7 ++++++- tests/unit/cli/test_cli.py | 16 ++++++++++++++++ tests/unit/test_local.py | 2 ++ 5 files changed, 36 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 34c4408bba..6960d3bfa0 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,13 @@ CHANGELOG ========= + +Next Release (TBD) +================== +* Fix local mode handling of routes with trailing slashes + (`#582 `__) + + 1.0.4 ===== diff --git a/chalice/cli/__init__.py b/chalice/cli/__init__.py index bdce5d3328..94f608a129 100644 --- a/chalice/cli/__init__.py +++ b/chalice/cli/__init__.py @@ -23,6 +23,7 @@ from chalice.utils import record_deployed_values from chalice.utils import remove_stage_from_deployed_values from chalice.deploy.deployer import validate_python_version +from chalice.deploy.deployer import validate_routes from chalice.utils import getting_started_prompt, UI, serialize_to_json from chalice.constants import CONFIG_VERSION, TEMPLATE_APP, GITIGNORE from chalice.constants import DEFAULT_STAGE_NAME @@ -94,6 +95,10 @@ def run_local_server(factory, port, stage, env): # app. env.update(config.environment_variables) app_obj = factory.load_chalice_app() + # Check that `chalice deploy` would let us deploy these routes, otherwise + # there is no point in testing locally. + routes = config.chalice_app.routes + validate_routes(routes) # When running `chalice local`, a stdout logger is configured # so you'll see the same stdout logging as you would when # running in lambda. This is configuring the root logger. diff --git a/chalice/local.py b/chalice/local.py index 48c4386fff..2779e70348 100644 --- a/chalice/local.py +++ b/chalice/local.py @@ -123,7 +123,12 @@ def match_route(self, url): # Otherwise we need to check for param substitution parsed_url = urlparse(url) query_params = {k: v[0] for k, v in parse_qs(parsed_url.query).items()} - parts = parsed_url.path.split('/') + path = parsed_url.path + # API Gateway removes the trailing slash if the route is not the root + # path. We do the same here so our route matching works the same way. + if path != '/' and path.endswith('/'): + path = path[:-1] + parts = path.split('/') captured = {} for route_url in self.route_urls: url_parts = route_url.split('/') diff --git a/tests/unit/cli/test_cli.py b/tests/unit/cli/test_cli.py index 0d282d85a2..f751816eb0 100644 --- a/tests/unit/cli/test_cli.py +++ b/tests/unit/cli/test_cli.py @@ -1,4 +1,5 @@ import mock +import pytest from chalice import cli from chalice.cli.factory import CLIFactory @@ -12,6 +13,7 @@ def test_run_local_server(): factory.create_config_obj.return_value.environment_variables = { 'foo': 'bar', } + factory.create_config_obj.return_value.chalice_app.routes = {} local_server = mock.Mock(spec=LocalDevServer) factory.create_local_server.return_value = local_server cli.run_local_server(factory, 8000, local_stage_test, env) @@ -19,3 +21,17 @@ def test_run_local_server(): local_server.serve_forever.assert_called_with() factory.create_config_obj.assert_called_with( chalice_stage_name=local_stage_test) + + +def test_cannot_run_local_mode_with_trailing_slash_route(): + local_stage_test = 'local_test' + factory = mock.Mock(spec=CLIFactory) + factory.create_config_obj.return_value.environment_variables = {} + factory.create_config_obj.return_value.chalice_app.routes = { + 'foobar/': None + } + local_server = mock.Mock(spec=LocalDevServer) + factory.create_local_server.return_value = local_server + with pytest.raises(ValueError) as e: + cli.run_local_server(factory, 8000, local_stage_test, {}) + assert str(e.value) == 'Route cannot end with a trailing slash: foobar/' diff --git a/tests/unit/test_local.py b/tests/unit/test_local.py index 27a2a456d7..86d03ea2fd 100644 --- a/tests/unit/test_local.py +++ b/tests/unit/test_local.py @@ -486,6 +486,8 @@ def test_can_deny_unauthed_request(auth_handler): ('/foo/other', '/foo/{capture}'), ('/names/foo', '/names/{capture}'), ('/names/bar', '/names/{capture}'), + ('/names/bar/', '/names/{capture}'), + ('/names/', None), ('/nomatch', None), ('/names/bar/wrong', None), ('/a/z/c', '/a/{capture}/c'),