Formatting from header acts like Versioning from header

CHANGELOG.md

@@ -15,6 +15,7 @@
 * [#2543](https://github.com/ruby-grape/grape/pull/2543): Fix array allocation on mount - [@ericproulx](https://github.com/ericproulx).
 * [#2546](https://github.com/ruby-grape/grape/pull/2546): Fix middleware with keywords - [@ericproulx](https://github.com/ericproulx).
 * [#2547](https://github.com/ruby-grape/grape/pull/2547): Remove jsonapi related code - [@ericproulx](https://github.com/ericproulx).
+* [#2548](https://github.com/ruby-grape/grape/pull/2548): Formatting from header acts like versioning from header - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 ### 2.3.0 (2025-02-08)

UPGRADING.md

@@ -6,6 +6,25 @@ Upgrading Grape
 - Passing a class to `build_with` or `Grape.config.param_builder` has been deprecated in favor of a symbolized short_name. See `SHORTNAME_LOOKUP` in [params_builder](lib/grape/params_builder.rb).
 - Including Grape's extensions like `Grape::Extensions::Hashie::Mash::ParamBuilder` has been deprecated in favor of using `build_with` at the route level.
 
+#### Accept Header Negotiation Harmonized
+
+[Accept](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Accept) header is now fully interpreted through `Rack::Utils.best_q_match` which is following [RFC2616 14.1](https://datatracker.ietf.org/doc/html/rfc2616#section-14.1).
+Since [Grape 2.1.0](https://github.com/ruby-grape/grape/blob/master/CHANGELOG.md#210-20240615), the [header versionning strategy](https://github.com/ruby-grape/grape?tab=readme-ov-file#header) is using embracing it
+but `Grape::Middleware::Formatter` never did.
+
+Your API might act differently since it will strictly follow the [RFC2616 14.1](https://datatracker.ietf.org/doc/html/rfc2616#section-14.1) when interpreting the `Accept` header.
+Here are the differences:
+
+###### Invalid or missing quality ranking
+The following will be interpreted that `application/json` is the preferred media type:
+- `application/json;q=invalid,application/xml;q=0.5`
+- `application/json,application/xml;q=1.0`
+
+###### Closest generic for custom vendored/versioned
+In a case where a custom vendored/versioned like `application/vnd.test+json` is provided and 
+your API is accepting `application/json`, Grape will not consider the last and default to `text/plain`.
+Use the [header versionning strategy](https://github.com/ruby-grape/grape?tab=readme-ov-file#header) or register it.
+
 ### Upgrading to >= 2.4.0
 
 #### Custom Validators

lib/grape/middleware/formatter.rb

@@ -122,56 +122,31 @@ def read_rack_input(body)
       def negotiate_content_type
         fmt = format_from_extension || format_from_params || options[:format] || format_from_header || options[:default_format]
         if content_type_for(fmt)
-          env[Grape::Env::API_FORMAT] = fmt
+          env[Grape::Env::API_FORMAT] = fmt.to_sym
         else
           throw :error, status: 406, message: "The requested format '#{fmt}' is not supported."
         end
       end
 
       def format_from_extension
-        parts = request.path.split('.')
+        request_path = request.path.try(:scrub)
+        dot_pos = request_path.rindex('.')
+        return unless dot_pos
 
-        if parts.size > 1
-          extension = parts.last
-          # avoid symbol memory leak on an unknown format
-          return extension.to_sym if content_type_for(extension)
-        end
-        nil
+        extension = request_path[dot_pos + 1..]
+        extension if content_type_for(extension)
       end
 
       def format_from_params
-        fmt = Rack::Utils.parse_nested_query(env[Rack::QUERY_STRING])[FORMAT]
-        # avoid symbol memory leak on an unknown format
-        return fmt.to_sym if content_type_for(fmt)
-
-        fmt
+        Rack::Utils.parse_nested_query(env[Rack::QUERY_STRING])[FORMAT]
       end
 
       def format_from_header
-        mime_array.each do |t|
-          return mime_types[t] if mime_types.key?(t)
-        end
-        nil
-      end
-
-      def mime_array
-        accept = env[Grape::Http::Headers::HTTP_ACCEPT]
-        return [] unless accept
-
-        accept_into_mime_and_quality = %r{
-          (
-            \w+/[\w+.-]+)     # eg application/vnd.example.myformat+xml
-          (?:
-           (?:;[^,]*?)?       # optionally multiple formats in a row
-           ;\s*q=([\w.]+)     # optional "quality" preference (eg q=0.5)
-          )?
-        }x
-
-        vendor_prefix_pattern = /vnd\.[^+]+\+/
+        accept_header = env[Grape::Http::Headers::HTTP_ACCEPT].try(:scrub)
+        return if accept_header.blank?
 
-        accept.scan(accept_into_mime_and_quality)
-              .sort_by { |_, quality_preference| -(quality_preference ? quality_preference.to_f : 1.0) }
-              .flat_map { |mime, _| [mime, mime.sub(vendor_prefix_pattern, '')] }
+        media_type = Rack::Utils.best_q_match(accept_header, mime_types.keys)
+        mime_types[media_type] if media_type
       end
     end
   end

spec/grape/middleware/formatter_spec.rb

@@ -82,6 +82,12 @@ def to_xml
   end
 
   context 'detection' do
+    context 'when path contains invalid byte sequence' do
+      it 'does not raise an exception' do
+        expect { subject.call(Rack::PATH_INFO => "/info.\x80") }.not_to raise_error
+      end
+    end
+
     it 'uses the xml extension if one is provided' do
       subject.call(Rack::PATH_INFO => '/info.xml')
       expect(subject.env[Grape::Env::API_FORMAT]).to eq(:xml)
@@ -95,8 +101,6 @@ def to_xml
     it 'uses the format parameter if one is provided' do
       subject.call(Rack::PATH_INFO => '/info', Rack::QUERY_STRING => 'format=json')
       expect(subject.env[Grape::Env::API_FORMAT]).to eq(:json)
-      subject.call(Rack::PATH_INFO => '/info', Rack::QUERY_STRING => 'format=xml')
-      expect(subject.env[Grape::Env::API_FORMAT]).to eq(:xml)
     end
 
     it 'uses the default format if none is provided' do
@@ -116,6 +120,12 @@ def to_xml
   end
 
   context 'accept header detection' do
+    context 'when header contains invalid byte sequence' do
+      it 'does not raise an exception' do
+        expect { subject.call(Rack::PATH_INFO => '/info', Grape::Http::Headers::HTTP_ACCEPT => "Hello \x80") }.not_to raise_error
+      end
+    end
+
     it 'detects from the Accept header' do
       subject.call(Rack::PATH_INFO => '/info', Grape::Http::Headers::HTTP_ACCEPT => 'application/xml')
       expect(subject.env[Grape::Env::API_FORMAT]).to eq(:xml)
@@ -131,10 +141,10 @@ def to_xml
 
     it 'handles quality rankings mixed with nothing' do
       subject.call(Rack::PATH_INFO => '/info', Grape::Http::Headers::HTTP_ACCEPT => 'application/json,application/xml; q=1.0')
-      expect(subject.env[Grape::Env::API_FORMAT]).to eq(:json)
+      expect(subject.env[Grape::Env::API_FORMAT]).to eq(:xml)
 
       subject.call(Rack::PATH_INFO => '/info', Grape::Http::Headers::HTTP_ACCEPT => 'application/xml; q=1.0,application/json')
-      expect(subject.env[Grape::Env::API_FORMAT]).to eq(:xml)
+      expect(subject.env[Grape::Env::API_FORMAT]).to eq(:json)
     end
 
     it 'handles quality rankings that have a default 1.0 value' do
@@ -156,30 +166,21 @@ def to_xml
       expect(subject.env[Grape::Env::API_FORMAT]).to eq(:xml)
     end
 
-    it 'ignores invalid quality rankings' do
-      subject.call(Rack::PATH_INFO => '/info', Grape::Http::Headers::HTTP_ACCEPT => 'application/json;q=invalid,application/xml;q=0.5')
-      expect(subject.env[Grape::Env::API_FORMAT]).to eq(:xml)
-      subject.call(Rack::PATH_INFO => '/info', Grape::Http::Headers::HTTP_ACCEPT => 'application/xml;q=0.5,application/json;q=invalid')
-      expect(subject.env[Grape::Env::API_FORMAT]).to eq(:xml)
-
-      subject.call(Rack::PATH_INFO => '/info', Grape::Http::Headers::HTTP_ACCEPT => 'application/json;q=,application/xml;q=0.5')
-      expect(subject.env[Grape::Env::API_FORMAT]).to eq(:json)
-
-      subject.call(Rack::PATH_INFO => '/info', Grape::Http::Headers::HTTP_ACCEPT => 'application/json;q=nil,application/xml;q=0.5')
-      expect(subject.env[Grape::Env::API_FORMAT]).to eq(:xml)
-    end
-
-    it 'parses headers with vendor and api version' do
-      subject.call(Rack::PATH_INFO => '/info', Grape::Http::Headers::HTTP_ACCEPT => 'application/vnd.test-v1+xml')
-      expect(subject.env[Grape::Env::API_FORMAT]).to eq(:xml)
-    end
-
     context 'with custom vendored content types' do
-      subject { described_class.new(app, content_types: { custom: 'application/vnd.test+json' }) }
+      context 'when registered' do
+        subject { described_class.new(app, content_types: { custom: 'application/vnd.test+json' }) }
 
-      it 'uses the custom type' do
-        subject.call(Rack::PATH_INFO => '/info', Grape::Http::Headers::HTTP_ACCEPT => 'application/vnd.test+json')
-        expect(subject.env[Grape::Env::API_FORMAT]).to eq(:custom)
+        it 'uses the custom type' do
+          subject.call(Rack::PATH_INFO => '/info', Grape::Http::Headers::HTTP_ACCEPT => 'application/vnd.test+json')
+          expect(subject.env[Grape::Env::API_FORMAT]).to eq(:custom)
+        end
+      end
+
+      context 'when unregistered' do
+        it 'returns the default content type text/plain' do
+          r = Rack::MockResponse[*subject.call(Rack::PATH_INFO => '/info', Grape::Http::Headers::HTTP_ACCEPT => 'application/vnd.test+json')]
+          expect(r.headers[Rack::CONTENT_TYPE]).to eq('text/plain')
+        end
       end
     end
 
@@ -216,11 +217,6 @@ def to_xml
       _, headers, = s.call(Rack::PATH_INFO => '/info', Grape::Http::Headers::HTTP_ACCEPT => 'application/vnd.test+json')
       expect(headers[Rack::CONTENT_TYPE]).to eq('application/vnd.test+json')
     end
-
-    it 'is set to closest generic for custom vendored/versioned without registered type' do
-      _, headers, = subject.call(Rack::PATH_INFO => '/info', Grape::Http::Headers::HTTP_ACCEPT => 'application/vnd.test+json')
-      expect(headers[Rack::CONTENT_TYPE]).to eq('application/json')
-    end
   end
 
   context 'format' do