fixing support for directories that have + in the name

directories that have + in the name should be served up if the browser
puts a + in the path.  + is a valid path character, so it should not be
translated to a space (like you do in query parameters).

references #265 rails/rails#11816

lib/rack/directory.rb

@@ -63,7 +63,7 @@ def initialize(root, app=nil)
 
     def call(env)
       script_name = env[SCRIPT_NAME]
-      path_info = Utils.unescape(env[PATH_INFO])
+      path_info = Utils.unescape_path(env[PATH_INFO])
 
       if forbidden = check_forbidden(path_info)
         forbidden
@@ -88,16 +88,16 @@ def list_directory(path_info, path, script_name)
       glob = ::File.join(path, '*')
 
       url_head = (script_name.split('/') + path_info.split('/')).map do |part|
-        Rack::Utils.escape part
+        Rack::Utils.escape_path part
       end
 
       Dir[glob].sort.each do |node|
         stat = stat(node)
-        next  unless stat
+        next unless stat
         basename = ::File.basename(node)
         ext = ::File.extname(node)
 
-        url = ::File.join(*url_head + [Rack::Utils.escape(basename)])
+        url = ::File.join(*url_head + [Rack::Utils.escape_path(basename)])
         size = stat.size
         type = stat.directory? ? 'directory' : Mime.mime_type(ext)
         size = stat.directory? ? '-' : filesize_format(size)
@@ -111,7 +111,7 @@ def list_directory(path_info, path, script_name)
       return [ 200, { CONTENT_TYPE =>'text/html; charset=utf-8'}, DirectoryBody.new(@root, path, files) ]
     end
 
-    def stat(node, max = 10)
+    def stat(node)
       ::File.stat(node)
     rescue Errno::ENOENT, Errno::ELOOP
       return nil

lib/rack/utils.rb

@@ -19,6 +19,8 @@ module Utils
     COMMON_SEP = QueryParser::COMMON_SEP
     KeySpaceConstrainedParams = QueryParser::Params
 
+    DEFAULT_PARSER = URI::Parser.new
+
     class << self
       attr_accessor :default_query_parser
     end
@@ -35,10 +37,18 @@ def escape(s)
     # Like URI escaping, but with %20 instead of +. Strictly speaking this is
     # true URI escaping.
     def escape_path(s)
-      escape(s).gsub('+', '%20')
+      DEFAULT_PARSER.escape s
     end
     module_function :escape_path
 
+    # Unescapes the **path** component of a URI.  See Rack::Utils.unescape for
+    # unescaping query parameters or form components.
+    def unescape_path(s)
+      DEFAULT_PARSER.unescape s
+    end
+    module_function :unescape_path
+
+
     # Unescapes a URI escaped string with +encoding+. +encoding+ will be the
     # target encoding of the string returned, and it defaults to UTF-8
     def unescape(s, encoding = Encoding::UTF_8)

test/spec_directory.rb

@@ -2,11 +2,36 @@
 require 'rack/directory'
 require 'rack/lint'
 require 'rack/mock'
+require 'tempfile'
+require 'fileutils'
 
 describe Rack::Directory do
   DOCROOT = File.expand_path(File.dirname(__FILE__)) unless defined? DOCROOT
   FILE_CATCH = proc{|env| [200, {'Content-Type'=>'text/plain', "Content-Length" => "7"}, ['passed!']] }
-  app = Rack::Lint.new(Rack::Directory.new(DOCROOT, FILE_CATCH))
+
+  attr_reader :app
+
+  def setup
+    @app = Rack::Lint.new(Rack::Directory.new(DOCROOT, FILE_CATCH))
+  end
+
+  it 'serves directories with + in the name' do
+    Dir.mktmpdir do |dir|
+      plus_dir = "foo+bar"
+      full_dir = File.join(dir, plus_dir)
+      FileUtils.mkdir full_dir
+      FileUtils.touch File.join(full_dir, "omg.txt")
+      app = Rack::Directory.new(dir, FILE_CATCH)
+      env = Rack::MockRequest.env_for("/#{plus_dir}/")
+      status,_,body = app.call env
+
+      assert_equal 200, status
+
+      str = ''
+      body.each { |x| str << x }
+      assert_match "foo+bar", str
+    end
+  end
 
   it "serve directory indices" do
     res = Rack::MockRequest.new(Rack::Lint.new(app)).
@@ -63,16 +88,35 @@
     res = mr.get("/cgi/test%2bdirectory")
 
     res.must_be :ok?
-    res.body.must_match(%r[/cgi/test%2Bdirectory/test%2Bfile])
+    res.body.must_match(%r[/cgi/test\+directory/test\+file])
 
     res = mr.get("/cgi/test%2bdirectory/test%2bfile")
     res.must_be :ok?
   end
 
+  it "correctly escape script name with spaces" do
+    Dir.mktmpdir do |dir|
+      space_dir = "foo bar"
+      full_dir = File.join(dir, space_dir)
+      FileUtils.mkdir full_dir
+      FileUtils.touch File.join(full_dir, "omg omg.txt")
+      app = Rack::Directory.new(dir, FILE_CATCH)
+      env = Rack::MockRequest.env_for(Rack::Utils.escape_path("/#{space_dir}/"))
+      status,_,body = app.call env
+
+      assert_equal 200, status
+
+      str = ''
+      body.each { |x| str << x }
+      assert_match "/foo%20bar/omg%20omg.txt", str
+    end
+  end
+
   it "correctly escape script name" do
+    _app = app
     app2 = Rack::Builder.new do
       map '/script-path' do
-        run app
+        run _app
       end
     end
 
@@ -81,9 +125,9 @@
     res = mr.get("/script-path/cgi/test%2bdirectory")
 
     res.must_be :ok?
-    res.body.must_match(%r[/script-path/cgi/test%2Bdirectory/test%2Bfile])
+    res.body.must_match(%r[/script-path/cgi/test\+directory/test\+file])
 
-    res = mr.get("/script-path/cgi/test%2bdirectory/test%2bfile")
+    res = mr.get("/script-path/cgi/test+directory/test+file")
     res.must_be :ok?
   end
 end