From bba1a9d6469b147583685afe210db62fc4827a31 Mon Sep 17 00:00:00 2001 From: "Hamed R. Nik" Date: Wed, 15 Jun 2016 12:40:41 +0100 Subject: [PATCH] Detect content type correctly if raised any low level errors\n If Rack level errors raise, content type of the request couldn't be detected correctly. Basically there are two types of errors might happen in Rack level, Rack::Utils::ParameterTypeError and Rack::Utils::InvalidParameterError.\n Passing query parameters like `x[y]=1&x[y]z=2` and `foo%81E=1` will raise the Rack level errors and the content type couldn't be detected correctly. --- CHANGELOG.md | 1 + lib/grape/middleware/formatter.rb | 6 +++++- spec/grape/middleware/formatter_spec.rb | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d4563faee..2c2dae4355 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ #### Fixes +* [#1427](https://github.com/ruby-grape/grape/pull/1427): Handling an invalid query string when trying to detect the response format - [@iCEAGE](https://github.com/iCEAGE). * [#1405](https://github.com/ruby-grape/grape/pull/1405): Fix priority of `rescue_from` clauses applying - [@hedgesky](https://github.com/hedgesky). * [#1365](https://github.com/ruby-grape/grape/pull/1365): Fix finding exception handler in error middleware - [@ktimothy](https://github.com/ktimothy). * [#1380](https://github.com/ruby-grape/grape/pull/1380): Fix `allow_blank: false` for `Time` attributes with valid values causes `NoMethodError` - [@ipkes](https://github.com/ipkes). diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index ea5705e1f8..d7438385c2 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -139,7 +139,11 @@ def format_from_extension end def format_from_params - fmt = Rack::Utils.parse_nested_query(env[Grape::Http::Headers::QUERY_STRING])[Grape::Http::Headers::FORMAT] + fmt = begin + Rack::Utils.parse_nested_query(env[Grape::Http::Headers::QUERY_STRING])[Grape::Http::Headers::FORMAT] + rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError + nil + end # avoid symbol memory leak on an unknown format return fmt.to_sym if content_type_for(fmt) fmt diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index 5a1fd1120b..ab8c6ead24 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -117,6 +117,24 @@ def to_xml expect(subject.env['api.format']).to eq(:json) end + it 'uses the requested format with invalid parameter type if provided in headers' do + _, headers, = subject.call( + 'PATH_INFO' => '/info', + 'QUERY_STRING' => 'id=12&id[]=12', + 'HTTP_ACCEPT' => 'application/json' + ) + expect(headers['Content-type']).to eq('application/json') + end + + it 'uses the requested format with invalid byte sequence in UTF-8 if provided in headers' do + _, headers, = subject.call( + 'PATH_INFO' => '/info', + 'QUERY_STRING' => 'foo%81E=1', + 'HTTP_ACCEPT' => 'application/json' + ) + expect(headers['Content-type']).to eq('application/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(:xml)