rack / rack-test

Rack::Test is a small, simple testing API for Rack apps.
MIT License
922 stars 250 forks source link

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

Closed postmodern closed 7 years ago

postmodern commented 14 years ago

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 commented 14 years ago

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

brynary commented 14 years ago

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 commented 7 years ago

@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: