From: Nobuyoshi Nakada Date: 2007-07-31T04:45:12+09:00 Subject: Re: URI::merge bug? Hi, At Sat, 19 May 2007 04:03:13 +0900, Daniel Martin wrote in [ruby-talk:252151]: > I've converted all the test examples in that section of RFC 3986 into > test code; see below. Thank you and sorry to be late. > And even with Nakada-san's patch, there's still a problem or two with > our resolver relative to the RFC 3986 test cases. However, with that > patch it passes everything labeled in the RFC as "Normal Cases" > (section 5.4.1). It still has issues with the abnormal cases of > section 5.4.2 URI.parse("ftp://example.com/pub").path returns "/pub" in 1.8 while "pub" in 1.9. Is 1.9 correct? Index: lib/uri/generic.rb =================================================================== --- lib/uri/generic.rb (revision 12858) +++ lib/uri/generic.rb (working copy) @@ -617,10 +617,6 @@ module URI def merge_path(base, rel) - # RFC2396, Section 5.2, 5) - if rel[0] == ?/ #/ - # RFC2396, Section 5.2, 5) - return rel - else + # RFC2396, Section 5.2, 5) # RFC2396, Section 5.2, 6) base_path = split_path(base) @@ -632,8 +628,8 @@ module URI base_path.slice!(i - 1, 2) end - if base_path.empty? - base_path = [''] # keep '/' for root directory - else - base_path.pop + + if (first = rel_path.first) and first.empty? + base_path.clear + rel_path.shift end @@ -654,10 +650,15 @@ module URI end - add_trailer_slash = true + add_trailer_slash = !tmp.empty? + if base_path.empty? + base_path = [''] # keep '/' for root directory + elsif add_trailer_slash + base_path.pop + end while x = tmp.shift - if x == '..' && base_path.size > 1 + if x == '..' # RFC2396, Section 4 # a .. or . in an absolute path has no special meaning - base_path.pop + base_path.pop if base_path.size > 1 else # if x == '..' @@ -676,5 +677,4 @@ module URI return base_path.join('/') end - end private :merge_path Index: test/uri/test_generic.rb =================================================================== --- test/uri/test_generic.rb (revision 12858) +++ test/uri/test_generic.rb (working copy) @@ -1,9 +1,7 @@ require 'test/unit' require 'uri' +require 'enumerator' -module URI - - -class TestGeneric < Test::Unit::TestCase +class URI::TestGeneric < Test::Unit::TestCase def setup @url = 'http://a/b/c/d;p?q' @@ -297,9 +295,9 @@ class TestGeneric < Test::Unit::TestCase # http://a/b/c/d;p?q -# ?y = http://a/b/c/?y +# ?y = http://a/b/c/d;p?y url = @base_url.merge('?y') assert_kind_of(URI::HTTP, url) - assert_equal('http://a/b/c/?y', url.to_s) - url = @base_url.route_to('http://a/b/c/?y') + assert_equal('http://a/b/c/d;p?y', url.to_s) + url = @base_url.route_to('http://a/b/c/d;p?y') assert_kind_of(URI::Generic, url) assert_equal('?y', url.to_s) @@ -453,8 +451,8 @@ class TestGeneric < Test::Unit::TestCase # http://a/b/c/d;p?q -# /./g = http://a/./g +# /./g = http://a/g url = @base_url.merge('/./g') assert_kind_of(URI::HTTP, url) - assert_equal('http://a/./g', url.to_s) + assert_equal('http://a/g', url.to_s) url = @base_url.route_to('http://a/./g') assert_kind_of(URI::Generic, url) @@ -465,5 +463,5 @@ class TestGeneric < Test::Unit::TestCase url = @base_url.merge('/../g') assert_kind_of(URI::HTTP, url) - assert_equal('http://a/../g', url.to_s) + assert_equal('http://a/g', url.to_s) url = @base_url.route_to('http://a/../g') assert_kind_of(URI::Generic, url) @@ -507,8 +505,8 @@ class TestGeneric < Test::Unit::TestCase # http://a/b/c/d;p?q -# ../../../g = http://a/../g +# ../../../g = http://a/g url = @base_url.merge('../../../g') assert_kind_of(URI::HTTP, url) - assert_equal('http://a/../g', url.to_s) + assert_equal('http://a/g', url.to_s) url = @base_url.route_to('http://a/../g') assert_kind_of(URI::Generic, url) @@ -520,5 +518,5 @@ class TestGeneric < Test::Unit::TestCase url = @base_url.merge('../../../../g') assert_kind_of(URI::HTTP, url) - assert_equal('http://a/../../g', url.to_s) + assert_equal('http://a/g', url.to_s) url = @base_url.route_to('http://a/../../g') assert_kind_of(URI::Generic, url) @@ -693,6 +691,56 @@ class TestGeneric < Test::Unit::TestCase assert_raises(URI::InvalidURIError) { uri.query = 'bar' } end + + def m(s) + @base_url.merge(s).to_s end + def test_rfc3986_examples + assert_equal("g:h", m("g:h")) + assert_equal("http://a/b/c/g", m("g")) + assert_equal("http://a/b/c/g", m("./g")) + assert_equal("http://a/b/c/g/", m("g/")) + assert_equal("http://a/g", m("/g")) + assert_equal("http://g", m("//g")) + assert_equal("http://a/b/c/d;p?y", m("?y")) + assert_equal("http://a/b/c/g?y", m("g?y")) + assert_equal("http://a/b/c/d;p?q#s", m("#s")) + assert_equal("http://a/b/c/g#s", m("g#s")) + assert_equal("http://a/b/c/g?y#s", m("g?y#s")) + assert_equal("http://a/b/c/;x", m(";x")) + assert_equal("http://a/b/c/g;x", m("g;x")) + assert_equal("http://a/b/c/g;x?y#s", m("g;x?y#s")) + assert_equal("http://a/b/c/d;p?q", m("")) + assert_equal("http://a/b/c/", m(".")) + assert_equal("http://a/b/c/", m("./")) + assert_equal("http://a/b/", m("..")) + assert_equal("http://a/b/", m("../")) + assert_equal("http://a/b/g", m("../g")) + assert_equal("http://a/", m("../..")) + assert_equal("http://a/", m("../../")) + assert_equal("http://a/g", m("../../g")) + assert_equal("http://a/g", m("../../../g")) + assert_equal("http://a/g", m("../../../../g")) + + assert_equal("http://a/g", m("/./g")) + assert_equal("http://a/g", m("/../g")) + assert_equal("http://a/b/c/g.", m("g.")) + assert_equal("http://a/b/c/.g", m(".g")) + assert_equal("http://a/b/c/g..", m("g..")) + assert_equal("http://a/b/c/..g", m("..g")) + + assert_equal("http://a/b/g", m("./../g")) + assert_equal("http://a/b/c/g/", m("./g/.")) + assert_equal("http://a/b/c/g/h", m("g/./h")) + assert_equal("http://a/b/c/h", m("g/../h")) + assert_equal("http://a/b/c/g;x=1/y", m("g;x=1/./y")) + assert_equal("http://a/b/c/y", m("g;x=1/../y")) + + assert_equal("http://a/b/c/g?y/./x", m("g?y/./x")) + assert_equal("http://a/b/c/g?y/../x", m("g?y/../x")) + assert_equal("http://a/b/c/g#s/./x", m("g#s/./x")) + assert_equal("http://a/b/c/g#s/../x", m("g#s/../x")) + assert_equal("http:g", m("http:g")) + end end -- Nobu Nakada