diff options
17 files changed, 71 insertions, 35 deletions
diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index d80d1d714c..ae875eb830 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -57,7 +57,7 @@ module ActionDispatch query_parameters.dup end params.merge!(path_parameters) - params = set_binary_encoding(params) + params = set_binary_encoding(params, params[:controller], params[:action]) set_header("action_dispatch.request.parameters", params) params end @@ -66,6 +66,7 @@ module ActionDispatch def path_parameters=(parameters) #:nodoc: delete_header("action_dispatch.request.parameters") + parameters = set_binary_encoding(parameters, parameters[:controller], parameters[:action]) # If any of the path parameters has an invalid encoding then # raise since it's likely to trigger errors further on. Request::Utils.check_param_encoding(parameters) @@ -85,9 +86,10 @@ module ActionDispatch private - def set_binary_encoding(params) - action = params[:action] - if binary_params_for?(action) + def set_binary_encoding(params, controller, action) + return params unless controller && controller.valid_encoding? + + if binary_params_for?(controller, action) ActionDispatch::Request::Utils.each_param_value(params) do |param| param.force_encoding ::Encoding::ASCII_8BIT end @@ -95,8 +97,8 @@ module ActionDispatch params end - def binary_params_for?(action) - controller_class.binary_params_for?(action) + def binary_params_for?(controller, action) + controller_class_for(controller).binary_params_for?(action) rescue NameError false end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 94b0d4880c..0b38ab7afb 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -76,10 +76,13 @@ module ActionDispatch def controller_class params = path_parameters + params[:action] ||= "index" + controller_class_for(params[:controller]) + end - if params.key?(:controller) - controller_param = params[:controller].underscore - params[:action] ||= "index" + def controller_class_for(name) + if name + controller_param = name.underscore const_name = "#{controller_param.camelize}Controller" ActiveSupport::Dependencies.constantize(const_name) else diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index 809bfcbe8a..9987a9bfa1 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -43,6 +43,10 @@ module ActionDispatch req.path_info = "/" + req.path_info unless req.path_info.start_with? "/" end + parameters = route.defaults.merge parameters.transform_values { |val| + val.dup.force_encoding(::Encoding::UTF_8) + } + req.path_parameters = set_params.merge parameters status, headers, body = route.app.serve(req) @@ -67,6 +71,7 @@ module ActionDispatch rails_req.path_info = match.post_match.sub(/^([^\/])/, '/\1') end + parameters = route.defaults.merge parameters yield(route, parameters) end end @@ -119,7 +124,7 @@ module ActionDispatch routes.map! { |r| match_data = r.path.match(req.path_info) - path_parameters = r.defaults.dup + path_parameters = {} match_data.names.zip(match_data.captures) { |name, val| path_parameters[name.to_sym] = Utils.unescape_uri(val) if val } diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index fefb84e095..f09051b306 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -96,6 +96,22 @@ class LegacyRouteSetTests < ActiveSupport::TestCase assert_equal({ "artist" => "journey", "song" => "faithfully" }, hash) end + def test_id_encoding + rs.draw do + get "/journey/:id", to: lambda { |env| + param = ActionDispatch::Request.new(env).path_parameters + resp = ActiveSupport::JSON.encode param + [200, {}, [resp]] + } + end + + # The encoding of the URL in production is *binary*, so we add a + # .b here. + hash = ActiveSupport::JSON.decode get(URI("http://example.org/journey/%E5%A4%AA%E9%83%8E".b)) + assert_equal({ "id" => "太郎" }, hash) + assert_equal ::Encoding::UTF_8, hash["id"].encoding + end + def test_id_with_dash rs.draw do get "/journey/:id", to: lambda { |env| diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 1af0edc1d3..446b65a9b9 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -4416,39 +4416,49 @@ end class TestInvalidUrls < ActionDispatch::IntegrationTest class FooController < ActionController::Base + def self.binary_params_for?(action) + action == "show" + end + def show render plain: "foo#show" end end - test "invalid UTF-8 encoding is treated as ASCII 8BIT encode" do + test "invalid UTF-8 encoding returns a bad request" do with_routing do |set| set.draw do get "/bar/:id", to: redirect("/foo/show/%{id}") - get "/foo/show(/:id)", to: "test_invalid_urls/foo#show" ok = lambda { |env| [200, { "Content-Type" => "text/plain" }, []] } get "/foobar/:id", to: ok ActiveSupport::Deprecation.silence do - get "/foo(/:action(/:id))", controller: "test_invalid_urls/foo" get "/:controller(/:action(/:id))" end end get "/%E2%EF%BF%BD%A6" - assert_response :not_found + assert_response :bad_request get "/foo/%E2%EF%BF%BD%A6" - assert_response :not_found - - get "/foo/show/%E2%EF%BF%BD%A6" - assert_response :ok + assert_response :bad_request get "/bar/%E2%EF%BF%BD%A6" - assert_response :redirect + assert_response :bad_request get "/foobar/%E2%EF%BF%BD%A6" + assert_response :bad_request + end + end + + test "params encoded with binary_params_for? are treated as ASCII 8bit" do + with_routing do |set| + set.draw do + get "/foo/show(/:id)", to: "test_invalid_urls/foo#show" + end + + get "/foo/show/%E2%EF%BF%BD%A6" assert_response :ok end end diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index 197f7f20b9..86f051f5ce 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -4,8 +4,8 @@ module ActiveModel module SecurePassword extend ActiveSupport::Concern - # BCrypt hash function can handle maximum 72 characters, and if we pass - # password of length more than 72 characters it ignores extra characters. + # BCrypt hash function can handle maximum 72 bytes, and if we pass + # password of length more than 72 bytes it ignores extra characters. # Hence need to put a restriction on password length. MAX_PASSWORD_LENGTH_ALLOWED = 72 @@ -20,7 +20,7 @@ module ActiveModel # # The following validations are added automatically: # * Password must be present on creation - # * Password length should be less than or equal to 72 characters + # * Password length should be less than or equal to 72 bytes # * Confirmation of password (using a +password_confirmation+ attribute) # # If password confirmation validation is not needed, simply leave out the diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index aa581ed1e6..8181d67816 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,4 +1,4 @@ -* When using #or, extract the common conditions and put them before the OR condition. +* When using `Relation#or`, extract the common conditions and put them before the OR condition. *Maxime Handfield Lapointe* diff --git a/activerecord/test/cases/comment_test.rb b/activerecord/test/cases/comment_test.rb index 9ab6607668..f2ec5d6518 100644 --- a/activerecord/test/cases/comment_test.rb +++ b/activerecord/test/cases/comment_test.rb @@ -119,8 +119,6 @@ if ActiveRecord::Base.connection.supports_comments? assert_match %r[t\.integer\s+"rating",\s+precision: 38,\s+comment: "I am running out of imagination"], output else assert_match %r[t\.integer\s+"rating",\s+comment: "I am running out of imagination"], output - end - unless current_adapter?(:OracleAdapter) assert_match %r[t\.index\s+.+\s+comment: "\\\"Very important\\\" index that powers all the performance.\\nAnd it's fun!"], output assert_match %r[t\.index\s+.+\s+name: "idx_obvious",\s+comment: "We need to see obvious comments"], output end diff --git a/activesupport/lib/active_support/testing/assertions.rb b/activesupport/lib/active_support/testing/assertions.rb index f6366bfd39..e2bc51ff7a 100644 --- a/activesupport/lib/active_support/testing/assertions.rb +++ b/activesupport/lib/active_support/testing/assertions.rb @@ -157,7 +157,7 @@ module ActiveSupport after = exp.call if to == UNTRACKED - error = "#{expression.inspect} didn't changed" + error = "#{expression.inspect} didn't change" error = "#{message}.\n#{error}" if message assert_not_equal before, after, error else diff --git a/guides/source/working_with_javascript_in_rails.md b/guides/source/working_with_javascript_in_rails.md index ed27752a06..304ac97b32 100644 --- a/guides/source/working_with_javascript_in_rails.md +++ b/guides/source/working_with_javascript_in_rails.md @@ -151,7 +151,7 @@ Because of Unobtrusive JavaScript, the Rails "Ajax helpers" are actually in two parts: the JavaScript half and the Ruby half. Unless you have disabled the Asset Pipeline, -[rails-ujs](https://github.com/rails/rails/blob/master/actionview/app/assets/javascripts/rails-ujs.coffee) +[rails-ujs](https://github.com/rails/rails/tree/master/actionview/app/assets/javascripts) provides the JavaScript half, and the regular Ruby view helpers add appropriate tags to your DOM. diff --git a/railties/lib/rails/generators/actions.rb b/railties/lib/rails/generators/actions.rb index 5cf0985050..2792d7636f 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -216,9 +216,9 @@ module Rails # Runs the supplied rake task (invoked with 'rails ...') # - # rails("db:migrate") - # rails("db:migrate", env: "production") - # rails("gems:install", sudo: true) + # rails_command("db:migrate") + # rails_command("db:migrate", env: "production") + # rails_command("gems:install", sudo: true) def rails_command(command, options = {}) execute_command :rails, command, options end diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 760cb2f81e..4899a63917 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -348,6 +348,8 @@ module Rails comment = "See https://github.com/rails/execjs#readme for more supported runtimes" if defined?(JRUBY_VERSION) GemfileEntry.version "therubyrhino", nil, comment + elsif RUBY_PLATFORM =~ /mingw|mswin/ + GemfileEntry.version "duktape", nil, comment else GemfileEntry.new "mini_racer", nil, comment, { platforms: :ruby }, true end diff --git a/railties/lib/rails/generators/rails/app/templates/bin/yarn b/railties/lib/rails/generators/rails/app/templates/bin/yarn index 44f75c22a4..c2f9b6768a 100644 --- a/railties/lib/rails/generators/rails/app/templates/bin/yarn +++ b/railties/lib/rails/generators/rails/app/templates/bin/yarn @@ -1,7 +1,7 @@ VENDOR_PATH = File.expand_path('..', __dir__) Dir.chdir(VENDOR_PATH) do begin - exec "yarnpkg #{ARGV.join(" ")}" + exec "yarnpkg #{ARGV.join(' ')}" rescue Errno::ENOENT $stderr.puts "Yarn executable was not detected in the system." $stderr.puts "Download Yarn at https://yarnpkg.com/en/docs/install" diff --git a/railties/lib/rails/generators/rails/app/templates/config/spring.rb b/railties/lib/rails/generators/rails/app/templates/config/spring.rb index c9119b40c0..9fa7863f99 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/spring.rb +++ b/railties/lib/rails/generators/rails/app/templates/config/spring.rb @@ -1,6 +1,6 @@ -%w( +%w[ .ruby-version .rbenv-vars tmp/restart.txt tmp/caching-dev.txt -).each { |path| Spring.watch(path) } +].each { |path| Spring.watch(path) } diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index a1209e4624..61c54b4222 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -120,7 +120,6 @@ task default: :test def test_dummy_clean inside dummy_path do remove_file "db/seeds.rb" - remove_file "doc" remove_file "Gemfile" remove_file "lib/tasks" remove_file "public/robots.txt" diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index fe5d302f3c..44c4688aa4 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -523,6 +523,8 @@ class AppGeneratorTest < Rails::Generators::TestCase run_generator if defined?(JRUBY_VERSION) assert_gem "therubyrhino" + elsif RUBY_PLATFORM =~ /mingw|mswin/ + assert_gem "duktape" else assert_file "Gemfile", /# gem 'mini_racer', platforms: :ruby/ end diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index e8372dea47..1fa40f74b9 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -491,7 +491,6 @@ class PluginGeneratorTest < Rails::Generators::TestCase assert_no_file "test/dummy/public/robots.txt" assert_no_file "test/dummy/README.md" assert_no_directory "test/dummy/lib/tasks" - assert_no_directory "test/dummy/doc" assert_no_directory "test/dummy/test" assert_no_directory "test/dummy/vendor" assert_no_directory "test/dummy/.git" |