serverless-adapter icon indicating copy to clipboard operation
serverless-adapter copied to clipboard

Possible issues with Cookies

Open H4ad opened this issue 2 years ago • 5 comments

Hi @laverdet and @ml27299,

Sorry to tag you both, but I saw you both creating issues on vendia-serverless: https://github.com/vendia/serverless-express/issues/609 and https://github.com/vendia/serverless-express/issues/554.

My library has the same intention of vendia/serverless-express but I construct it be more extensible and easier to maintain and customize than vendia.

Because my library is a refactored version of vendia/serverless-express, maybe it could have some issues that you both describe in the issues above.

I'm proposing that you try my library and let me know if the problems with cookies will persist, if so, I can work on trying to fix these problems. If so, I'll have more description and also someone to test with because I currently don't have APIs that use ALB or cookies, so it's hard for me to test those scenarios.

For ALB, I have two problems to fix from what I saw from your issues:

  • Force all headers values to be string.
  • Add support to return multiple headers based on this stack overflow answer(suggestion from @laverdet).

H4ad avatar Dec 20 '22 16:12 H4ad

About the problem with ALB stringify headers, I already added PR to fix this issue at #74.

H4ad avatar Dec 20 '22 16:12 H4ad

I won't be able to test your package but you should also take a look at https://github.com/vendia/serverless-express/issues/554 which has some information on broken header ordering under AWS ELB. Under this environment [when event.version === undefined] you need to reverse the order of multiValueHeaders in order to send back headers in the correct order. This has ramifications, for example, when your response deletes a cookie and then sets a new value on that same cookie. Without this workaround an application which works correctly on ALB will fail in strange ways under ELB. Included in the issue is a CloudFormation template and shell script which demonstrates the problem clearly.

Also if I remember correctly the StackOverflow answer, while certainly clever, doesn't help that much. Under ELB it does let you set multiple headers when lambda.multi_value_headers.enabled is false, but it should be preferred to always enable this option and use multiValueHeaders instead.

laverdet avatar Dec 20 '22 16:12 laverdet

Thanks for your time, I will look carefully to see which things I can improve based on that issue. You already give a lot of information, I will try to run CloudFormation to reproduce the same problem.

H4ad avatar Dec 20 '22 20:12 H4ad

Thanks for the quick update! In my case turning on the multi-value headers via alb worked

ml27299 avatar Dec 21 '22 23:12 ml27299

The issue is with ELB not ALB

laverdet avatar Dec 21 '22 23:12 laverdet