Skip to content

Commit

Permalink
Fix handling of trailing slash to match API Gateway
Browse files Browse the repository at this point in the history
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 aws#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.
  • Loading branch information
stealthycoin committed Nov 6, 2017
1 parent f95051d commit cbda2fd
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 1 deletion.
8 changes: 8 additions & 0 deletions CHANGELOG.rst
Expand Up @@ -2,6 +2,14 @@
CHANGELOG
=========


Next Release (TBD)
==================

* Fix local mode handling of routes with trailing slashes
(`#582 <https://github.com/aws/chalice/issues/582>`__)


1.0.4
=====

Expand Down
5 changes: 5 additions & 0 deletions chalice/cli/__init__.py
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
7 changes: 6 additions & 1 deletion chalice/local.py
Expand Up @@ -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('/')
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/cli/test_cli.py
@@ -1,4 +1,5 @@
import mock
import pytest

from chalice import cli
from chalice.cli.factory import CLIFactory
Expand All @@ -12,10 +13,25 @@ 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)
assert env['foo'] == 'bar'
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/'
2 changes: 2 additions & 0 deletions tests/unit/test_local.py
Expand Up @@ -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'),
Expand Down

0 comments on commit cbda2fd

Please sign in to comment.