From a8b29428012a6c05eb446359930e1e16b07e3d36 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Thu, 10 Sep 2026 21:09:11 +0200 Subject: [PATCH] Answer the header versioner's common Accept headers from a table built once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header versioner ran Rack's q-value match (`Rack::Utils.best_q_match`, which splits both strings inside `Rack::Mime.match?` for every candidate) and a media-type parse on every request: about 3.6 µs of a 12.4 µs request. Its answer depends only on the Accept header and the middleware's `available_media_types`, which is fixed once the middleware is built. `MediaTypeForAcceptCache` works that answer out once for every declared media type, for `*/*` (what curl, Net::HTTP and most HTTP libraries send) and for no Accept header at all, and the versioner looks the header up there. Any other header, such as a q-value list or another casing, misses and takes the full path unchanged. Only declared media types are keys, so client input cannot grow a table, and each entry is computed by the real function, so a hit answers exactly what the full path would. Every endpoint builds its own versioner, so the table is shared per list through `Grape::Util::Cache`, as `ContentTypes::MimeTypesCache` already is. Built per instance it cost 3.5 MB and 120 ms of compile time on a 500-endpoint API; shared, 16 KB and 1 ms. Handing one parsed media type to many requests means the strings it gives out must not be alterable in place, so `Grape::Util::MediaType` is now immutable, strings included, on the full path too so both paths agree. `#initialize` copies its arguments rather than freezing the caller's. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + UPGRADING.md | 14 +++++++ lib/grape/middleware/versioner/header.rb | 38 ++++++++++++++++++- lib/grape/util/media_type.rb | 17 ++++++--- .../grape/middleware/versioner/header_spec.rb | 11 ++++++ 5 files changed, 74 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fdc7d3a3e..d5c52bd9e 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). +* [#2922](https://github.com/ruby-grape/grape/pull/2922): Answer the header versioner's common Accept headers from a table built once instead of negotiating them per request - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 4.0.0 (2026-09-07) diff --git a/UPGRADING.md b/UPGRADING.md index 965065fc5..2ceb2102f 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,6 +1,20 @@ Upgrading Grape =============== +### Upgrading to >= 4.1.0 + +#### The header versioner's `api.*` env values are frozen + +`version ..., using: :header` now answers the Accept headers most requests send from a table built once, so every request sending the same header is handed the same parsed media type ([#2922](https://github.com/ruby-grape/grape/pull/2922)). The strings it writes into the env — `api.type`, `api.subtype`, `api.vendor`, `api.version` and `api.format` — are therefore frozen, as is `Grape::Util::MediaType` itself. Code that altered one of them in place now raises `FrozenError`; build a new String instead: + +```ruby +# Before +env['api.version'] << '-beta' + +# After +env['api.version'] = "#{env['api.version']}-beta" +``` + ### Upgrading to >= 4.0.0 #### A positional options Hash is no longer accepted by `auth`, `http_basic` or `desc` diff --git a/lib/grape/middleware/versioner/header.rb b/lib/grape/middleware/versioner/header.rb index 26d83050c..2fd9f70ab 100644 --- a/lib/grape/middleware/versioner/header.rb +++ b/lib/grape/middleware/versioner/header.rb @@ -22,6 +22,42 @@ module Versioner # X-Cascade header to alert Grape::Router to attempt the next matched # route. class Header < Base + # Accept headers answered up front besides the declared media types: + # +*/*+, what curl, Net::HTTP and most HTTP libraries send unless told + # otherwise, and no Accept header at all. + COMMON_ACCEPT_HEADERS = ['*/*', nil].freeze + + # +MediaType.best_quality+ answers from the header and + # +available_media_types+ alone, and the latter is fixed once the + # middleware is built. Most requests send one of a handful of headers -- + # a declared media type spelled as declared (+application/vnd.acme-v1+json+) + # or one of COMMON_ACCEPT_HEADERS -- so their answers are worked out + # once instead of re-running Rack's q-value match and a parse on every + # request. Any other header (a q-value list, another casing) is not a + # key and takes the full path, so a client cannot grow a table. + # + # Shared per list: every endpoint builds its own versioner, and all of + # an API's declare the same media types. The key is a frozen copy, as + # the middleware's own list stays reachable through + # +available_media_types+. + class MediaTypeForAcceptCache < Grape::Util::Cache + def initialize + super + @cache = Hash.new do |h, available_media_types| + declared = available_media_types.map(&:-@).freeze + h[declared] = [*declared, *COMMON_ACCEPT_HEADERS].each_with_object({}) do |accept, media_types| + media_type = Grape::Util::MediaType.best_quality(accept, declared) + media_types[accept] = media_type if media_type + end.freeze + end + end + end + + def initialize(app, **options) + super + @media_type_for_accept = MediaTypeForAcceptCache[available_media_types] + end + def before match_best_quality_media_type! do |media_type| env.update( @@ -40,7 +76,7 @@ def match_best_quality_media_type! return unless vendor strict_header_checks! - media_type = Grape::Util::MediaType.best_quality(accept_header, available_media_types) + media_type = @media_type_for_accept[accept_header] || Grape::Util::MediaType.best_quality(accept_header, available_media_types) return yield media_type if media_type fail! diff --git a/lib/grape/util/media_type.rb b/lib/grape/util/media_type.rb index e4f041f19..05331f834 100644 --- a/lib/grape/util/media_type.rb +++ b/lib/grape/util/media_type.rb @@ -13,14 +13,19 @@ class MediaType # in the case they will be compared in. VENDOR_VERSION_HEADER_REGEX = /\Avnd\.(?[a-z0-9.\-_!^]+?)(?:-(?[a-z0-9*.]+))?(?:\+(?[a-z0-9*\-.]+))?\z/ + # Immutable, strings included: the header versioner shares one instance + # per declared media type across every request that sends it, and these + # strings are what it writes into the env. The arguments are copied + # rather than frozen, as they are the caller's. def initialize(type:, subtype:) - @type = type - @subtype = subtype - VENDOR_VERSION_HEADER_REGEX.match(subtype) do |m| - @vendor = m[:vendor] - @version = m[:version] - @format = m[:format] + @type = -type + @subtype = -subtype + VENDOR_VERSION_HEADER_REGEX.match(@subtype) do |m| + @vendor = m[:vendor].freeze + @version = m[:version].freeze + @format = m[:format].freeze end + freeze end def ==(other) diff --git a/spec/grape/middleware/versioner/header_spec.rb b/spec/grape/middleware/versioner/header_spec.rb index 3e46f1133..62d38d8e3 100644 --- a/spec/grape/middleware/versioner/header_spec.rb +++ b/spec/grape/middleware/versioner/header_spec.rb @@ -145,6 +145,17 @@ expect(exception.message).to include('API version not found') end end + + # Requests sending the same Accept header are all answered from one parsed + # media type, so what a request is handed must not be alterable in place. + it 'cannot be altered by one request for the next' do + keys = [Grape::Env::API_TYPE, Grape::Env::API_SUBTYPE, Grape::Env::API_VENDOR, Grape::Env::API_VERSION, Grape::Env::API_FORMAT] + _, _, env = subject.call('HTTP_ACCEPT' => 'application/vnd.vendor-v1+json') + keys.each { |key| expect { env[key] << '-altered' }.to raise_error(FrozenError) } + + _, _, env = subject.call('HTTP_ACCEPT' => 'application/vnd.vendor-v1+json') + expect(env.values_at(*keys)).to eq(%w[application vnd.vendor-v1+json vendor v1 json]) + end end it 'succeeds if :strict is not set' do