Merge branch 'master' into active-storage-import

actionpack/lib/action_dispatch/http/parameters.rb

@@ -57,7 +57,7 @@ def parameters
                    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 @@ def parameters
       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 @@ def path_parameters
 
       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 @@ def set_binary_encoding(params)
           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

actionpack/lib/action_dispatch/http/request.rb

@@ -76,10 +76,13 @@ def self.binary_params_for?(action); false; end
 
     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

actionpack/lib/action_dispatch/journey/router.rb

@@ -43,6 +43,10 @@ def serve(req)
             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 @@ def recognize(rails_req)
             rails_req.path_info   = match.post_match.sub(/^([^\/])/, '/\1')
           end
 
+          parameters = route.defaults.merge parameters
           yield(route, parameters)
         end
       end
@@ -119,7 +124,7 @@ def find_routes(req)
 
           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
             }

actionpack/test/controller/routing_test.rb

@@ -96,6 +96,22 @@ def test_symbols_with_dashes
     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|

actionpack/test/dispatch/routing_test.rb

@@ -4416,39 +4416,49 @@ def app; APP 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

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 ClassMethods
       #
       # 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

activerecord/CHANGELOG.md

-*   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*
 

activerecord/test/cases/comment_test.rb

@@ -119,8 +119,6 @@ def test_schema_dump_with_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

activesupport/lib/active_support/testing/assertions.rb

@@ -157,7 +157,7 @@ def assert_changes(expression, message = nil, from: UNTRACKED, to: UNTRACKED, &b
         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

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.
 

railties/lib/rails/generators/actions.rb

@@ -216,9 +216,9 @@ def rake(command, options = {})
 
       # 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

railties/lib/rails/generators/app_base.rb

@@ -348,6 +348,8 @@ def javascript_runtime_gemfile_entry
         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

railties/lib/rails/generators/rails/app/templates/bin/yarn

 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"

railties/lib/rails/generators/rails/app/templates/config/spring.rb

-%w(
+%w[
   .ruby-version
   .rbenv-vars
   tmp/restart.txt
   tmp/caching-dev.txt
-).each { |path| Spring.watch(path) }
+].each { |path| Spring.watch(path) }

railties/lib/rails/generators/rails/plugin/plugin_generator.rb

@@ -120,7 +120,6 @@ def test_dummy_assets
     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"

railties/test/generators/app_generator_test.rb

@@ -523,6 +523,8 @@ def test_inclusion_of_javascript_runtime
     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

railties/test/generators/plugin_generator_test.rb

@@ -491,7 +491,6 @@ def test_unnecessary_files_are_not_generated_in_dummy_application
     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"