Skip to content

Commit

Permalink
Avoid coercing parameter with multiple types to an empty Array
Browse files Browse the repository at this point in the history
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.

Closes #2018
  • Loading branch information
stanhu committed Mar 17, 2020
1 parent 8deebb5 commit 10b15fe
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#### Features
* Your contribution here.
* [#2018](https://github.com/ruby-grape/grape/pull/2018): Avoid coercing parameter with multiple types to an empty Array - [@stanhu](https://github.com/stanhu).
* [#2015](https://github.com/ruby-grape/grape/pull/2014): Reduce MatchData allocation - [@ericproulx](https://github.com/ericproulx).
* [#2014](https://github.com/ruby-grape/grape/pull/2014): Reduce total allocated arrays - [@ericproulx](https://github.com/ericproulx).
* [#2011](https://github.com/ruby-grape/grape/pull/2011): Reduce total retained regexes - [@ericproulx](https://github.com/ericproulx).
Expand Down
3 changes: 2 additions & 1 deletion lib/grape/validations/validators/coerce.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,10 @@ 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 [] if type == Array
return Set.new if type == Set
return {} if type == Hash
return val if type.is_a?(Array)
end

converter.call(val)
Expand Down
13 changes: 13 additions & 0 deletions spec/grape/validations/validators/coerce_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,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]
Expand Down

0 comments on commit 10b15fe

Please sign in to comment.