From 7d1c7e1688f94cf967fe11a69e958d798b8725fb Mon Sep 17 00:00:00 2001
From: Tim Perkins <tperkins@salsify.com>
Date: Wed, 23 Mar 2016 10:08:31 -0400
Subject: [PATCH] Do not modify a Hash argument to error!

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.
---
 CHANGELOG.md                      |  1 +
 lib/grape/error_formatter/base.rb | 12 ++++++++----
 spec/grape/api_spec.rb            |  2 +-
 spec/grape/endpoint_spec.rb       | 10 ++++++++++
 4 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f2827c24bc..ad0ceba6a8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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)
 =================
diff --git a/lib/grape/error_formatter/base.rb b/lib/grape/error_formatter/base.rb
index b3cecbf99f..9879849983 100644
--- a/lib/grape/error_formatter/base.rb
+++ b/lib/grape/error_formatter/base.rb
@@ -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
@@ -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
diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb
index d57a41c0e3..1d3e32a70a 100644
--- a/spec/grape/api_spec.rb
+++ b/spec/grape/api_spec.rb
@@ -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
diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb
index 00273796f8..139200e378 100644
--- a/spec/grape/endpoint_spec.rb
+++ b/spec/grape/endpoint_spec.rb
@@ -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')