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 for double-slash uri request #9723

Conversation

calvernaz
Copy link
Contributor

@calvernaz calvernaz commented Nov 24, 2023

Motivation

The RAW_URI can contain a path with a double slash for example, for the request https://28e9f096.execute-api.localhost.localstack.cloud:4566//hello, RAW_URI will be //hello.

This will cause an issue parsing the url using the function urlparse. With this fix, we replace // with / to proceed with the parsing.

Changes

Normalizes the RAW_URI by replacing the // with /. It does it in two places, on the http package and on the apigateway router_asf.

Testing

Unit tests for the specific parsing function.

localstack/http/request.py Outdated Show resolved Hide resolved
@calvernaz calvernaz added the semver: patch Non-breaking changes which can be included in patch releases label Nov 24, 2023
@calvernaz calvernaz force-pushed the 8170-bug-apigateway-v2-httpapi-does-not-handle-trailing-slashes-and-raises-authorizer-exception branch from 8890c50 to f01cdf9 Compare November 24, 2023 19:15
Copy link

github-actions bot commented Nov 24, 2023

LocalStack Community integration with Pro

       2 files         2 suites   1h 8m 0s ⏱️
2 371 tests 2 056 ✔️ 315 💤 0
2 372 runs  2 056 ✔️ 316 💤 0

Results for commit 8b6b226.

♻️ This comment has been updated with latest results.

@calvernaz calvernaz added this to the 3.1 milestone Nov 24, 2023
@coveralls
Copy link

coveralls commented Nov 26, 2023

Coverage Status

coverage: 84.159% (+0.03%) from 84.132%
when pulling 8b6b226 on 8170-bug-apigateway-v2-httpapi-does-not-handle-trailing-slashes-and-raises-authorizer-exception
into a3d8c9d on master.

Copy link
Contributor

@bentsku bentsku left a comment

Choose a reason for hiding this comment

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

LGTM! This is great because it does not change current behavior, but allows us to use the new raw_uri field. This is a really good solution!

Just a small question in comment, but nothing major really. Thanks a lot for fixing this!

localstack/services/apigateway/router_asf.py Outdated Show resolved Hide resolved
@calvernaz calvernaz force-pushed the 8170-bug-apigateway-v2-httpapi-does-not-handle-trailing-slashes-and-raises-authorizer-exception branch from 47f0e7c to 7f78fc6 Compare November 28, 2023 23:49
@calvernaz calvernaz force-pushed the 8170-bug-apigateway-v2-httpapi-does-not-handle-trailing-slashes-and-raises-authorizer-exception branch from 7f78fc6 to 8b6b226 Compare November 29, 2023 08:25
@calvernaz calvernaz merged commit a65e7bb into master Nov 29, 2023
26 checks passed
@calvernaz calvernaz deleted the 8170-bug-apigateway-v2-httpapi-does-not-handle-trailing-slashes-and-raises-authorizer-exception branch November 29, 2023 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: apigateway v2 HttpApi does not handle trailing slashes and raises authorizer exception
4 participants