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

Allow for multiple HTTP headers with same field name in ResponseHeader #1000

Closed
bmontuelle opened this issue Sep 19, 2017 · 5 comments
Closed

Comments

@bmontuelle
Copy link

bmontuelle commented Sep 19, 2017

Are you looking for help?

Following up the discussion on glitter channel https://gitter.im/lagom/lagom?at=59bfcc2d7b7d98d30d0d3005

Lagom Version (1.2.x / 1.3.x / etc)

1.4.0-M2

API (Scala / Java / Neither / Both)

Scala

Operating System (Ubuntu 15.10 / MacOS 10.10 / Windows 10)

MacOS 10.12.6 (16G29)

JDK (Oracle 1.8.0_112, OpenJDK 1.8.x, Azul Zing)

Oracle 1.8.0_111

java version "1.8.0_111"
Java(TM) SE Runtime Environment (build 1.8.0_111-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.111-b14, mixed mode)

Expected Behavior

  1. Define a service implementation such as
def method: ServerServiceCall[NotUsed, Done] = ServerServiceCall { (_, _) =>
    Future.successful((ResponseHeader.Ok
        .withHeader("Set-Cookie", s"cookie1=One; Path=/;")
        .addHeader("Set-cookie", s"cookie2=Two; Path=/;"),
      Done))
  }

Suspected behavior also appears with this definition

def method: ServerServiceCall[NotUsed, Done] = ServerServiceCall { (_, _) =>
    Future.successful((ResponseHeader.Ok.withHeaders(immutable.Seq(
          ("Set-Cookie", s"cookie1=One; Path=/;"),
          ("Set-cookie", s"cookie2=Two; Path=/;")
      )), 
    Done))
}
  1. Wire it as a REST call with the descriptor in the api
override final def descriptor = {
    import Service._
    named("SomeService").withCalls(
      restCall(Method.GET, "/api/method", method _)
    ).withAutoAcl(true)
  }
  1. Start lagom server
$ sbt runAll
  1. Call the rest endpoint and get both set-cookieheaders in the response
$ curl -v -X GET   http://localhost:9000/api/method
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 9000 (#0)
> PUT /api/logout HTTP/1.1
> Host: localhost:9000
> User-Agent: curl/7.54.0
> Accept: */*
>
< HTTP/1.1 200 OK
< set-cookie: cookie1=One; Path=/
< set-cookie: cookie2=Two; Path=/
< Date: Mon, 18 Sep 2017 13:39:41 GMT
< Server: akka-http/10.0.9
< Content-Length: 0
<
* Connection #0 to host localhost left intact 

Actual Behavior

$ curl -v -X GET   http://localhost:9000/api/method
*   Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 9000 (#0)
> PUT /api/logout HTTP/1.1
> Host: localhost:9000
> User-Agent: curl/7.54.0
> Accept: */*
>
< HTTP/1.1 200 OK
< set-cookie: cookie1=One; Path=/
< Date: Mon, 18 Sep 2017 13:39:41 GMT
< Server: akka-http/10.0.9
< Content-Length: 0
<
* Connection #0 to host localhost left intact 

The response HTTP headers only contains one occurence of "set-cookie" header, the first defined.

As RFC 6265 states its required to use multiple occurence of headers with the same field name for set-cookie . Others HTTP headers will probably fall in the same case (most notably those from X- non-standards headers)

@gmethvin
Copy link
Contributor

gmethvin commented Sep 20, 2017

At its core I believe this is a Play issue, not a Lagom one. There is some context here: playframework/playframework#3279

This is not really a problem for Play users because Play already has a high-level API to handle cookies, for example:

Ok("Hello world").withCookies(Cookie("cookie1", "One"), Cookie("cookie2", "Two"))

Lagom has it's own ResponseHeader that wraps Play's, and doesn't have any explicit handling for cookies.

As a workaround, I think it should work to use a single Set-Cookie header with ;; separating the cookies: cookie1=One;Path=/;;cookie2=Two;Path=/. The server should "unfold" those cookies and serve them as separate cookie headers.

@jroper
Copy link
Member

jroper commented Sep 20, 2017

I was just about to reply with exactly the same thing. Play unfolds Set-Cookie headers here:

https://github.com/playframework/playframework/blob/8a660fc12d359e8e005ffc368883ee5124262d0e/framework/src/play-server/src/main/scala/play/core/server/common/ServerResultUtils.scala#L237

@bmontuelle
Copy link
Author

Thanks, the solution with a single Set-Cookie header with the actual content separated with ;; does works

@ignasi35
Copy link
Contributor

@bmontuelle I will close this issue. Please reopen if you want to discuss further.

@bmontuelle
Copy link
Author

@ignasi35 Thats fine to me, we ended up moving all the cookie management into the web gateway and using play Cookies api for it

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

No branches or pull requests

4 participants