From be0cd76eba083180c7a9d13a5ede1ec2455aff5a 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 | 7 ++++++- spec/grape/middleware/formatter_spec.rb | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d4563faee..e1b39160aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ #### Fixes +* [#number](https://github.com/ruby-grape/grape/pull/number): Detect content type in case of raising Rack level errors - [@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..ac08c4b03a 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -1,4 +1,5 @@ require 'grape/middleware/base' +require 'rack/utils' module Grape module Middleware @@ -139,7 +140,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)