Add rack_request to Grape::Middleware::Base

Add some rescues when dealing with rack_params and GET functions
Add exceptions related to rescues

Author: ericproulx

lib/grape/exceptions/conflicting_types.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module Grape
+  module Exceptions
+    class ConflictingTypes < Base
+      def initialize
+        super(message: compose_message(:conflicting_types), status: 400)
+      end
+    end
+  end
+end

lib/grape/exceptions/invalid_parameters.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module Grape
+  module Exceptions
+    class InvalidParameters < Base
+      def initialize
+        super(message: compose_message(:invalid_parameters), status: 400)
+      end
+    end
+  end
+end

lib/grape/exceptions/too_deep_parameters.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module Grape
+  module Exceptions
+    class TooDeepParameters < Base
+      def initialize(limit)
+        super(message: compose_message(:too_deep_parameters, limit: limit), status: 400)
+      end
+    end
+  end
+end

lib/grape/locale/en.yml

@@ -58,4 +58,6 @@ en:
           problem: 'invalid version header'
           resolution: '%{message}'
         invalid_response: 'Invalid response'
-        query_parsing: 'query params are not parsable'
+        conflicting_types: 'query params contains conflicting types'
+        invalid_parameters: 'query params contains invalid format or byte sequence'
+        too_deep_parameters: 'query params are recursively nested over the specified limit (%{limit})'

lib/grape/middleware/base.rb

@@ -6,9 +6,7 @@ class Base
       include Helpers
       include Grape::DSL::Headers
 
-      attr_reader :app, :env, :options
-
-      TEXT_HTML = 'text/html'
+      attr_reader :app, :env, :options, :rack_request
 
       # @param [Rack Application] app The standard argument for a Rack middleware.
       # @param [Hash] options A hash of options, simply stored for use by subclasses.
@@ -54,6 +52,10 @@ def before; end
       # @return [Response, nil] a Rack SPEC response or nil to call the application afterwards.
       def after; end
 
+      def rack_request
+        @rack_request ||= Rack::Request.new(env)
+      end
+
       def response
         return @app_response if @app_response.is_a?(Rack::Response)
 
@@ -73,7 +75,15 @@ def content_type_for(format)
       end
 
       def content_type
-        content_type_for(env[Grape::Env::API_FORMAT] || options[:format]) || TEXT_HTML
+        content_type_for(env[Grape::Env::API_FORMAT] || options[:format]) || 'text/html'
+      end
+
+      def query_params
+        @rack_request.GET
+      rescue Rack::QueryParser::ParamsTooDeepError
+        raise Grape::Exceptions::TooDeepParameters.new(Rack::Utils.param_depth_limit)
+      rescue Rack::Utils::ParameterTypeError
+        raise Grape::Exceptions::ConflictingTypes
       end
 
       private

lib/grape/middleware/error.rb

@@ -39,7 +39,7 @@ def call!(env)
       private
 
       def rack_response(status, headers, message)
-        message = Rack::Utils.escape_html(message) if headers[Rack::CONTENT_TYPE] == TEXT_HTML
+        message = Rack::Utils.escape_html(message) if headers[Rack::CONTENT_TYPE] == 'text/html'
         Rack::Response.new(Array.wrap(message), Rack::Utils.status_code(status), Grape::Util::Header.new.merge(headers))
       end
 

lib/grape/middleware/formatter.rb

@@ -3,8 +3,6 @@
 module Grape
   module Middleware
     class Formatter < Base
-      CHUNKED = 'chunked'
-      FORMAT = 'format'
 
       def default_options
         {
@@ -69,34 +67,27 @@ def ensure_content_type(headers)
         end
       end
 
-      def request
-        @request ||= Rack::Request.new(env)
-      end
-
-      # store read input in env['api.request.input']
       def read_body_input
-        return unless
-          (request.post? || request.put? || request.patch? || request.delete?) &&
-          (!request.form_data? || !request.media_type) &&
-          !request.parseable_data? &&
-          (request.content_length.to_i.positive? || request.env[Grape::Http::Headers::HTTP_TRANSFER_ENCODING] == CHUNKED)
-
-        return unless (input = env[Rack::RACK_INPUT])
+        input = rack_request.body # reads RACK_INPUT
+        return if input.nil?
+        return unless read_body_input?
 
         input.try(:rewind)
         body = env[Grape::Env::API_REQUEST_INPUT] = input.read
         begin
-          read_rack_input(body) if body && !body.empty?
+          read_rack_input(body)
         ensure
           input.try(:rewind)
         end
       end
 
-      # store parsed input in env['api.request.body']
       def read_rack_input(body)
-        fmt = request.media_type ? mime_types[request.media_type] : options[:default_format]
+        return if body.empty?
+
+        media_type = rack_request.media_type
+        fmt = media_type ? mime_types[media_type] : options[:default_format]
 
-        throw :error, status: 415, message: "The provided content-type '#{request.media_type}' is not supported." unless content_type_for(fmt)
+        throw :error, status: 415, message: "The provided content-type '#{media_type}' is not supported." unless content_type_for(fmt)
         parser = Grape::Parser.parser_for fmt, options[:parsers]
         if parser
           begin
@@ -119,8 +110,21 @@ def read_rack_input(body)
         end
       end
 
+      # this middleware will not try to format the following content-types since Rack already handles them
+      # when calling Rack's `params` function
+      # - application/x-www-form-urlencoded
+      # - multipart/form-data
+      # - multipart/related
+      # - multipart/mixed
+      def read_body_input?
+        (rack_request.post? || rack_request.put? || rack_request.patch? || rack_request.delete?) &&
+          !(rack_request.form_data? && rack_request.content_type) &&
+          !rack_request.parseable_data? &&
+          (rack_request.content_length.to_i.positive? || rack_request.env[Grape::Http::Headers::HTTP_TRANSFER_ENCODING] == 'chunked')
+      end
+
       def negotiate_content_type
-        fmt = format_from_extension || format_from_params || options[:format] || format_from_header || options[:default_format]
+        fmt = format_from_extension || query_params['format'] || options[:format] || format_from_header || options[:default_format]
         if content_type_for(fmt)
           env[Grape::Env::API_FORMAT] = fmt.to_sym
         else
@@ -129,18 +133,14 @@ def negotiate_content_type
       end
 
       def format_from_extension
-        request_path = request.path.try(:scrub)
+        request_path = rack_request.path.try(:scrub)
         dot_pos = request_path.rindex('.')
         return unless dot_pos
 
         extension = request_path[dot_pos + 1..]
         extension if content_type_for(extension)
       end
 
-      def format_from_params
-        Rack::Utils.parse_nested_query(env[Rack::QUERY_STRING])[FORMAT]
-      end
-
       def format_from_header
         accept_header = env[Grape::Http::Headers::HTTP_ACCEPT].try(:scrub)
         return if accept_header.blank?

lib/grape/middleware/versioner/param.rb

@@ -20,12 +20,11 @@ module Versioner
       #   env['api.version'] => 'v1'
       class Param < Base
         def before
-          potential_version = Rack::Utils.parse_nested_query(env[Rack::QUERY_STRING])[parameter_key]
+          potential_version = query_params[parameter_key]
           return if potential_version.blank?
 
           version_not_found! unless potential_version_match?(potential_version)
-          env[Grape::Env::API_VERSION] = potential_version
-          env[Rack::RACK_REQUEST_QUERY_HASH].delete(parameter_key) if env.key? Rack::RACK_REQUEST_QUERY_HASH
+          env[Grape::Env::API_VERSION] = env[Rack::RACK_REQUEST_QUERY_HASH].delete(parameter_key)
         end
       end
     end

lib/grape/request.rb

@@ -37,8 +37,14 @@ def make_params
       @params_builder.call(rack_params).deep_merge!(grape_routing_args)
     rescue EOFError
       raise Grape::Exceptions::EmptyMessageBody.new(content_type)
-    rescue Rack::Multipart::MultipartPartLimitError
+    rescue Rack::Multipart::MultipartPartLimitError, Rack::Multipart::MultipartTotalPartLimitError
       raise Grape::Exceptions::TooManyMultipartFiles.new(Rack::Utils.multipart_part_limit)
+    rescue Rack::QueryParser::ParamsTooDeepError
+      raise Grape::Exceptions::TooDeepParameters.new(Rack::Utils.param_depth_limit)
+    rescue Rack::Utils::ParameterTypeError
+      raise Grape::Exceptions::ConflictingTypes
+    rescue Rack::Utils::InvalidParameterError
+      raise Grape::Exceptions::InvalidParameters
     end
 
     def build_headers

spec/grape/middleware/base_spec.rb

@@ -223,4 +223,20 @@ def after
       expect(last_response.headers['X-Test-Overwriting']).to eq('Bye')
     end
   end
+
+  describe 'query_params' do
+    context 'when query params are conflicting' do
+      it 'raises an ConflictingTypes error' do
+        expect { get '/?x[y]=1&x[y]z=2' } .to raise_error(Grape::Exceptions::ConflictingTypes)
+      end
+    end
+
+    context 'when query params is over the specified limit' do
+      let(:query_params) { "foo#{"[a]" * Rack::Utils.param_depth_limit}=bar" }
+
+      it 'raises an ConflictingTypes error' do
+        expect { get "/?foo#{"[a]" * Rack::Utils.param_depth_limit}=bar" } .to raise_error(Grape::Exceptions::TooDeepParameters)
+      end
+    end
+  end
 end