Skip to content

Commit

Permalink
Do not modify a Hash argument to error!
Browse files Browse the repository at this point in the history
This change allows an immutable hash, for example a frozen constant, to be passed as the message to `error!`.

If the message is a Hash, then it is always duplicated, and any `:with` key is removed from the copy.
  • Loading branch information
Tim Perkins committed Mar 25, 2016
1 parent dc77375 commit 7d1c7e1
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* [#1325](https://github.com/ruby-grape/grape/pull/1325): Params: Fix coerce_with helper with Array types - [@ngonzalez](https://github.com/ngonzalez).
* [#1326](https://github.com/ruby-grape/grape/pull/1326): Fix wrong behavior for OPTIONS and HEAD requests with catch-all - [@ekampp](https://github.com/ekampp), [@namusyaka](https://github.com/namusyaka).
* [#1330](https://github.com/ruby-grape/grape/pull/1330): Add `register` keyword for adding customized parsers and formatters - [@namusyaka](https://github.com/namusyaka).
* [#1336](https://github.com/ruby-grape/grape/pull/1336): Do not modify Hash argument to `error!` - [@tjwp](https://github.com/tjwp).

0.15.0 (3/8/2016)
=================
Expand Down
12 changes: 8 additions & 4 deletions lib/grape/error_formatter/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@ module ErrorFormatter
module Base
def present(message, env)
present_options = {}
present_options[:with] = message.delete(:with) if message.is_a?(Hash)
presented_message = message
if presented_message.is_a?(Hash)
presented_message = presented_message.dup
present_options[:with] = presented_message.delete(:with)
end

presenter = env[Grape::Env::API_ENDPOINT].entity_class_for_obj(message, present_options)
presenter = env[Grape::Env::API_ENDPOINT].entity_class_for_obj(presented_message, present_options)

unless presenter || env[Grape::Env::GRAPE_ROUTING_ARGS].nil?
# env['api.endpoint'].route does not work when the error occurs within a middleware
Expand All @@ -21,10 +25,10 @@ def present(message, env)
if presenter
embeds = { env: env }
embeds[:version] = env[Grape::Env::API_VERSION] if env[Grape::Env::API_VERSION]
message = presenter.represent(message, embeds).serializable_hash
presented_message = presenter.represent(presented_message, embeds).serializable_hash
end

message
presented_message
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2046,7 +2046,7 @@ def static
end

it 'presented with' do
error = { code: 408, with: error_presenter }
error = { code: 408, with: error_presenter }.freeze
subject.get '/exception' do
error! error, 408
end
Expand Down
10 changes: 10 additions & 0 deletions spec/grape/endpoint_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,16 @@ def app
expect(last_response.body).to eq('{"dude":"rad"}')
end

it 'accepts a frozen object' do
subject.get '/hey' do
error!({ 'dude' => 'rad' }.freeze, 403)
end

get '/hey.json'
expect(last_response.status).to eq(403)
expect(last_response.body).to eq('{"dude":"rad"}')
end

it 'can specifiy headers' do
subject.get '/hey' do
error!({ 'dude' => 'rad' }, 403, 'X-Custom' => 'value')
Expand Down

0 comments on commit 7d1c7e1

Please sign in to comment.