Skip to content

Commit

Permalink
Fix Encoding::CompatibilityError when public path is UTF-8
Browse files Browse the repository at this point in the history
In rails#5337 we forced the path encoding to ASCII-8BIT to prevent static
file handling from blowing up before an application has had chance to
deal with possibly invalid urls. However this has a negative side
effect of making it an incompatible encoding if the application's
public path has UTF-8 characters in it.

To work around the problem we check to see if the path has a valid
encoding once it has been unescaped. If it is not valid then we can
return early since it will not match any file anyway.

Fixes rails#13518
  • Loading branch information
pixeltrix committed Dec 29, 2013
1 parent 54ccc58 commit 436ed51
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 6 deletions.
15 changes: 15 additions & 0 deletions actionpack/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,18 @@
* Fix `Encoding::CompatibilityError` when public path is UTF-8

In #5337 we forced the path encoding to ASCII-8BIT to prevent static file handling
from blowing up before an application has had chance to deal with possibly invalid
urls. However this has a negative side effect of making it an incompatible encoding
if the application's public path has UTF-8 characters in it.

To work around the problem we check to see if the path has a valid encoding once
it has been unescaped. If it is not valid then we can return early since it will
not match any file anyway.

Fixes #13518

*Andrew White*

* `ActionController::Parameters#permit!` permits hashes in array values.

*Xavier Noria*
Expand Down
6 changes: 3 additions & 3 deletions actionpack/lib/action_dispatch/middleware/static.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ def initialize(root, cache_control)
end

def match?(path)
path = path.dup
path = unescape_path(path)
return false unless path.valid_encoding?

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

matches = Dir[paths]
Expand All @@ -40,7 +41,6 @@ def unescape_path(path)
end

def escape_glob_chars(path)
path.force_encoding('binary') if path.respond_to? :force_encoding
path.gsub(/[*?{}\[\]]/, "\\\\\\&")
end
end
Expand Down
19 changes: 16 additions & 3 deletions actionpack/test/dispatch/static_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def get(path)
end

def with_static_file(file)
path = "#{FIXTURE_LOAD_PATH}/public" + file
path = "#{FIXTURE_LOAD_PATH}/#{public_path}" + file
File.open(path, "wb+") { |f| f.write(file) }
yield file
ensure
Expand All @@ -149,11 +149,24 @@ class StaticTest < ActiveSupport::TestCase
DummyApp = lambda { |env|
[200, {"Content-Type" => "text/plain"}, ["Hello, World!"]]
}
App = ActionDispatch::Static.new(DummyApp, "#{FIXTURE_LOAD_PATH}/public", "public, max-age=60")

def setup
@app = App
@app = ActionDispatch::Static.new(DummyApp, "#{FIXTURE_LOAD_PATH}/public", "public, max-age=60")
end

def public_path
"public"
end

include StaticTests
end

class StaticEncodingTest < StaticTest
def setup
@app = ActionDispatch::Static.new(DummyApp, "#{FIXTURE_LOAD_PATH}/公共", "public, max-age=60")
end

def public_path
"公共"
end
end
1 change: 1 addition & 0 deletions actionpack/test/fixtures/公共/foo/bar.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/foo/bar.html
3 changes: 3 additions & 0 deletions actionpack/test/fixtures/公共/foo/baz.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
body {
background: #000;
}
1 change: 1 addition & 0 deletions actionpack/test/fixtures/公共/foo/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/foo/index.html
1 change: 1 addition & 0 deletions actionpack/test/fixtures/公共/foo/こんにちは.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
means hello in Japanese
1 change: 1 addition & 0 deletions actionpack/test/fixtures/公共/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/index.html

0 comments on commit 436ed51

Please sign in to comment.