From 6a21f8080056593cfb5cc3eebc2a5985d7618fec Mon Sep 17 00:00:00 2001 From: Ben Schmeckpeper Date: Wed, 10 Feb 2021 10:41:48 -0600 Subject: [PATCH] Handle EOFError raised by Rack (#2161) --- .github/workflows/test.yml | 5 +-- CHANGELOG.md | 2 ++ gemfiles/rack2_2.gemfile | 39 ++++++++++++++++++++++ lib/grape.rb | 1 + lib/grape/exceptions/empty_message_body.rb | 11 ++++++ lib/grape/locale/en.yml | 2 +- lib/grape/request.rb | 2 ++ spec/grape/endpoint_spec.rb | 13 ++++++++ 8 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 gemfiles/rack2_2.gemfile create mode 100644 lib/grape/exceptions/empty_message_body.rb diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 188e01c6d2..44a7541f9f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -33,6 +33,7 @@ jobs: - Gemfile - gemfiles/rack1.gemfile - gemfiles/rack2.gemfile + - gemfiles/rack2_2.gemfile - gemfiles/rack_edge.gemfile - gemfiles/rails_5.gemfile - gemfiles/rails_6.gemfile @@ -76,7 +77,7 @@ jobs: - name: Run tests run: bundle exec rake spec - + - name: Run tests (spec/integration/eager_load) if: ${{ matrix.gemfile == 'Gemfile' }} run: bundle exec rspec spec/integration/eager_load @@ -84,7 +85,7 @@ jobs: - name: Run tests (spec/integration/multi_json) if: ${{ matrix.gemfile == 'gemfiles/multi_json.gemfile' }} run: bundle exec rspec spec/integration/multi_json - + - name: Run tests (spec/integration/multi_xml) if: ${{ matrix.gemfile == 'gemfiles/multi_xml.gemfile' }} run: bundle exec rspec spec/integration/multi_xml diff --git a/CHANGELOG.md b/CHANGELOG.md index 2793496880..b610254e7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ * Your contribution here. +* [#2161](https://github.com/ruby-grape/grape/pull/2157): Handle EOFError from Rack when given an empty multipart body - [@bschmeck](https://github.com/bschmeck). + ### 1.5.2 (2021/02/06) #### Features diff --git a/gemfiles/rack2_2.gemfile b/gemfiles/rack2_2.gemfile new file mode 100644 index 0000000000..88cfa240d0 --- /dev/null +++ b/gemfiles/rack2_2.gemfile @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +# This file was generated by Appraisal + +source 'https://rubygems.org' + +gem 'rack', '~> 2.2' + +group :development, :test do + gem 'bundler' + gem 'hashie' + gem 'rake' + gem 'rubocop', '1.7.0' + gem 'rubocop-ast', '1.3.0' + gem 'rubocop-performance', '1.9.1', require: false +end + +group :development do + gem 'appraisal' + gem 'benchmark-ips' + gem 'benchmark-memory' + gem 'guard' + gem 'guard-rspec' + gem 'guard-rubocop' +end + +group :test do + gem 'cookiejar' + gem 'coveralls_reborn' + gem 'grape-entity', '~> 0.6' + gem 'maruku' + gem 'mime-types' + gem 'rack-jsonp', require: 'rack/jsonp' + gem 'rack-test', '~> 1.1.0' + gem 'rspec', '~> 3.0' + gem 'ruby-grape-danger', '~> 0.2.0', require: false +end + +gemspec path: '../' diff --git a/lib/grape.rb b/lib/grape.rb index c1f313c2f9..818c8f6bc2 100644 --- a/lib/grape.rb +++ b/lib/grape.rb @@ -76,6 +76,7 @@ module Exceptions autoload :InvalidVersionHeader autoload :MethodNotAllowed autoload :InvalidResponse + autoload :EmptyMessageBody end end diff --git a/lib/grape/exceptions/empty_message_body.rb b/lib/grape/exceptions/empty_message_body.rb new file mode 100644 index 0000000000..c4fd431767 --- /dev/null +++ b/lib/grape/exceptions/empty_message_body.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Grape + module Exceptions + class EmptyMessageBody < Base + def initialize(body_format) + super(message: compose_message(:empty_message_body, body_format: body_format), status: 400) + end + end + end +end diff --git a/lib/grape/locale/en.yml b/lib/grape/locale/en.yml index 10fa381f7d..036eb93b80 100644 --- a/lib/grape/locale/en.yml +++ b/lib/grape/locale/en.yml @@ -44,6 +44,7 @@ en: "when specifying %{body_format} as content-type, you must pass valid %{body_format} in the request's 'body' " + empty_message_body: 'Empty message body supplied with %{body_format} content-type' invalid_accept_header: problem: 'Invalid accept header' resolution: '%{message}' @@ -51,4 +52,3 @@ en: problem: 'Invalid version header' resolution: '%{message}' invalid_response: 'Invalid response' - diff --git a/lib/grape/request.rb b/lib/grape/request.rb index b547797482..0357477d50 100644 --- a/lib/grape/request.rb +++ b/lib/grape/request.rb @@ -15,6 +15,8 @@ def initialize(env, **options) def params @params ||= build_params + rescue EOFError + raise Grape::Exceptions::EmptyMessageBody, content_type end def headers diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index 4bbeb070c3..487fbb0d3e 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -420,6 +420,19 @@ def app expect(last_response.status).to eq(201) expect(last_response.body).to eq('Bob') end + + # Rack swallowed this error until v2.2.0 + it 'returns a 400 if given an invalid multipart body', if: Gem::Version.new(Rack.release) >= Gem::Version.new('2.2.0') do + subject.params do + requires :file, type: Rack::Multipart::UploadedFile + end + subject.post '/upload' do + params[:file][:filename] + end + post '/upload', { file: '' }, 'CONTENT_TYPE' => 'multipart/form-data; boundary=foobar' + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('Empty message body supplied with multipart/form-data; boundary=foobar content-type') + end end it 'responds with a 415 for an unsupported content-type' do