ruby-dev

Akinori MUSHA さんは書きました:
> 　Ruby 1.8.7-preview1 をリリースしました。伸び伸びのスケジュール

おつかれさまです。
uriについての変更をバックポートしました(遅くなってすいません)。
<http://rubyforge.org/tracker/index.php?func=detail&aid=19013&group_id=426&atid=22040>

パッチを添付しましたので取り込みの検討をお願いします。

基本的にはtrunkと同じにしています。
ただしテストについてはtrunkのように追加するのではなく
変更部分の書き換えを行いました。
# trunkでもそうしたほうがよいのではないかと思っています。

-- 
ay

Attachments (1)

ruby187-19013.diff (17.3 KB, text/x-diff)

Index: NEWS
===================================================================
--- NEWS	(revision 16058)
+++ NEWS	(working copy)
@@ -293,6 +293,17 @@
 * uri
 
   * added LDAPS scheme.
+  * Change for RFC3986:
+    * FTP
+      * URI('ftp://example.com/foo').path #=> 'foo'
+      * URI('ftp://example.com/%2Ffoo').path #=> '/foo'
+      * URI::FTP.build([nil, 'example.com', nil, '/foo', 'i').to_s #=> 'ftp://example.com/%2Ffoo;type=i'
+    * URI merge
+      * URI('http://a/b/c/d;p?q').merge('?y') == URI('http://a/b/c/d;p?y')
+      * URI('http://a/b/c/d;p?q').merge('/./g') == URI('http://a/g')
+      * URI('http://a/b/c/d;p?q').merge('/../g') == URI('http://a/g')
+      * URI('http://a/b/c/d;p?q').merge('../../../g') == URI('http://a/g')
+      * URI('http://a/b/c/d;p?q').merge('../../../../g') == URI('http://a/g')
 
 * rss
 
Index: lib/uri/ftp.rb
===================================================================
--- lib/uri/ftp.rb	(revision 16058)
+++ lib/uri/ftp.rb	(working copy)
@@ -11,7 +11,7 @@
 module URI
 
   #
-  # RFC1738 section 3.2.
+  # FTP URI syntax is defined by RFC1738 section 3.2.
   #
   class FTP < Generic
     DEFAULT_PORT = 21
@@ -22,13 +22,12 @@
       :path, :typecode
     ].freeze
     #
-    # Typecode is, "a", "i" or "d". 
-    # As for "a" the text, as for "i" binary, 
-    # as for "d" the directory is displayed. 
-    # "A" with the text, as for "i" being binary, 
-    # is because the respective data type was called ASCII and 
-    # IMAGE with the protocol of FTP.
+    # Typecode is "a", "i" or "d". 
     #
+    # * "a" indicates a text file (the FTP command was ASCII)
+    # * "i" indicates a binary file (FTP command IMAGE)
+    # * "d" indicates the contents of a directory should be displayed
+    #
     TYPECODE = ['a', 'i', 'd'].freeze
     TYPECODE_PREFIX = ';type='.freeze
 
@@ -52,11 +51,43 @@
     #
     # == Description
     #
-    # Creates a new URI::FTP object from components of URI::FTP with
-    # check.  It is scheme, userinfo, host, port, path and typecode. It
-    # provided by an Array or a Hash. typecode is "a", "i" or "d".
+    # Creates a new URI::FTP object from components, with syntax checking.  
     #
+    # The components accepted are +userinfo+, +host+, +port+, +path+ and 
+    # +typecode+.
+    #
+    # The components should be provided either as an Array, or as a Hash 
+    # with keys formed by preceding the component names with a colon. 
+    #
+    # If an Array is used, the components must be passed in the order
+    # [userinfo, host, port, path, typecode]
+    #
+    # If the path supplied is absolute, it will be escaped in order to
+    # make it absolute in the URI. Examples:
+    #
+    #     require 'uri'
+    #
+    #     uri = URI::FTP.build(['user:password', 'ftp.example.com', nil, 
+    #       '/path/file.> zip', 'i'])
+    #     puts uri.to_s  ->  ftp://user:password@ftp.example.com/%2Fpath/file.zip;type=a
+    #
+    #     uri2 = URI::FTP.build({:host => 'ftp.example.com', 
+    #       :path => 'ruby/src'})
+    #     puts uri2.to_s  ->  ftp://ftp.example.com/ruby/src
+    #
     def self.build(args)
+
+      # Fix the incoming path to be generic URL syntax
+      # FTP path  ->  URL path
+      # foo/bar       /foo/bar
+      # /foo/bar      /%2Ffoo/bar
+      #
+      if args.kind_of?(Array)
+        args[3] = '/' + args[3].sub(/^\//, '%2F')
+      else
+        args[:path] = '/' + args[:path].sub(/^\//, '%2F')
+      end
+
       tmp = Util::make_components_hash(self, args)
 
       if tmp[:typecode]
@@ -72,16 +103,14 @@
     #
     # == Description
     #
-    # Create a new URI::FTP object from ``generic'' components with no
-    # check.
+    # Creates a new URI::FTP object from generic URL components with no
+    # syntax checking.
     #
-    # == Usage
+    # Unlike build(), this method does not escape the path component as
+    # required by RFC1738; instead it is treated as per RFC2396.
     #
-    #   require 'uri'
-    #   p ftp = URI.parse("ftp://ftp.ruby-lang.org/pub/ruby/;type=d") 
-    #   # => #<URI::FTP:0x201fad08 URL:ftp://ftp.ruby-lang.org/pub/ruby/;type=d>
-    #   p ftp.typecode
-    #   # => "d"
+    # Arguments are +scheme+, +userinfo+, +host+, +port+, +registry+, +path+, 
+    # +opaque+, +query+ and +fragment+, in that order.
     #
     def initialize(*arg)
       super(*arg)
@@ -130,6 +159,27 @@
       return tmp
     end
 
+    # Returns the path from an FTP URI.
+    #
+    # RFC 1738 specifically states that the path for an FTP URI does not
+    # include the / which separates the URI path from the URI host. Example:
+    #
+    #     ftp://ftp.example.com/pub/ruby 
+    #
+    # The above URI indicates that the client should connect to 
+    # ftp.example.com then cd pub/ruby from the initial login directory.
+    #
+    # If you want to cd to an absolute directory, you must include an
+    # escaped / (%2F) in the path. Example:
+    #
+    #     ftp://ftp.example.com/%2Fpub/ruby
+    #
+    # This method will then return "/pub/ruby"
+    #
+    def path
+      return @path.sub(/^\//,'').sub(/^%2F/i,'/')
+    end
+
     def to_s
       save_path = nil
       if @typecode
Index: lib/uri/generic.rb
===================================================================
--- lib/uri/generic.rb	(revision 16058)
+++ lib/uri/generic.rb	(working copy)
@@ -616,65 +616,65 @@
     private :split_path
 
     def merge_path(base, rel)
+
       # RFC2396, Section 5.2, 5)
-      if rel[0] == ?/ #/
-        # RFC2396, Section 5.2, 5)
-        return rel
+      # RFC2396, Section 5.2, 6)
+      base_path = split_path(base)
+      rel_path  = split_path(rel)
 
-      else
-        # RFC2396, Section 5.2, 6)
-        base_path = split_path(base)
-        rel_path  = split_path(rel)
+      # RFC2396, Section 5.2, 6), a)
+      base_path << '' if base_path.last == '..'
+      while i = base_path.index('..')
+        base_path.slice!(i - 1, 2)
+      end
 
-        # RFC2396, Section 5.2, 6), a)
-	base_path << '' if base_path.last == '..'
-	while i = base_path.index('..')
-	  base_path.slice!(i - 1, 2)
-        end
-        if base_path.empty?
-          base_path = [''] # keep '/' for root directory
-        else
-	  base_path.pop
-        end
+      if (first = rel_path.first) and first.empty?
+        base_path.clear
+        rel_path.shift
+      end
 
-        # RFC2396, Section 5.2, 6), c)
-        # RFC2396, Section 5.2, 6), d)
-        rel_path.push('') if rel_path.last == '.' || rel_path.last == '..'
-        rel_path.delete('.')
+      # RFC2396, Section 5.2, 6), c)
+      # RFC2396, Section 5.2, 6), d)
+      rel_path.push('') if rel_path.last == '.' || rel_path.last == '..'
+      rel_path.delete('.')
 
-        # RFC2396, Section 5.2, 6), e)
-        tmp = []
-        rel_path.each do |x|
-          if x == '..' &&
-              !(tmp.empty? || tmp.last == '..')
-            tmp.pop
-          else
-            tmp << x
-          end
+      # RFC2396, Section 5.2, 6), e)
+      tmp = []
+      rel_path.each do |x|
+        if x == '..' &&
+            !(tmp.empty? || tmp.last == '..')
+          tmp.pop
+        else
+          tmp << x
         end
+      end
 
-        add_trailer_slash = true
-        while x = tmp.shift
-          if x == '..' && base_path.size > 1
-            # RFC2396, Section 4
-            # a .. or . in an absolute path has no special meaning
-            base_path.pop
-          else
-            # if x == '..'
-            #   valid absolute (but abnormal) path "/../..."
-            # else
-            #   valid absolute path
-            # end
-            base_path << x
-            tmp.each {|t| base_path << t}
-            add_trailer_slash = false
-            break
-          end
+      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 == '..'
+          # RFC2396, Section 4
+          # a .. or . in an absolute path has no special meaning
+          base_path.pop if base_path.size > 1
+        else
+          # if x == '..'
+          #   valid absolute (but abnormal) path "/../..."
+          # else
+          #   valid absolute path
+          # end
+          base_path << x
+          tmp.each {|t| base_path << t}
+          add_trailer_slash = false
+          break
         end
-        base_path.push('') if add_trailer_slash
+      end
+      base_path.push('') if add_trailer_slash
 
-        return base_path.join('/')
-      end
+      return base_path.join('/')
     end
     private :merge_path
 
Index: test/uri/test_common.rb
===================================================================
--- test/uri/test_common.rb	(revision 16058)
+++ test/uri/test_common.rb	(working copy)
@@ -12,7 +12,6 @@
   end
 
   def test_extract
-    # ruby-list:36086
     assert_equal(['http://example.com'], 
 		 URI.extract('http://example.com'))
     assert_equal(['http://example.com'], 
@@ -20,9 +19,9 @@
     assert_equal(['http://example.com/foo)'], 
 		 URI.extract('(http://example.com/foo)'))
     assert_equal(['http://example.jphttp://example.jp'], 
-		 URI.extract('http://example.jphttp://example.jp'))
+		 URI.extract('http://example.jphttp://example.jp'), "[ruby-list:36086]")
     assert_equal(['http://example.jphttp://example.jp'], 
-		 URI.extract('http://example.jphttp://example.jp', ['http']))
+		 URI.extract('http://example.jphttp://example.jp', ['http']), "[ruby-list:36086]")
     assert_equal(['http://', 'mailto:'].sort, 
 		 URI.extract('ftp:// http:// mailto: https://', ['http', 'mailto']).sort)
     # reported by Doug Kearns <djkea2@mugca.its.monash.edu.au>
Index: test/uri/test_ftp.rb
===================================================================
--- test/uri/test_ftp.rb	(revision 16058)
+++ test/uri/test_ftp.rb	(working copy)
@@ -15,7 +15,7 @@
     exp = [
       'ftp',
       'user:pass', 'host.com', URI::FTP.default_port, 
-      '/abc/def', nil,
+      'abc/def', nil,
     ]
     ary = [
       url.scheme, url.userinfo, url.host, url.port,
@@ -27,6 +27,26 @@
     assert_equal('pass', url.password)
   end
 
+  def test_paths
+    # If you think what's below is wrong, please read RubyForge bug 2055, 
+    # RFC 1738 section 3.2.2, and RFC 2396.
+    u = URI.parse('ftp://ftp.example.com/foo/bar/file.ext')
+    assert_equal(u.path, 'foo/bar/file.ext')
+    u = URI.parse('ftp://ftp.example.com//foo/bar/file.ext')
+    assert_equal(u.path, '/foo/bar/file.ext')
+    u = URI.parse('ftp://ftp.example.com/%2Ffoo/bar/file.ext')
+    assert_equal(u.path, '/foo/bar/file.ext')
+  end
+
+  def test_assemble
+    # uri/ftp is conservative and uses the older RFC 1738 rules, rather than
+    # assuming everyone else has implemented RFC 2396.
+    uri = URI::FTP.build(['user:password', 'ftp.example.com', nil, 
+                         '/path/file.zip', 'i'])
+    assert_equal(uri.to_s,
+           'ftp://user:password@ftp.example.com/%2Fpath/file.zip;type=i')
+  end
+
   def test_select
     assert_equal(['ftp', 'a.b.c', 21], URI.parse('ftp://a.b.c/').select(:scheme, :host, :port))
     u = URI.parse('ftp://a.b.c/')
Index: test/uri/test_generic.rb
===================================================================
--- test/uri/test_generic.rb	(revision 16058)
+++ test/uri/test_generic.rb	(working copy)
@@ -38,6 +38,17 @@
     exp = [
       'ftp', 
       nil, 'ftp.is.co.za', URI::FTP.default_port, 
+      'rfc/rfc1808.txt', nil,
+    ]
+    ary = uri_to_ary(url)
+    assert_equal(exp, ary)
+    # 1'
+    url = URI.parse('ftp://ftp.is.co.za/%2Frfc/rfc1808.txt')
+    assert_kind_of(URI::FTP, url)
+
+    exp = [
+      'ftp', 
+      nil, 'ftp.is.co.za', URI::FTP.default_port, 
       '/rfc/rfc1808.txt', nil,
     ]
     ary = uri_to_ary(url)
@@ -124,11 +135,10 @@
     assert_kind_of(URI::Generic, url)
 
     # 9
-    # [ruby-dev:25667]
     url = URI.parse('ftp://:pass@localhost/')
-    assert_equal('', url.user)
+    assert_equal('', url.user, "[ruby-dev:25667]")
     assert_equal('pass', url.password)
-    assert_equal(':pass', url.userinfo)
+    assert_equal(':pass', url.userinfo, "[ruby-dev:25667]")
     url = URI.parse('ftp://user@localhost/')
     assert_equal('user', url.user)
     assert_equal(nil, url.password)
@@ -155,9 +165,8 @@
     assert_equal(URI.parse('http://foo/baz'), u3 + '/baz')
     assert_equal(URI.parse('http://foo/baz'), u4 + '/baz')
 
-    # from [ruby-dev:11508] Re: uri
     url = URI.parse('http://hoge/a.html') + 'b.html'
-    assert_equal('http://hoge/b.html', url.to_s)
+    assert_equal('http://hoge/b.html', url.to_s, "[ruby-dev:11508]")
 
     # reported by Mr. Kubota <em6t-kbt@asahi-net.or.jp>
     url = URI.parse('http://a/b') + 'http://x/y'
@@ -174,15 +183,13 @@
     assert(nil != u.merge!("../baz"))
     assert_equal('http://foo/baz', u.to_s)
 
-    # [ruby-dev:23628]
     u0 = URI.parse('mailto:foo@example.com')
     u1 = URI.parse('mailto:foo@example.com#bar')
-    assert_equal(uri_to_ary(u0 + '#bar'), uri_to_ary(u1))
+    assert_equal(uri_to_ary(u0 + '#bar'), uri_to_ary(u1), "[ruby-dev:23628]")
 
-    # [ruby-list:39838]
     u0 = URI.parse('http://www.example.com/')
     u1 = URI.parse('http://www.example.com/foo/..') + './'
-    assert_equal(u0, u1)
+    assert_equal(u0, u1, "[ruby-list:39838]")
     u0 = URI.parse('http://www.example.com/foo/')
     u1 = URI.parse('http://www.example.com/foo/bar/..') + './'
     assert_equal(u0, u1)
@@ -196,11 +203,10 @@
     u1 = URI.parse('http://www.example.com/foo/bar/baz/../..') + './'
     assert_equal(u0, u1)
 
-    # [ruby-list:39844]
     u = URI.parse('http://www.example.com/')
     u0 = u + './foo/'
     u1 = u + './foo/bar/..'
-    assert_equal(u0, u1)
+    assert_equal(u0, u1, "[ruby-list:39844]")
     u = URI.parse('http://www.example.com/')
     u0 = u + './'
     u1 = u + './foo/bar/../..'
@@ -238,7 +244,7 @@
     assert_equal('', url.to_s)
   end
 
-  def test_rfc2396_examples
+  def test_rfc3986_examples
 #  http://a/b/c/d;p?q
 #        g:h           =  g:h
     url = @base_url.merge('g:h')
@@ -296,11 +302,11 @@
     assert_equal('//g', url.to_s)
 
 #  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)
 
@@ -314,11 +320,11 @@
     assert_equal('g?y', url.to_s)
 
 #  http://a/b/c/d;p?q
-#        #s            =  (current document)#s
+#        #s            =  http://a/b/c/d;p?q#s
     url = @base_url.merge('#s')
     assert_kind_of(URI::HTTP, url)
-    assert_equal(@base_url.to_s + '#s', url.to_s)
-    url = @base_url.route_to(@base_url.to_s + '#s')
+    assert_equal('http://a/b/c/d;p?q#s', url.to_s)
+    url = @base_url.route_to('http://a/b/c/d;p?q#s')
     assert_kind_of(URI::Generic, url)
     assert_equal('#s', url.to_s)
 
@@ -452,22 +458,22 @@
     assert_equal('', url.to_s)
 
 #  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)
-    url = @base_url.route_to('http://a/./g')
-    assert_kind_of(URI::Generic, url)
-    assert_equal('/./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)
+#    assert_equal('/./g', url.to_s)
 
 #  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)
-    url = @base_url.route_to('http://a/../g')
-    assert_kind_of(URI::Generic, url)
-    assert_equal('/../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)
+#    assert_equal('/../g', url.to_s)
 
 #  http://a/b/c/d;p?q
 #        g.            =  http://a/b/c/g.
@@ -506,24 +512,24 @@
     assert_equal('..g', url.to_s)
 
 #  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)
-    url = @base_url.route_to('http://a/../g')
+    assert_equal('http://a/g', url.to_s)
+    url = @base_url.route_to('http://a/g')
     assert_kind_of(URI::Generic, url)
-    assert('../../../g' != url.to_s) # ok? yes, it confuses you
-    assert_equal('/../g', url.to_s)  # and it is clearly
+    assert('../../../g' != url.to_s)  # ok? yes, it confuses you
+    assert_equal('../../g', url.to_s) # and it is clearly
 
 #  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)
-    url = @base_url.route_to('http://a/../../g')
+    assert_equal('http://a/g', url.to_s)
+    url = @base_url.route_to('http://a/g')
     assert_kind_of(URI::Generic, url)
     assert('../../../../g' != url.to_s) # ok? yes, it confuses you
-    assert_equal('/../../g', url.to_s)  # and it is clearly
+    assert_equal('../../g', url.to_s)   # and it is clearly
 
 #  http://a/b/c/d;p?q
 #        ./../g        =  http://a/b/g

Thread

Prev Next

In This Thread

Prev Next