Fix: Regex issue and Keyerror crash #1794
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aahung
approved these changes
Nov 13, 2020
Comment on lines
+315
to
+317
| # ignore leading and trailing `/` in the path name | ||
| m = re.search(r"[a-zA-Z0-9]+[\-\_]?[a-zA-Z0-9]+", path) | ||
| path = m.string[m.start(0) : m.end(0)] |
There was a problem hiding this comment.
nit: since we are just ignoring the leading and trailing /, I think we can simplify the code as
path = path.strip('/')
...
if not path:
raise ...There was a problem hiding this comment.
Thanks, it does look better, however even with this i won't be able to get rid of the if else as i would still have to check if path is "" before I do re.search. so I don't think it will be that useful.
CoshUS
approved these changes
Nov 13, 2020
hoffa
reviewed
Nov 13, 2020
| else: | ||
| # ignore leading and trailing `/` in the path name | ||
| m = re.search(r"[a-zA-Z0-9]+[\-\_]?[a-zA-Z0-9]+", path) | ||
| path = m.string[m.start(0) : m.end(0)] |
There was a problem hiding this comment.
It's not clear from reading the code what path is, and why it's reassigned to in such a way (e.g. foo-bar-baz becomes foo-bar... why?).
If it's generating a logical ID from an arbitrary string, we should refactor in such a way; it seems to be a fairly generic use-case.
There was a problem hiding this comment.
That's a good point. I am not sure about the reason behind creating logical_id like this. But we can try to understand where we use the logical id further and modify the logic accordingly.
I will merge this PR for now and can track this later.
mndeveci
added a commit
that referenced
this pull request
Nov 23, 2020
* Fix: Updated Slack Invite Link (#1712) * Updated Slack Invite Link * Restricted jsonschema to Python 2 * Forced pyrsistent to 0.16 in Python 2 * Reverted Changes to enum34 * Merge master back to develop (#1734) * Release v1.26.0 (#1680) * feat: add support for VPCEndpointIds in EndpointConfiguration * fix: update formatting with black * docs: update 2016-10-31.md * docs: added api endpointconfiguration example * docs: make example more generic * fix: remove nested EndpointConfiguration types from output * fix: only allow one EndpointConfiguration Type * doc: update example to reflect only allowing one EndpointConfiguration Type * fix : missing UserPool properties (#1506) (#1581) * fix: resource policy generation for {path+} (#1580) * refactor: Remove 2016-10-31 examples * update PR template * adjust pr template * Adding authorization scopes as list validation in ApiGatewayAuthorizer (v1 and v2). (#1670) * Adding authorization scopes as list validation in ApiGatewayAuthorizer and ApiGatewayV2Authorizer. * make black. * Adding functional test for invalid auth scope. * adding error condition for invalid test. * removing test template file. * feat: MSK event type support for AWS::Serverless::Function (#52) Co-authored-by: Steve Brown <[email protected]> Co-authored-by: jtaylor00 <[email protected]> Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Alex Wood <[email protected]> Co-authored-by: Tarun <[email protected]> * Update __init__.py (#1704) * Release/v1.27.0 resolveconflict (#1717) * Release v1.26.0 (#1680) * feat: add support for VPCEndpointIds in EndpointConfiguration * fix: update formatting with black * docs: update 2016-10-31.md * docs: added api endpointconfiguration example * docs: make example more generic * fix: remove nested EndpointConfiguration types from output * fix: only allow one EndpointConfiguration Type * doc: update example to reflect only allowing one EndpointConfiguration Type * fix : missing UserPool properties (#1506) (#1581) * fix: resource policy generation for {path+} (#1580) * refactor: Remove 2016-10-31 examples * update PR template * adjust pr template * Adding authorization scopes as list validation in ApiGatewayAuthorizer (v1 and v2). (#1670) * Adding authorization scopes as list validation in ApiGatewayAuthorizer and ApiGatewayV2Authorizer. * make black. * Adding functional test for invalid auth scope. * adding error condition for invalid test. * removing test template file. * feat: MSK event type support for AWS::Serverless::Function (#52) Co-authored-by: Steve Brown <[email protected]> Co-authored-by: jtaylor00 <[email protected]> Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Alex Wood <[email protected]> Co-authored-by: Tarun <[email protected]> * Fix: Updated Slack Invite Link (#1712) * Updated Slack Invite Link * Restricted jsonschema to Python 2 * Forced pyrsistent to 0.16 in Python 2 * Reverted Changes to enum34 Co-authored-by: Sriram Madapusi Vasudevan <[email protected]> Co-authored-by: Steve Brown <[email protected]> Co-authored-by: jtaylor00 <[email protected]> Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Alex Wood <[email protected]> Co-authored-by: Tarun <[email protected]> Co-authored-by: Cosh_ <[email protected]> Co-authored-by: Sriram Madapusi Vasudevan <[email protected]> Co-authored-by: Steve Brown <[email protected]> Co-authored-by: jtaylor00 <[email protected]> Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Alex Wood <[email protected]> Co-authored-by: Tarun <[email protected]> Co-authored-by: Mehmet Nuri Deveci <[email protected]> Co-authored-by: Cosh_ <[email protected]> * Lambdaauth (#1733) * Add support for Lambda Authorizers in HttpAPI * Address comments and fix formatting * fix version * Validate input parameters. Update tests * black reformat Co-authored-by: Raymond Wang <[email protected]> * Feature toggle (#1737) * Adding logic to pipe app config providers. Unit test pending * Adding some documentation to config providers. * Adding some unit tests and making black ignore json files. * minor cleanup. * Addressing PR comments. * feature: Support MTLS auth properties in REST and HTTP API domain names (#1725) * feature: Support MTLS auth properties in REST and HTTP API domain names * fix unicode != str issue in py2.7 * add SecurityPolicy because default RESTAPI is using TLS1.0 * Add new property DisableExecuteApiEndpoint * black reformat * Address comments * Add tests on invalid templates * address test failures in py2.7 * restart travis tests * fix: adding support for passing target id to EventBridgeRule (#1747) * Manage black version using requirement file (#1748) * chore: Manage black version in dev.txt - config pre-commit - config development guide - config travis Refer to the commit below in aws-sam-cli aws/aws-sam-cli@d725db5fbfc698a9f0c7582 * Format using black 20.8b1 * Opt-out black fron dev.txt for Python 2 * fix: Validate API request models (#1757) * Backward merge master branch into develop (#1761) * Release v1.26.0 (#1680) * feat: add support for VPCEndpointIds in EndpointConfiguration * fix: update formatting with black * docs: update 2016-10-31.md * docs: added api endpointconfiguration example * docs: make example more generic * fix: remove nested EndpointConfiguration types from output * fix: only allow one EndpointConfiguration Type * doc: update example to reflect only allowing one EndpointConfiguration Type * fix : missing UserPool properties (#1506) (#1581) * fix: resource policy generation for {path+} (#1580) * refactor: Remove 2016-10-31 examples * update PR template * adjust pr template * Adding authorization scopes as list validation in ApiGatewayAuthorizer (v1 and v2). (#1670) * Adding authorization scopes as list validation in ApiGatewayAuthorizer and ApiGatewayV2Authorizer. * make black. * Adding functional test for invalid auth scope. * adding error condition for invalid test. * removing test template file. * feat: MSK event type support for AWS::Serverless::Function (#52) Co-authored-by: Steve Brown <[email protected]> Co-authored-by: jtaylor00 <[email protected]> Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Alex Wood <[email protected]> Co-authored-by: Tarun <[email protected]> * Update __init__.py (#1704) * Release/v1.27.0 resolveconflict (#1717) * Release v1.26.0 (#1680) * feat: add support for VPCEndpointIds in EndpointConfiguration * fix: update formatting with black * docs: update 2016-10-31.md * docs: added api endpointconfiguration example * docs: make example more generic * fix: remove nested EndpointConfiguration types from output * fix: only allow one EndpointConfiguration Type * doc: update example to reflect only allowing one EndpointConfiguration Type * fix : missing UserPool properties (#1506) (#1581) * fix: resource policy generation for {path+} (#1580) * refactor: Remove 2016-10-31 examples * update PR template * adjust pr template * Adding authorization scopes as list validation in ApiGatewayAuthorizer (v1 and v2). (#1670) * Adding authorization scopes as list validation in ApiGatewayAuthorizer and ApiGatewayV2Authorizer. * make black. * Adding functional test for invalid auth scope. * adding error condition for invalid test. * removing test template file. * feat: MSK event type support for AWS::Serverless::Function (#52) Co-authored-by: Steve Brown <[email protected]> Co-authored-by: jtaylor00 <[email protected]> Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Alex Wood <[email protected]> Co-authored-by: Tarun <[email protected]> * Fix: Updated Slack Invite Link (#1712) * Updated Slack Invite Link * Restricted jsonschema to Python 2 * Forced pyrsistent to 0.16 in Python 2 * Reverted Changes to enum34 Co-authored-by: Sriram Madapusi Vasudevan <[email protected]> Co-authored-by: Steve Brown <[email protected]> Co-authored-by: jtaylor00 <[email protected]> Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Alex Wood <[email protected]> Co-authored-by: Tarun <[email protected]> Co-authored-by: Cosh_ <[email protected]> * Release/v1.28.0 (#1754) (#1756) * Lambdaauth (#1733) * Add support for Lambda Authorizers in HttpAPI * Address comments and fix formatting * fix version * Validate input parameters. Update tests * black reformat Co-authored-by: Raymond Wang <[email protected]> * bump sam-translator version to v1.28.0 Co-authored-by: Tolledo <[email protected]> Co-authored-by: Tolledo <[email protected]> * bump version to 1.28.1 (#1758) (#1759) Co-authored-by: Sriram Madapusi Vasudevan <[email protected]> Co-authored-by: Steve Brown <[email protected]> Co-authored-by: jtaylor00 <[email protected]> Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Alex Wood <[email protected]> Co-authored-by: Tarun <[email protected]> Co-authored-by: Mehmet Nuri Deveci <[email protected]> Co-authored-by: Cosh_ <[email protected]> Co-authored-by: Tolledo <[email protected]> * chore: Upgrade outdated dependencies in base.txt and dev.txt (#1744) * chore: Update versions in in base.txt (no effective difference) * chore: Update versions specified with ">=" in dev.txt * Fallback pytest to 4.6.x for python2 * chore: Use Compatible release clause in requirement files (#1762) * Fix: SAM Crashes Invalid swagger models exception (#1765) * Fix: Invalid swagger models exception * Fix: Invalid resource policy exception Co-authored-by: Mufaddal Makati <[email protected]> * Fix: DefaultAuth not a string exception (#1774) * Fix: DefaultAuth not a string exception * fix: userpool ref not a string Co-authored-by: Mufaddal Makati <[email protected]> * Adding PermissionsBoundary property for State Machine resource (#1772) * Adding PermissionsBoundary property for State Machine resource * Add permissions_boundary to StateMachineGenerator and _construct_role docstrings Co-authored-by: Vaib Suri <[email protected]> * fix: use newer policy name in gov & cn regions for xray (#1767) * Update __init__.py (#1775) * Release/v1.29.0 (#1769) (#1773) * Mq event source (#60) * Support AmazonMQ as event source * Set black hook language version to python3 * chore: version bump (#64) * Black reformat Co-authored-by: Kaidi He <[email protected]> Co-authored-by: Wing Fung Lau <[email protected]> Co-authored-by: Kaidi He <[email protected]> * Add Description property to Api and HttApi resources (#1719) * Update DEVELOPMENT_GUIDE; moved from rst to md (#1778) * fix: default 'DefinitionBody' is conflict with disableExecuteApiEndpoint (#1790) * fix: default 'DefinitionBody' is conflict with disableExecuteApiEndpoint * update swagger format * Add comment to explain why definitionbody is always defined * Add comments * Fix: Regex issue and Keyerror crash (#1794) * Fix: Regex issue when basepath is / * fix: keyerror in open_api when method is ANY * fixed python 2.7 compatibility issue Co-authored-by: Mufaddal Makati <[email protected]> * feature: Support for Lambda Code Signing (#53) (#1825) * feature: add support for CFN fields for lambda signing (#53) * feature: add support for CFN fields for lambda signing * feature: add support for CFN fields for lambda signing (update formatting) * feature: add support for CFN fields for lambda signing (update patching) * feature: add support for CFN fields for lambda signing (update template) * Revert "feat: add explicit UpdateReplacePolicy (#1481)" (#1568) * docs: document IpV6 option on Domain Configuration object (#1588) * chore: Exclude test modules in whl (#1597) * feat: Add Step Function Resource (#1601) Co-authored-by: Jacob Fuss <[email protected]> * Release Changes for 1.25.0 * feature: add support for CFN fields for lambda signing * feature: add support for CFN fields for lambda signing (slight code update) * feature: add support for CFN fields for lambda signing (update globals.py) Co-authored-by: Shreya <[email protected]> Co-authored-by: Timo Schilling <[email protected]> Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Alex Wood <[email protected]> * Move Tests to Appveyor (#1801) * print python version * update path vars * update linux cmd * update linux cmd * update linux cmd * update whitelist in tox * update passenv * update tox whitelisting * update tox whitelisting Co-authored-by: Shreya <[email protected]> Co-authored-by: Timo Schilling <[email protected]> Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Alex Wood <[email protected]> * merge with master Co-authored-by: Cosh_ <[email protected]> Co-authored-by: Raymond Wang <[email protected]> Co-authored-by: Sriram Madapusi Vasudevan <[email protected]> Co-authored-by: Steve Brown <[email protected]> Co-authored-by: jtaylor00 <[email protected]> Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Alex Wood <[email protected]> Co-authored-by: Tarun <[email protected]> Co-authored-by: Tolledo <[email protected]> Co-authored-by: _sam <[email protected]> Co-authored-by: Mufaddal Makati <[email protected]> Co-authored-by: Mufaddal Makati <[email protected]> Co-authored-by: Adam Wong <[email protected]> Co-authored-by: Vaib Suri <[email protected]> Co-authored-by: Wing Fung Lau <[email protected]> Co-authored-by: Kaidi He <[email protected]> Co-authored-by: Anton Grübel <[email protected]> Co-authored-by: Shreya <[email protected]> Co-authored-by: Timo Schilling <[email protected]> Co-authored-by: Jacob Fuss <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
Description of changes:
Fix: Regex issue when basepath is /
Fix: keyerror in open_api when method is ANY
Description of how you validated changes:
Checklist:
make prpassesExamples?
Please reach out in the comments, if you want to add an example. Examples will be
added to
sam initthrough https://github.com/awslabs/aws-sam-cli-app-templates/By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.