Skip to content

Commit

Permalink
Set response headers based on Rack version
Browse files Browse the repository at this point in the history
  • Loading branch information
schinery committed Oct 16, 2023
1 parent 4e2a2ff commit 608bd18
Show file tree
Hide file tree
Showing 12 changed files with 81 additions and 35 deletions.
10 changes: 10 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ jobs:
if: ${{ matrix.gemfile == 'multi_xml' }}
run: bundle exec rspec spec/integration/multi_xml

- name: Run tests (spec/integration/rack/v2)
# rack_2_0.gemfile is equals to Gemfile
if: ${{ matrix.gemfile == 'rack_2_0' }}
run: bundle exec rspec spec/integration/rack/v2

- name: Run tests (spec/integration/rack/v3)
# rack_2_0.gemfile is equals to Gemfile
if: ${{ matrix.gemfile == 'rack_3_0' }}
run: bundle exec rspec spec/integration/rack/v3

- name: Coveralls
uses: coverallsapp/github-action@master
with:
Expand Down
2 changes: 2 additions & 0 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,8 @@ RSpec/FilePath:
- 'spec/integration/eager_load/eager_load_spec.rb'
- 'spec/integration/multi_json/json_spec.rb'
- 'spec/integration/multi_xml/xml_spec.rb'
- 'spec/integration/rack/v2/headers_spec.rb'
- 'spec/integration/rack/v3/headers_spec.rb'

# Offense count: 12
# Configuration parameters: Max.
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#### Features

* [#2353](https://github.com/ruby-grape/grape/pull/2353): Added Rails 7.1 support - [@ericproulx](https://github.com/ericproulx).
* [#2355](https://github.com/ruby-grape/grape/pull/2355): Set response headers based on Rack version - [@schinery](https://github.com/schinery).
* Your contribution here.

#### Fixes
Expand Down
10 changes: 8 additions & 2 deletions lib/grape/http/headers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,14 @@ module Headers
PATH_INFO = 'PATH_INFO'
REQUEST_METHOD = 'REQUEST_METHOD'
QUERY_STRING = 'QUERY_STRING'
CONTENT_TYPE = 'Content-Type'

if Gem::Version.new(Rack::RELEASE) < Gem::Version.new('3')
CONTENT_TYPE = 'Content-Type'
X_CASCADE = 'X-Cascade'
else
CONTENT_TYPE = 'content-type'
X_CASCADE = 'x-cascade'
end

GET = 'GET'
POST = 'POST'
Expand All @@ -24,7 +31,6 @@ module Headers
SUPPORTED_METHODS_WITHOUT_OPTIONS = Grape::Util::LazyObject.new { [GET, POST, PUT, PATCH, DELETE, HEAD].freeze }

HTTP_ACCEPT_VERSION = 'HTTP_ACCEPT_VERSION'
X_CASCADE = 'X-Cascade'
HTTP_TRANSFER_ENCODING = 'HTTP_TRANSFER_ENCODING'
HTTP_ACCEPT = 'HTTP_ACCEPT'

Expand Down
42 changes: 21 additions & 21 deletions spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ class DummyFormatClass
'example'
end
put '/example'
expect(last_response.headers['Content-Type']).to eql 'text/plain'
expect(last_response.headers[content_type_header]).to eql 'text/plain'
end

describe 'adds an OPTIONS route that' do
Expand Down Expand Up @@ -1196,7 +1196,7 @@ class DummyFormatClass

it 'sets content type for txt format' do
get '/foo'
expect(last_response.headers['Content-Type']).to eq('text/plain')
expect(last_response.headers[content_type_header]).to eq('text/plain')
end

it 'does not set Cache-Control' do
Expand All @@ -1206,22 +1206,22 @@ class DummyFormatClass

it 'sets content type for xml' do
get '/foo.xml'
expect(last_response.headers['Content-Type']).to eq('application/xml')
expect(last_response.headers[content_type_header]).to eq('application/xml')
end

it 'sets content type for json' do
get '/foo.json'
expect(last_response.headers['Content-Type']).to eq('application/json')
expect(last_response.headers[content_type_header]).to eq('application/json')
end

it 'sets content type for serializable hash format' do
get '/foo.serializable_hash'
expect(last_response.headers['Content-Type']).to eq('application/json')
expect(last_response.headers[content_type_header]).to eq('application/json')
end

it 'sets content type for binary format' do
get '/foo.binary'
expect(last_response.headers['Content-Type']).to eq('application/octet-stream')
expect(last_response.headers[content_type_header]).to eq('application/octet-stream')
end

it 'returns raw data when content type binary' do
Expand All @@ -1230,7 +1230,7 @@ class DummyFormatClass
subject.format :binary
subject.get('/binary_file') { File.binread(image_filename) }
get '/binary_file'
expect(last_response.headers['Content-Type']).to eq('application/octet-stream')
expect(last_response.headers[content_type_header]).to eq('application/octet-stream')
expect(last_response.body).to eq(file)
end

Expand All @@ -1243,7 +1243,7 @@ class DummyFormatClass
subject.get('/file') { file test_file }
get '/file'
expect(last_response.headers['Content-Length']).to eq('25')
expect(last_response.headers['Content-Type']).to eq('text/plain')
expect(last_response.headers[content_type_header]).to eq('text/plain')
expect(last_response.body).to eq(file_content)
end

Expand All @@ -1257,7 +1257,7 @@ class DummyFormatClass
subject.get('/stream') { stream test_stream }
get '/stream', {}, 'HTTP_VERSION' => 'HTTP/1.1', 'SERVER_PROTOCOL' => 'HTTP/1.1'

expect(last_response.headers['Content-Type']).to eq('text/plain')
expect(last_response.headers[content_type_header]).to eq('text/plain')
expect(last_response.headers['Content-Length']).to be_nil
expect(last_response.headers['Cache-Control']).to eq('no-cache')
expect(last_response.headers['Transfer-Encoding']).to eq('chunked')
Expand All @@ -1268,23 +1268,23 @@ class DummyFormatClass
it 'sets content type for error' do
subject.get('/error') { error!('error in plain text', 500) }
get '/error'
expect(last_response.headers['Content-Type']).to eql 'text/plain'
expect(last_response.headers[content_type_header]).to eql 'text/plain'
end

it 'sets content type for json error' do
subject.format :json
subject.get('/error') { error!('error in json', 500) }
get '/error.json'
expect(last_response.status).to be 500
expect(last_response.headers['Content-Type']).to eql 'application/json'
expect(last_response.headers[content_type_header]).to eql 'application/json'
end

it 'sets content type for xml error' do
subject.format :xml
subject.get('/error') { error!('error in xml', 500) }
get '/error'
expect(last_response.status).to be 500
expect(last_response.headers['Content-Type']).to eql 'application/xml'
expect(last_response.headers[content_type_header]).to eql 'application/xml'
end

it 'includes extension in format' do
Expand Down Expand Up @@ -1314,12 +1314,12 @@ class DummyFormatClass

it 'sets content type' do
get '/custom.custom'
expect(last_response.headers['Content-Type']).to eql 'application/custom'
expect(last_response.headers[content_type_header]).to eql 'application/custom'
end

it 'sets content type for error' do
get '/error.custom'
expect(last_response.headers['Content-Type']).to eql 'application/custom'
expect(last_response.headers[content_type_header]).to eql 'application/custom'
end
end

Expand All @@ -1339,7 +1339,7 @@ class DummyFormatClass
image_filename = 'grape.png'
post url, file: Rack::Test::UploadedFile.new(image_filename, 'image/png', true)
expect(last_response.status).to eq(201)
expect(last_response.headers['Content-Type']).to eq('image/png')
expect(last_response.headers[content_type_header]).to eq('image/png')
expect(last_response.headers['Content-Disposition']).to eq("attachment; filename*=UTF-8''grape.png")
File.open(image_filename, 'rb') do |io|
expect(last_response.body).to eq io.read
Expand All @@ -1351,7 +1351,7 @@ class DummyFormatClass
filename = __FILE__
post '/attachment.rb', file: Rack::Test::UploadedFile.new(filename, 'application/x-ruby', true)
expect(last_response.status).to eq(201)
expect(last_response.headers['Content-Type']).to eq('application/x-ruby')
expect(last_response.headers[content_type_header]).to eq('application/x-ruby')
expect(last_response.headers['Content-Disposition']).to eq("attachment; filename*=UTF-8''api_spec.rb")
File.open(filename, 'rb') do |io|
expect(last_response.body).to eq io.read
Expand Down Expand Up @@ -3311,7 +3311,7 @@ def static
it 'is able to cascade' do
subject.mount lambda { |env|
headers = {}
headers['X-Cascade'] == 'pass' if env['PATH_INFO'].exclude?('boo')
headers[x_cascade_header] == 'pass' if env['PATH_INFO'].exclude?('boo')
[200, headers, ['Farfegnugen']]
} => '/'

Expand Down Expand Up @@ -4081,14 +4081,14 @@ def before
subject.version 'v1', using: :path, cascade: true
get '/v1/hello'
expect(last_response.status).to eq(404)
expect(last_response.headers['X-Cascade']).to eq('pass')
expect(last_response.headers[x_cascade_header]).to eq('pass')
end

it 'does not cascade' do
subject.version 'v2', using: :path, cascade: false
get '/v2/hello'
expect(last_response.status).to eq(404)
expect(last_response.headers.keys).not_to include 'X-Cascade'
expect(last_response.headers.keys).not_to include x_cascade_header
end
end

Expand All @@ -4097,14 +4097,14 @@ def before
subject.cascade true
get '/hello'
expect(last_response.status).to eq(404)
expect(last_response.headers['X-Cascade']).to eq('pass')
expect(last_response.headers[x_cascade_header]).to eq('pass')
end

it 'does not cascade' do
subject.cascade false
get '/hello'
expect(last_response.status).to eq(404)
expect(last_response.headers.keys).not_to include 'X-Cascade'
expect(last_response.headers.keys).not_to include x_cascade_header
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/grape/endpoint_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ def app
end

it 'responses with given content type in headers' do
expect(last_response.headers['Content-Type']).to eq 'application/json; charset=utf-8'
expect(last_response.headers[content_type_header]).to eq 'application/json; charset=utf-8'
end
end

Expand Down
4 changes: 2 additions & 2 deletions spec/grape/exceptions/invalid_accept_header_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

shared_examples_for 'a not-cascaded request' do
it 'does not include the X-Cascade=pass header' do
expect(last_response.headers['X-Cascade']).to be_nil
expect(last_response.headers[x_cascade_header]).to be_nil
end

it 'does not accept the request' do
Expand All @@ -29,7 +29,7 @@

shared_examples_for 'a rescued request' do
it 'does not include the X-Cascade=pass header' do
expect(last_response.headers['X-Cascade']).to be_nil
expect(last_response.headers[x_cascade_header]).to be_nil
end

it 'does show rescue handler processing' do
Expand Down
6 changes: 3 additions & 3 deletions spec/grape/middleware/versioner/accept_version_header_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
end.to throw_symbol(
:error,
status: 406,
headers: { 'X-Cascade' => 'pass' },
headers: { x_cascade_header => 'pass' },
message: 'The requested version is not supported.'
)
end
Expand Down Expand Up @@ -65,7 +65,7 @@
end.to throw_symbol(
:error,
status: 406,
headers: { 'X-Cascade' => 'pass' },
headers: { x_cascade_header => 'pass' },
message: 'Accept-Version header must be set.'
)
end
Expand All @@ -76,7 +76,7 @@
end.to throw_symbol(
:error,
status: 406,
headers: { 'X-Cascade' => 'pass' },
headers: { x_cascade_header => 'pass' },
message: 'Accept-Version header must be set.'
)
end
Expand Down
12 changes: 6 additions & 6 deletions spec/grape/middleware/versioner/header_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
expect { subject.call('HTTP_ACCEPT' => 'application/vnd.othervendor+json').last }
.to raise_exception do |exception|
expect(exception).to be_a(Grape::Exceptions::InvalidAcceptHeader)
expect(exception.headers).to eql('X-Cascade' => 'pass')
expect(exception.headers).to eql(x_cascade_header => 'pass')
expect(exception.status).to be 406
expect(exception.message).to include 'API vendor not found'
end
Expand All @@ -115,7 +115,7 @@
expect { subject.call('HTTP_ACCEPT' => 'application/vnd.othervendor-v1+json').last }
.to raise_exception do |exception|
expect(exception).to be_a(Grape::Exceptions::InvalidAcceptHeader)
expect(exception.headers).to eql('X-Cascade' => 'pass')
expect(exception.headers).to eql(x_cascade_header => 'pass')
expect(exception.status).to be 406
expect(exception.message).to include('API vendor not found')
end
Expand Down Expand Up @@ -143,7 +143,7 @@
it 'fails with 406 Not Acceptable if version is invalid' do
expect { subject.call('HTTP_ACCEPT' => 'application/vnd.vendor-v2+json').last }.to raise_exception do |exception|
expect(exception).to be_a(Grape::Exceptions::InvalidVersionHeader)
expect(exception.headers).to eql('X-Cascade' => 'pass')
expect(exception.headers).to eql(x_cascade_header => 'pass')
expect(exception.status).to be 406
expect(exception.message).to include('API version not found')
end
Expand Down Expand Up @@ -176,7 +176,7 @@
it 'fails with 406 Not Acceptable if header is not set' do
expect { subject.call({}).last }.to raise_exception do |exception|
expect(exception).to be_a(Grape::Exceptions::InvalidAcceptHeader)
expect(exception.headers).to eql('X-Cascade' => 'pass')
expect(exception.headers).to eql(x_cascade_header => 'pass')
expect(exception.status).to be 406
expect(exception.message).to include('Accept header must be set.')
end
Expand All @@ -185,7 +185,7 @@
it 'fails with 406 Not Acceptable if header is empty' do
expect { subject.call('HTTP_ACCEPT' => '').last }.to raise_exception do |exception|
expect(exception).to be_a(Grape::Exceptions::InvalidAcceptHeader)
expect(exception.headers).to eql('X-Cascade' => 'pass')
expect(exception.headers).to eql(x_cascade_header => 'pass')
expect(exception.status).to be 406
expect(exception.message).to include('Accept header must be set.')
end
Expand Down Expand Up @@ -262,7 +262,7 @@
it 'fails with another version' do
expect { subject.call('HTTP_ACCEPT' => 'application/vnd.vendor-v3+json') }.to raise_exception do |exception|
expect(exception).to be_a(Grape::Exceptions::InvalidVersionHeader)
expect(exception.headers).to eql('X-Cascade' => 'pass')
expect(exception.headers).to eql(x_cascade_header => 'pass')
expect(exception.status).to be 406
expect(exception.message).to include('API version not found')
end
Expand Down
6 changes: 6 additions & 0 deletions spec/integration/rack/v2/headers_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# frozen_string_literal: true

describe Grape::Http::Headers do
it { expect(described_class::CONTENT_TYPE).to eq('Content-Type') }
it { expect(described_class::X_CASCADE).to eq('X-Cascade') }
end
6 changes: 6 additions & 0 deletions spec/integration/rack/v3/headers_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# frozen_string_literal: true

describe Grape::Http::Headers do
it { expect(described_class::CONTENT_TYPE).to eq('content-type') }
it { expect(described_class::X_CASCADE).to eq('x-cascade') }
end
15 changes: 15 additions & 0 deletions spec/support/headers_helpers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# frozen_string_literal: true

module Spec
module Support
module Helpers
def content_type_header
Grape::Http::Headers::CONTENT_TYPE
end

def x_cascade_header
Grape::Http::Headers::X_CASCADE
end
end
end
end

0 comments on commit 608bd18

Please sign in to comment.