diff --git a/CHANGELOG.md b/CHANGELOG.md index d2d99ec1ba..1581dfca54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -### 1.0.4 (Next) +### 1.1.0 (Next) #### Features @@ -10,6 +10,7 @@ * [#1762](https://github.com/ruby-grape/grape/pull/1763): Fix unsafe HTML rendering on errors - [@ctennis](https://github.com/ctennis). * [#1759](https://github.com/ruby-grape/grape/pull/1759): Update appraisal for rails_edge - [@zvkemp](https://github.com/zvkemp). * [#1758](https://github.com/ruby-grape/grape/pull/1758): Fix expanding load_path in gemspec - [@2maz](https://github.com/2maz). +* [#1765](https://github.com/ruby-grape/grape/pull/1765): Use 415 when request body is of an unsupported media type - [@jdmurphy](https://github.com/jdmurphy). * Your contribution here. ### 1.0.3 (4/23/2018) diff --git a/README.md b/README.md index fef5729f02..3d7bc63631 100644 --- a/README.md +++ b/README.md @@ -145,7 +145,7 @@ content negotiation, versioning and much more. ## Stable Release -You're reading the documentation for the next release of Grape, which should be **1.0.4**. +You're reading the documentation for the next release of Grape, which should be **1.1.0**. Please read [UPGRADING](UPGRADING.md) when upgrading from a previous version. The current stable release is [1.0.3](https://github.com/ruby-grape/grape/blob/v1.0.3/README.md). @@ -2551,6 +2551,9 @@ Built-in formatters are the following. * `:serializable_hash`: use object's `serializable_hash` when available, otherwise fallback to `:json` * `:binary`: data will be returned "as is" +If a body is present in a request to an API, with a Content-Type header value that is of an unsupported type a +"415 Unsupported Media Type" error code will be returned by Grape. + Response statuses that indicate no content as defined by [Rack](https://github.com/rack) [here](https://github.com/rack/rack/blob/master/lib/rack/utils.rb#L567) will bypass serialization and the body entity - though there should be none - diff --git a/UPGRADING.md b/UPGRADING.md index 41cd80eb40..d63fa23e64 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,6 +1,12 @@ Upgrading Grape =============== +### Upgrading to >= 1.1.0 + +#### Changes in HTTP Response Code for Unsupported Content Type + +For PUT, POST, PATCH, and DELETE requests where a non-empty body and a "Content-Type" header is supplied that is not supported by the Grape API, Grape will no longer return a 406 "Not Acceptable" HTTP status code and will instead return a 415 "Unsupported Media Type" so that the usage of HTTP status code falls more in line with the specification of [RFC 2616](https://www.ietf.org/rfc/rfc2616.txt). + ### Upgrading to >= 1.0.0 #### Changes in XML and JSON Parsers diff --git a/gemfiles/multi_json.gemfile b/gemfiles/multi_json.gemfile index 90f62fa9b1..af5797e5c5 100644 --- a/gemfiles/multi_json.gemfile +++ b/gemfiles/multi_json.gemfile @@ -22,7 +22,7 @@ end group :test do gem 'cookiejar' gem 'coveralls', '~> 0.8.17', require: false - gem 'danger-toc', '~> 0.1.0' + gem 'danger-toc', '~> 0.1.2' gem 'grape-entity', '~> 0.6' gem 'maruku' gem 'mime-types' diff --git a/gemfiles/multi_xml.gemfile b/gemfiles/multi_xml.gemfile index f3391d7eb3..b323389cfb 100644 --- a/gemfiles/multi_xml.gemfile +++ b/gemfiles/multi_xml.gemfile @@ -22,7 +22,7 @@ end group :test do gem 'cookiejar' gem 'coveralls', '~> 0.8.17', require: false - gem 'danger-toc', '~> 0.1.0' + gem 'danger-toc', '~> 0.1.2' gem 'grape-entity', '~> 0.6' gem 'maruku' gem 'mime-types' diff --git a/gemfiles/rack_1.5.2.gemfile b/gemfiles/rack_1.5.2.gemfile index 8b4ed8f396..a9b955f017 100644 --- a/gemfiles/rack_1.5.2.gemfile +++ b/gemfiles/rack_1.5.2.gemfile @@ -22,7 +22,7 @@ end group :test do gem 'cookiejar' gem 'coveralls', '~> 0.8.17', require: false - gem 'danger-toc', '~> 0.1.0' + gem 'danger-toc', '~> 0.1.2' gem 'grape-entity', '~> 0.6' gem 'maruku' gem 'mime-types' diff --git a/gemfiles/rack_edge.gemfile b/gemfiles/rack_edge.gemfile index c614016562..ee2b967748 100644 --- a/gemfiles/rack_edge.gemfile +++ b/gemfiles/rack_edge.gemfile @@ -22,7 +22,7 @@ end group :test do gem 'cookiejar' gem 'coveralls', '~> 0.8.17', require: false - gem 'danger-toc', '~> 0.1.0' + gem 'danger-toc', '~> 0.1.2' gem 'grape-entity', '~> 0.6' gem 'maruku' gem 'mime-types' diff --git a/gemfiles/rails_3.gemfile b/gemfiles/rails_3.gemfile index 4110319231..5c70608a9a 100644 --- a/gemfiles/rails_3.gemfile +++ b/gemfiles/rails_3.gemfile @@ -23,7 +23,7 @@ end group :test do gem 'cookiejar' gem 'coveralls', '~> 0.8.17', require: false - gem 'danger-toc', '~> 0.1.0' + gem 'danger-toc', '~> 0.1.2' gem 'grape-entity', '~> 0.6' gem 'maruku' gem 'mime-types' diff --git a/gemfiles/rails_4.gemfile b/gemfiles/rails_4.gemfile index 67656db8e8..7a4d248c1d 100644 --- a/gemfiles/rails_4.gemfile +++ b/gemfiles/rails_4.gemfile @@ -22,7 +22,7 @@ end group :test do gem 'cookiejar' gem 'coveralls', '~> 0.8.17', require: false - gem 'danger-toc', '~> 0.1.0' + gem 'danger-toc', '~> 0.1.2' gem 'grape-entity', '~> 0.6' gem 'maruku' gem 'mime-types' diff --git a/gemfiles/rails_5.gemfile b/gemfiles/rails_5.gemfile index f04ab8d67a..2b59c1add2 100644 --- a/gemfiles/rails_5.gemfile +++ b/gemfiles/rails_5.gemfile @@ -22,7 +22,7 @@ end group :test do gem 'cookiejar' gem 'coveralls', '~> 0.8.17', require: false - gem 'danger-toc', '~> 0.1.0' + gem 'danger-toc', '~> 0.1.2' gem 'grape-entity', '~> 0.6' gem 'maruku' gem 'mime-types' diff --git a/gemfiles/rails_edge.gemfile b/gemfiles/rails_edge.gemfile index 0e4c76fc2c..bce0e1942d 100644 --- a/gemfiles/rails_edge.gemfile +++ b/gemfiles/rails_edge.gemfile @@ -22,7 +22,7 @@ end group :test do gem 'cookiejar' gem 'coveralls', '~> 0.8.17', require: false - gem 'danger-toc', '~> 0.1.0' + gem 'danger-toc', '~> 0.1.2' gem 'grape-entity', '~> 0.6' gem 'maruku' gem 'mime-types' diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index a3ab82b4cb..4f5fccc2ec 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -95,7 +95,7 @@ def read_rack_input(body) 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." + throw :error, status: 415, message: "The provided content-type '#{request.media_type}' is not supported." end parser = Grape::Parser.parser_for fmt, options if parser diff --git a/lib/grape/version.rb b/lib/grape/version.rb index b244ba4357..47a2dda13a 100644 --- a/lib/grape/version.rb +++ b/lib/grape/version.rb @@ -1,4 +1,4 @@ module Grape # The current version of Grape. - VERSION = '1.0.4'.freeze + VERSION = '1.1.0'.freeze end diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index a736efe045..5614eea76e 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -941,15 +941,15 @@ def app end end - it 'responds with a 406 for an unsupported content-type' do + it 'responds with a 415 for an unsupported content-type' do subject.format :json # subject.content_type :json, "application/json" subject.put '/request_body' do params[:user] end put '/request_body', 'Bobby T.', 'CONTENT_TYPE' => 'application/xml' - expect(last_response.status).to eq(406) - expect(last_response.body).to eq('{"error":"The requested content-type \'application/xml\' is not supported."}') + expect(last_response.status).to eq(415) + expect(last_response.body).to eq('{"error":"The provided 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 @@ -960,8 +960,8 @@ def app end put '/request_body', ::Grape::Json.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."}') + expect(last_response.status).to eq(415) + expect(last_response.body).to eq('{"error":"The provided content-type \'text/plain\' is not supported."}') end context 'content type with params' do diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index dafef25a85..aaa0488acf 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -224,6 +224,80 @@ def to_xml context 'input' do %w[POST PATCH PUT DELETE].each do |method| + context 'when body is not nil or empty' do + context 'when Content-Type is supported' do + let(:io) { StringIO.new('{"is_boolean":true,"string":"thing"}') } + let(:content_type) { 'application/json' } + + it "parses the body from #{method} and copies values into rack.request.form_hash" do + subject.call( + 'PATH_INFO' => '/info', + 'REQUEST_METHOD' => method, + 'CONTENT_TYPE' => content_type, + 'rack.input' => io, + 'CONTENT_LENGTH' => io.length + ) + expect(subject.env['rack.request.form_hash']['is_boolean']).to be true + expect(subject.env['rack.request.form_hash']['string']).to eq('thing') + end + end + + context 'when Content-Type is not supported' do + let(:io) { StringIO.new('{"is_boolean":true,"string":"thing"}') } + let(:content_type) { 'application/atom+xml' } + + it 'returns a 415 HTTP error status' do + error = catch(:error) { + subject.call( + 'PATH_INFO' => '/info', + 'REQUEST_METHOD' => method, + 'CONTENT_TYPE' => content_type, + 'rack.input' => io, + 'CONTENT_LENGTH' => io.length + ) + } + expect(error[:status]).to eq(415) + expect(error[:message]).to eq("The provided content-type 'application/atom+xml' is not supported.") + end + end + end + + context 'when body is nil' do + let(:io) { double } + before do + allow(io).to receive_message_chain(:rewind, :read).and_return(nil) + end + + it 'does not read and parse the body' do + expect(subject).not_to receive(:read_rack_input) + subject.call( + 'PATH_INFO' => '/info', + 'REQUEST_METHOD' => method, + 'CONTENT_TYPE' => 'application/json', + 'rack.input' => io, + 'CONTENT_LENGTH' => 0 + ) + end + end + + context 'when body is empty' do + let(:io) { double } + before do + allow(io).to receive_message_chain(:rewind, :read).and_return('') + end + + it 'does not read and parse the body' do + expect(subject).not_to receive(:read_rack_input) + subject.call( + 'PATH_INFO' => '/info', + 'REQUEST_METHOD' => method, + 'CONTENT_TYPE' => 'application/json', + 'rack.input' => io, + 'CONTENT_LENGTH' => 0 + ) + end + end + ['application/json', 'application/json; charset=utf-8'].each do |content_type| context content_type do it "parses the body from #{method} and copies values into rack.request.form_hash" do