Quality arguments to Accept header should default to 1.0 (#2366)

* Quality arguments to Accept header should default to 1.0

* space

* Fix spec that results in XML

* sort only once

* Simplify

* Ensure empty and invalid quality values also parse correctly, specs

* Update CHANGELOG.md

* Comments for readme and changelog

Author: hiddewie

CHANGELOG.md

@@ -7,10 +7,11 @@
 * [#2360](https://github.com/ruby-grape/grape/pull/2360): Reduce gem size by removing specs - [@ericproulx](https://github.com/ericproulx).
 * [#2361](https://github.com/ruby-grape/grape/pull/2361): Remove `Rack::Auth::Digest` - [@ninoseki](https://github.com/ninoseki).
 * Your contribution here.
-  
+
 #### Fixes
 
 * [#2364](https://github.com/ruby-grape/grape/pull/2364): Add missing requires - [@ericproulx](https://github.com/ericproulx).
+* [#2366](https://github.com/ruby-grape/grape/pull/2366): Default quality to 1.0 in the `Accept` header when omitted - [@hiddewie](https://github.com/hiddewie).
 * Your contribution here.
 
 ### 1.8.0 (2023/08/30)

README.md

@@ -596,6 +596,10 @@ When an invalid `Accept` header is supplied, a `406 Not Acceptable` error is ret
 option is set to `false`. Otherwise a `404 Not Found` error is returned by Rack if no other route
 matches.
 
+Grape will evaluate the relative quality preference included in Accept headers and default to a quality of 1.0 when omitted. In the following example a Grape API that supports XML and JSON in that order will return JSON:
+
+    curl -H "Accept: text/xml;q=0.8, application/json;q=0.9" localhost:1234/resource
+
 ### Accept-Version Header
 
 ```ruby
@@ -1600,7 +1604,7 @@ Note endless ranges are also supported with ActiveSupport >= 6.0, but they requi
 ```ruby
 params do
   requires :minimum, type: Integer, values: 10..
-  optional :maximum, type: Integer, values: ..10 
+  optional :maximum, type: Integer, values: ..10
 end
 ```
 

lib/grape/middleware/formatter.rb

@@ -164,14 +164,14 @@ def mime_array
             \w+/[\w+.-]+)     # eg application/vnd.example.myformat+xml
           (?:
            (?:;[^,]*?)?       # optionally multiple formats in a row
-           ;\s*q=([\d.]+)     # optional "quality" preference (eg q=0.5)
+           ;\s*q=([\w.]+)     # optional "quality" preference (eg q=0.5)
           )?
         }x
 
         vendor_prefix_pattern = /vnd\.[^+]+\+/
 
         accept.scan(accept_into_mime_and_quality)
-              .sort_by { |_, quality_preference| -quality_preference.to_f }
+              .sort_by { |_, quality_preference| -(quality_preference ? quality_preference.to_f : 1.0) }
               .flat_map { |mime, _| [mime, mime.sub(vendor_prefix_pattern, '')] }
       end
     end

spec/grape/middleware/formatter_spec.rb

@@ -125,20 +125,51 @@ def to_xml
     it 'uses quality rankings to determine formats' do
       subject.call('PATH_INFO' => '/info', 'HTTP_ACCEPT' => 'application/json; q=0.3,application/xml; q=1.0')
       expect(subject.env['api.format']).to eq(:xml)
+
       subject.call('PATH_INFO' => '/info', 'HTTP_ACCEPT' => 'application/json; q=1.0,application/xml; q=0.3')
       expect(subject.env['api.format']).to eq(:json)
     end
 
     it 'handles quality rankings mixed with nothing' do
       subject.call('PATH_INFO' => '/info', 'HTTP_ACCEPT' => 'application/json,application/xml; q=1.0')
+      expect(subject.env['api.format']).to eq(:json)
+
+      subject.call('PATH_INFO' => '/info', 'HTTP_ACCEPT' => 'application/xml; q=1.0,application/json')
       expect(subject.env['api.format']).to eq(:xml)
     end
 
+    it 'handles quality rankings that have a default 1.0 value' do
+      subject.call('PATH_INFO' => '/info', 'HTTP_ACCEPT' => 'application/json,application/xml;q=0.5')
+      expect(subject.env['api.format']).to eq(:json)
+      subject.call('PATH_INFO' => '/info', 'HTTP_ACCEPT' => 'application/xml;q=0.5,application/json')
+      expect(subject.env['api.format']).to eq(:json)
+    end
+
     it 'parses headers with other attributes' do
       subject.call('PATH_INFO' => '/info', 'HTTP_ACCEPT' => 'application/json; abc=2.3; q=1.0,application/xml; q=0.7')
       expect(subject.env['api.format']).to eq(:json)
     end
 
+    it 'ensures that a quality of 0 is less preferred than any other content type' do
+      subject.call('PATH_INFO' => '/info', 'HTTP_ACCEPT' => 'application/json;q=0.0,application/xml')
+      expect(subject.env['api.format']).to eq(:xml)
+      subject.call('PATH_INFO' => '/info', 'HTTP_ACCEPT' => 'application/xml,application/json;q=0.0')
+      expect(subject.env['api.format']).to eq(:xml)
+    end
+
+    it 'ignores invalid quality rankings' do
+      subject.call('PATH_INFO' => '/info', 'HTTP_ACCEPT' => 'application/json;q=invalid,application/xml;q=0.5')
+      expect(subject.env['api.format']).to eq(:xml)
+      subject.call('PATH_INFO' => '/info', 'HTTP_ACCEPT' => 'application/xml;q=0.5,application/json;q=invalid')
+      expect(subject.env['api.format']).to eq(:xml)
+
+      subject.call('PATH_INFO' => '/info', 'HTTP_ACCEPT' => 'application/json;q=,application/xml;q=0.5')
+      expect(subject.env['api.format']).to eq(:json)
+
+      subject.call('PATH_INFO' => '/info', 'HTTP_ACCEPT' => 'application/json;q=nil,application/xml;q=0.5')
+      expect(subject.env['api.format']).to eq(:xml)
+    end
+
     it 'parses headers with vendor and api version' do
       subject.call('PATH_INFO' => '/info', 'HTTP_ACCEPT' => 'application/vnd.test-v1+xml')
       expect(subject.env['api.format']).to eq(:xml)