Skip to content

Commit

Permalink
Fix ActionDispatch::Static to serve files with unencoded PCHAR
Browse files Browse the repository at this point in the history
RFC 3986[1] allows sub-delim characters in path segments unencoded,
however Rack::File requires them to be encoded so we use URI's
unescape method to leave them alone and then escape them again.

Also since the path gets passed to Dir[] we need to escape any glob
characters in the path.

[1]: http://www.ietf.org/rfc/rfc3986.txt
  • Loading branch information
pixeltrix committed Feb 17, 2012
1 parent 1a0bc2a commit 86d3bc3
Show file tree
Hide file tree
Showing 14 changed files with 50 additions and 2 deletions.
12 changes: 10 additions & 2 deletions actionpack/lib/action_dispatch/middleware/static.rb
Expand Up @@ -11,14 +11,14 @@ def initialize(root, cache_control)
def match?(path)
path = path.dup

full_path = path.empty? ? @root : File.join(@root, ::Rack::Utils.unescape(path))
full_path = path.empty? ? @root : File.join(@root, escape_glob_chars(unescape_path(path)))
paths = "#{full_path}#{ext}"

matches = Dir[paths]
match = matches.detect { |m| File.file?(m) }
if match
match.sub!(@compiled_root, '')
match
::Rack::Utils.escape(match)
end
end

Expand All @@ -32,6 +32,14 @@ def ext
"{,#{ext},/index#{ext}}"
end
end

def unescape_path(path)
URI.parser.unescape(path)
end

def escape_glob_chars(path)
path.gsub(/(\*|\?|\[|\]|\{|\})/, "\\\\\\1")
end
end

class Static
Expand Down
28 changes: 28 additions & 0 deletions actionpack/test/dispatch/static_test.rb
Expand Up @@ -30,6 +30,34 @@ def test_serves_static_index_file_in_directory
assert_html "/foo/index.html", get("/foo")
end

def test_serves_static_file_with_encoded_pchar
assert_html "/foo/foo!bar.html", get("/foo/foo%21bar.html")
assert_html "/foo/foo$bar.html", get("/foo/foo%24bar.html")
assert_html "/foo/foo&bar.html", get("/foo/foo%26bar.html")
assert_html "/foo/foo'bar.html", get("/foo/foo%27bar.html")
assert_html "/foo/foo(bar).html", get("/foo/foo%28bar%29.html")
assert_html "/foo/foo*bar.html", get("/foo/foo%2Abar.html")
assert_html "/foo/foo+bar.html", get("/foo/foo%2Bbar.html")
assert_html "/foo/foo,bar.html", get("/foo/foo%2Cbar.html")
assert_html "/foo/foo;bar.html", get("/foo/foo%3Bbar.html")
assert_html "/foo/foo:bar.html", get("/foo/foo%3Abar.html")
assert_html "/foo/foo@bar.html", get("/foo/foo%40bar.html")
end

def test_serves_static_file_with_unencoded_pchar
assert_html "/foo/foo!bar.html", get("/foo/foo!bar.html")
assert_html "/foo/foo$bar.html", get("/foo/foo$bar.html")
assert_html "/foo/foo&bar.html", get("/foo/foo&bar.html")
assert_html "/foo/foo'bar.html", get("/foo/foo'bar.html")
assert_html "/foo/foo(bar).html", get("/foo/foo(bar).html")
assert_html "/foo/foo*bar.html", get("/foo/foo*bar.html")
assert_html "/foo/foo+bar.html", get("/foo/foo+bar.html")
assert_html "/foo/foo,bar.html", get("/foo/foo,bar.html")
assert_html "/foo/foo;bar.html", get("/foo/foo;bar.html")
assert_html "/foo/foo:bar.html", get("/foo/foo:bar.html")
assert_html "/foo/foo@bar.html", get("/foo/foo@bar.html")
end

private

def assert_html(body, response)
Expand Down
1 change: 1 addition & 0 deletions actionpack/test/fixtures/public/foo/foo!bar.html
@@ -0,0 +1 @@
/foo/foo!bar.html
1 change: 1 addition & 0 deletions actionpack/test/fixtures/public/foo/foo$bar.html
@@ -0,0 +1 @@
/foo/foo$bar.html
1 change: 1 addition & 0 deletions actionpack/test/fixtures/public/foo/foo&bar.html
@@ -0,0 +1 @@
/foo/foo&bar.html
1 change: 1 addition & 0 deletions actionpack/test/fixtures/public/foo/foo'bar.html
@@ -0,0 +1 @@
/foo/foo'bar.html
1 change: 1 addition & 0 deletions actionpack/test/fixtures/public/foo/foo(bar).html
@@ -0,0 +1 @@
/foo/foo(bar).html
1 change: 1 addition & 0 deletions actionpack/test/fixtures/public/foo/foo*bar.html
@@ -0,0 +1 @@
/foo/foo*bar.html
1 change: 1 addition & 0 deletions actionpack/test/fixtures/public/foo/foo+bar.html
@@ -0,0 +1 @@
/foo/foo+bar.html
1 change: 1 addition & 0 deletions actionpack/test/fixtures/public/foo/foo,bar.html
@@ -0,0 +1 @@
/foo/foo,bar.html
1 change: 1 addition & 0 deletions actionpack/test/fixtures/public/foo/foo:bar.html
@@ -0,0 +1 @@
/foo/foo:bar.html
1 change: 1 addition & 0 deletions actionpack/test/fixtures/public/foo/foo;bar.html
@@ -0,0 +1 @@
/foo/foo;bar.html
1 change: 1 addition & 0 deletions actionpack/test/fixtures/public/foo/foo=bar.html
@@ -0,0 +1 @@
/foo/foo=bar.html
1 change: 1 addition & 0 deletions actionpack/test/fixtures/public/foo/foo@bar.html
@@ -0,0 +1 @@
/foo/foo@bar.html

0 comments on commit 86d3bc3

Please sign in to comment.