Bug #1699

URI::FTP to_s problem after modification

Added by mizyo (Norihisa Fujita) almost 3 years ago. Updated about 1 year ago.

[ruby-core:24077]
Status:Closed Start date:06/29/2009
Priority:Normal Due date:
Assignee:akira (akira yamada) % Done:

100%

Category:-
Target version:1.9.2
ruby -v:ruby 1.9.2dev (2009-06-29 trunk 23886)

Description

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

patch (477 Bytes) mizyo (Norihisa Fujita), 06/29/2009 12:13 pm

Associated revisions

Revision 27350
Added by mame (Yusuke Endoh) about 2 years ago

* lib/uri/ftp.rb (URI::FTP#set_path): added to correct handling of special case where path of ftp is relative. This converts relative path to absolute one, because external representation of ftp path is relative and internal representation is absolute. [ruby-core:24077] * lib/uri/ftp.rb (URI::FTP#initialize): converts absolute to relative. * lib/uri/generic.rb (URI::Generic#check_path): allow relative path when scheme is ftp.

History

Updated by yugui (Yuki Sonoda) almost 3 years ago

  • Status changed from Open to Assigned
  • Assignee set to akira (akira yamada)
  • Target version set to 1.9.2

Updated by naruse (Yui NARUSE) over 2 years ago

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

Updated by mame (Yusuke Endoh) about 2 years ago

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>

Updated by mame (Yusuke Endoh) about 2 years ago

  • 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.

Also available in: Atom PDF