Skip to content

Bug: APIGatewayRestResolver does not capture path segment with + sign #3006

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

Closed
nejcskofic opened this issue Aug 25, 2023 · 5 comments · Fixed by #3026
Closed

Bug: APIGatewayRestResolver does not capture path segment with + sign #3006

nejcskofic opened this issue Aug 25, 2023 · 5 comments · Fixed by #3026
Assignees
Labels
bug Something isn't working event_handlers

Comments

@nejcskofic
Copy link
Contributor

Expected Behaviour

If handler is registered for dynamic path, I would expect that segment containing plus sign is captured.

Example:

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

Resolver is invoked with: GET /dummy/a+b

Current Behaviour

Resolver returns 404 error.

Code snippet

from dataclasses import dataclass

import pytest
from aws_lambda_powertools.event_handler import APIGatewayRestResolver


@pytest.fixture
def lambda_context():
    @dataclass
    class LambdaContext:
        function_name: str = "test"
        memory_limit_in_mb: int = 128
        invoked_function_arn: str = "arn:aws:lambda:eu-west-1:123456789012:function:test"
        aws_request_id: str = "da658bd3-2d6f-4e7b-8ec2-937234644fdc"

    return LambdaContext()


def test_api_gateway_plus_path_capturing(lambda_context):
    app = APIGatewayRestResolver()

    @app.get("/dummy/<value>")
    def handler(value: str):
        assert value == "a+b"
        return {}

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

    assert response["statusCode"] == 200

Possible Solution

api_gateway.pyL44 contains definition of safe characters. This list is incomplete. By rfc3986 Section 3.3 path is built from segments where segments can contain sub-delims which are defined as:

sub-delims  = "!" / "$" / "&" / "'" / "(" / ")"
                  / "*" / "+" / "," / ";" / "="

API Gateway resolver is using this list of characters when building route regex:

_SAFE_URI = "-._~()'!*:@,;="

Plus sign (+), dollar sign ($) and ampersand sign (&) are missing from that list. Adding them would resolve this issue.

Steps to Reproduce

Run above code snippet as pytest test.

Powertools for AWS Lambda (Python) version

latest

AWS Lambda function runtime

3.10

Packaging format used

PyPi

Debugging logs

No response

@nejcskofic nejcskofic added bug Something isn't working triage Pending triage from maintainers labels Aug 25, 2023
@leandrodamascena
Copy link
Contributor

Looking at this now.

@leandrodamascena leandrodamascena removed the triage Pending triage from maintainers label Aug 28, 2023
@leandrodamascena leandrodamascena self-assigned this Aug 28, 2023
@leandrodamascena leandrodamascena moved this from Triage to Pending review in Powertools for AWS Lambda (Python) Aug 28, 2023
@leandrodamascena leandrodamascena moved this from Pending review to Pending customer in Powertools for AWS Lambda (Python) Aug 28, 2023
@leandrodamascena
Copy link
Contributor

Hello @nejcskofic! I can confirm that this is indeed a bug and we need to fix it. Our Resolver must be able to handle these additional characters: $/&.

Do you want to submit a fix + tests for this issue? We plan to launch a release on Friday and we'd love it if you would send in a PR to contribute to Powertools for AWS Lambda.

Don't worry if you can't, We know everyone's been very busy and I can fix that.

@nejcskofic
Copy link
Contributor Author

Hello @leandrodamascena. Thanks for the response. I can take a look at it later today. One question though: in your response you mention only $ and &. What about + (which was my original issue)?

@leandrodamascena
Copy link
Contributor

Sorry, I made a mistake! Yes, include + as well. 😑.

Thanks and let me know if you need any help.

@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

@leandrodamascena leandrodamascena moved this from Coming soon to Shipped in Powertools for AWS Lambda (Python) Sep 11, 2023
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
Projects
Status: Shipped
Development

Successfully merging a pull request may close this issue.

2 participants