diff options
17 files changed, 137 insertions, 52 deletions
diff --git a/.travis.yml b/.travis.yml index 303c838693..dd1d911b63 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,6 @@ script: 'ci/travis.rb' before_install: - travis_retry gem install bundler - "rvm current | grep 'jruby' && export AR_JDBC=true || echo" -install: "bundle install --without test" rvm: - 1.9.3 - 2.0.0 diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 3e3df19a84..2490282d6e 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -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* diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index c6a7d9c415..2764584fe9 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -11,9 +11,10 @@ module ActionDispatch 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] @@ -40,7 +41,6 @@ module ActionDispatch end def escape_glob_chars(path) - path.force_encoding('binary') if path.respond_to? :force_encoding path.gsub(/[*?{}\[\]]/, "\\\\\\&") end end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index bfba8d143d..4bf2dc6e23 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -3,6 +3,7 @@ require 'active_support/core_ext/hash/reverse_merge' require 'active_support/core_ext/hash/slice' require 'active_support/core_ext/enumerable' require 'active_support/core_ext/array/extract_options' +require 'active_support/core_ext/module/remove_method' require 'active_support/inflector' require 'action_dispatch/routing/redirection' @@ -546,11 +547,11 @@ module ActionDispatch _routes = @set app.routes.define_mounted_helper(name) app.routes.singleton_class.class_eval do - define_method :mounted? do + redefine_method :mounted? do true end - define_method :_generate_prefix do |options| + redefine_method :_generate_prefix do |options| prefix_options = options.slice(*_route.segment_keys) # we must actually delete prefix segment keys to avoid passing them to next url_for _route.segment_keys.each { |k| options.delete(k) } diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb index acccbcb2e6..d83461e52f 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -137,7 +137,7 @@ module StaticTests 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 @@ -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 diff --git a/actionpack/test/fixtures/公共/foo/bar.html b/actionpack/test/fixtures/公共/foo/bar.html new file mode 100644 index 0000000000..9a35646205 --- /dev/null +++ b/actionpack/test/fixtures/公共/foo/bar.html @@ -0,0 +1 @@ +/foo/bar.html
\ No newline at end of file diff --git a/actionpack/test/fixtures/公共/foo/baz.css b/actionpack/test/fixtures/公共/foo/baz.css new file mode 100644 index 0000000000..b5173fbef2 --- /dev/null +++ b/actionpack/test/fixtures/公共/foo/baz.css @@ -0,0 +1,3 @@ +body { +background: #000; +} diff --git a/actionpack/test/fixtures/公共/foo/index.html b/actionpack/test/fixtures/公共/foo/index.html new file mode 100644 index 0000000000..497a2e898f --- /dev/null +++ b/actionpack/test/fixtures/公共/foo/index.html @@ -0,0 +1 @@ +/foo/index.html
\ No newline at end of file diff --git a/actionpack/test/fixtures/公共/foo/こんにちは.html b/actionpack/test/fixtures/公共/foo/こんにちは.html new file mode 100644 index 0000000000..1df9166522 --- /dev/null +++ b/actionpack/test/fixtures/公共/foo/こんにちは.html @@ -0,0 +1 @@ +means hello in Japanese diff --git a/actionpack/test/fixtures/公共/index.html b/actionpack/test/fixtures/公共/index.html new file mode 100644 index 0000000000..525950ba6b --- /dev/null +++ b/actionpack/test/fixtures/公共/index.html @@ -0,0 +1 @@ +/index.html
\ No newline at end of file diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb index 7e16156408..a1664509df 100644 --- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb @@ -13,6 +13,83 @@ module ActiveRecord @config = original.config.dup end + # Expands a connection string into a hash + class ConnectionUrlResolver # :nodoc: + + # == Example + # url = 'postgresql://foo:bar@localhost:9000/foo_test?pool=5&timeout=3000' + # ConnectionUrlResolver.new(url).to_hash + # # => { + # "adapter" => "postgresql", + # "host" => "localhost", + # "port" => 9000, + # "database" => "foo_test", + # "username" => "foo", + # "password" => "bar", + # "pool" => "5", + # "timeout" => "3000" + # } + def initialize(url) + raise "Database URL cannot be empty" if url.blank? + @uri = URI.parse(url) + @adapter = @uri.scheme + @adapter = "postgresql" if @adapter == "postgres" + @query = @uri.query || '' + end + + # Converts the given url to a full connection hash + def to_hash + config = raw_config.reject { |_,value| value.blank? } + config.map { |key,value| config[key] = uri_parser.unescape(value) if value.is_a? String } + config + end + + private + + def uri + @uri + end + + def uri_parser + @uri_parser ||= URI::Parser.new + end + + # Converts the query parameters of the uri into a hash + # "localhost?pool=5&reap_frequency=2" + # # => {"pool" => "5", "reap_frequency" => "2"} + # + # returns empty hash if no query present + # "localhost" + # # => {} + def query_hash + Hash[@query.split("&").map { |pair| pair.split("=") }] + end + + def raw_config + query_hash.merge({ + "adapter" => @adapter, + "username" => uri.user, + "password" => uri.password, + "port" => uri.port, + "database" => database, + "host" => uri.host }) + end + + # Returns name of the database + # sqlite3 expects this to be a full path or `:memory` + def database + if @adapter == 'sqlite3' + if '/:memory:' == uri.path + ':memory:' + else + uri.path + end + else + uri.path.sub(%r{^/},"") + end + end + end + ## # Builds a ConnectionSpecification from user input class Resolver # :nodoc: @@ -84,40 +161,8 @@ module ActiveRecord spec end - def resolve_string_connection(spec) # :nodoc: - config = URI.parse spec - adapter = config.scheme - adapter = "postgresql" if adapter == "postgres" - - database = if adapter == 'sqlite3' - if '/:memory:' == config.path - ':memory:' - else - config.path - end - else - config.path.sub(%r{^/},"") - end - - spec = { "adapter" => adapter, - "username" => config.user, - "password" => config.password, - "port" => config.port, - "database" => database, - "host" => config.host } - - spec.reject!{ |_,value| value.blank? } - - uri_parser = URI::Parser.new - - spec.map { |key,value| spec[key] = uri_parser.unescape(value) if value.is_a?(String) } - - if config.query - options = Hash[config.query.split("&").map{ |pair| pair.split("=") }] - spec.merge!(options) - end - - spec + def resolve_string_connection(url) # :nodoc: + ConnectionUrlResolver.new(url).to_hash end end end diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 90dfc177e5..4b769aa11c 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -179,9 +179,6 @@ db_namespace = namespace :db do require 'active_record/fixtures' base_dir = if ENV['FIXTURES_PATH'] - STDERR.puts "Using FIXTURES_PATH env variable is deprecated, please use " + - "ActiveRecord::Tasks::DatabaseTasks.fixtures_path = '/path/to/fixtures' " + - "instead." File.join [Rails.root, ENV['FIXTURES_PATH'] || %w{test fixtures}].flatten else ActiveRecord::Tasks::DatabaseTasks.fixtures_path @@ -204,9 +201,6 @@ db_namespace = namespace :db do puts %Q(The fixture ID for "#{label}" is #{ActiveRecord::FixtureSet.identify(label)}.) if label base_dir = if ENV['FIXTURES_PATH'] - STDERR.puts "Using FIXTURES_PATH env variable is deprecated, please use " + - "ActiveRecord::Tasks::DatabaseTasks.fixtures_path = '/path/to/fixtures' " + - "instead." File.join [Rails.root, ENV['FIXTURES_PATH'] || %w{test fixtures}].flatten else ActiveRecord::Tasks::DatabaseTasks.fixtures_path diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 3d0709266a..78da6a83ec 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -37,6 +37,8 @@ module ActiveRecord def not(opts, *rest) where_value = @scope.send(:build_where, opts, rest).map do |rel| case rel + when NilClass + raise ArgumentError, 'Invalid argument for .where.not(), got nil.' when Arel::Nodes::In Arel::Nodes::NotIn.new(rel.left, rel.right) when Arel::Nodes::Equality diff --git a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb index a85f3d4298..605a2d86e4 100644 --- a/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb +++ b/activerecord/test/cases/adapters/mysql/mysql_adapter_test.rb @@ -18,7 +18,8 @@ module ActiveRecord def test_bad_connection_mysql assert_raise ActiveRecord::NoDatabaseError do - connection = ActiveRecord::Base.mysql_connection(adapter: "mysql", database: "should_not_exist-cinco-dog-db") + configuration = ActiveRecord::Base.configurations['arunit'].merge(database: 'should_not_exist-cinco-dog-db') + connection = ActiveRecord::Base.mysql_connection(configuration) connection.exec_query('drop table if exists ex') end end diff --git a/activerecord/test/cases/adapters/mysql2/connection_test.rb b/activerecord/test/cases/adapters/mysql2/connection_test.rb index 91b780a7ee..8fe8bd7df3 100644 --- a/activerecord/test/cases/adapters/mysql2/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql2/connection_test.rb @@ -15,7 +15,8 @@ class MysqlConnectionTest < ActiveRecord::TestCase def test_bad_connection assert_raise ActiveRecord::NoDatabaseError do - connection = ActiveRecord::Base.mysql2_connection(adapter: "mysql2", database: "should_not_exist-cinco-dog-db") + configuration = ActiveRecord::Base.configurations['arunit'].merge(database: 'should_not_exist-cinco-dog-db') + connection = ActiveRecord::Base.mysql2_connection(configuration) connection.exec_query('drop table if exists ex') end end diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index 778175e2e8..131080913c 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -12,7 +12,8 @@ module ActiveRecord def test_bad_connection assert_raise ActiveRecord::NoDatabaseError do - connection = ActiveRecord::Base.postgresql_connection(database: "should_not_exist-cinco-dog-db", adapter: "postgresql") + configuration = ActiveRecord::Base.configurations['arunit'].merge(database: 'should_not_exist-cinco-dog-db') + connection = ActiveRecord::Base.postgresql_connection(configuration) connection.exec_query('drop table if exists ex') end end diff --git a/activerecord/test/cases/relation/where_chain_test.rb b/activerecord/test/cases/relation/where_chain_test.rb index d44b4dfe3d..fd2420cb88 100644 --- a/activerecord/test/cases/relation/where_chain_test.rb +++ b/activerecord/test/cases/relation/where_chain_test.rb @@ -23,6 +23,12 @@ module ActiveRecord assert_equal([expected], relation.where_values) end + def test_not_with_nil + assert_raise ArgumentError do + Post.where.not(nil) + end + end + def test_not_in expected = Arel::Nodes::NotIn.new(Post.arel_table[@name], %w[hello goodbye]) relation = Post.where.not(title: %w[hello goodbye]) |