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

feat: support both enable/disable attribute for multi value headers on ALB #392

Conversation

fredericgermain
Copy link
Contributor

@fredericgermain fredericgermain commented Mar 25, 2021

Settings in lambda target group.

Issue #, if available:

Description of changes:

Checklist

  • Tests have been added and are passing
  • Documentation has been updated

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…n ALB

Setting found in target groups of lambda type.
@fredericgermain fredericgermain force-pushed the pr/any_multi_value_headers_alb branch from 64e9b7f to 9da7504 Compare March 25, 2021 19:10
@fredericgermain fredericgermain changed the title Support both enable/disable attribute for multi value headers on ALB. feat: support both enable/disable attribute for multi value headers on ALB Mar 25, 2021
Comment on lines +11 to +17
const multiValueHeaders = !event.headers ? getMultiValueHeaders({ headers: responseHeaders }) : undefined
const headers = event.headers
? Object.entries(responseHeaders).reduce((acc, [k, v]) => {
acc[k] = Array.isArray(v) ? v[0] : v
return acc
}, {})
: undefined
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add some comments on what we're trying to do here? Why are we using event.headers (request headers) for response headers?

@brettstack
Copy link
Collaborator

Thanks for the PR. Could you add more context on why this is needed? Thanks!

@fredericgermain
Copy link
Contributor Author

When multi header value setting is disabled in target group (the default, in an ALB config), we receive this kind of event :

{ requestContext: { elb: { targetGroupArn: 'arn:aws:elasticloadbalancing:us-east-1:1234:targetgroup/my-cool-lambda/0fe13e0123456789' } }, httpMethod: 'GET', path: '/', queryStringParameters: { data: '%7B%22type%22%3D%3E%22web%22%7D' }, headers: { accept: '*/*', host: 'my-cool-lambda.my-awesome-domain.com', 'user-agent': 'curl/7.68.0', 'x-amzn-trace-id': 'Root=1-605c80b5-16f003190233a8156c949c15', 'x-forwarded-for': '1.2.3.4', 'x-forwarded-port': '443', 'x-forwarded-proto': 'https' }, body: '', isBase64Encoded: false }

When multi value headers setting is enabled, we have this kind of event

{ requestContext: { elb: { targetGroupArn: 'arn:aws:elasticloadbalancing:us-east-1:1234:targetgroup/my-cool-lambda/0fe13e0123456789' } }, httpMethod: 'GET', path: '/', multiValueQueryStringParameters: { data: [ '%7B%22type%22%3D%3E%22web%22%7D' ] }, multiValueHeaders: { accept: [ '*/*' ], host: [ 'my-cool-lambda.my-awesome-domain.com' ], 'user-agent': [ 'curl/7.68.0' ], 'x-amzn-trace-id': [ 'Root=1-605c8726-68350c5341138f4d0e54f2e0' ], 'x-forwarded-for': [ '1.2.3.4' ], 'x-forwarded-port': [ '443' ], 'x-forwarded-proto': [ 'https' ] }, body: '', isBase64Encoded: false }

This patch accomodates both configurations dynamically.

@peebles
Copy link

peebles commented Aug 10, 2021

Was this ever implemented? I find that I must enable multi-value header option on ALB target group, or headers are not recognized at all in express server. At least, I know for sure that CORs does not work at all unless multi-value headers are enabled.

@brett-vendia brett-vendia merged commit a5cb5b5 into CodeGenieApp:mainline Oct 13, 2021
github-actions bot pushed a commit that referenced this pull request Oct 13, 2021
# [4.5.0](v4.4.0...v4.5.0) (2021-10-13)

### Features

* support both enable/disable attribute for multi value headers on ALB ([#392](#392)) ([a5cb5b5](a5cb5b5))
@github-actions
Copy link

🎉 This PR is included in version 4.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

glasser added a commit to apollographql/apollo-server that referenced this pull request Oct 20, 2021
CodeGenieApp/serverless-express#392 makes the handler
return headers in the field matching the one where it receives request
headers: `headers` or `multiValueHeaders`. (I guess this is a preference
for ALB?) Our mock server needs to be consistent in which one it looks
at. This is a test-only change.
OneDev0411 added a commit to OneDev0411/serverless-express that referenced this pull request Mar 16, 2023
# [4.5.0](CodeGenieApp/serverless-express@v4.4.0...v4.5.0) (2021-10-13)

### Features

* support both enable/disable attribute for multi value headers on ALB ([#392](CodeGenieApp/serverless-express#392)) ([a5cb5b5](CodeGenieApp/serverless-express@a5cb5b5))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants