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

[BUG] Tapir Endpoint Incorrectly Merging Set-Cookie Headers in HTTP Response #3654

Closed
voteslaugh opened this issue Apr 4, 2024 · 3 comments
Assignees
Labels

Comments

@voteslaugh
Copy link

Tapir version: 1.0.1 - 1.10.0

Scala version: 2.13.10

The syntax for setting cookies is clearly defined and can be found in the documentation here:

There's an issue with one of my Tapir endpoints where the response's Set-Cookie header is not being handled correctly by browsers. The header appears to be consolidated into a single line:

Set-Cookie: name1=value1, name2=value2

However, browsers expect each cookie to be set with a separate Set-Cookie header like so:

Set-Cookie: name1=value1
Set-Cookie: name2=value2

Would you be able to assist in resolving this?

object Main extends ZIOAppDefault{

  val app: HttpApp[Any] = ZioHttpInterpreter()
    .toHttp(endpoint.in("text").get.out(setCookies).out(stringBody).zServerLogic(_ =>
      ZIO.succeed(List(CookieWithMeta.unsafeApply("name1", "value1", path = Some("/path1")), CookieWithMeta.unsafeApply("name2", "value2", path = Some("/path2"))), "sometext")
    ))

  override val run = Server.serve(app).provide(Server.default)
}
curl -i "http://localhost:8080/text"
HTTP/1.1 200 OK
content-length: 8
Content-Type: text/plain; charset=UTF-8
Set-Cookie: name1=value1; Path=/path1, name2=value2; Path=/path2

sometext
@voteslaugh
Copy link
Author

It work incorrectly with the ZIO HTTP server, but it works properly with the Vert.x server

@adamw
Copy link
Member

adamw commented Apr 18, 2024

I think setting multiple cookies in one header is correct per the HTTP spec (see here), but the "living standard" might be different.

As a work-around, maybe you can try including two setCookie(name) outputs.

More generally, I think it might make sense to include special rules for Set-Cookie so that server interpreters don't combine them using ,.

@voteslaugh
Copy link
Author

I think setting multiple cookies in one header is correct per the HTTP spec (see here), but the "living standard" might be different.

As a work-around, maybe you can try including two setCookie(name) outputs.

More generally, I think it might make sense to include special rules for Set-Cookie so that server interpreters don't combine them using ,.

Thank you for the response. Regarding the temporary solution: using two setCookie(name) did not work, just as using two header didn't work either - they still end up being combined into one with a comma when using zio http server. The fact that using Set-Cookie headers separated by commas doesn't work was confirmed when I noticed that only the first cookie is stored in the browser, while the second one cannot be recognized and saved by the browser.

Additionally, I want to mention that pure zio http with its methods for adding cookies works correctly with multiple Set-Cookie headers.

@mkubala mkubala self-assigned this Apr 22, 2024
@mkubala mkubala added the server label Apr 22, 2024
mkubala added a commit that referenced this issue Apr 25, 2024
This change makes the behaviour of handling multiple Set-Cookie headers consistent among multiple server backends.

See #3654 for more details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants