Skip to content

Commit

Permalink
Use 415 status code when content type is not supported
Browse files Browse the repository at this point in the history
  • Loading branch information
Josh Murphy committed Jun 5, 2018
1 parent d845a3a commit 6209297
Show file tree
Hide file tree
Showing 15 changed files with 101 additions and 17 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
### 1.0.4 (Next)
### 1.1.0 (Next)

#### Features

Expand All @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down Expand Up @@ -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 -
Expand Down
6 changes: 6 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/multi_json.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/multi_xml.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rack_1.5.2.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rack_edge.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rails_3.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rails_4.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rails_5.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/rails_edge.gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/middleware/formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/version.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module Grape
# The current version of Grape.
VERSION = '1.0.4'.freeze
VERSION = '1.1.0'.freeze
end
10 changes: 5 additions & 5 deletions spec/grape/endpoint_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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', '<user>Bobby T.</user>', '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
Expand All @@ -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
Expand Down
74 changes: 74 additions & 0 deletions spec/grape/middleware/formatter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 6209297

Please sign in to comment.