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

Rack::Test::CookieJar cannot handle multiple 'path' values in the Cookie. #16

Closed
postmodern opened this issue Aug 31, 2010 · 3 comments · Fixed by #197
Closed

Rack::Test::CookieJar cannot handle multiple 'path' values in the Cookie. #16

postmodern opened this issue Aug 31, 2010 · 3 comments · Fixed by #197

Comments

@postmodern
Copy link

While testing a Rack middleware that proxies requests, I noticed that rack-test was giving me the following exception:

/home/hal/.rvm/gems/ruby-1.9.2-p0/gems/rack-test-0.5.4/lib/rack/test/cookie_jar.rb:53:in `path': undefined method `strip' for #<Array:0x0000000377c4e8> (NoMethodError)
from /home/hal/.rvm/gems/ruby-1.9.2-p0/gems/rack-test-0.5.4/lib/rack/test/cookie_jar.rb:77:in `valid?'
from /home/hal/.rvm/gems/ruby-1.9.2-p0/gems/rack-test-0.5.4/lib/rack/test/cookie_jar.rb:128:in `block in merge'
from /home/hal/.rvm/gems/ruby-1.9.2-p0/gems/rack-test-0.5.4/lib/rack/test/cookie_jar.rb:126:in `each'
from /home/hal/.rvm/gems/ruby-1.9.2-p0/gems/rack-test-0.5.4/lib/rack/test/cookie_jar.rb:126:in `merge'
from /home/hal/.rvm/gems/ruby-1.9.2-p0/gems/rack-test-0.5.4/lib/rack/mock_session.rb:35:in `request'
from /home/hal/.rvm/gems/ruby-1.9.2-p0/gems/rack-test-0.5.4/lib/rack/test.rb:207:in `process_request'
from /home/hal/.rvm/gems/ruby-1.9.2-p0/gems/rack-test-0.5.4/lib/rack/test.rb:57:in `get'
from /vault/1/code/ronin/ronin-web/spec/web/middleware/proxy_spec.rb:29:in `block (2 levels) in <top (required)>'

The problem stems from CookieJar#path assuming the path option will always be a String:

  # :api: private
  def path
    @options["path"].strip || "/"
  end

The path option is originally generated by Rack::Utils#parse_query, which will return an Array if query params are repeated:

include Rack::Utils

parse_query("csrf_id=8d0c781f207dfcc0e8db55bd467b01e0; path=/, _github_ses=BAh7BzoRbG9jYWxlX2d1ZXNzMCIKZmxhc2hJQzonQWN0aW9uQ29udHJvbGxlcjo6Rmxhc2g6OkZsYXNoSGFzaHsABjoKQHVzZWR7AA%3D%3D--e10506e0f6935897cafe4f56774e20aa35e579a5; path=/; expires=Wed, 01 Jan 2020 08:00:00 GMT; HttpOnly")['path']
# => ["/, _github_ses=BAh7BzoRbG9jYWxlX2d1ZXNzMCIKZmxhc2hJQzonQWN0aW9uQ29udHJvbGxlcjo6Rmxhc2g6OkZsYXNoSGFzaHsABjoKQHVzZWR7AA==--e10506e0f6935897cafe4f56774e20aa35e579a5", "/"]
@postmodern
Copy link
Author

Tested with rack-1.2.1 and rack-test-0.5.4.

@brynary
Copy link
Collaborator

brynary commented Sep 26, 2010

Thanks for the bug report. Can you take a stab at a failing test case? That would help me ensure this stays fixed for all time.

Cheers,

-Bryan

@dennissivia
Copy link

@perlun @junaruga I am not completely sure about the implementation.
The RFC seems to be a little ambiguous.
See this link and RFC 6265
My reading is that: client should send paths in the right order, but servers should not assume that clients actually do that. Thus we have two options:

  • Use the first entry
  • Sort by path length + age and pick the first of the result list.
    My recommendation is to just use the first approach, since we are implementing a testing library
    and not rack itself, so the focus should be fixing the missing array support.
    In addition I think we should fix this issue according to the suggestion.
    What do you think?

perlun pushed a commit that referenced this issue Aug 3, 2017
* Bugfix for Cookies with multiple paths

closes #16

* Fixed spec names
alex-damian-negru pushed a commit to alex-damian-negru/rack-test that referenced this issue Apr 5, 2021
* Bugfix for Cookies with multiple paths

closes rack#16

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

Successfully merging a pull request may close this issue.

3 participants