Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(api_gateway): allow whitespace in routes' path parameter #1099

Merged
merged 3 commits into from
Apr 8, 2022
Merged

fix(api_gateway): allow whitespace in routes' path parameter #1099

merged 3 commits into from
Apr 8, 2022

Conversation

sthuber90
Copy link
Contributor

Issue number: #1098

Summary

Changes

Please provide a summary of what's being changed

Updated the _UNSAFE_URI constant to support whitespace in path parameter

User experience

Please share what the user experience looks like before and after this change

User facing nothing changed

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Apr 8, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Apr 8, 2022

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.

@sthuber90 sthuber90 changed the title fix(parameters): allow whitespace in routes' path parmeter fix(api_gateway): allow whitespace in routes' path parmeter Apr 8, 2022
@github-actions github-actions bot added the bug Something isn't working label Apr 8, 2022
@codecov-commenter
Copy link

codecov-commenter commented Apr 8, 2022

Codecov Report

Merging #1099 (9208667) into develop (8c4eca0) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #1099   +/-   ##
========================================
  Coverage    99.96%   99.96%           
========================================
  Files          119      119           
  Lines         5378     5378           
  Branches       613      613           
========================================
  Hits          5376     5376           
  Partials         2        2           
Impacted Files Coverage Δ
aws_lambda_powertools/event_handler/api_gateway.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c4eca0...9208667. Read the comment docs.

@sthulb
Copy link
Contributor

sthulb commented Apr 8, 2022

@sthuber90 Mind creating a separate test out the tests? So there's a separate test for whitespace checks? Want to make sure that the tests are easy to spot if we get a regression in the future

@sthuber90
Copy link
Contributor Author

Sure, will do

@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 8, 2022
@heitorlessa heitorlessa changed the title fix(api_gateway): allow whitespace in routes' path parmeter fix(api_gateway): allow whitespace in routes' path parameter Apr 8, 2022
@heitorlessa heitorlessa merged commit eeb127c into aws-powertools:develop Apr 8, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Apr 8, 2022

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!

@sthuber90 sthuber90 deleted the fix-1098 branch April 8, 2022 14:03
heitorlessa added a commit to heitorlessa/aws-lambda-powertools-python that referenced this pull request Apr 8, 2022
heitorlessa added a commit to heitorlessa/aws-lambda-powertools-python that referenced this pull request Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants