diff --git a/CHANGELOG.md b/CHANGELOG.md index fdc7d3a3e..5d84bbbd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * [#2918](https://github.com/ruby-grape/grape/pull/2918): Skip the dry-types round trip when a value already is the declared type - [@ericproulx](https://github.com/ericproulx). * [#2917](https://github.com/ruby-grape/grape/pull/2917): Read path captures out of the router's union match instead of re-running the route's pattern - [@ericproulx](https://github.com/ericproulx). * [#2921](https://github.com/ruby-grape/grape/pull/2921): Pin the router's request-time isolation regressions through requests instead of its instance variables - [@ericproulx](https://github.com/ericproulx). +* [#2929](https://github.com/ruby-grape/grape/pull/2929): Skip a header copy, a message re-encode and a Proc conversion on every error response - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 4.0.0 (2026-09-07) diff --git a/lib/grape/error_formatter/json.rb b/lib/grape/error_formatter/json.rb index fe34a904b..5b3ef469a 100644 --- a/lib/grape/error_formatter/json.rb +++ b/lib/grape/error_formatter/json.rb @@ -23,8 +23,11 @@ def wrap_message(message) { error: ensure_utf8(message) } end + # Re-encoding a String that already is valid UTF-8 only copies it, for + # about 260 ns on every error response. def ensure_utf8(message) return message unless message.respond_to? :encode + return message if message.is_a?(String) && message.encoding == Encoding::UTF_8 && message.valid_encoding? message.encode('UTF-8', invalid: :replace, undef: :replace) end diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index a2dd7c48a..09774a40a 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -66,9 +66,13 @@ def call!(env) private + # +headers+ is handed over as it is: Rack::Response copies it into a + # Headers of its own on every supported Rack (Rack 2.2 through + # +HeaderHash[]+, which only adopts a Hash that already is one), and every + # caller builds it fresh for this response. 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), headers) end # Escaping must key off the media type only, case-insensitively. Comparing @@ -268,7 +272,7 @@ def rescue_handler_for_any_class(klass) def run_rescue_handler(handler, error, endpoint, redispatched: false) callable = handler.is_a?(Symbol) ? endpoint.public_method(handler) : handler response = catch(:error) do - callable.arity.zero? ? endpoint.instance_exec(&callable) : endpoint.instance_exec(error, &callable) + call_rescue_handler(callable, error, endpoint) rescue StandardError => e return redispatch(e, endpoint, redispatched) end @@ -279,6 +283,18 @@ def run_rescue_handler(handler, error, endpoint, redispatched: false) run_rescue_handler(method(:default_rescue_handler), Grape::Exceptions::InvalidResponse.new, endpoint) end + # A +rescue_from+ block runs as the endpoint. A Method (the middleware's + # own handlers, or the endpoint's for a +with:+ Symbol) is already bound + # to a receiver that instance_exec cannot change, so it is called as it + # is rather than turned into a Proc first. + def call_rescue_handler(callable, error, endpoint) + if callable.is_a?(Method) + callable.arity.zero? ? callable.call : callable.call(error) + else + callable.arity.zero? ? endpoint.instance_exec(&callable) : endpoint.instance_exec(error, &callable) + end + end + # Route an exception raised inside a +rescue_from+ block. # # * If we have already redispatched once (the redispatched handler diff --git a/spec/grape/error_formatter/json_spec.rb b/spec/grape/error_formatter/json_spec.rb new file mode 100644 index 000000000..2b9676a4a --- /dev/null +++ b/spec/grape/error_formatter/json_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +describe Grape::ErrorFormatter::Json do + let(:app) do + Class.new(Grape::API) do + format :json + get('/utf8') { error!('café', 400) } + get('/binary') { error!("caf\xC3\xA9".b, 400) } + get('/malformed') { error!("caf\xC3", 400) } + end + end + + # A String message goes out as UTF-8 whatever it arrived as: one that already + # is valid UTF-8 as it is, anything else converted, with what does not + # convert replaced rather than failing the response. + it 'renders a UTF-8 message as it is' do + get '/utf8' + expect(JSON.parse(last_response.body)).to eq('error' => 'café') + end + + it 'converts a binary message, replacing the bytes it cannot map' do + get '/binary' + expect(JSON.parse(last_response.body)).to eq('error' => 'caf��') + end + + it 'replaces the invalid bytes of a malformed UTF-8 message' do + get '/malformed' + expect(JSON.parse(last_response.body)).to eq('error' => 'caf�') + end +end