From 134255931f5568d7ac546991f34453ff56b2479c Mon Sep 17 00:00:00 2001
From: Dmitriy Nesteryuk <dmitriy.nesteryuk@gmail.com>
Date: Sun, 22 Dec 2019 13:01:53 +0200
Subject: [PATCH] careful check for empty params

Integers don't respond to the `empty?` method. It caused issue with optional
params as described in #1847.
---
 CHANGELOG.md                                  |  1 +
 lib/grape/validations/attributes_iterator.rb  |  4 +---
 .../validations/single_attribute_iterator.rb  | 13 ++++++++++--
 lib/grape/validations/validators/base.rb      | 10 +++++-----
 .../single_attribute_iterator_spec.rb         | 20 +++++++++++++++----
 5 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index dc21ebf1b1..22b66f3f15 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@
 
 #### Fixes
 
+* [#1947](https://github.com/ruby-grape/grape/pull/1947): Careful check for empty params - [@dnesteryuk](https://github.com/dnesteryuk).
 * [#1931](https://github.com/ruby-grape/grape/pull/1946): Fixes issue when using namespaces in `Grape::API::Instance` mounted directly - [@myxoh](https://github.com/myxoh).
 
 ### 1.2.5 (2019/12/01)
diff --git a/lib/grape/validations/attributes_iterator.rb b/lib/grape/validations/attributes_iterator.rb
index 40dc91edfd..6c53d469a0 100644
--- a/lib/grape/validations/attributes_iterator.rb
+++ b/lib/grape/validations/attributes_iterator.rb
@@ -31,9 +31,7 @@ def do_each(params_to_process, parent_indicies = [], &block)
 
           if @scope.type == Array
             next unless @original_params.is_a?(Array) # do not validate content of array if it isn't array
-            inside_array = true
-          end
-          if inside_array
+
             # fill current and parent scopes with correct array indicies
             parent_scope = @scope.parent
             parent_indicies.each do |parent_index|
diff --git a/lib/grape/validations/single_attribute_iterator.rb b/lib/grape/validations/single_attribute_iterator.rb
index a76cfa8360..f281598965 100644
--- a/lib/grape/validations/single_attribute_iterator.rb
+++ b/lib/grape/validations/single_attribute_iterator.rb
@@ -5,11 +5,20 @@ module Validations
     class SingleAttributeIterator < AttributesIterator
       private
 
-      def yield_attributes(resource_params, attrs)
+      def yield_attributes(val, attrs)
         attrs.each do |attr_name|
-          yield resource_params, attr_name
+          yield val, attr_name, empty?(val)
         end
       end
+
+      # Primitives like Integers and Booleans don't respond to +empty?+.
+      # It could be possible to use +blank?+ instead, but
+      #
+      #     false.blank?
+      #     => true
+      def empty?(val)
+        val.respond_to?(:empty?) ? val.empty? : val.nil?
+      end
     end
   end
 end
diff --git a/lib/grape/validations/validators/base.rb b/lib/grape/validations/validators/base.rb
index 22997e8bc6..1d7bb90e8d 100644
--- a/lib/grape/validations/validators/base.rb
+++ b/lib/grape/validations/validators/base.rb
@@ -42,12 +42,12 @@ def validate!(params)
         # there may be more than one error per field
         array_errors = []
 
-        attributes.each do |resource_params, attr_name|
-          next if !@scope.required? && resource_params.empty?
-          next unless @scope.meets_dependency?(resource_params, params)
+        attributes.each do |val, attr_name, empty_val|
+          next if !@scope.required? && empty_val
+          next unless @scope.meets_dependency?(val, params)
           begin
-            if @required || resource_params.respond_to?(:key?) && resource_params.key?(attr_name)
-              validate_param!(attr_name, resource_params)
+            if @required || val.respond_to?(:key?) && val.key?(attr_name)
+              validate_param!(attr_name, val)
             end
           rescue Grape::Exceptions::Validation => e
             array_errors << e
diff --git a/spec/grape/validations/single_attribute_iterator_spec.rb b/spec/grape/validations/single_attribute_iterator_spec.rb
index abce54d86b..2b3edbf064 100644
--- a/spec/grape/validations/single_attribute_iterator_spec.rb
+++ b/spec/grape/validations/single_attribute_iterator_spec.rb
@@ -6,7 +6,7 @@
   describe '#each' do
     subject(:iterator) { described_class.new(validator, scope, params) }
     let(:scope) { Grape::Validations::ParamsScope.new(api: Class.new(Grape::API)) }
-    let(:validator) { double(attrs: %i[first second third]) }
+    let(:validator) { double(attrs: %i[first second]) }
 
     context 'when params is a hash' do
       let(:params) do
@@ -15,7 +15,7 @@
 
       it 'yields params and every single attribute from the list' do
         expect { |b| iterator.each(&b) }
-          .to yield_successive_args([params, :first], [params, :second], [params, :third])
+          .to yield_successive_args([params, :first, false], [params, :second, false])
       end
     end
 
@@ -26,10 +26,22 @@
 
       it 'yields every single attribute from the list for each of the array elements' do
         expect { |b| iterator.each(&b) }.to yield_successive_args(
-          [params[0], :first], [params[0], :second], [params[0], :third],
-          [params[1], :first], [params[1], :second], [params[1], :third]
+          [params[0], :first, false], [params[0], :second, false],
+          [params[1], :first, false], [params[1], :second, false]
         )
       end
+
+      context 'empty values' do
+        let(:params) { [{}, '', 10] }
+
+        it 'marks params with empty values' do
+          expect { |b| iterator.each(&b) }.to yield_successive_args(
+            [params[0], :first, true], [params[0], :second, true],
+            [params[1], :first, true], [params[1], :second, true],
+            [params[2], :first, false], [params[2], :second, false]
+          )
+        end
+      end
     end
   end
 end