dakrone / clj-http

An idiomatic clojure http client wrapping the apache client. Officially supported version.
http://clojars.org/clj-http
MIT License
1.78k stars 408 forks source link

Get-cookies from clj-http.cookies doesn't reflect cookies with the same names #590

Open matthewlisp opened 3 years ago

matthewlisp commented 3 years ago

I've discovered this while working on issue #589

When the server sends cookies with the same name, for example:

Set-Cookie: foo=bar;expires=Wed, 21 Oct 2021 07:28:00 GMT;
Set-Cookie: foo=qux;expires=Wed, 21 Dec 2021 07:28:00 GMT;

The cookie-store will update normally and will contain the cookies with the same names, however when calling https://github.com/dakrone/clj-http/blob/3.x/src/clj_http/cookies.clj#L140-L144

The conversion to clojure map won't contain duplicated keys, since this is impossible for clj maps.

I'm not sure if this is an issue at all, but what i have in mind is, if someone is relying on get-cookies for cookie serialization at some point, you're going to lose information, which is currently my case, so i'll probably avoid using get-cookies and try something directly on the java object of cookie-store.

rymndhng commented 3 years ago

The conversion to clojure map won't contain duplicated keys, since this is impossible for clj maps.

With the scenario you've described, a client that implements a cookie store should update cookie value foo to be bar. The Java BasicCookieStore does this as well and will overwrite the entry for Cookie foo, as demonstrated below.

  (def my-cookie-store (cookie-store))
  (.addCookie my-cookie-store (org.apache.http.impl.cookie.BasicClientCookie. "foo" "bar"))
  (.getCookies my-cookie-store)
  ;;[#object[org.apache.http.impl.cookie.BasicClientCookie 0x57f88a45 "[version: 0][name: foo][value: bar][domain: null][path: null][expiry: null]"]]

  (.addCookie my-cookie-store (org.apache.http.impl.cookie.BasicClientCookie. "foo" "qux"))
  (.getCookies my-cookie-store)
  ;; [#object[org.apache.http.impl.cookie.BasicClientCookie 0x70e828a "[version: 0][name: foo][value: qux][domain: null][path: null][expiry: null]"]]

The server should be concatenating the cookie values together if you want the client cookie store to remember both bar and qux. The Cookie Store is behaving in the way that matches the cookie specification.

If you can't change the server response, your best bet is to serialize the response headers directly, rather than using a cookie store.