Forum: Ruby-core [Bug #1699] URI::FTP to_s problem after modification

Bug #1699: URI::FTP to_s problem after modification
http://redmine.ruby-lang.org/issues/show/1699

Author: Norihisa Fujita
Status: Open, Priority: Normal
ruby -v: ruby 1.9.2dev (2009-06-29 trunk 23886)

After modification URI::FTP object by +, a slash is missing after host.
ruby 1.8.7 also has this problem, but 1.8.6 does not.

I think we should use self.path instead of @path in to_s. (please see 
attached patch)

Reproduction code is:
require 'uri'
uri = 'ftp://host/path'
puts URI.parse(uri)  #=> ftp://host/path
puts URI.parse(uri) + './foobar' #=> ftp://hostfoobar

Issue #1699 has been updated by Yui NARUSE.


This is derived from the spec level bug of "ftp" URI scheme and uri/ftp 
lib.

http://tools.ietf.org/html/rfc1738
http://tools.ietf.org/html/rfc1808
http://tools.ietf.org/html/draft-hoffman-ftp-uri-04
http://lists.w3.org/Archives/Public/public-iri/2010Jan/0002.html
----------------------------------------
http://redmine.ruby-lang.org/issues/show/1699

Issue #1699 has been updated by Yusuke Endoh.


Hi,

> This is derived from the spec level bug of "ftp" URI scheme and uri/ftp lib.

Even so, the reported behavior is absolutely wrong.

Indeed, the spec seems to be bizarre.  And, to conform the
bizarre spec, URI was modified by defining URI::FTP#path
(at r9028).

I agree with the purpose of the fix, but the way of the fix
is incomplete and wrong.

URI::FTP#path removes preceding '/' and decodes preceding
'%2F' to '/'.  If it does so, URI::FTP#set_path MUST be
defined and do the opposite.

I'll soon commit my patch which is approved by naruse.
I confirmed that make test, test-all and test-rubyspec are
passed.


BTW, the fix I mentioned above was commited at r9028.
Its commit log is:

  Author: ryan <ryan@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>
  Date:   Wed Aug 24 05:08:00 2005 +0000

      Lovely RDOC patches from mathew (metaATpoboxDOTcom) on URI/* and 
getoptlong.rb

But in fact, it includes code, which changed the spec and
even caused the regression actually.

I believe that this is accident.  I don't know who mathew
is and how he sent the patch to Ryan.  But Ryan, please be
careful to check a patch written by others when you import.


diff --git a/lib/uri/ftp.rb b/lib/uri/ftp.rb
index 1ffd909..85efccd 100644
--- a/lib/uri/ftp.rb
+++ b/lib/uri/ftp.rb
@@ -87,11 +87,6 @@ module URI
       # 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)

@@ -118,6 +113,7 @@ module URI
     # +opaque+, +query+ and +fragment+, in that order.
     #
     def initialize(*arg)
+      arg[5] = arg[5].sub(/^\//,'').sub(/^%2F/,'/')
       super(*arg)
       @typecode = nil
       tmp = @path.index(TYPECODE_PREFIX)
@@ -185,6 +181,11 @@ module URI
       return @path.sub(/^\//,'').sub(/^%2F/,'/')
     end

+    def set_path(v)
+      super("/" + v.sub(/^\//, "%2F"))
+    end
+    protected :set_path
+
     def to_s
       save_path = nil
       if @typecode
diff --git a/lib/uri/generic.rb b/lib/uri/generic.rb
index 14ca973..4fdfd14 100644
--- a/lib/uri/generic.rb
+++ b/lib/uri/generic.rb
@@ -482,7 +482,9 @@ module URI
           "path conflicts with opaque"
       end

-      if @scheme
+      # If scheme is ftp, path may be relative.
+      # See RFC 1738 section 3.2.2, and RFC 2396.
+      if @scheme && @scheme != "ftp"
         if v && v != '' && parser.regexp[:ABS_PATH] !~ v
           raise InvalidComponentError,
             "bad component(expected absolute path component): #{v}"

--
Yusuke Endoh <mame@tsg.ne.jp>
----------------------------------------
http://redmine.ruby-lang.org/issues/show/1699

Issue #1699 has been updated by Yusuke Endoh.

Status changed from Assigned to Closed
% Done changed from 0 to 100

This issue was solved with changeset r27350.
Norihisa, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

----------------------------------------
http://redmine.ruby-lang.org/issues/show/1699