Change ActionController::Parameters#require behavior when value is empty

When the value for the required key is empty an ActionController::ParameterMissing is raised which gets caught by ActionController::Base and turned into a 400 Bad Request reply with a message in the body saying the key is missing, which is misleading.

With these changes, ActionController::EmptyParameter will be raised which ActionController::Base will catch and turn into a 400 Bad Request reply with a message in the body saying the key value is empty.

3 files changed, 32 insertions, 1 deletions

diff --git a/actionpack/test/controller/parameters/parameters_require_test.rb b/actionpack/test/controller/parameters/parameters_require_test.rb
index bdaba8d2d8..21b3eaa6b5 100644
--- a/actionpack/test/controller/parameters/parameters_require_test.rb
+++ b/actionpack/test/controller/parameters/parameters_require_test.rb
@@ -2,8 +2,14 @@ require 'abstract_unit'
 require 'action_controller/metal/strong_parameters'
 
 class ParametersRequireTest < ActiveSupport::TestCase
-  test "required parameters must be present not merely not nil" do
+  test "required parameters must be present" do
     assert_raises(ActionController::ParameterMissing) do
+      ActionController::Parameters.new(name: {}).require(:person)
+    end
+  end
+
+  test "required parameters can't be blank" do
+    assert_raises(ActionController::EmptyParameter) do
       ActionController::Parameters.new(person: {}).require(:person)
     end
   end
diff --git a/actionpack/test/controller/required_params_test.rb b/actionpack/test/controller/required_params_test.rb
index 343d57c300..b27069140b 100644
--- a/actionpack/test/controller/required_params_test.rb
+++ b/actionpack/test/controller/required_params_test.rb
@@ -5,6 +5,11 @@ class BooksController < ActionController::Base
     params.require(:book).require(:name)
     head :ok
   end
+
+  def update
+    params.require(:book)
+    head :ok
+  end
 end
 
 class ActionControllerRequiredParamsTest < ActionController::TestCase
@@ -20,6 +25,20 @@ class ActionControllerRequiredParamsTest < ActionController::TestCase
     end
   end
 
+  test "empty required parameters will raise an exception" do
+    assert_raise ActionController::EmptyParameter do
+      put :update, {book: {}}
+    end
+
+    assert_raise ActionController::EmptyParameter do
+      put :update, {book: ''}
+    end
+
+    assert_raise ActionController::EmptyParameter do
+      put :update, {book: nil}
+    end
+  end
+
   test "required parameters that are present will not raise" do
     post :create, { book: { name: "Mjallo!" } }
     assert_response :ok
diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb
index 6035f0361e..9fafc3a0e9 100644
--- a/actionpack/test/dispatch/debug_exceptions_test.rb
+++ b/actionpack/test/dispatch/debug_exceptions_test.rb
@@ -41,6 +41,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
         raise ActionController::UrlGenerationError, "No route matches"
       when "/parameter_missing"
         raise ActionController::ParameterMissing, :missing_param_key
+      when "/required_key_empty_value"
+        raise ActionController::EmptyParameter, :empty_param_key
       else
         raise "puke!"
       end
@@ -120,6 +122,10 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
     get "/parameter_missing", {}, {'action_dispatch.show_exceptions' => true}
     assert_response 400
     assert_match(/ActionController::ParameterMissing/, body)
+
+    get "/required_key_empty_value", {}, {'action_dispatch.show_exceptions' => true}
+    assert_response 400
+    assert_match(/ActionController::EmptyParameter/, body)
   end
 
   test "does not show filtered parameters" do