Skip to content

Commit 6c79be7

Browse files
ericproulxclaude
andauthored
Match Accept media types case-insensitively (#2847)
Media types are case-insensitive (RFC 9110 section 8.3.1), but the registered ones are spelled in lower case and matched literally, so a differently-cased Accept header found nothing: Accept: TEXT/PLAIN -> served application/json Accept: APPLICATION/VND.TWITTER-V1+JSON -> api.version nil Neither failed loudly. Content negotiation fell through to the default format, and header versioning behaved as though no version had been asked for, so the request was served by whichever version matched first -- the client quietly got something other than what it asked for. Three sites decided this, all comparing against lower-case registered types: the formatter's Accept lookup, MediaType.best_quality_media_type, and the vendor pattern in MediaType.parse / .match?. Down-case the incoming media type at each. The vendor pattern stays lower-case, which is the case a vendor and version are declared in and therefore compared in. Grape already treats media types this way when deciding whether to escape an error body (Middleware::Error#html_content_type?, from #2789). Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 9b5f638 commit 6c79be7

5 files changed

Lines changed: 61 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
* [#2827](https://github.com/ruby-grape/grape/pull/2827): Make the `cascade` DSL getter return the configured value (`cascade false` read back as `true`) - [@ericproulx](https://github.com/ericproulx).
5757
* [#2829](https://github.com/ruby-grape/grape/pull/2829): Fix a cascading route handing over only to the last route registered for the path, making a middle version (3+ mounted versions with a catch-all) answer 406 - [@ericproulx](https://github.com/ericproulx).
5858
* [#2826](https://github.com/ruby-grape/grape/pull/2826): Fix `api.version` not being set for the root route of a path-versioned API (`GET /v1`) - [@ericproulx](https://github.com/ericproulx).
59+
* [#2847](https://github.com/ruby-grape/grape/pull/2847): Match `Accept` media types case-insensitively, so a differently-cased header still negotiates the content type and resolves a vendor version - [@ericproulx](https://github.com/ericproulx).
5960
* [#2834](https://github.com/ruby-grape/grape/pull/2834): Restore the #2824 fix for cascaded routes leaking `route_info` and path captures, silently reverted by #2829 - [@ericproulx](https://github.com/ericproulx).
6061
* [#2838](https://github.com/ruby-grape/grape/pull/2838): Reject request params nested in more arrays than the block declares, instead of silently unwrapping them and passing validation, and report `type: Array[JSON]` errors against the element that failed - [@ericproulx](https://github.com/ericproulx).
6162
* [#2842](https://github.com/ruby-grape/grape/pull/2842): Warn at definition time when a `rescue_from` class is already covered by one registered earlier in the same scope, since the later handler never runs - [@ericproulx](https://github.com/ericproulx).

lib/grape/middleware/formatter.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,15 @@ def format_from_extension
156156
extension if content_type_for(extension)
157157
end
158158

159+
# Media types are case-insensitive (RFC 9110 §8.3.1) but the registered
160+
# ones are spelled in lower case and Rack matches them literally, so an
161+
# `Accept: TEXT/PLAIN` found nothing and fell through to the default
162+
# format — the client quietly got something other than what it asked for.
159163
def format_from_header
160164
accept_header = try_scrub(env['HTTP_ACCEPT'])
161165
return if accept_header.blank? || accept_header == ALL_MEDIA_TYPES
162166

163-
media_type = Rack::Utils.best_q_match(accept_header, mime_types.keys)
167+
media_type = Rack::Utils.best_q_match(accept_header.downcase, mime_types.keys)
164168
mime_types[media_type] if media_type
165169
end
166170
end

lib/grape/util/media_type.rb

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ class MediaType
77

88
# based on the HTTP Accept header with the pattern:
99
# application/vnd.:vendor-:version+:format
10+
#
11+
# Matched against a down-cased media type: they are case-insensitive
12+
# (RFC 9110 §8.3.1), while a vendor and version are declared in the DSL
13+
# in the case they will be compared in.
1014
VENDOR_VERSION_HEADER_REGEX = /\Avnd\.(?<vendor>[a-z0-9.\-_!^]+?)(?:-(?<version>[a-z0-9*.]+))?(?:\+(?<format>[a-z0-9*\-.]+))?\z/
1115

1216
def initialize(type:, subtype:)
@@ -41,7 +45,7 @@ def best_quality(header, available_media_types)
4145
def parse(media_type)
4246
return if media_type.blank?
4347

44-
type, subtype = media_type.split('/', 2)
48+
type, subtype = media_type.downcase.split('/', 2)
4549
return if type.blank? || subtype.blank?
4650

4751
new(type:, subtype:)
@@ -50,14 +54,17 @@ def parse(media_type)
5054
def match?(media_type)
5155
return false if media_type.blank?
5256

53-
subtype = media_type.split('/', 2).last
57+
subtype = media_type.downcase.split('/', 2).last
5458
return false if subtype.blank?
5559

5660
VENDOR_VERSION_HEADER_REGEX.match?(subtype)
5761
end
5862

63+
# The available types are registered in lower case and Rack matches them
64+
# literally, so the header has to be down-cased to be compared against
65+
# them at all.
5966
def best_quality_media_type(header, available_media_types)
60-
header.blank? ? available_media_types.first : Rack::Utils.best_q_match(header, available_media_types)
67+
header.blank? ? available_media_types.first : Rack::Utils.best_q_match(header.downcase, available_media_types)
6168
end
6269
end
6370

spec/grape/api_spec.rb

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4183,6 +4183,32 @@ def my_method
41834183
end
41844184
end
41854185

4186+
# Media types are case-insensitive (RFC 9110 §8.3.1). The registered ones are
4187+
# spelled in lower case and matched literally, so a differently-cased Accept
4188+
# used to find nothing: content negotiation fell through to the default format
4189+
# and header versioning behaved as though no version had been asked for.
4190+
describe 'a differently-cased Accept header' do
4191+
it 'still negotiates the content type' do
4192+
subject.content_type :json, 'application/json'
4193+
subject.content_type :txt, 'text/plain'
4194+
subject.default_format :json
4195+
subject.get('/x') { { a: 1 } }
4196+
4197+
get '/x', {}, 'HTTP_ACCEPT' => 'TEXT/PLAIN'
4198+
expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('text/plain')
4199+
end
4200+
4201+
it 'still resolves the version of a vendor media type' do
4202+
subject.version 'v1', using: :header, vendor: 'twitter'
4203+
subject.format :json
4204+
subject.get('/x') { env[Grape::Env::API_VERSION] }
4205+
4206+
get '/x', {}, 'HTTP_ACCEPT' => 'APPLICATION/VND.TWITTER-V1+JSON'
4207+
expect(last_response.status).to eq(200)
4208+
expect(last_response.body).to eq('v1'.to_json)
4209+
end
4210+
end
4211+
41864212
describe '.format' do
41874213
context ':txt' do
41884214
before do

spec/grape/util/media_type_spec.rb

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,25 @@
4141
it_behaves_like 'MediaType'
4242
end
4343
end
44+
45+
# Media types are case-insensitive (RFC 9110 §8.3.1); the vendor pattern is
46+
# written in lower case, so anything else used to parse as no vendor at all.
47+
context 'when the header is not in lower case' do
48+
subject(:media_type) { described_class.parse(header) }
49+
50+
let(:header) { 'APPLICATION/VND.TEST-V1+JSON' }
51+
52+
it 'parses the vendor, version and format' do
53+
expect(media_type.vendor).to eq('test')
54+
expect(media_type.version).to eq('v1')
55+
expect(media_type.format).to eq('json')
56+
end
57+
58+
it 'down-cases the type and subtype' do
59+
expect(media_type.type).to eq('application')
60+
expect(media_type.subtype).to eq('vnd.test-v1+json')
61+
end
62+
end
4463
end
4564

4665
describe '.match?' do

0 commit comments

Comments
 (0)