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

aws-lambda duplicate headers in API Gateway V1 response #3870

Open
jagregory opened this issue Jan 30, 2025 · 3 comments · May be fixed by #3883
Open

aws-lambda duplicate headers in API Gateway V1 response #3870

jagregory opened this issue Jan 30, 2025 · 3 comments · May be fixed by #3883
Labels

Comments

@jagregory
Copy link

What version of Hono are you using?

4.6.17

What runtime/platform is your app running on? (with version if possible)

AWS Lambda/NodeJS 20

What steps can reproduce the bug?

If an incoming request event has multiValueHeaders set, then headers will be set in both headers and multiValueHeaders in the response, resulting in duplicated header values in the final HTTP response. The AWS docs describe this behaviour:

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.

This whole block looks suspicious to me:

const result: APIGatewayProxyResult = {
body: body,
headers: {},
multiValueHeaders: event.multiValueHeaders ? {} : undefined,
statusCode: res.status,
isBase64Encoded,
}
this.setCookies(event, res, result)
res.headers.forEach((value, key) => {
result.headers[key] = value
if (event.multiValueHeaders && result.multiValueHeaders) {
result.multiValueHeaders[key] = [value]
}
})

  1. The initialization of multiValueHeaders in result seems wrong, why does the incoming event alter the structure of the outgoing response? Surely multiValueHeaders should be initialized if there's multi-valued headers in res.headers.
  2. If result.multiValueHeaders is initialized (because the incoming event had multiValueHeaders) then the subsequent loop will set a header in both result.headers and result.multiValueHeaders which is incorrect. API Gateway will merge both these fields in the final HTTP response.

I think the correct behaviour should be to either:

  1. Detect if there a header has multiple values in res.headers and only set it in multiValueHeaders, otherwise only set it in headers. Never set it in both.
  2. Just set every header in multiValueHeaders even if there's only a single value, and never set headers.

The AWS docs suggest (2) is acceptable:

The multiValueHeaders key can contain multi-value headers as well as single-value headers. You can use the multiValueHeaders key to specify all of your extra headers, including any single-value ones.

What is the expected behavior?

Headers should not be duplicated.

What do you see instead?

No response

Additional information

No response

@yusukebe
Copy link
Member

yusukebe commented Feb 1, 2025

@watany-dev or @exoego, Can you take a look at this?

@exoego
Copy link
Contributor

exoego commented Feb 2, 2025

I would say this is "bug" since AWS APIGW's headers/multiValueHeaders merge may be confusing to users.

In addition to APIGW, ALB should be taken care of, since both shares implementation.

https://docs.aws.amazon.com/elasticloadbalancing/latest/application/lambda-functions.html#multi-value-headers-response
You must use multiValueHeaders if you have enabled multi-value headers and headers otherwise.

It appears that headers and multiValueHeaders are mutually-exclusive in ALB response.
So it's but to ALB.

Opened #3883
But I need to check it with real ALB and APIGW in weekday.

@yusukebe yusukebe added bug and removed triage labels Feb 2, 2025
@yusukebe
Copy link
Member

yusukebe commented Feb 2, 2025

Hi @exoego

Thank you for the super quick response! I added a bug label. Please ping me if the PR is ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants