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

Bug: Open API Validation not validating response serialization when body is Falsy. #5887

Open
amin-farjadi opened this issue Jan 20, 2025 · 7 comments · Fixed by #6119
Open
Assignees
Labels

Comments

@amin-farjadi
Copy link

amin-farjadi commented Jan 20, 2025

Expected Behaviour

When:

  • resolver (of type APIGatewayResolver) has enable_validation=True and,
  • route's function is annotated with a return type X (e.g. a Pydantic model) and,
  • route's function return value is not of type X (and cannot be casted)

Expected:
raise a 422 (HTTPStatus.UNPROCESSABLE_ENTITY) as the result of error in response serialization

Current Behaviour

If function's return value is Falsy, the route is resolved to a 200 OK.

Code snippet

from typing import Literal

from aws_lambda_powertools.event_handler.api_gateway import APIGatewayRestResolver
from pydantic import BaseModel
import pytest

app = APIGatewayRestResolver(enable_validation=True)


class Todo(BaseModel):
    title: str
    status: Literal["Pending", "Done"]


@app.get("/none")
def return_none() -> Todo:
    return None


@app.get("/empty_list")
def return_empty_list() -> Todo:
    return list()


@app.get("/empty_dict")
def return_empty_dict() -> Todo:
    return dict()


@app.get("/empty_string")
def return_empty_string() -> Todo:
    return ""


def lambda_handler(event, context):
    return app.resolve(event, context)


# --- Tests below ---


@pytest.fixture()
def event_factory():
    def _factory(path: str):
        return {
            "httpMethod": "GET",
            "path": path,
        }

    yield _factory


@pytest.mark.parametrize(
    "path, body",
    [
        ("/empty_dict", "{}"),
        ("/empty_list", "[]"),
        ("/none", "null"),
        ("/empty_string", ""),
    ],
    ids=["empty_dict", "empty_list", "none", "empty_string"],
)
def test_unexpected_response_validation(path, body, event_factory):
    """Test to demonstrate cases where a non-OK response is expected from APIGatewayResolver."""
    event = event_factory(path)
    response = lambda_handler(event, None)
    assert response["statusCode"] == 200
    assert response["body"] == body


if __name__ == "__main__":
    pytest.main(["-v", __file__])

Possible Solution

Replace the falsy valuation of the response body in the _handle_response method of the OpenAPIValidationMiddleware class with a more robust class/type comparison.

Steps to Reproduce

  • save snippet above to a file
  • install the following dependencies
    • "aws-lambda-powertools>=3.4.1"
    • "pydantic>=2.10.5"
    • "pytest>=8.3.4"
  • run the file

Powertools for AWS Lambda (Python) version

latest, 3.4.1

AWS Lambda function runtime

3.12

Packaging format used

PyPi

Debugging logs

@amin-farjadi amin-farjadi added bug Something isn't working triage Pending triage from maintainers labels Jan 20, 2025
Copy link

boring-cyborg bot commented Jan 20, 2025

Thanks for opening your first issue here! We'll come back to you as soon as we can.
In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

@leandrodamascena
Copy link
Contributor

Hey @amin-farjadi! Thanks for opening this, will check that.

@leandrodamascena leandrodamascena added event_handlers openapi-schema and removed triage Pending triage from maintainers labels Jan 21, 2025
@leandrodamascena leandrodamascena moved this from Triage to Pending review in Powertools for AWS Lambda (Python) Jan 21, 2025
@anafalcao anafalcao moved this from Pending review to Backlog in Powertools for AWS Lambda (Python) Jan 29, 2025
@anafalcao
Copy link
Contributor

Hey @amin-farjadi , we were able to reproduce the bug, and we'll work on it. Thank you!

@leandrodamascena
Copy link
Contributor

Assigning to @anafalcao to see if it is possible to deliver in this current iteration.

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.

@github-actions github-actions bot added the pending-release Fix or implementation already in dev waiting to be released label Feb 20, 2025
Copy link
Contributor

This is now released under 3.7.0 version!

@github-actions github-actions bot removed the pending-release Fix or implementation already in dev waiting to be released label Feb 25, 2025
@leandrodamascena leandrodamascena moved this from Coming soon to Shipped in Powertools for AWS Lambda (Python) Feb 25, 2025
@leandrodamascena
Copy link
Contributor

Reopening this since it created a regression: #6216

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Pending review
Development

Successfully merging a pull request may close this issue.

3 participants