Skip to content

Conversation

nejcskofic
Copy link
Contributor

Issue number: #3006

Summary

Added +$& as safe characters to be captured by dynamic path in API Gateway resolver.

Changes

  • Updated _SAFE_URI in api_gateway.pyL46 to include characters +, $ and &
  • Updated existing test in test_api_gateway.pyL992 to test capturing with above characters

User experience

app = APIGatewayRestResolver()

@app.get("/dummy/<value>")
def handler(value: str):
	return {}

response = app.resolve(
	{
		"path": "/dummy/a+b",
		"httpMethod": "GET",
		"requestContext": {"requestId": "227b78aa-779d-47d4-a48e-ce62120393b8"}
	},
	lambda_context
)

# This invokes above handler instead of returning 404
assert response["statusCode"] == 200

Checklist

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

Is this a breaking change? Not 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.

@nejcskofic nejcskofic requested a review from a team August 29, 2023 18:26
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 29, 2023
@leandrodamascena leandrodamascena changed the title fix(api-gateway): updated safe uri characters in API GW resolver to include +$& fix(event_handler): update safe uri characters to include +$& Aug 29, 2023
@leandrodamascena leandrodamascena self-requested a review August 29, 2023 18:39
@github-actions github-actions bot added the bug Something isn't working label Aug 29, 2023
@leandrodamascena leandrodamascena changed the title fix(event_handler): update safe uri characters to include +$& fix(event_handler): expanding safe URI characters to include +$& Aug 29, 2023
@codecov-commenter
Copy link

codecov-commenter commented Aug 29, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (2cac2b5) 96.56% compared to head (a7526a8) 96.56%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #3026   +/-   ##
========================================
  Coverage    96.56%   96.56%           
========================================
  Files          175      175           
  Lines         7825     7825           
  Branches      1476     1476           
========================================
  Hits          7556     7556           
  Misses         217      217           
  Partials        52       52           
Files Changed Coverage Δ
aws_lambda_powertools/event_handler/api_gateway.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @nejcskofic! Thanks a lot for fixing this! I just changed the title to follow our standards.

Approved!!
image

@leandrodamascena leandrodamascena merged commit e51f6ff into aws-powertools:develop Aug 29, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented Aug 29, 2023

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

@boring-cyborg
Copy link

boring-cyborg bot commented Aug 29, 2023

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@nejcskofic nejcskofic deleted the fix/api-resolver-capture-plus-sign branch August 29, 2023 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working event_handlers size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: APIGatewayRestResolver does not capture path segment with + sign
3 participants