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

fix: return correct header keys for each integration #877

Merged
merged 1 commit into from
May 17, 2024

Conversation

fluxth
Copy link
Contributor

@fluxth fluxth commented May 16, 2024

Description of changes

Fixes #871
Fixes #876

Each integration handle header keys differently, this patch tries to address these differences so that we have proper headers in responses.

Note: I don't currently have time to spin up all these integrations on AWS to test this, so this patch is entirely based on documentation and reference code from awslabs/aws-lambda-go-api-proxy repo. Please feel free to test and leave feedback in the reviews.

References

ALB Integration
https://docs.aws.amazon.com/elasticloadbalancing/latest/application/lambda-functions.html#multi-value-headers-response

The names of the fields used for headers differ depending on whether you enable multi-value headers for the target group. You must use multiValueHeaders if you have enabled multi-value headers and headers otherwise.

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

If you specify values for both headers and multiValueHeaders, API Gateway merges them into a single list. If the same key-value pair is specified in both, only the values from multiValueHeaders will appear in the merged list.

APIGW v2 Integration
https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-lambda.html

Format 2.0 doesn't have multiValueHeaders or multiValueQueryStringParameters fields. Duplicate headers are combined with commas and included in the headers field. Duplicate query strings are combined with commas and included in the queryStringParameters field.

awslabs/aws-lambda-go-api-proxy source code

By submitting this pull request

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I confirm that I've made a best effort attempt to update all relevant documentation.

// API gateway v2 doesn't have multiValueHeaders field. Duplicate headers are
// combined with commas and included in the headers field.
headers: headers,
multi_value_headers: HeaderMap::new(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field should probably be removed from ApiGatewayV2httpResponse struct but for simplicity of this fix I'm going to leave it there for now.

@calavera
Copy link
Contributor

Thanks a lot for diving deep in this issue!

Each integration handle header keys differently, this patch tries to
address these differences so that we have proper headers in responses.

**ALB Integration**
https://docs.aws.amazon.com/elasticloadbalancing/latest/application/lambda-functions.html#multi-value-headers-response

    The names of the fields used for headers differ depending on
    whether you enable multi-value headers for the target group.
    You must use multiValueHeaders if you have enabled multi-value
    headers and headers otherwise.

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

    If you specify values for both headers and multiValueHeaders,
    API Gateway merges them into a single list. If the same key-value
    pair is specified in both, only the values from multiValueHeaders
    will appear in the merged list.

**APIGW v2 Integration**
https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-lambda.html

    Format 2.0 doesn't have multiValueHeaders or
    multiValueQueryStringParameters fields. Duplicate headers are
    combined with commas and included in the headers field.
    Duplicate query strings are combined with commas and included
    in the queryStringParameters field.

**`awslabs/aws-lambda-go-api-proxy` source code**

- https://github.com/awslabs/aws-lambda-go-api-proxy/blob/3f6c8160ae0c22b0bd05b2e3a9122736f035c74b/core/response.go#L117
- https://github.com/awslabs/aws-lambda-go-api-proxy/blob/3f6c8160ae0c22b0bd05b2e3a9122736f035c74b/core/responseALB.go#L108
- https://github.com/awslabs/aws-lambda-go-api-proxy/blob/3f6c8160ae0c22b0bd05b2e3a9122736f035c74b/core/responsev2.go#L117
Copy link
Contributor

@calavera calavera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I've been looking at the CDK, and I found a way to create automated integration tests to prevent future regressions. I'll create tests for all these http integrations soon.

@calavera calavera merged commit 447d9fb into awslabs:main May 17, 2024
4 checks passed
@fluxth fluxth deleted the fix-headers branch May 17, 2024 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants