awslabs / aws-lambda-go-api-proxy

lambda-go-api-proxy makes it easy to port APIs written with Go frameworks such as Gin (https://gin-gonic.github.io/gin/ ) to AWS Lambda and Amazon API Gateway.
Apache License 2.0
1.05k stars 197 forks source link

Add support for multi-value headers in API Gateway #30

Closed huyphan closed 5 years ago

huyphan commented 5 years ago

Since November 2018, AWS supported multi-value parameters in API Gateway which includes multi-value headers. We would need to add support for this feature. One notable use case of this feature is allowing Lambda functions to return multiple "Set-Cookie" header to client. Currently only the last cookie is captured by the library.

Based on that AWS articles, looks like we can just get rid of Headers field entirely and replaced it by MultiValueHeaders, even for single-value headers.

sapessi commented 5 years ago

We merged #26 a week or so ago to add support for multi-value query string parameters. I plan to do the same for headers. Hopefully in the next couple of weeks.

huyphan commented 5 years ago

I can probably send a PR for this, just wondering if you prefer to just throw all the headers to MultiValueHeaders (ie. not using Headers field at all) or keep the single-value ones in Headers and multi-value ones in MultivalueHeaders.

I prefer the former as -- although it doesn't make the response look elegant -- it doesn't break functionality and the code is more simple.

sapessi commented 5 years ago

I'm with you. Throw everything in the MultiValueHeaders structure. Keeps the code simple/maintainable.

sapessi commented 5 years ago

Resolving this since you PR is merged. Thank you so much for the contribution @huyphan!

KilleR commented 5 years ago

The update here has no reverse compatibility with any application which was using the proxy Headers field to set headers globally across all handlers

func Handler(request events.APIGatewayProxyRequest) (events.APIGatewayProxyResponse, error) {
    proxyRes, err := muxLambda.Proxy(request)
    if proxyRes.Headers == nil {
        proxyRes.Headers = make(map[string]string)
    }
    proxyRes.Headers["Access-Control-Allow-Origin"] = "*"
    proxyRes.Headers["Access-Control-Allow-Headers"] = "*"
    proxyRes.Headers["Access-Control-Allow-Methods"] = "*"
    return proxyRes, err
}

I was using the above to globally add CORS headers (as this was a solid shortcut to adding it on every function, and AWS SAM doesn't support CORS on simple API requests).

This no longer works with the latest code update

sapessi commented 5 years ago

Hi @KilleR - the change is indeed breaking for customers using the proxy request/response objects directly. We strive not to make breaking changes in how we generate HTTP requests and handle responses from the frameworks. However, supporting both the old Headers and new multi value headers field would have introduced additional complexity we did not need to take on based on our tenet of not breaking the HTTP objects. My recommendation, would be to use middlewares in the HTTP framework (gin example) to add additional headers.

KilleR commented 5 years ago

Thanks for the response. I ended up rewriting it myself to use the new multi-value headers, which works fine. I can't find any information on how best to do this with Gorrilla Mux, which is what I'm using. Gorilla middleware for the same effect wraps the HTTP server, rather than the router. Unfortunately, the HTTP server doesn't seem to be exposed by the lambda proxy so the middleware I'd previously used doesn't work here, which is why I rolled my own method to add the appropriate headers.

The update, in case you're interested:

func Handler(request events.APIGatewayProxyRequest) (events.APIGatewayProxyResponse, error) {
    proxyRes, err := muxLambda.Proxy(request)
    proxyRes.MultiValueHeaders["Access-Control-Allow-Origin"] = []string{"*"}
    proxyRes.MultiValueHeaders["Access-Control-Allow-Headers"] = []string{"*"}
    proxyRes.MultiValueHeaders["Access-Control-Allow-Methods"] = []string{"*"}
    return proxyRes, err
}
sapessi commented 5 years ago

I'm going by the example here. If you are initializing the router in the init() function, why not Use the CORS middleware there? Do you have some sample code of what you tried?