Merge branch 'lax-uri-2'

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)

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
+    (?: (?<scheme> [^:/?\#]+ ) : )?
+    (?: // (?<authority> [^/?\#]* ) )?
+    (?<path> [^?\#]* )
+    (?: \? (?<query> [^\#]* ) )?
+    (?: \# (?<fragment> .* ) )?
+    \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

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
 

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]
         }
       }
     }

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)