diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cb4abcb2..cc00e85b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,7 @@ * [#2856](https://github.com/ruby-grape/grape/pull/2856): Update simplecov - [@ericproulx](https://github.com/ericproulx). * [#2841](https://github.com/ruby-grape/grape/pull/2841): Stop `use`, `helpers`, `rescue_from` and other registrations declared below a route from reaching it when an earlier registration had seeded the same key (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). * [#2846](https://github.com/ruby-grape/grape/pull/2846): Keep a `:version` path capture in `params` when the API declares no version, instead of always dropping it as Grape's own - [@ericproulx](https://github.com/ericproulx). +* [#2839](https://github.com/ruby-grape/grape/pull/2839): Tag path params as UTF-8 instead of leaving them ASCII-8BIT, so they compare equal to the non-ASCII literals an API declares (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/UPGRADING.md b/UPGRADING.md index a9f6e0059..9be905bcb 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -38,6 +38,65 @@ end Nothing changes for the ordinary arrangement — registrations declared before a route, or inherited from an enclosing namespace or a mounting API, still apply exactly as before, including values an enclosing scope gains after the nested scope was created. +#### Path params are tagged UTF-8 instead of ASCII-8BIT + +Params captured from the request path — `route_param`, `:id`-style segments, splats — now come back tagged `UTF-8`. They used to carry the `ASCII-8BIT` encoding of Rack's `PATH_INFO`, because Mustermann decodes the path against that raw string and nothing re-tagged the result. Query and body params were already `UTF-8`, since Rack tags those itself. + +**Why UTF-8.** Nothing obliges a client to send it — HTTP treats the request target as octets, and Rack's SPEC has CGI keys carry non-ASCII as `ASCII-8BIT`. UTF-8 is a convention rather than a guarantee. But it is the convention Rack itself already applies to everything *except* the path: `Rack::QueryParser#unescape` decodes the query string and form bodies with `URI.decode_www_form_component(string, Encoding::UTF_8)`, which tags the result without validating it. + +One request carrying the same invalid octets in three places, before this change: + +| source | encoding | `valid_encoding?` | +| --- | --- | --- | +| query — `?q=%C3%28` | `UTF-8` | `false` | +| form body — `form=%C3%28` | `UTF-8` | `false` | +| path — `/%C3%28` | **`ASCII-8BIT`** | `true` | + +The bytes are equally malformed in all three; only the label differed. The path reads `true` merely because `ASCII-8BIT` considers every byte sequence valid. So this change is not Grape adopting an outside convention — it is Grape agreeing with the library handing it the request. The path param was the odd one out only because Mustermann decodes against `PATH_INFO` directly and never had Rack's `unescape` applied to it. + +Grape does exactly what Rack does: re-tag, do not validate. The bytes are untouched, so octets that are *not* UTF-8 stay detectably invalid rather than being scrubbed into something the client never sent. (Rails takes the same approach for path captures — `ActionDispatch::Journey::Router` force-encodes each one to UTF-8 after unescaping.) + +**What this fixes.** An API's own declarations used to disagree with themselves depending on where a value arrived from — a binary string never equals the UTF-8 literal it was written as: + +```ruby +params { requires :id, type: String, values: ['café'] } +``` + +| request | before | 4.0 | +| --- | --- | --- | +| `GET /?id=café` (query) | `200` | unchanged | +| `GET /café` (path) | `400 "id does not have a valid value"` | `200` | + +The same held for `same_as`, `except_values`, and any comparison an endpoint made against a non-ASCII literal. Serialization was affected too: a non-ASCII path param rendered into a JSON response emitted an encoding warning from the `json` gem, and is slated to raise there in json 3.0. + +**What can break.** Only the encoding tag changes; the bytes are untouched, and an invalid byte sequence stays invalid rather than being scrubbed. Comparisons against pure-ASCII strings are unaffected. Code that relied on a path param being binary — concatenating one with genuinely binary data, for instance — can now raise `Encoding::CompatibilityError`, and should call `.b` on the param to opt back into binary: + +```ruby +params[:id].b + binary_blob +``` + +An application that worked around the old behavior with its own `force_encoding(Encoding::UTF_8)` needs no change; that call is now a no-op. + +**Restoring the old behavior wholesale.** If enough code depends on binary path params that patching each site is impractical, a `before` filter at the top of your root API re-tags them all back. `env['grape.routing_args']` holds exactly the params that came from the path, so query and body params are left alone, and `before` runs ahead of validation, so declarations see the binary strings too: + +```ruby +class API < Grape::API + before do + env['grape.routing_args']&.each_key do |key| + next if key == :route_info + + value = params[key] + params[key] = value.b if value.is_a?(String) + params[key] = value.map(&:b) if value.is_a?(Array) + end + end + + # ... mounts and routes +end +``` + +Declared in the root API, this covers mounted APIs too. Treat it as a migration aid rather than a permanent setting: it restores the inconsistency this change fixes, so `values: ['café']` will keep rejecting `GET /café` while accepting `?id=café`. + #### `Array`/`Set` of an unsupported type is rejected when the API is defined Declaring a collection whose element type Grape cannot coerce — `type: Array[Foo]` or `type: Set[Foo]` where `Foo` is neither a primitive, a structure, nor a valid custom type — now raises as soon as the `params` block is evaluated, i.e. while the API class is being loaded: diff --git a/lib/grape/router/route.rb b/lib/grape/router/route.rb index 7afa40bc1..07a03073a 100644 --- a/lib/grape/router/route.rb +++ b/lib/grape/router/route.rb @@ -49,7 +49,9 @@ def params_for(input) parsed = pattern.params(input) return unless parsed - parsed.compact.symbolize_keys + params = {} + parsed.each { |name, value| params[name.to_sym] = tag_utf8!(value) unless value.nil? } + params end protected @@ -60,6 +62,38 @@ def convert_to_head_request! private + # Mustermann decodes path captures out of +PATH_INFO+, which Rack hands us + # tagged ASCII-8BIT, so path params came back binary while Rack tags query + # and body params UTF-8. That split makes an API's own declarations + # disagree with themselves: `values: ['café']` matched `?id=café` but not + # `/café`, since a binary string never equals the UTF-8 literal it was + # written as. + # + # Re-tag as UTF-8. Nothing obliges a client to send UTF-8: the request + # target is octets to HTTP, and Rack's SPEC has CGI keys carry non-ASCII + # as ASCII-8BIT. But UTF-8 is what browsers percent-encode with, what an + # IRI maps to, and what Rails settles on — ActionDispatch::Journey::Router + # force_encodes every path capture to UTF-8 after unescaping it. + # + # Only the encoding changes; the bytes are untouched. Octets that are not + # UTF-8 therefore stay invalid and are caught downstream rather than being + # silently scrubbed into something the client never sent. + # + # The re-tag is in place, hence the bang. Mustermann's +Pattern#params+ + # builds a fresh Hash of fresh, unfrozen strings on every call and skips + # its own Match cache, so the mutation cannot escape this request — which + # is also what lets the Array branch re-tag its elements with +each+. + def tag_utf8!(value) + case value + when String + value.force_encoding(Encoding::UTF_8) + when Array + value.each { |v| tag_utf8!(v) } + else + value + end + end + def upcase_method(method) method_s = method.to_s Grape::HTTP_SUPPORTED_METHODS.detect { |m| m.casecmp(method_s).zero? } || method_s.upcase diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 04ca5af1f..ef7a805e8 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -381,6 +381,68 @@ def first = raise('#first should not have been called') expect(last_response.body).to eq('{"foo":1234}') end end + + context 'with a non-ascii segment' do + it 'tags the param as UTF-8 rather than leaving it binary' do + subject.route_param :id do + get { params[:id].encoding.name } + end + + get '/caf%C3%A9' + expect(last_response.body).to eq('UTF-8') + end + + it 'tags every captured segment' do + subject.namespace :a do + route_param :one do + route_param :two do + get { [params[:one].encoding.name, params[:two].encoding.name].join(',') } + end + end + end + + get '/a/caf%C3%A9/th%C3%A9' + expect(last_response.body).to eq('UTF-8,UTF-8') + end + + it 'tags a splat capture' do + subject.get('/files/*path') { params[:path].encoding.name } + + get '/files/a/b/caf%C3%A9' + expect(last_response.body).to eq('UTF-8') + end + + # An unnamed splat captures into an Array rather than a String. + it 'tags every element of an unnamed splat capture' do + subject.get('/files/*') { params[:splat].map { |s| s.encoding.name }.join(',') } + + get '/files/caf%C3%A9' + expect(last_response.body).to eq('UTF-8') + end + + # A path param used to arrive binary while the same value arrived UTF-8 + # through the query string, so an API's own declaration disagreed with + # itself depending on where the value came from. + it 'compares equal to the utf-8 literal the API declares' do + subject.params { requires :id, type: String, values: ['café'] } + subject.route_param :id do + get { 'matched' } + end + + get '/caf%C3%A9' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('matched') + end + + it 'leaves the bytes untouched' do + subject.route_param :id do + get { params[:id] } + end + + get '/caf%C3%A9' + expect(last_response.body.b).to eq('caf%C3%A9'.b.gsub('%C3%A9', "\xC3\xA9".b)) + end + end end describe '.route' do