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.

README.md

@@ -14,8 +14,9 @@ class PeopleController < ActionController::Base
   end
 
   # This will pass with flying colors as long as there's a person key in the parameters, otherwise
-  # it'll raise a ActionController::MissingParameter exception, which will get caught by
-  # ActionController::Base and turned into that 400 Bad Request reply.
+  # it'll raise an ActionController::ParameterMissing exception if the key is not present or
+  # it'll raise an ActionController:EmptyParameter exception if the key value is blank,
+  # which will get caught by ActionController::Base and turned into that 400 Bad Request reply.
   def update
     person = current_account.people.find(params[:id])
     person.update_attributes!(person_params)

lib/action_controller/parameters.rb

@@ -17,6 +17,15 @@ def initialize(param)
     end
   end
 
+  class EmptyParameter < IndexError
+    attr_reader :param
+
+    def initialize(param)
+      @param = param
+      super("value is empty for required key: #{param}")
+    end
+  end
+
   class UnpermittedParameters < IndexError
     attr_reader :params
 
@@ -29,7 +38,7 @@ def initialize(params)
   class Parameters < ActiveSupport::HashWithIndifferentAccess
     attr_accessor :permitted
     alias :permitted? :permitted
-    
+
     cattr_accessor :action_on_unpermitted_parameters, :instance_accessor => false
 
     # Never raise an UnpermittedParameters exception because of these params
@@ -52,7 +61,8 @@ def permit!
     end
 
     def require(key)
-      self[key].presence || raise(ActionController::ParameterMissing.new(key))
+      raise(ActionController::ParameterMissing.new(key)) unless self.key?(key)
+      self[key].presence || raise(ActionController::EmptyParameter.new(key))
     end
 
     alias :required :require
@@ -215,23 +225,23 @@ def fields_for_style?(object)
         object.is_a?(Hash) && object.all? { |k, v| k =~ /\A-?\d+\z/ && v.is_a?(Hash) }
       end
 
-      def unpermitted_parameters!(params)  
+      def unpermitted_parameters!(params)
         return unless self.class.action_on_unpermitted_parameters
-        
+
         unpermitted_keys = unpermitted_keys(params)
 
-        if unpermitted_keys.any?  
-          case self.class.action_on_unpermitted_parameters  
+        if unpermitted_keys.any?
+          case self.class.action_on_unpermitted_parameters
           when :log
             name = "unpermitted_parameters.action_controller"
             ActiveSupport::Notifications.instrument(name, :keys => unpermitted_keys)
-          when :raise  
-            raise ActionController::UnpermittedParameters.new(unpermitted_keys)  
-          end  
-        end  
-      end  
-  
-      def unpermitted_keys(params)  
+          when :raise
+            raise ActionController::UnpermittedParameters.new(unpermitted_keys)
+          end
+        end
+      end
+
+      def unpermitted_keys(params)
         self.keys - params.keys - NEVER_UNPERMITTED_PARAMS
       end
   end
@@ -243,6 +253,10 @@ module StrongParameters
       rescue_from(ActionController::ParameterMissing) do |parameter_missing_exception|
         render :text => "Required parameter missing: #{parameter_missing_exception.param}", :status => :bad_request
       end
+
+      rescue_from(ActionController::EmptyParameter) do |empty_parameter_exception|
+        render :text => "Required parameter value is empty: #{empty_parameter_exception.param}", :status => :bad_request
+      end
     end
 
     def params

test/action_controller_required_params_test.rb

@@ -5,6 +5,11 @@ def create
     params.require(:book).require(:name)
     head :ok
   end
+
+  def update
+    params.require(:book)
+    head :ok
+  end
 end
 
 class ActionControllerRequiredParamsTest < ActionController::TestCase
@@ -27,4 +32,20 @@ class ActionControllerRequiredParamsTest < ActionController::TestCase
     post :create, { :magazine => { :name => "Mjallo!" } }
     assert_equal "Required parameter missing: book", response.body
   end
+
+  test "empty required parameters will raise an exception" do
+    put :update, { :book => {} }
+    assert_response :bad_request
+
+    put :update, { :book => '' }
+    assert_response :bad_request
+
+    put :update, { :book => nil }
+    assert_response :bad_request
+  end
+
+  test "empty parameters will be mentioned in the return" do
+    put :update, { :book => {} }
+    assert_equal "Required parameter value is empty: book", response.body
+  end
 end

test/parameters_require_test.rb

@@ -2,9 +2,23 @@
 require 'action_controller/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
+
+    assert_raises(ActionController::EmptyParameter) do
+      ActionController::Parameters.new(:person => '').require(:person)
+    end
+
+    assert_raises(ActionController::EmptyParameter) do
+      ActionController::Parameters.new(:person => nil).require(:person)
+    end
   end
 end