From ff7901bc887845d2153a97d9caf05cf411abbabe Mon Sep 17 00:00:00 2001 From: "hidde.wieringa" Date: Thu, 2 Nov 2023 16:27:29 +0100 Subject: [PATCH 1/8] Quality arguments to Accept header should default to 1.0 --- lib/grape/middleware/formatter.rb | 1 + spec/grape/middleware/formatter_spec.rb | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index 0f8e7cdb3c..6db61820a1 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -172,6 +172,7 @@ def mime_array accept.scan(accept_into_mime_and_quality) .sort_by { |_, quality_preference| -quality_preference.to_f } + .sort_by { |_, quality_preference| -(quality_preference || '1.0').to_f } .flat_map { |mime, _| [mime, mime.sub(vendor_prefix_pattern, '')] } end end diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index 29f49f88ba..2dad51efc1 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -131,9 +131,18 @@ def to_xml 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) From 1245611c2f8d135b66d835266ca63fa5cce12a4e Mon Sep 17 00:00:00 2001 From: "hidde.wieringa" Date: Thu, 2 Nov 2023 16:27:57 +0100 Subject: [PATCH 2/8] space --- spec/grape/middleware/formatter_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index 2dad51efc1..7f47d6001f 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -125,6 +125,7 @@ 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 @@ -132,6 +133,7 @@ def to_xml 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 From 4285cfb4888c48b78d1f2e603b12e5a947128022 Mon Sep 17 00:00:00 2001 From: "hidde.wieringa" Date: Thu, 2 Nov 2023 16:34:39 +0100 Subject: [PATCH 3/8] Fix spec that results in XML --- spec/grape/middleware/formatter_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index 7f47d6001f..4fa08f2f93 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -132,7 +132,7 @@ def to_xml 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) + expect(subject.env['api.format']).to eq(:xml) subject.call('PATH_INFO' => '/info', 'HTTP_ACCEPT' => 'application/xml; q=1.0,application/json') expect(subject.env['api.format']).to eq(:xml) From 1dd47a1f6534a56c3d0cedcf8ba156e48817cb30 Mon Sep 17 00:00:00 2001 From: "hidde.wieringa" Date: Thu, 2 Nov 2023 16:38:13 +0100 Subject: [PATCH 4/8] sort only once --- lib/grape/middleware/formatter.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index 6db61820a1..c0c30fc72f 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -171,7 +171,6 @@ def mime_array 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 || '1.0').to_f } .flat_map { |mime, _| [mime, mime.sub(vendor_prefix_pattern, '')] } end From 5e4c2fb5ab0dd31f6ad06a2a920cb388857f329b Mon Sep 17 00:00:00 2001 From: "hidde.wieringa" Date: Thu, 2 Nov 2023 16:38:44 +0100 Subject: [PATCH 5/8] Simplify --- spec/grape/middleware/formatter_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index 4fa08f2f93..7f47d6001f 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -132,7 +132,7 @@ def to_xml 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(:xml) + 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) From fb6aa640284c4ebb746682b83aaf045e1b26e6ba Mon Sep 17 00:00:00 2001 From: Hidde Wieringa Date: Fri, 3 Nov 2023 21:31:23 +0100 Subject: [PATCH 6/8] Ensure empty and invalid quality values also parse correctly, specs --- lib/grape/middleware/formatter.rb | 4 ++-- spec/grape/middleware/formatter_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index c0c30fc72f..0e87373d0b 100644 --- a/lib/grape/middleware/formatter.rb +++ b/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 || '1.0').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 diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index 7f47d6001f..59245ad113 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -150,6 +150,26 @@ def to_xml 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) From 613e5f0d6704e51a423f729352454619091c6567 Mon Sep 17 00:00:00 2001 From: Hidde Wieringa Date: Tue, 7 Nov 2023 08:09:06 +0100 Subject: [PATCH 7/8] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6618fadeb3..333a303e3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ #### 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): Quality arguments to `accept` header should default to 1.0 - [@hiddewie](https://github.com/hiddewie). * Your contribution here. ### 1.8.0 (2023/08/30) From 7a6cefc9c69e1cb2b7fd14067eab68297bac3382 Mon Sep 17 00:00:00 2001 From: Hidde Wieringa Date: Tue, 7 Nov 2023 16:51:15 +0100 Subject: [PATCH 8/8] Comments for readme and changelog --- CHANGELOG.md | 4 ++-- README.md | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 333a303e3b..d17ce560c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +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): Quality arguments to `accept` header should default to 1.0 - [@hiddewie](https://github.com/hiddewie). +* [#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) diff --git a/README.md b/README.md index f0d66f8094..e1d1295afe 100644 --- a/README.md +++ b/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 ```