fix #726 - Throw an error for content types not specified via format …

…and fallback to default_format only if content-type is not specified.
ruby-grape · Mar 10, 2017 · 132f232 · 132f232
1 parent 92c173b
commit 132f232
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,7 @@
 * [#1564](https://github.com/ruby-grape/grape/pull/1564): Fix declared params bug with nested namespaces - [@bmarini](https://github.com/bmarini).
 * [#1567](https://github.com/ruby-grape/grape/pull/1567): Fix values validator when value is empty array and apply except to input array - [@jlfaber](https://github.com/jlfaber).
 * [#1569](https://github.com/ruby-grape/grape/pull/1569), [#1511](https://github.com/ruby-grape/grape/issues/1511): Upgrade mustermann-grape to 1.0.0 - [@namusyaka](https://github.com/namusyaka).
+* [#1589](https://github.com/ruby-grape/grape/pull/1589): [#726](https://github.com/ruby-grape/grape/issues/726): Use default_format when Content-type is missing and respond with 406 when Content-type is invalid - [@inclooder](https://github.com/inclooder).
 * Your contribution here.
 
 ### 0.19.1 (1/9/2017)

diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb
@@ -90,31 +90,30 @@ def read_body_input
 
       # store parsed input in env['api.request.body']
       def read_rack_input(body)
-        fmt = mime_types[request.media_type] if request.media_type
-        fmt ||= options[:default_format]
-        if content_type_for(fmt)
-          parser = Grape::Parser.parser_for fmt, options
-          if parser
-            begin
-              body = (env[Grape::Env::API_REQUEST_BODY] = parser.call(body, env))
-              if body.is_a?(Hash)
-                env[Grape::Env::RACK_REQUEST_FORM_HASH] = if env[Grape::Env::RACK_REQUEST_FORM_HASH]
-                                                            env[Grape::Env::RACK_REQUEST_FORM_HASH].merge(body)
-                                                          else
-                                                            body
-                                                          end
-                env[Grape::Env::RACK_REQUEST_FORM_INPUT] = env[Grape::Env::RACK_INPUT]
-              end
-            rescue Grape::Exceptions::Base => e
-              raise e
-            rescue StandardError => e
-              throw :error, status: 400, message: e.message
+        fmt = request.media_type ? mime_types[request.media_type] : options[:default_format]
+
+        unless content_type_for(fmt)
+          throw :error, status: 406, message: "The requested content-type '#{request.media_type}' is not supported."
+        end
+        parser = Grape::Parser.parser_for fmt, options
+        if parser
+          begin
+            body = (env[Grape::Env::API_REQUEST_BODY] = parser.call(body, env))
+            if body.is_a?(Hash)
+              env[Grape::Env::RACK_REQUEST_FORM_HASH] = if env[Grape::Env::RACK_REQUEST_FORM_HASH]
+                                                          env[Grape::Env::RACK_REQUEST_FORM_HASH].merge(body)
+                                                        else
+                                                          body
+                                                        end
+              env[Grape::Env::RACK_REQUEST_FORM_INPUT] = env[Grape::Env::RACK_INPUT]
             end
-          else
-            env[Grape::Env::API_REQUEST_BODY] = body
+          rescue Grape::Exceptions::Base => e
+            raise e
+          rescue StandardError => e
+            throw :error, status: 400, message: e.message
           end
         else
-          throw :error, status: 406, message: "The requested content-type '#{request.media_type}' is not supported."
+          env[Grape::Env::API_REQUEST_BODY] = body
         end
       end
 

diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb
@@ -843,6 +843,18 @@ def app
       expect(last_response.body).to eq('{"error":"The requested content-type \'application/xml\' is not supported."}')
     end
 
+    it 'does not accept text/plain in JSON format if application/json is specified as content type' do
+      subject.format :json
+      subject.default_format :json
+      subject.put '/request_body' do
+        params[:user]
+      end
+      put '/request_body', MultiJson.dump(user: 'Bob'), 'CONTENT_TYPE' => 'text/plain'
+
+      expect(last_response.status).to eq(406)
+      expect(last_response.body).to eq('{"error":"The requested content-type \'text/plain\' is not supported."}')
+    end
+
     context 'content type with params' do
       before do
         subject.format :json