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

Different cookies in later requests overwrite first cookie #257

Closed
kjg opened this issue Jan 25, 2012 · 19 comments
Closed

Different cookies in later requests overwrite first cookie #257

kjg opened this issue Jan 25, 2012 · 19 comments
Milestone

Comments

@kjg
Copy link
Contributor

kjg commented Jan 25, 2012

When I make multiple requests to a service the cookie set in the first request is overwritten by different cookies in subsequent requests even through the subsequent cookies have different names.

It seems that the problem is this line https://github.com/rubiii/savon/blob/master/lib/savon/client.rb#L94 where the entire cookie header is overwritten when a new Set-Cookie header is detected

def set_cookie(headers)
  http.headers["Cookie"] = headers["Set-Cookie"] if headers["Set-Cookie"]
end
@kjg
Copy link
Contributor Author

kjg commented Jan 26, 2012

Here is a fix for this. I'll have time to create a proper pull-request including specs in a few days if no one else does it before then.

def set_cookie(headers)
  if headers["Set-Cookie"]
    @cookies ||= {}
    #handle single or multiple Set-Cookie Headers as returned by Rack::Utils::HeaderHash in HTTPI
    set_cookies = [headers["Set-Cookie"]].flatten 
    set_cookies.each do |set_cookie|
      # use the cookie name as the key to the hash to allow for cookie updates and seperation
      # set the value to name=value (for easy joining), stopping when we hit the Cookie options
      @cookies[set_cookie.split('=').first] = set_cookie.split(';').first
    end

    http.headers["Cookie"] = @cookies.values.join(';')
  end
end

@rubiii
Copy link
Contributor

rubiii commented Feb 17, 2012

if you could create a pull requests (with tests), i'll make sure to fix this.

@kjg
Copy link
Contributor Author

kjg commented Apr 6, 2012

Are you still planning on pulling this in?

@rubiii
Copy link
Contributor

rubiii commented Jun 6, 2012

merged. thank you very much!

@ruby2day
Copy link

Hmmm I still cant get more than one Set-Cookie . An api im consuming using savon sends back 2 "Set-Cookie" values, and only the last is set in the header response. I thought that multiple values of the same cookie name would be stored in an array for access?

I can see the two cookies coming over the wire from the api response, but i can only get the last value. supwitdat

@sbu
Copy link

sbu commented Jan 15, 2013

I have the same problem, I only get one cookie. More details at: https://groups.google.com/forum/#!topic/savonrb/fOAJYIWCj8M

@rubiii
Copy link
Contributor

rubiii commented Jan 15, 2013

version 2 i guess?

@rubiii rubiii reopened this Jan 15, 2013
@sbu
Copy link

sbu commented Jan 15, 2013

Yes, version 2.0.2.

@rubiii
Copy link
Contributor

rubiii commented Jan 15, 2013

i'll try to come up with a spec to verify the problem later today.
if someone else has the time to help out, there's already a cookie spec
which should be pretty easy to reuse for multiple cookies:

https://github.com/savonrb/savon/blob/master/spec/savon/client_spec.rb#L98

@rubiii
Copy link
Contributor

rubiii commented Jan 15, 2013

i need more information to debug this. could you please double-check and paste the output of:

response.http.headers["Set-Cookies"]

also this might be interesting as well:

response.http.cookies

plus which httpi adapter are you using and could you try another one to isolate the problem?

@sbu
Copy link

sbu commented Jan 15, 2013

response.http.headers["Set-Cookies"]
=> nil

response.http.headers["set-cookie"]({KEY} instead of real value for security reasons)
=> "ASP.NET_SessionId={KEY}; path=/; HttpOnly"

response.http.cookies ({KEY} instead of real value for security reasons)
=> [#<HTTPI::Cookie:0x007ffc26081a90 @cookie="ASP.NET_SessionId={KEY}; path=/; HttpOnly">]

Using httpi 2.0.0 (automatically installed as a dependence of savon 2.0.2)

.ASPXAUTH is missing, it is returned in the header of the response if the request is issued by SoapUI

@sbu
Copy link

sbu commented Jan 15, 2013

If I issue the request twice (using Savon), then the second response contains the .ASPXAUTH cookie ... strange ...

@rogerleite
Copy link
Member

@rubiii i think this is a bug from httpi, and this pull request savonrb/httpi@4ebba6d fix it, returning cookies as Array when is more than one. We have to check if savon won't break in this case.

This fix was after releasing httpi 2.0.0.

@rubiii
Copy link
Contributor

rubiii commented Jan 15, 2013

@rogerleite thanks! unfortunately httpi master has lots of changes (we should discuss how and when to release them).
i'll try to integrate the pull request with v2.0.0 for this problem.

@rogerleite
Copy link
Member

@rubiii we can checkout v2.0.0 tag, apply the commit and release httpi v2.0.1

@rubiii
Copy link
Contributor

rubiii commented Jan 15, 2013

@rogerleite yep, savon seems to work fine with your change. just not sure about #363 right now.

@rubiii
Copy link
Contributor

rubiii commented Jan 25, 2013

applied the commit and released v2.0.1. let me know if this fixes the problem.

@rubiii
Copy link
Contributor

rubiii commented Jan 26, 2013

savon 2.1 will depend on httpi 2.0.2 which contains the fix for this problem.
@ruby2day @sbu please test against master to make sure this is fixed.

@rubiii
Copy link
Contributor

rubiii commented Feb 3, 2013

released v2.1.0. please refer to the updated changelog and documentation for details. let me know how it works!

@rubiii rubiii closed this as completed Feb 3, 2013
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

5 participants