Skip to content

Commit

Permalink
Validate you don't have OPTIONS when cors=True
Browse files Browse the repository at this point in the history
You'll get an error message from API gateway about a conflict
exception which isn't that helpful.  By adding it to the
validation step here, we get a much clearer error message:

```
ValueError: Route entry cannot have both cors=True and
methods=['OPTIONS', ...] configured.  When CORS is enabled, an OPTIONS
method is automatically added for you.  Please remove 'OPTIONS' from the
list of configured HTTP methods for: /badview
```

Closes aws#142.
  • Loading branch information
jamesls committed Oct 24, 2016
1 parent 010976e commit c65ac17
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 6 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ Next Release (TBD)
(`#139 <https://github.com/awslabs/chalice/issues/139>`__)
* Raise errors when unknown kwargs are provided to ``app.route(...)``
(`#144 <https://github.com/awslabs/chalice/pull/144>`__)
* Raise validation error when configuring CORS and an OPTIONS method
(`#142 <https://github.com/awslabs/chalice/issues/142>`__)


0.3.0
Expand Down
33 changes: 27 additions & 6 deletions chalice/deployer.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def build_url_trie(routes):
:return: A prefix trie of URL patterns.
"""
_validate_routes(routes)
validate_routes(routes)
root = node('', '/')
for route in routes:
if route == '/':
Expand Down Expand Up @@ -170,21 +170,42 @@ def validate_configuration(config):
"""
routes = config.chalice_app.routes
_validate_routes(routes)
validate_routes(routes)
_validate_manage_iam_role(config)


def _validate_routes(routes):
def validate_routes(routes):
# type: (Dict[str, Any]) -> None
# We're trying to validate any kind of route that will fail
# when we send the request to API gateway.
# We check for:
#
# * any routes that end with a trailing slash.
for route in routes:
if route != '/' and route.endswith('/'):
for route_name, route_entry in routes.items():
if route_name != '/' and route_name.endswith('/'):
raise ValueError("Route cannot end with a trailing slash: %s"
% route)
% route_name)
if route_entry is not None:
# This 'is not None' check is not strictly needed.
# It's used because some of the tests don't populate
# a route_entry when creating test routes.
# This should be cleaned up.
_validate_route_entry(route_name, route_entry)


def _validate_route_entry(route_url, route_entry):
# type: (str, app.RouteEntry) -> None
if route_entry.cors:
# If the user has enabled CORS, they can't also have an OPTIONS method
# because we'll create one for them. API gateway will raise an error
# about duplicate methods.
if 'OPTIONS' in route_entry.methods:
raise ValueError(
"Route entry cannot have both cors=True and "
"methods=['OPTIONS', ...] configured. When "
"CORS is enabled, an OPTIONS method is automatically "
"added for you. Please remove 'OPTIONS' from the list of "
"configured HTTP methods for: %s" % route_url)


def _validate_manage_iam_role(config):
Expand Down
10 changes: 10 additions & 0 deletions tests/unit/test_deployer.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from chalice.deployer import APIGatewayMethods
from chalice.deployer import FULL_PASSTHROUGH, ERROR_MAPPING
from chalice.deployer import validate_configuration
from chalice.deployer import validate_routes
from chalice.deployer import Deployer
from chalice.app import RouteEntry, ALL_ERRORS
from chalice.app import Chalice
Expand Down Expand Up @@ -427,3 +428,12 @@ def test_lambda_deployer_repeated_deploy():
# And should result in the lambda function being updated with the API.
aws_client.update_function_code.assert_called_with(
'appname', 'package contents')


def test_cant_have_options_with_cors(sample_app):
@sample_app.route('/badcors', methods=['GET', 'OPTIONS'], cors=True)
def badview():
pass

with pytest.raises(ValueError):
validate_routes(sample_app.routes)

0 comments on commit c65ac17

Please sign in to comment.