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

API Gateway does not provide Content-Length header, breaks strict checking #143

Open
antdking opened this issue Oct 14, 2020 · 5 comments
Open
Labels
improvement Improve an existing feature

Comments

@antdking
Copy link

We're having some issues at the moment, where running Flask via the Starlette WSGIMiddleware ends up dropping the Body due to no Content-Length being present.

API Gateway doesn't provide the Content-Length header, and the main lambda -> wsgi adaptors handle this by setting the header based on the length of the decoded body:

Looking at this commit, it seemed to be an active decision to move to this method, and completely override the provided Content-Length (I'm assuming this was either historic APIGW behaviour, or someone manually mapped the header).

Can the same functionality be added to mangum, to add support for frameworks that have stricter checking on Content-Length please?

@antdking
Copy link
Author

We're happy to contribute the fix if this is something you're happy to accommodate.

Are there any contributor guidelines we should be aware of?

@jordaneremieff
Copy link
Collaborator

@cybojenix sure, happy for you to PR this change. 👍

I'm not very strict on contributions, just so long as there is some test coverage and the changes conform to black formatting, then it should be fine. Mypy is run in the build, but don't worry too much about type hints, sprinkling # type: ignore is acceptable in cases where the correct typing isn't obvious.

This reminds me I should add a contributor guidelines to the repo at some point. :)

@antdking
Copy link
Author

antdking commented Sep 2, 2021

This is still in our internal issue tracker, however during the porting, we implemented a small hack to patch the functionality.

This is only tested with Flask/Werkzeug 1.x, so your mileage may vary:

def create_app():
    flask_app = create_flask_app()

    app = FastAPI()

    # This mount must always go last, so that FastAPI routes take precedence
    app.mount("/", WSGIMiddleware(_patch_input_stream(flask_app)))

    return app, flask_app


def _patch_input_stream(app):
    """
    Support requests that don't provide a Content-Length.

    This happens with API-Gateway.

    We're safe to disable the protection Werkzeug provides, as
    Starlette unpacks the entire request for us already.

    It's also a nice little speedup, as extra checks are skipped, and
    we don't rewrap the input.

    https://github.com/pallets/werkzeug/blob/f3b081634c744ae1e0ae3f76034ee90bfdc2a808/src/werkzeug/wsgi.py#L217-L221
    """

    def inner(environ, start_response):
        # safe to change directly, it's been cloned.
        environ["wsgi.input_terminated"] = True

        return app(environ, start_response)

    return inner

@druska
Copy link

druska commented Oct 13, 2021

If anyone is Googling for this, it also breaks parsing request bodies with Django ASGI. This is using the API Gateway lambda-proxy integration.

A couple of workarounds are here: https://stackoverflow.com/a/62382607/403844

@dgilmanAIDENTIFIED
Copy link

I'm having success with just intercepting the Lambda payload before it makes it to Mangum:

asgi_handler = mangum.Mangum(app)

def handler(event, context):
    if event.get("body") is not None:
        content_length = str(len(event["body"]))
        if "multiValueHeaders" in event:
            event["multiValueHeaders"]["Content-Length"] = [content_length]
        if "headers" in event:
            event["headers"]["Content-Length"] = content_length

    return asgi_handler(event, context)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improve an existing feature
Projects
None yet
Development

No branches or pull requests

4 participants