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] Manually passed Cookie header overrides http.CookieJar cookies #15

Open
tebruno99 opened this issue Dec 13, 2022 · 2 comments · May be fixed by #17
Open

[bug] Manually passed Cookie header overrides http.CookieJar cookies #15

tebruno99 opened this issue Dec 13, 2022 · 2 comments · May be fixed by #17
Labels
bug Something isn't working waiting on new maintainer

Comments

@tebruno99
Copy link

From websocket created by yauheni-chaburanau: gorilla#597

Description
If you manually pass Cookie header in DialContext(..., http.Header), cookies from Dialer.Jar will be overwritten.

Steps to Reproduce

dialer := websocket.Dialer{
	Jar: jar,
}

header := http.Header{}
header.Set("Cookie", "some_cookie_name=some_cookie_value")

... = dialer.DialContext(ctx, url, header)

Possible reason
From the first look I would say that this is happening because the part of code which is responsible for setting up all the passed headers ignores already applied Cookie header from http.CookieJar.

@tebruno99
Copy link
Author

tebruno99 commented Dec 13, 2022

Original Comment: @garyburd gorilla#597 (comment)

There are several options here:

  1. The jar is ignored when cookie header is specified (current behavior).
  2. Cookie header is ignored when a jar is specified.
  3. It is an error to specify a cookie header and a jar.
  4. Append cookies from jar to cookies specified in header (Fix Manually passed Cookie header overrides http.CookieJar cookies gorilla/websocket#599)
  5. Append cookies form header to cookies from jar.

Changing from option 1 to another option is an incompatible change, but it’s not likely to be a problem in practice.

Applications are not blocked by this issue. An application can implement whatever logic it wants by constructing a cookie header before the call to dial.

There is a similar situation with subprotocols. It is an error to specify subprotocols in the dialer and the header.

Let’s wait for new maintainer to make call on the right approach. Applications are not blocked by current code.

@tebruno99
Copy link
Author

I intend to replicate how the http package handles this situation, if the PR fits that implementation I'll merge. (Need to review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working waiting on new maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant