From 303cb295932065fdd73061ce7b27c3f253db46a8 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sun, 6 Sep 2026 13:29:26 +0200 Subject: [PATCH] Keep content-type negotiation in place as ensure_content_type! #2908 stopped `Grape::Middleware::Formatter#ensure_content_type` from writing into the headers Hash it was handed, returning `headers.merge(Rack::CONTENT_TYPE => ...)` instead. That copy runs on almost every response, and the inline pair made it cost three allocations rather than one: Ruby builds the one-pair Hash literal, then copies it again converting the implicit keyword Hash into a positional argument for `Hash#merge`, and `Rack::Headers#merge` dups the receiver on top of that. Measured over 2000 requests through a formatted endpoint: merge 19906.25 kB / 196,000 objects in place 18968.75 kB / 190,000 objects Three objects per response on the hot path, for a header the caller is about to send anyway. The negotiation goes back to writing into `headers`, and the method carries a `!` so the mutation is visible at the call site. `build_formatted_response` drops the `typed_headers` local it only needed in order to hold the copy, so `headers` is still never reassigned, which is what #2908 was after. The trade is that a mounted plain Rack app returning a frozen or shared headers Hash is written into again, as it was before #2908. Co-authored-by: Claude Opus 5 --- CHANGELOG.md | 3 ++- lib/grape/middleware/formatter.rb | 29 +++++++++++++++-------------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a95ec6a66..d2d9c2951 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -113,7 +113,8 @@ * [#2902](https://github.com/ruby-grape/grape/pull/2902): Make `Grape::ErrorFormatter.formatter_for` a lookup that answers `nil` for an unregistered format, like `Grape::Parser.parser_for`, and move the `default_error_formatter` / `Grape::ErrorFormatter::Txt` fallback to `Grape::Middleware::Error`, which owns it; `default_error_formatter` naming nothing registered now raises `Grape::Exceptions::UnknownErrorFormatter` instead of silently storing `Txt` (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). * [#2904](https://github.com/ruby-grape/grape/pull/2904): Document and spec route `requirements` given as a Mustermann capture type, which constrains the match and hands the endpoint the converted value - [@ericproulx](https://github.com/ericproulx). * [#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; `Grape::Middleware::Formatter#ensure_content_type` and the `oneof` collection in `Grape::Validations::ParamsScope` no longer write into the Hash they were given - [@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). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index 64c985a8e..775c026fd 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -56,25 +56,25 @@ def after private def build_formatted_response(status, headers, bodies) - typed_headers = ensure_content_type(headers) + ensure_content_type!(headers) if bodies.is_a?(Grape::ServeStream::StreamResponse) - Grape::ServeStream::SendfileResponse.new([], status, typed_headers) do |resp| + Grape::ServeStream::SendfileResponse.new([], status, headers) do |resp| resp.body = bodies.stream end else # Allow content-type to be explicitly overwritten - formatter = fetch_formatter(typed_headers) + formatter = fetch_formatter(headers) bodymap = instrument_format_response(formatter) do bodies.map { |body| formatter.call(body, env) } end - # A bare Rack tuple rather than a Rack::Response: +typed_headers+ is already + # A bare Rack tuple rather than a Rack::Response: +headers+ is already # a Grape::Util::Header (a Rack::Headers on Rack 3), so wrapping only # re-normalizes the same keys into a second Headers hash that # +Middleware::Base#call+ unwraps again with +to_a+ on the way out. # The 204/304 bodies Rack::Response#finish would blank are returned # above, before this point. - [status, typed_headers, bodymap] + [status, headers, bodymap] end rescue Grape::Exceptions::InvalidFormatter => e throw :error, Grape::Exceptions::ErrorResponse.new(status: 500, message: e.message, backtrace: e.backtrace, original_exception: e) @@ -96,15 +96,16 @@ def fetch_formatter(headers) # Set the content type header for the API format if it is not already present. # - # @param headers [Hash] - # @return [Hash] - def ensure_content_type(headers) - return headers if headers[Rack::CONTENT_TYPE] - - # Merged rather than written in place: +headers+ belongs to the response - # the app returned, and negotiating a content type for it is not a reason - # to reach back into it. - headers.merge(Rack::CONTENT_TYPE => content_type_for(env[Grape::Env::API_FORMAT])) + # Written into +headers+ rather than returned as a copy, hence the +!+: + # this runs on every response, and copying the hash costs an allocation + # per request for a header the caller is about to send anyway. + # + # @param headers [Hash] the response headers, mutated in place + # @return [void] + def ensure_content_type!(headers) + return if headers[Rack::CONTENT_TYPE] + + headers[Rack::CONTENT_TYPE] = content_type_for(env[Grape::Env::API_FORMAT]) end def read_body_input