From 5b78957e198c747eb81194187d74fc95bcb055de Mon Sep 17 00:00:00 2001 From: Akinori MUSHA Date: Sat, 23 Mar 2013 02:02:56 +0900 Subject: [PATCH] Conform to RFC 6265 5.1.4 in that path=/a matches /a/* but not /ab. Remove HTTP::Cookie.normalize_path and add HTTP::Cookie.path_match? instead for comparison. --- lib/http/cookie.rb | 39 ++++++++++++++++++++++--------- lib/http/cookie_jar/hash_store.rb | 4 ++-- test/test_http_cookie.rb | 38 +++++++++++++++++++++++++++++- 3 files changed, 67 insertions(+), 14 deletions(-) diff --git a/lib/http/cookie.rb b/lib/http/cookie.rb index b6c2cf9..70336ad 100644 --- a/lib/http/cookie.rb +++ b/lib/http/cookie.rb @@ -198,15 +198,32 @@ class HTTP::Cookie autoload :Scanner, 'http/cookie/scanner' class << self - # Normalizes a given path. If it is empty or it is a relative - # path, the root path '/' is returned. + # Tests if +target_path+ is under +base_path+ as described in RFC + # 6265 5.1.4. +base_path+ must be an absolute path. + # +target_path+ may be empty, in which case it is treated as the + # root path. # - # If a URI object is given, returns a new URI object with the path - # part normalized. - def normalize_path(path) - return path + normalize_path(path.path) if URI === path + # e.g. + # + # path_match?('/admin/', '/admin/index') == true + # path_match?('/admin/', '/Admin/index') == false + # path_match?('/admin/', '/admin/') == true + # path_match?('/admin/', '/admin') == false + # + # path_match?('/admin', '/admin') == true + # path_match?('/admin', '/Admin') == false + # path_match?('/admin', '/admins') == false + # path_match?('/admin', '/admin/') == true + # path_match?('/admin', '/admin/index') == true + def path_match?(base_path, target_path) + base_path.start_with?('/') or return false # RFC 6265 5.1.4 - path.start_with?('/') ? path : '/' + bsize = base_path.size + tsize = target_path.size + return bsize == 1 if tsize == 0 # treat empty target_path as "/" + return false unless target_path.start_with?(base_path) + return true if bsize == tsize || base_path.end_with?('/') + target_path[bsize] == ?/ end # Parses a Set-Cookie header value `set_cookie` into an array of @@ -386,7 +403,7 @@ class HTTP::Cookie def path=(path) path = check_string_type(path) or raise TypeError, "#{path.class} is not a String" - @path = HTTP::Cookie.normalize_path(path) + @path = path.start_with?('/') ? path : '/' end attr_reader :origin @@ -397,7 +414,7 @@ class HTTP::Cookie raise ArgumentError, "origin cannot be changed once it is set" origin = URI(origin) self.domain ||= origin.host - self.path ||= (HTTP::Cookie.normalize_path(origin) + './').path + self.path ||= (origin + './').path acceptable_from_uri?(origin) or raise ArgumentError, "unacceptable cookie sent from URI #{origin}" @origin = origin @@ -513,7 +530,7 @@ class HTTP::Cookie end uri = URI(uri) return false if secure? && !(URI::HTTPS === uri) - acceptable_from_uri?(uri) && HTTP::Cookie.normalize_path(uri.path).start_with?(@path) + acceptable_from_uri?(uri) && HTTP::Cookie.path_match?(@path, uri.path) end # Returns a string for use in a Cookie header value, @@ -537,7 +554,7 @@ class HTTP::Cookie if @for_domain || @domain != DomainName.new(origin.host).hostname string << "; Domain=#{@domain}" end - if (HTTP::Cookie.normalize_path(origin) + './').path != @path + if (origin + './').path != @path string << "; Path=#{@path}" end if @max_age diff --git a/lib/http/cookie_jar/hash_store.rb b/lib/http/cookie_jar/hash_store.rb index 87af8f0..55e581b 100644 --- a/lib/http/cookie_jar/hash_store.rb +++ b/lib/http/cookie_jar/hash_store.rb @@ -54,11 +54,11 @@ class HTTP::CookieJar def each(uri = nil) if uri thost = DomainName.new(uri.host) - tpath = HTTP::Cookie.normalize_path(uri.path) + tpath = uri.path @jar.each { |domain, paths| next unless thost.cookie_domain?(domain) paths.each { |path, hash| - next unless tpath.start_with?(path) + next unless HTTP::Cookie.path_match?(path, tpath) hash.delete_if { |name, cookie| if cookie.expired? true diff --git a/test/test_http_cookie.rb b/test/test_http_cookie.rb index d34e2e1..d0b43a4 100644 --- a/test/test_http_cookie.rb +++ b/test/test_http_cookie.rb @@ -502,9 +502,10 @@ class TestHTTPCookie < Test::Unit::TestCase { :created_at => time }, { :created_at => time, :path => '/foo/bar/' }, { :created_at => time, :path => '/foo/' }, + { :created_at => time, :path => '/foo' }, ].map { |attrs| HTTP::Cookie.new(cookie_values(attrs)) } - assert_equal([3, 4, 1, 2, 0], cookies.sort.map { |i| + assert_equal([3, 4, 5, 1, 2, 0], cookies.sort.map { |i| cookies.find_index { |j| j.equal?(i) } }) end @@ -739,6 +740,28 @@ class TestHTTPCookie < Test::Unit::TestCase 'file:///', ] }, + HTTP::Cookie.parse('a6=b; path=/dir', + :origin => 'http://example.com/dir/file.html').first => { + true => [ + 'http://example.com/dir', + 'http://example.com/dir/', + 'http://example.com/dir/test.html', + 'https://example.com/dir', + 'https://example.com/dir/', + 'https://example.com/dir/test.html', + ], + false => [ + 'file:///dir/test.html', + 'http://example.com/dir2', + 'http://example.com/dir2/test.html', + 'http://www.example.com/dir/test.html', + 'http://www.example.com/dir2/test.html', + 'https://example.com/dir2', + 'https://example.com/dir2/test.html', + 'https://www.example.com/dir/test.html', + 'https://www.example.com/dir2/test.html', + ] + }, }.each { |cookie, hash| hash.each { |expected, urls| urls.each { |url| @@ -748,4 +771,17 @@ class TestHTTPCookie < Test::Unit::TestCase } } end + + def test_s_path_match? + assert_equal true, HTTP::Cookie.path_match?('/admin/', '/admin/index') + assert_equal false, HTTP::Cookie.path_match?('/admin/', '/Admin/index') + assert_equal true, HTTP::Cookie.path_match?('/admin/', '/admin/') + assert_equal false, HTTP::Cookie.path_match?('/admin/', '/admin') + + assert_equal true, HTTP::Cookie.path_match?('/admin', '/admin') + assert_equal false, HTTP::Cookie.path_match?('/admin', '/Admin') + assert_equal false, HTTP::Cookie.path_match?('/admin', '/admins') + assert_equal true, HTTP::Cookie.path_match?('/admin', '/admin/') + assert_equal true, HTTP::Cookie.path_match?('/admin', '/admin/index') + end end