diff --git a/CHANGELOG.md b/CHANGELOG.md index d2d9c2951..89766e950 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -115,6 +115,7 @@ * [#2905](https://github.com/ruby-grape/grape/pull/2905): Nest any `route_param` requirement under the param name, not just a Regexp, and reject the `requirements` shapes that have no param to attach to where they are written rather than on the first request - [@ericproulx](https://github.com/ericproulx). * [#2908](https://github.com/ruby-grape/grape/pull/2908): Stop reassigning method parameters across `lib`, so a parameter keeps the value its caller passed for the whole method; the `oneof` collection in `Grape::Validations::ParamsScope` no longer writes into the Hash it was given - [@ericproulx](https://github.com/ericproulx). * [#2910](https://github.com/ruby-grape/grape/pull/2910): Restore `Grape::Middleware::Formatter`'s in-place content-type negotiation as `ensure_content_type!`, which #2908 had turned into a copy of the response headers on every response - [@ericproulx](https://github.com/ericproulx). +* [#2911](https://github.com/ruby-grape/grape/pull/2911): Copy response headers with `merge!` instead of `merge` in `Grape::API::Instance#call` and `Grape::Middleware::Error`, which allocated a `Grape::Util::Header` only to discard it - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/lib/grape/api/instance.rb b/lib/grape/api/instance.rb index 8a3e2f1e7..2b43d5a8e 100644 --- a/lib/grape/api/instance.rb +++ b/lib/grape/api/instance.rb @@ -118,7 +118,12 @@ def initialize def call(env) status, headers, response = @router.call(env) unless @cascade - headers = Grape::Util::Header.new.merge(headers) + # +merge!+, not +merge+: the latter is a `dup` plus a `merge!`, so the + # Header built on this line would be allocated only to be discarded. + # The copy stays because +headers+ can come from a mounted Rack app, + # which is free to hand back a frozen or shared Hash that the delete + # below must not reach into. + headers = Grape::Util::Header.new.merge!(headers) headers.delete('X-Cascade') end diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 0b43730c0..a2dd7c48a 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -68,7 +68,7 @@ def call!(env) def rack_response(status, headers, message) body = html_content_type?(headers[Rack::CONTENT_TYPE]) ? Rack::Utils.escape_html(message) : message - Rack::Response.new(Array.wrap(body), Rack::Utils.status_code(status), Grape::Util::Header.new.merge(headers)) + Rack::Response.new(Array.wrap(body), Rack::Utils.status_code(status), Grape::Util::Header.new.merge!(headers)) end # Escaping must key off the media type only, case-insensitively. Comparing diff --git a/spec/grape/api/instance_spec.rb b/spec/grape/api/instance_spec.rb index 2af3df688..e7deff512 100644 --- a/spec/grape/api/instance_spec.rb +++ b/spec/grape/api/instance_spec.rb @@ -117,4 +117,29 @@ def app expect(an_instance.compile!.cascade?).to be(true) end end + + describe '#call' do + context 'when cascade is false' do + let(:rack_headers) { { 'content-type' => 'text/plain', 'x-cascade' => 'pass' }.freeze } + let(:root_api) do + headers = rack_headers + Class.new(Grape::API::Instance) do + cascade false + mount ->(_env) { [200, headers, ['from rack']] } => '/rack' + end + end + + it 'strips X-Cascade from the response' do + get '/rack' + expect(last_response.headers).not_to have_key('x-cascade') + end + + # The mounted app owns the Hash it returned and is free to hand back a + # frozen or shared one, so removing X-Cascade has to happen on a copy. + it 'does not write into the headers the mounted app returned' do + get '/rack' + expect(rack_headers).to eq('content-type' => 'text/plain', 'x-cascade' => 'pass') + end + end + end end