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

Support multiValueHeaders on Response #129

Merged

Conversation

koxudaxi
Copy link
Collaborator

@jordaneremieff
I suggest to support multiValueHeaders on Response.
I want to set the same-name cookies.
FastAPI + uvicorn can return same-name cookies.
But, Mangum returned only the last cookie. the first cookie is dropped.
Mangum should support multiValueHeaders to resolve the problem.

This change affects a little bit of performance.
If You prefer this PR, then I will add test the code.

How do you think about it?

https://docs.aws.amazon.com/apigateway/latest/developerguide/set-up-lambda-proxy-integrations.html#api-gateway-simple-proxy-for-lambda-output-format

example

from fastapi import FastAPI
from starlette.responses import Response
from mangum import Mangum
app = FastAPI()

@app.get('/')
def get_root():
    response = Response()
    response.set_cookie('test', 'value1')
    response.set_cookie('test', 'value2')
    return response


if __name__ == '__main__':
    import uvicorn
    uvicorn.run(app)
else:
    handler = Mangum(app)

output

curl -v http://127.0.0.1:8000/
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8000 (#0)
> GET / HTTP/1.1
> Host: 127.0.0.1:8000
> User-Agent: curl/7.58.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< date: Tue, 23 Jun 2020 15:19:09 GMT
< server: uvicorn
< set-cookie: test=value1; Path=/; SameSite=lax
< set-cookie: test=value2; Path=/; SameSite=lax
< transfer-encoding: chunked
< 
* Connection #0 to host 127.0.0.1 left intact

@jordaneremieff
Copy link
Owner

@koxudaxi Looks like a good addition, I will merge this when it is ready. Thanks!

@koxudaxi koxudaxi changed the title [WIP] Support multiValueHeaders on Response Support multiValueHeaders on Response Jun 24, 2020
@koxudaxi
Copy link
Collaborator Author

@jordaneremieff
I have added unit tests.

By the way, Why CI does not run for this PR?

@jordaneremieff
Copy link
Owner

@koxudaxi thanks. Not sure why it doesn't appear on Github, but the build does run successfully in CI https://travis-ci.org/github/erm/mangum/builds/701506590. I'll try to review this soon then will merge.

@jordaneremieff jordaneremieff merged commit bac03b7 into jordaneremieff:main Jun 25, 2020
four43 pushed a commit to four43/mangum that referenced this pull request Mar 27, 2021
four43 pushed a commit to four43/mangum that referenced this pull request Aug 20, 2021
khamaileon pushed a commit to khamaileon/mangum that referenced this pull request Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants