Skip to content

Commit

Permalink
Try to dup non-frozen default params with each use.
Browse files Browse the repository at this point in the history
Closes #1438.
  • Loading branch information
Joe Faber authored and dblock committed Jul 20, 2016
1 parent 21925fc commit 8b72144
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#### Fixes

* [#1446](https://github.com/ruby-grape/grape/pull/1446): Fix for `env` inside `before` when using not allowed method - [@leifg](https://github.com/leifg).
* [#1438](https://github.com/ruby-grape/grape/pull/1439): Try to dup non-frozen default params with each use - [@jlfaber](https://github.com/jlfaber).
* [#1430](https://github.com/ruby-grape/grape/pull/1430): Fix for `declared(params)` inside `route_param` - [@Arkanain](https://github.com/Arkanain).
* [#1405](https://github.com/ruby-grape/grape/pull/1405): Fix priority of `rescue_from` clauses applying - [@hedgesky](https://github.com/hedgesky).
* [#1365](https://github.com/ruby-grape/grape/pull/1365): Fix finding exception handler in error middleware - [@ktimothy](https://github.com/ktimothy).
Expand Down
27 changes: 26 additions & 1 deletion lib/grape/validations/validators/default.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,14 @@ def initialize(attrs, options, required, scope)
end

def validate_param!(attr_name, params)
params[attr_name] = @default.is_a?(Proc) ? @default.call : @default unless params.key?(attr_name)
return if params.key? attr_name
params[attr_name] = if @default.is_a? Proc
@default.call
elsif @default.frozen? || !duplicatable?(@default)
@default
else
duplicate(@default)
end
end

def validate!(params)
Expand All @@ -20,6 +27,24 @@ def validate!(params)
end
end
end

private

# return true if we might be able to dup this object
def duplicatable?(obj)
!obj.nil? &&
obj != true &&
obj != false &&
!obj.is_a?(Symbol) &&
!obj.is_a?(Numeric)
end

# make a best effort to dup the object
def duplicate(obj)
obj.dup
rescue TypeError
obj
end
end
end
end
41 changes: 41 additions & 0 deletions spec/grape/api/parameters_modification_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
require 'spec_helper'

describe Grape::Endpoint do
subject { Class.new(Grape::API) }

def app
subject
end

before do
subject.namespace :test do
params do
optional :foo, default: '-abcdef'
end
get do
params[:foo].slice!(0)
params[:foo]
end
end
end

context 'when route modifies param value' do
it 'param default should not change' do
get '/test'
expect(last_response.status).to eq 200
expect(last_response.body).to eq 'abcdef'

get '/test'
expect(last_response.status).to eq 200
expect(last_response.body).to eq 'abcdef'

get '/test?foo=-123456'
expect(last_response.status).to eq 200
expect(last_response.body).to eq '123456'

get '/test'
expect(last_response.status).to eq 200
expect(last_response.body).to eq 'abcdef'
end
end
end

0 comments on commit 8b72144

Please sign in to comment.