From 633d7cf529d5aed8a27e2e59364de1e8ca0737d0 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Mon, 16 Mar 2020 22:20:09 -0700
Subject: [PATCH] Avoid coercing parameter with multiple types to an empty
 Array

If an optional parameter has multiple types (e.g. [File, String]) and a
nil parameter is passed, Grape would previously coerce the value to an
empty Array. This can break certain APIs that treat nil values
differently from an empty Array.

To fix this, we refactor the nil handling into a `coerce_nil` method and
remove the special case check for handling an Array of types.

Closes #2018
---
 CHANGELOG.md                                  |  1 +
 lib/grape/validations/validators/coerce.rb    | 20 ++++++-------
 .../validations/validators/coerce_spec.rb     | 30 +++++++++++++++++++
 3 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1203fafc36..26d901d23a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@
 #### Fixes
 
 * Your contribution here.
+* [#2019](https://github.com/ruby-grape/grape/pull/2018): Avoid coercing parameter with multiple types to an empty Array - [@stanhu](https://github.com/stanhu).
 
 ### 1.3.1 (2020/03/11)
 
diff --git a/lib/grape/validations/validators/coerce.rb b/lib/grape/validations/validators/coerce.rb
index 561ec1bcf2..99317632f0 100644
--- a/lib/grape/validations/validators/coerce.rb
+++ b/lib/grape/validations/validators/coerce.rb
@@ -65,21 +65,21 @@ def valid_type?(val)
       end
 
       def coerce_value(val)
-        # define default values for structures, the dry-types lib which is used
-        # for coercion doesn't accept nil as a value, so it would fail
-        if val.nil?
-          return []      if type == Array || type.is_a?(Array)
-          return Set.new if type == Set
-          return {}      if type == Hash
-        end
-
-        converter.call(val)
-
+        val.nil? ? coerce_nil(val) : converter.call(val)
       # Some custom types might fail, so it should be treated as an invalid value
       rescue StandardError
         Types::InvalidValue.new
       end
 
+      def coerce_nil(val)
+        # define default values for structures, the dry-types lib which is used
+        # for coercion doesn't accept nil as a value, so it would fail
+        return []      if type == Array
+        return Set.new if type == Set
+        return {}      if type == Hash
+        val
+      end
+
       # Type to which the parameter will be coerced.
       #
       # @return [Class]
diff --git a/spec/grape/validations/validators/coerce_spec.rb b/spec/grape/validations/validators/coerce_spec.rb
index dbf57c9d57..2cfaceea83 100644
--- a/spec/grape/validations/validators/coerce_spec.rb
+++ b/spec/grape/validations/validators/coerce_spec.rb
@@ -180,6 +180,23 @@ def self.parsed?(value)
         expect(last_response.body).to eq(integer_class_name)
       end
 
+      it 'String' do
+        subject.params do
+          requires :string, coerce: String
+        end
+        subject.get '/string' do
+          params[:string].class
+        end
+
+        get '/string', string: 45
+        expect(last_response.status).to eq(200)
+        expect(last_response.body).to eq('String')
+
+        get '/string', string: nil
+        expect(last_response.status).to eq(200)
+        expect(last_response.body).to eq('NilClass')
+      end
+
       it 'is a custom type' do
         subject.params do
           requires :uri, coerce: SecureURIOnly
@@ -647,6 +664,19 @@ def self.parsed?(value)
         expect(last_response.body).to eq('String')
       end
 
+      it 'respects nil values' do
+        subject.params do
+          optional :a, types: [File, String]
+        end
+        subject.get '/' do
+          params[:a].class.to_s
+        end
+
+        get '/', a: nil
+        expect(last_response.status).to eq(200)
+        expect(last_response.body).to eq('NilClass')
+      end
+
       it 'fails when no coercion is possible' do
         subject.params do
           requires :a, types: [Boolean, Integer]