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 for multiValueHeaders locally #830

Closed
Sarke opened this issue Dec 5, 2018 · 7 comments
Closed

Support for multiValueHeaders locally #830

Sarke opened this issue Dec 5, 2018 · 7 comments
Labels
area/local/start-api sam local start-api command stage/waiting-for-release Fix has been merged to develop and is waiting for a release type/feature Feature request type/ux

Comments

@Sarke
Copy link

Sarke commented Dec 5, 2018

Description:
sam local start-api doesn't detect or pass along any of the multiValueHeaders from the lambda response.

This is most notable (for me) because it defaults to application/json content type even though another header is sent, so for example HTML will be displayed as plain text in the browser.

This is because it only checks the headers key in the lamba response, and not the multiValueHeaders key.

The code related to this issue is the _parse_lambda_output method, found here:
https://github.com/awslabs/aws-sam-cli/blob/develop/samcli/local/apigw/local_apigw_service.py#L182-L214

Here's an example lambda response (using https://github.com/stackery/php-lambda-layer).

{
  "body": "<html><body>test</body></html>",
  "multiValueHeaders": {
    "Connection": ["close"],
    "Content-type": ["text/html;charset=UTF-8"],
    "Date": ["Wed, 05 Dec 2018 00:51:05 +0000"],
    "Host": ["localhost:8000"],
    "X-Powered-By": ["PHP/7.1.7"]
    },
  "statusCode": 200
}

This works as expected once deployed, just not locally with sam.

See also:

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

https://docs.aws.amazon.com/apigateway/latest/developerguide/set-up-lambda-proxy-integrations.html#apigateway-multivalue-headers-and-parameters

Steps to reproduce the issue:

  1. Create template.yaml like this:
AWSTemplateFormatVersion: 2010-09-09
Description: PHP Lambda test
Transform: AWS::Serverless-2016-10-31
Resources:
  phpserver:
    Type: AWS::Serverless::Function
    Properties:
      FunctionName: phpserver
      Description: PHP Webserver
      CodeUri: src/php
      Runtime: provided
      Handler: index.php
      MemorySize: 256
      Timeout: 15
      Tracing: Active
      Layers:
        - 'arn:aws:lambda:us-west-2:887080169480:layer:php71:4'
      Events:
        api:
          Type: Api
          Properties:
            Path: /{proxy+}
            Method: ANY
  1. Create src/php/index.php with any content, e.g.:
<html><body>OK</body></html>
  1. Run sam local start-api and go to http://127.0.0.1:3000/test (or whatever the address).

Observed result:
None of the headers are sent to the browser. Content is displayed as plain text.

The response headers are:

Content-Type: application/json
Content-Length: 4
Server: Werkzeug/0.14.1 Python/2.7.15rc1
Date: Wed, 05 Dec 2018 00:28:36 GMT

Expected result:
sam local start-api detects the Content-Type header from the multiValueHeaders and the correct content type and other headers are sent to the browser. Content in this example is displayed properly as HTML.

Response headers similar to this, which is what I get from AWS once deployed:

content-type: text/html; charset=UTF-8
content-length: 4
date: Wed, 05 Dec 2018 01:36:09 GMT
x-powered-by: PHP/7.1.7

Additional environment details (Ex: Windows, Mac, Amazon Linux etc)
Linux

Output of sam --version:
SAM CLI, version 0.8.1

Optional Debug logs:

2018-12-04 17:41:24 No Content-Type given. Defaulting to 'application/json'.
2018-12-04 17:41:24 127.0.0.1 - - [04/Dec/2018 17:41:24] "GET /test HTTP/1.1" 200 -
@Sarke
Copy link
Author

Sarke commented Dec 5, 2018

This is related to #735 and #741 however they seem to only deal with the request headers and not the response headers which my issue covers.

@martysweet
Copy link

I ran across this issue as well when using the Stackery layer so have worked on a quick fix.

If you look at the DEVELOPER.md, pull my branch and use samdev it should work 😄
https://github.com/awslabs/aws-sam-cli/compare/develop...martysweet:multivalue-response-headers?expand=1

Tomorrow I will add tests and will make a proper PR in this repo 🎉

@btipling
Copy link

btipling commented May 21, 2019

Given that #1190 was closed as a duplicate, I thought I should mention here that solving this issue may have become more urgent since #1154 was merged. As of that code merge, sending MultiValue headers results in 502 HTTP errors. The problem is that official aws golang libraries automatically send multivalue headers, specifically:

aws-lambda-go-api-proxy:
https://github.com/awslabs/aws-lambda-go-api-proxy/blob/master/core/response.go#L90

and various mentions of it here in aws/aws-lambda-go are not automatically omitted when not explicitly given when JSON is marshalled:

https://github.com/aws/aws-lambda-go/blob/master/events/apigw.go
https://github.com/aws/aws-lambda-go/blob/master/events/alb.go

@jfuss
Copy link
Contributor

jfuss commented May 24, 2019

#1166 was merged today and will go out in the next release. What that PR supported was the ability for you Lambda Function to return the multiValueHeaders key in the response. On mac, you can use brew upgrade --head aws-sam-cli (this will build from source).

#741 is handling the other side of multiValueHeaders. That is sending multiValueHeaders in the event to your function.

Marking this as waiting-for-release.

@jfuss jfuss added the stage/waiting-for-release Fix has been merged to develop and is waiting for a release label May 24, 2019
@btipling
Copy link

btipling commented Jun 13, 2019

@jfuss both #1166 and #741 have merged, does that mean this issue is fixed? It looks like those changes were released?

https://github.com/awslabs/aws-sam-cli/releases/tag/v0.17.0

Thank you.

@jfuss
Copy link
Contributor

jfuss commented Jun 13, 2019

@btipling Please see my previous comment above

@jfuss
Copy link
Contributor

jfuss commented Sep 17, 2019

This was released in v0.17.0

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/local/start-api sam local start-api command stage/waiting-for-release Fix has been merged to develop and is waiting for a release type/feature Feature request type/ux
Projects
None yet
Development

No branches or pull requests

4 participants