diff --git a/lib/http/cookie.rb b/lib/http/cookie.rb index 01e8254..e6d4d32 100644 --- a/lib/http/cookie.rb +++ b/lib/http/cookie.rb @@ -1,5 +1,6 @@ # :markup: markdown require 'http/cookie/version' +require 'http/cookie/uri_parser' require 'time' require 'uri' require 'domain_name' @@ -275,7 +276,7 @@ def parse(set_cookie, origin, options = nil, &block) logger = options[:logger] created_at = options[:created_at] end - origin = URI(origin) + origin = HTTP::Cookie::URIParser.parse(origin) [].tap { |cookies| Scanner.new(set_cookie, logger).scan_set_cookie { |name, value, attrs| @@ -455,7 +456,7 @@ def origin= origin @origin.nil? or raise ArgumentError, "origin cannot be changed once it is set" # Delay setting @origin because #domain= or #path= may fail - origin = URI(origin) + origin = HTTP::Cookie::URIParser.parse(origin) if URI::HTTP === origin self.domain ||= origin.host self.path ||= (origin + './').path @@ -548,7 +549,7 @@ def expire! # Tests if it is OK to accept this cookie if it is sent from a given # URI/URL, `uri`. def acceptable_from_uri?(uri) - uri = URI(uri) + uri = HTTP::Cookie::URIParser.parse(uri) return false unless URI::HTTP === uri && uri.host host = DomainName.new(uri.host) @@ -585,7 +586,7 @@ def valid_for_uri?(uri) if @domain.nil? raise "cannot tell if this cookie is valid because the domain is unknown" end - uri = URI(uri) + uri = HTTP::Cookie::URIParser.parse(uri) # RFC 6265 5.4 return false if secure? && !(URI::HTTPS === uri) acceptable_from_uri?(uri) && HTTP::Cookie.path_match?(@path, uri.path) diff --git a/lib/http/cookie/uri_parser.rb b/lib/http/cookie/uri_parser.rb new file mode 100644 index 0000000..3dfc99e --- /dev/null +++ b/lib/http/cookie/uri_parser.rb @@ -0,0 +1,36 @@ +module HTTP::Cookie::URIParser + module_function + + # Regular Expression taken from RFC 3986 Appendix B + URIREGEX = %r{ + \A + (?: (? [^:/?\#]+ ) : )? + (?: // (? [^/?\#]* ) )? + (? [^?\#]* ) + (?: \? (? [^\#]* ) )? + (?: \# (? .* ) )? + \z + }x + + # Escape RFC 3986 "reserved" characters minus valid characters for path + # More specifically, gen-delims minus "/" / "?" / "#" + def escape_path(path) + path.sub(/\A[^?#]+/) { |p| p.gsub(/[:\[\]@]+/) { |r| CGI.escape(r) } } + end + + # Parse a URI string or object, relaxing the constraints on the path component + def parse(uri) + URI(uri) + rescue URI::InvalidURIError + str = String.try_convert(uri) or + raise ArgumentError, "bad argument (expected URI object or URI string)" + + m = URIREGEX.match(str) or raise + + path = m[:path] + str[m.begin(:path)...m.end(:path)] = escape_path(path) + uri = URI.parse(str) + uri.__send__(:set_path, path) + uri + end +end diff --git a/lib/http/cookie_jar.rb b/lib/http/cookie_jar.rb index ef5429b..97910fa 100644 --- a/lib/http/cookie_jar.rb +++ b/lib/http/cookie_jar.rb @@ -156,7 +156,7 @@ def each(uri = nil, &block) # :yield: cookie block_given? or return enum_for(__method__, uri) if uri - uri = URI(uri) + uri = HTTP::Cookie::URIParser.parse(uri) return self unless URI::HTTP === uri && uri.host end diff --git a/test/test_http_cookie.rb b/test/test_http_cookie.rb index 2454099..aeb3710 100644 --- a/test/test_http_cookie.rb +++ b/test/test_http_cookie.rb @@ -302,7 +302,8 @@ def test_parse_many "name=Aaron; Domain=localhost; Expires=Sun, 06 Nov 2011 00:29:51 GMT; Path=/, " \ "name=Aaron; Domain=localhost; Expires=Sun, 06 Nov 2011 00:29:51 GMT; Path=/; HttpOnly, " \ "expired=doh; Expires=Fri, 04 Nov 2011 00:29:51 GMT; Path=/, " \ - "a_path=some_path; Expires=Sun, 06 Nov 2011 00:29:51 GMT; Path=/some_path, " \ + "a_path1=some_path; Expires=Sun, 06 Nov 2011 00:29:51 GMT; Path=/some_path, " \ + "a_path2=some_path; Expires=Sun, 06 Nov 2011 00:29:51 GMT; Path=/some_path[], " \ "no_path1=no_path; Expires=Sun, 06 Nov 2011 00:29:52 GMT, no_expires=nope; Path=/, " \ "no_path2=no_path; Expires=Sun, 06 Nov 2011 00:29:52 GMT; no_expires=nope; Path, " \ "no_path3=no_path; Expires=Sun, 06 Nov 2011 00:29:52 GMT; no_expires=nope; Path=, " \ @@ -313,17 +314,22 @@ def test_parse_many "no_domain3=no_domain; Expires=Sun, 06 Nov 2011 00:29:53 GMT; no_expires=nope; Domain=" cookies = HTTP::Cookie.parse cookie_str, url - assert_equal 15, cookies.length + assert_equal 16, cookies.length name = cookies.find { |c| c.name == 'name' } assert_equal "Aaron", name.value assert_equal "/", name.path assert_equal Time.at(1320539391), name.expires - a_path = cookies.find { |c| c.name == 'a_path' } - assert_equal "some_path", a_path.value - assert_equal "/some_path", a_path.path - assert_equal Time.at(1320539391), a_path.expires + a_path1 = cookies.find { |c| c.name == 'a_path1' } + assert_equal "some_path", a_path1.value + assert_equal "/some_path", a_path1.path + assert_equal Time.at(1320539391), a_path1.expires + + a_path2 = cookies.find { |c| c.name == 'a_path2' } + assert_equal "some_path", a_path2.value + assert_equal "/some_path[]", a_path2.path + assert_equal Time.at(1320539391), a_path2.expires no_expires = cookies.find { |c| c.name == 'no_expires' } assert_equal "nope", no_expires.value @@ -941,6 +947,10 @@ def test_origin= cookie.origin = URI.parse('http://www.example.com/') } + cookie = HTTP::Cookie.new('a', 'b') + cookie.origin = HTTP::Cookie::URIParser.parse('http://example.com/path[]/') + assert_equal '/path[]/', cookie.path + cookie = HTTP::Cookie.new('a', 'b', :domain => '.example.com') cookie.origin = URI.parse('http://example.org/') assert_equal false, cookie.acceptable? @@ -1022,6 +1032,18 @@ def test_valid_for_uri? 'file:///dir2/test.html', ] }, + HTTP::Cookie.parse('a4=b; domain=example.com; path=/dir2[]/', + HTTP::Cookie::URIParser.parse('http://example.com/dir[]/file.html')).first => { + true => [ + HTTP::Cookie::URIParser.parse('https://example.com/dir2[]/file.html'), + HTTP::Cookie::URIParser.parse('http://example.com/dir2[]/file.html'), + ], + false => [ + HTTP::Cookie::URIParser.parse('https://example.com/dir[]/file.html'), + HTTP::Cookie::URIParser.parse('http://example.com/dir[]/file.html'), + 'file:///dir2/test.html', + ] + }, HTTP::Cookie.parse('a4=b; secure', URI('https://example.com/dir/file.html')).first => { true => [ @@ -1069,7 +1091,7 @@ def test_valid_for_uri? hash.each { |expected, urls| urls.each { |url| assert_equal expected, cookie.valid_for_uri?(url), '%s: %s' % [cookie.name, url] - assert_equal expected, cookie.valid_for_uri?(URI(url)), "%s: URI(%s)" % [cookie.name, url] + assert_equal expected, cookie.valid_for_uri?(HTTP::Cookie::URIParser.parse(url)), "%s: URI(%s)" % [cookie.name, url] } } } diff --git a/test/test_http_cookie_jar.rb b/test/test_http_cookie_jar.rb index 8f57abf..b84fcc6 100644 --- a/test/test_http_cookie_jar.rb +++ b/test/test_http_cookie_jar.rb @@ -465,7 +465,7 @@ def test_save_session_cookies_yaml end def test_save_and_read_cookiestxt - url = URI 'http://rubyforge.org/foo/' + url = HTTP::Cookie::URIParser.parse('https://rubyforge.org/foo[]/') # Add one cookie with an expiration date in the future cookie = HTTP::Cookie.new(cookie_values) @@ -474,7 +474,7 @@ def test_save_and_read_cookiestxt :expires => nil)) cookie2 = HTTP::Cookie.new(cookie_values(:name => 'Baz', :value => 'Foo#Baz', - :path => '/foo/', + :path => '/foo[]/', :for_domain => false)) h_cookie = HTTP::Cookie.new(cookie_values(:name => 'Quux', :value => 'Foo#Quux', @@ -523,7 +523,7 @@ def test_save_and_read_cookiestxt assert_equal 'Foo#Baz', cookie.value assert_equal 'rubyforge.org', cookie.domain assert_equal false, cookie.for_domain - assert_equal '/foo/', cookie.path + assert_equal '/foo[]/', cookie.path assert_equal false, cookie.httponly? when 'Quux' assert_equal 'Foo#Quux', cookie.value @@ -656,6 +656,34 @@ def test_paths assert_equal(0, @jar.cookies(url).length) end + def test_non_rfc3986_compliant_paths + url = HTTP::Cookie::URIParser.parse('http://RubyForge.org/login[]') + + values = cookie_values(:path => "/login[]", :expires => nil, :origin => url) + + # Add one cookie with an expiration date in the future + cookie = HTTP::Cookie.new(values) + @jar.add(cookie) + assert_equal(1, @jar.cookies(url).length) + + # Add a second cookie + @jar.add(HTTP::Cookie.new(values.merge( :name => 'Baz' ))) + assert_equal(2, @jar.cookies(url).length) + + # Make sure we don't get the cookie in a different path + assert_equal(0, @jar.cookies(HTTP::Cookie::URIParser.parse('http://RubyForge.org/hello[]')).length) + assert_equal(0, @jar.cookies(HTTP::Cookie::URIParser.parse('http://RubyForge.org/')).length) + + # Expire the first cookie + @jar.add(HTTP::Cookie.new(values.merge( :expires => Time.now - (10 * 86400)))) + assert_equal(1, @jar.cookies(url).length) + + # Expire the second cookie + @jar.add(HTTP::Cookie.new(values.merge( :name => 'Baz', + :expires => Time.now - (10 * 86400)))) + assert_equal(0, @jar.cookies(url).length) + end + def test_ssl_cookies # thanks to michal "ocher" ochman for reporting the bug responsible for this test. values = cookie_values(:expires => nil)