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