From 0b1a87f73cca3da23b65f3dfb19daeac436ab2ee Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 18 Aug 2014 11:20:06 -0500 Subject: Refactor out Dir.glob from ActionDispatch::Static Dir.glob can be a security concern. The original use was to provide logic of fallback files. Example a request to `/` should render the file from `/public/index.html`. We can replace the dir glob with the specific logic it represents. The glob {,index,index.html} will look for the current path, then in the directory of the path with index file and then in the directory of the path with index.html. This PR replaces the glob logic by manually checking each potential match. Best case scenario this results in one less file API request, worst case, this has one more file API request. Related to #16464 Update: added a test for when a file of a given name (`public/bar.html` and a directory `public/bar` both exist in the same root directory. Changed logic to accommodate this scenario. --- .../lib/action_dispatch/middleware/static.rb | 25 +++++----------------- actionpack/test/dispatch/static_test.rb | 4 ++++ actionpack/test/fixtures/public/bar.html | 1 + actionpack/test/fixtures/public/bar/index.html | 1 + .../fixtures/\345\205\254\345\205\261/bar.html" | 1 + .../\345\205\254\345\205\261/bar/index.html" | 1 + 6 files changed, 13 insertions(+), 20 deletions(-) create mode 100644 actionpack/test/fixtures/public/bar.html create mode 100644 actionpack/test/fixtures/public/bar/index.html create mode 100644 "actionpack/test/fixtures/\345\205\254\345\205\261/bar.html" create mode 100644 "actionpack/test/fixtures/\345\205\254\345\205\261/bar/index.html" (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index 0733132277..e66c21ef85 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -21,17 +21,13 @@ module ActionDispatch end def match?(path) - path = unescape_path(path) + path = URI.parser.unescape(path) return false unless path.valid_encoding? - full_path = path.empty? ? @root : File.join(@root, escape_glob_chars(path)) - paths = "#{full_path}#{ext}" + paths = [path, "#{path}#{ext}", "#{path}/index#{ext}"] - matches = Dir[paths] - match = matches.detect { |m| File.file?(m) } - if match - match.sub!(@compiled_root, '') - ::Rack::Utils.escape(match) + if match = paths.detect {|p| File.file?(File.join(@root, p)) } + return ::Rack::Utils.escape(match) end end @@ -57,18 +53,7 @@ module ActionDispatch private def ext - @ext ||= begin - ext = ::ActionController::Base.default_static_extension - "{,#{ext},/index#{ext}}" - end - end - - def unescape_path(path) - URI.parser.unescape(path) - end - - def escape_glob_chars(path) - path.gsub(/[*?{}\[\]]/, "\\\\\\&") + ::ActionController::Base.default_static_extension end def content_type(path) diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb index 97affc7b21..6f7373201c 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -37,6 +37,10 @@ module StaticTests assert_html "/foo/index.html", get("/foo") end + def test_serves_file_with_same_name_before_index_in_directory + assert_html "/bar.html", get("/bar") + end + def test_served_static_file_with_non_english_filename jruby_skip "Stop skipping if following bug gets fixed: " \ "http://jira.codehaus.org/browse/JRUBY-7192" diff --git a/actionpack/test/fixtures/public/bar.html b/actionpack/test/fixtures/public/bar.html new file mode 100644 index 0000000000..67fc57079b --- /dev/null +++ b/actionpack/test/fixtures/public/bar.html @@ -0,0 +1 @@ +/bar.html \ No newline at end of file diff --git a/actionpack/test/fixtures/public/bar/index.html b/actionpack/test/fixtures/public/bar/index.html new file mode 100644 index 0000000000..d5bb8f898d --- /dev/null +++ b/actionpack/test/fixtures/public/bar/index.html @@ -0,0 +1 @@ +/bar/index.html \ No newline at end of file diff --git "a/actionpack/test/fixtures/\345\205\254\345\205\261/bar.html" "b/actionpack/test/fixtures/\345\205\254\345\205\261/bar.html" new file mode 100644 index 0000000000..67fc57079b --- /dev/null +++ "b/actionpack/test/fixtures/\345\205\254\345\205\261/bar.html" @@ -0,0 +1 @@ +/bar.html \ No newline at end of file diff --git "a/actionpack/test/fixtures/\345\205\254\345\205\261/bar/index.html" "b/actionpack/test/fixtures/\345\205\254\345\205\261/bar/index.html" new file mode 100644 index 0000000000..d5bb8f898d --- /dev/null +++ "b/actionpack/test/fixtures/\345\205\254\345\205\261/bar/index.html" @@ -0,0 +1 @@ +/bar/index.html \ No newline at end of file -- cgit v1.2.3