From 2f62365d4c526fe436019478a29d9c0982bc6596 Mon Sep 17 00:00:00 2001 From: Stuart Chinery <163900+schinery@users.noreply.github.com> Date: Tue, 24 Oct 2023 14:22:09 +0100 Subject: [PATCH] Set response headers based on Rack version (#2355) * Set response headers based on Rack version * Use Rack.release instead of Rack::RELEASE * Bumped to 1.9.0 and updated UPGRADING.md * Added .rack3? method * Removed headers_helper * Updated README and UPGRADING --- .github/workflows/test.yml | 10 ++++ .rubocop_todo.yml | 2 + CHANGELOG.md | 5 +- README.md | 7 +-- UPGRADING.md | 29 +++++++++++ lib/grape.rb | 4 ++ lib/grape/dsl/inside_route.rb | 8 +-- lib/grape/endpoint.rb | 2 +- lib/grape/http/headers.rb | 20 ++++++- lib/grape/request.rb | 10 +++- lib/grape/version.rb | 2 +- spec/grape/api/custom_validations_spec.rb | 12 +++-- spec/grape/api_spec.rb | 52 +++++++++---------- spec/grape/dsl/inside_route_spec.rb | 44 ++++++++-------- spec/grape/endpoint_spec.rb | 8 +-- .../exceptions/invalid_accept_header_spec.rb | 4 +- .../versioner/accept_version_header_spec.rb | 6 +-- .../grape/middleware/versioner/header_spec.rb | 12 ++--- spec/grape/request_spec.rb | 10 +++- spec/integration/rack/v2/headers_spec.rb | 11 ++++ spec/integration/rack/v3/headers_spec.rb | 11 ++++ 21 files changed, 187 insertions(+), 82 deletions(-) create mode 100644 spec/integration/rack/v2/headers_spec.rb create mode 100644 spec/integration/rack/v3/headers_spec.rb diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 40bd96b671..150f588407 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -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: diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index b2ba49521c..57e7a83bd8 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -272,6 +272,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. diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e1200ea8a..54906d7d12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,12 @@ -### 1.8.1 (Next) +### 1.9.0 (Next) #### 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). * [#2360](https://github.com/ruby-grape/grape/pull/2360): Reduce gem size by removing specs - [@ericproulx](https://github.com/ericproulx). * Your contribution here. - + #### Fixes * Your contribution here. diff --git a/README.md b/README.md index b0fe3375f5..0e28d24c1f 100644 --- a/README.md +++ b/README.md @@ -160,7 +160,7 @@ content negotiation, versioning and much more. ## Stable Release -You're reading the documentation for the next release of Grape, which should be **1.8.1**. +You're reading the documentation for the next release of Grape, which should be **1.9.0**. Please read [UPGRADING](UPGRADING.md) when upgrading from a previous version. The current stable release is [1.8.0](https://github.com/ruby-grape/grape/blob/v1.8.0/README.md). @@ -2130,8 +2130,9 @@ curl -H "secret_PassWord: swordfish" ... The header name will have been normalized for you. -- In the `header` helper names will be coerced into a capitalized kebab case. -- In the `env` collection they appear in all uppercase, in snake case, and prefixed with 'HTTP_'. +- In the `header` helper names will be coerced into a downcased kebab case as `secret-password` if using Rack 3. +- In the `header` helper names will be coerced into a capitalized kebab case as `Secret-PassWord` if using Rack < 3. +- In the `env` collection they appear in all uppercase, in snake case, and prefixed with 'HTTP_' as `HTTP_SECRET_PASSWORD` The header name will have been normalized per HTTP standards defined in [RFC2616 Section 4.2](https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2) regardless of what is being sent by a client. diff --git a/UPGRADING.md b/UPGRADING.md index 620e1d6ec9..c2f856c8f9 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,6 +1,35 @@ Upgrading Grape =============== +### Upgrading to >= 1.9.0 + +#### Headers + +As per [rack/rack#1592](https://github.com/rack/rack/issues/1592) Rack 3.0 is enforcing the HTTP/2 semantics, and thus treats all headers as lowercase. Starting with Grape 1.9.0, headers will be cased based on what version of Rack you are using. + +Given this request: + +```shell +curl -H "Content-Type: application/json" -H "Secret-Password: foo" ... +``` + +If you are using Rack 3 in your application then the headers will be set to: + +```ruby +{ "content-type" => "application/json", "secret-password" => "foo"} +``` + +This means if you are checking for header values in your application, you would need to change your code to use downcased keys. + +```ruby +get do + # This would use headers['Secret-Password'] in Rack < 3 + error!('Unauthorized', 401) unless headers['secret-password'] == 'swordfish' +end +``` + +See [#2355](https://github.com/ruby-grape/grape/pull/2355) for more information. + ### Upgrading to >= 1.7.0 #### Exceptions renaming diff --git a/lib/grape.rb b/lib/grape.rb index 680bea819f..f4ec5701fa 100644 --- a/lib/grape.rb +++ b/lib/grape.rb @@ -42,6 +42,10 @@ def self.deprecator @deprecator ||= ActiveSupport::Deprecation.new('2.0', 'Grape') end + def self.rack3? + Gem::Version.new(::Rack.release) >= Gem::Version.new('3') + end + eager_autoload do autoload :API autoload :Endpoint diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index 7023d69dd9..e2bc9169ad 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -185,7 +185,7 @@ def redirect(url, permanent: false, body: nil, **_options) status 302 body_message ||= "This resource has been moved temporarily to #{url}." end - header 'Location', url + header Grape::Http::Headers::LOCATION, url content_type 'text/plain' body body_message end @@ -328,9 +328,9 @@ def sendfile(value = nil) def stream(value = nil) return if value.nil? && @stream.nil? - header 'Content-Length', nil - header 'Transfer-Encoding', nil - header 'Cache-Control', 'no-cache' # Skips ETag generation (reading the response up front) + header Grape::Http::Headers::CONTENT_LENGTH, nil + header Grape::Http::Headers::TRANSFER_ENCODING, nil + header Grape::Http::Headers::CACHE_CONTROL, 'no-cache' # Skips ETag generation (reading the response up front) if value.is_a?(String) file_body = Grape::ServeStream::FileBody.new(value) @stream = Grape::ServeStream::StreamResponse.new(file_body) diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 8dfe915bce..663a3e8f1d 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -250,7 +250,7 @@ def run if (allowed_methods = env[Grape::Env::GRAPE_ALLOWED_METHODS]) raise Grape::Exceptions::MethodNotAllowed.new(header.merge('Allow' => allowed_methods)) unless options? - header 'Allow', allowed_methods + header Grape::Http::Headers::ALLOW, allowed_methods response_object = '' status 204 else diff --git a/lib/grape/http/headers.rb b/lib/grape/http/headers.rb index 564d97ff8e..442f6b65cc 100644 --- a/lib/grape/http/headers.rb +++ b/lib/grape/http/headers.rb @@ -10,7 +10,24 @@ module Headers PATH_INFO = 'PATH_INFO' REQUEST_METHOD = 'REQUEST_METHOD' QUERY_STRING = 'QUERY_STRING' - CONTENT_TYPE = 'Content-Type' + + if Grape.rack3? + ALLOW = 'allow' + CACHE_CONTROL = 'cache-control' + CONTENT_LENGTH = 'content-length' + CONTENT_TYPE = 'content-type' + LOCATION = 'location' + TRANSFER_ENCODING = 'transfer-encoding' + X_CASCADE = 'x-cascade' + else + ALLOW = 'Allow' + CACHE_CONTROL = 'Cache-Control' + CONTENT_LENGTH = 'Content-Length' + CONTENT_TYPE = 'Content-Type' + LOCATION = 'Location' + TRANSFER_ENCODING = 'Transfer-Encoding' + X_CASCADE = 'X-Cascade' + end GET = 'GET' POST = 'POST' @@ -24,7 +41,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' diff --git a/lib/grape/request.rb b/lib/grape/request.rb index 10fd28cc62..f522ea923b 100644 --- a/lib/grape/request.rb +++ b/lib/grape/request.rb @@ -46,8 +46,14 @@ def build_headers end end - def transform_header(header) - -header[5..].split('_').each(&:capitalize!).join('-') + if Grape.rack3? + def transform_header(header) + -header[5..].tr('_', '-').downcase + end + else + def transform_header(header) + -header[5..].split('_').map(&:capitalize).join('-') + end end end end diff --git a/lib/grape/version.rb b/lib/grape/version.rb index d0632f9dfd..9b8501a1ed 100644 --- a/lib/grape/version.rb +++ b/lib/grape/version.rb @@ -2,5 +2,5 @@ module Grape # The current version of Grape. - VERSION = '1.8.1' + VERSION = '1.9.0' end diff --git a/spec/grape/api/custom_validations_spec.rb b/spec/grape/api/custom_validations_spec.rb index bfb821de86..121e44f918 100644 --- a/spec/grape/api/custom_validations_spec.rb +++ b/spec/grape/api/custom_validations_spec.rb @@ -162,13 +162,19 @@ def validate(request) return unless request.params.key? @attrs.first # check if admin flag is set to true return unless @option + # check if user is admin or not # as an example get a token from request and check if it's admin or not - raise Grape::Exceptions::Validation.new(params: @attrs, message: 'Can not set Admin only field.') unless request.headers['X-Access-Token'] == 'admin' + raise Grape::Exceptions::Validation.new(params: @attrs, message: 'Can not set Admin only field.') unless request.headers[access_header] == 'admin' + end + + def access_header + Grape.rack3? ? 'x-access-token' : 'X-Access-Token' end end end let(:app) { Rack::Builder.new(subject) } + let(:x_access_token_header) { Grape.rack3? ? 'x-access-token' : 'X-Access-Token' } before do described_class.register_validator('admin', admin_validator) @@ -197,14 +203,14 @@ def validate(request) end it 'does not fail when we send admin fields and we are admin' do - header 'X-Access-Token', 'admin' + header x_access_token_header, 'admin' get '/', admin_field: 'tester', non_admin_field: 'toaster', admin_false_field: 'test' expect(last_response.status).to eq 200 expect(last_response.body).to eq 'bacon' end it 'fails when we send admin fields and we are not admin' do - header 'X-Access-Token', 'user' + header x_access_token_header, 'user' get '/', admin_field: 'tester', non_admin_field: 'toaster', admin_false_field: 'test' expect(last_response.status).to eq 400 expect(last_response.body).to include 'Can not set Admin only field.' diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index c02e0b6e88..5e31f51cfc 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -689,7 +689,7 @@ class DummyFormatClass 'example' end put '/example' - expect(last_response.headers['Content-Type']).to eql 'text/plain' + expect(last_response.headers[Grape::Http::Headers::CONTENT_TYPE]).to eql 'text/plain' end describe 'adds an OPTIONS route that' do @@ -1196,32 +1196,32 @@ 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[Grape::Http::Headers::CONTENT_TYPE]).to eq('text/plain') end it 'does not set Cache-Control' do get '/foo' - expect(last_response.headers['Cache-Control']).to be_nil + expect(last_response.headers[Grape::Http::Headers::CACHE_CONTROL]).to be_nil end it 'sets content type for xml' do get '/foo.xml' - expect(last_response.headers['Content-Type']).to eq('application/xml') + expect(last_response.headers[Grape::Http::Headers::CONTENT_TYPE]).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[Grape::Http::Headers::CONTENT_TYPE]).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[Grape::Http::Headers::CONTENT_TYPE]).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[Grape::Http::Headers::CONTENT_TYPE]).to eq('application/octet-stream') end it 'returns raw data when content type binary' do @@ -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[Grape::Http::Headers::CONTENT_TYPE]).to eq('application/octet-stream') expect(last_response.body).to eq(file) end @@ -1242,8 +1242,8 @@ 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[Grape::Http::Headers::CONTENT_LENGTH]).to eq('25') + expect(last_response.headers[Grape::Http::Headers::CONTENT_TYPE]).to eq('text/plain') expect(last_response.body).to eq(file_content) end @@ -1257,10 +1257,10 @@ 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-Length']).to be_nil - expect(last_response.headers['Cache-Control']).to eq('no-cache') - expect(last_response.headers['Transfer-Encoding']).to eq('chunked') + expect(last_response.headers[Grape::Http::Headers::CONTENT_TYPE]).to eq('text/plain') + expect(last_response.headers[Grape::Http::Headers::CONTENT_LENGTH]).to be_nil + expect(last_response.headers[Grape::Http::Headers::CACHE_CONTROL]).to eq('no-cache') + expect(last_response.headers[Grape::Http::Headers::TRANSFER_ENCODING]).to eq('chunked') expect(last_response.body).to eq("c\r\nThis is some\r\nd\r\n file content\r\n0\r\n\r\n") end @@ -1268,7 +1268,7 @@ 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[Grape::Http::Headers::CONTENT_TYPE]).to eql 'text/plain' end it 'sets content type for json error' do @@ -1276,7 +1276,7 @@ class DummyFormatClass 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[Grape::Http::Headers::CONTENT_TYPE]).to eql 'application/json' end it 'sets content type for xml error' do @@ -1284,7 +1284,7 @@ class DummyFormatClass 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[Grape::Http::Headers::CONTENT_TYPE]).to eql 'application/xml' end it 'includes extension in format' do @@ -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[Grape::Http::Headers::CONTENT_TYPE]).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[Grape::Http::Headers::CONTENT_TYPE]).to eql 'application/custom' end end @@ -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[Grape::Http::Headers::CONTENT_TYPE]).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 @@ -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[Grape::Http::Headers::CONTENT_TYPE]).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 @@ -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[Grape::Http::Headers::X_CASCADE] == 'pass' if env['PATH_INFO'].exclude?('boo') [200, headers, ['Farfegnugen']] } => '/' @@ -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[Grape::Http::Headers::X_CASCADE]).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 Grape::Http::Headers::X_CASCADE end end @@ -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[Grape::Http::Headers::X_CASCADE]).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 Grape::Http::Headers::X_CASCADE end end end diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index 9547af46b2..6dff2d15c8 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -73,7 +73,7 @@ def initialize end it 'sets location header' do - expect(subject.header['Location']).to eq '/' + expect(subject.header[Grape::Http::Headers::LOCATION]).to eq '/' end end @@ -87,7 +87,7 @@ def initialize end it 'sets location header' do - expect(subject.header['Location']).to eq '/' + expect(subject.header[Grape::Http::Headers::LOCATION]).to eq '/' end end end @@ -263,9 +263,9 @@ def initialize end before do - subject.header 'Cache-Control', 'cache' - subject.header 'Content-Length', 123 - subject.header 'Transfer-Encoding', 'base64' + subject.header Grape::Http::Headers::CACHE_CONTROL, 'cache' + subject.header Grape::Http::Headers::CONTENT_LENGTH, 123 + subject.header Grape::Http::Headers::TRANSFER_ENCODING, 'base64' end it 'sends no deprecation warnings' do @@ -283,19 +283,19 @@ def initialize it 'does not change the Cache-Control header' do subject.sendfile file_path - expect(subject.header['Cache-Control']).to eq 'cache' + expect(subject.header[Grape::Http::Headers::CACHE_CONTROL]).to eq 'cache' end it 'does not change the Content-Length header' do subject.sendfile file_path - expect(subject.header['Content-Length']).to eq 123 + expect(subject.header[Grape::Http::Headers::CONTENT_LENGTH]).to eq 123 end it 'does not change the Transfer-Encoding header' do subject.sendfile file_path - expect(subject.header['Transfer-Encoding']).to eq 'base64' + expect(subject.header[Grape::Http::Headers::TRANSFER_ENCODING]).to eq 'base64' end end @@ -324,9 +324,9 @@ def initialize end before do - subject.header 'Cache-Control', 'cache' - subject.header 'Content-Length', 123 - subject.header 'Transfer-Encoding', 'base64' + subject.header Grape::Http::Headers::CACHE_CONTROL, 'cache' + subject.header Grape::Http::Headers::CONTENT_LENGTH, 123 + subject.header Grape::Http::Headers::TRANSFER_ENCODING, 'base64' end it 'emits no deprecation warnings' do @@ -344,25 +344,25 @@ def initialize it 'sets Cache-Control header to no-cache' do subject.stream file_path - expect(subject.header['Cache-Control']).to eq 'no-cache' + expect(subject.header[Grape::Http::Headers::CACHE_CONTROL]).to eq 'no-cache' end it 'does not change Cache-Control header' do subject.stream - expect(subject.header['Cache-Control']).to eq 'cache' + expect(subject.header[Grape::Http::Headers::CACHE_CONTROL]).to eq 'cache' end it 'sets Content-Length header to nil' do subject.stream file_path - expect(subject.header['Content-Length']).to be_nil + expect(subject.header[Grape::Http::Headers::CONTENT_LENGTH]).to be_nil end it 'sets Transfer-Encoding header to nil' do subject.stream file_path - expect(subject.header['Transfer-Encoding']).to be_nil + expect(subject.header[Grape::Http::Headers::TRANSFER_ENCODING]).to be_nil end end @@ -374,9 +374,9 @@ def initialize end before do - subject.header 'Cache-Control', 'cache' - subject.header 'Content-Length', 123 - subject.header 'Transfer-Encoding', 'base64' + subject.header Grape::Http::Headers::CACHE_CONTROL, 'cache' + subject.header Grape::Http::Headers::CONTENT_LENGTH, 123 + subject.header Grape::Http::Headers::TRANSFER_ENCODING, 'base64' end it 'emits no deprecation warnings' do @@ -394,19 +394,19 @@ def initialize it 'sets Cache-Control header to no-cache' do subject.stream stream_object - expect(subject.header['Cache-Control']).to eq 'no-cache' + expect(subject.header[Grape::Http::Headers::CACHE_CONTROL]).to eq 'no-cache' end it 'sets Content-Length header to nil' do subject.stream stream_object - expect(subject.header['Content-Length']).to be_nil + expect(subject.header[Grape::Http::Headers::CONTENT_LENGTH]).to be_nil end it 'sets Transfer-Encoding header to nil' do subject.stream stream_object - expect(subject.header['Transfer-Encoding']).to be_nil + expect(subject.header[Grape::Http::Headers::TRANSFER_ENCODING]).to be_nil end end @@ -421,7 +421,7 @@ def initialize it 'returns default' do expect(subject.stream).to be_nil - expect(subject.header['Cache-Control']).to be_nil + expect(subject.header[Grape::Http::Headers::CACHE_CONTROL]).to be_nil end end diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index 6eb37227f3..e81fbc806d 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -146,14 +146,16 @@ def app it 'includes additional request headers' do get '/headers', nil, 'HTTP_X_GRAPE_CLIENT' => '1' - expect(JSON.parse(last_response.body)['X-Grape-Client']).to eq('1') + x_grape_client_header = Grape.rack3? ? 'x-grape-client' : 'X-Grape-Client' + expect(JSON.parse(last_response.body)[x_grape_client_header]).to eq('1') end it 'includes headers passed as symbols' do env = Rack::MockRequest.env_for('/headers') env[:HTTP_SYMBOL_HEADER] = 'Goliath passes symbols' body = read_chunks(subject.call(env)[2]).join - expect(JSON.parse(body)['Symbol-Header']).to eq('Goliath passes symbols') + symbol_header = Grape.rack3? ? 'symbol-header' : 'Symbol-Header' + expect(JSON.parse(body)[symbol_header]).to eq('Goliath passes symbols') end end @@ -497,7 +499,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[Grape::Http::Headers::CONTENT_TYPE]).to eq 'application/json; charset=utf-8' end end diff --git a/spec/grape/exceptions/invalid_accept_header_spec.rb b/spec/grape/exceptions/invalid_accept_header_spec.rb index 5017807cbf..ca4aec5ec7 100644 --- a/spec/grape/exceptions/invalid_accept_header_spec.rb +++ b/spec/grape/exceptions/invalid_accept_header_spec.rb @@ -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[Grape::Http::Headers::X_CASCADE]).to be_nil end it 'does not accept the request' do @@ -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[Grape::Http::Headers::X_CASCADE]).to be_nil end it 'does show rescue handler processing' do diff --git a/spec/grape/middleware/versioner/accept_version_header_spec.rb b/spec/grape/middleware/versioner/accept_version_header_spec.rb index a67c26f249..c2a66f2157 100644 --- a/spec/grape/middleware/versioner/accept_version_header_spec.rb +++ b/spec/grape/middleware/versioner/accept_version_header_spec.rb @@ -36,7 +36,7 @@ end.to throw_symbol( :error, status: 406, - headers: { 'X-Cascade' => 'pass' }, + headers: { Grape::Http::Headers::X_CASCADE => 'pass' }, message: 'The requested version is not supported.' ) end @@ -65,7 +65,7 @@ end.to throw_symbol( :error, status: 406, - headers: { 'X-Cascade' => 'pass' }, + headers: { Grape::Http::Headers::X_CASCADE => 'pass' }, message: 'Accept-Version header must be set.' ) end @@ -76,7 +76,7 @@ end.to throw_symbol( :error, status: 406, - headers: { 'X-Cascade' => 'pass' }, + headers: { Grape::Http::Headers::X_CASCADE => 'pass' }, message: 'Accept-Version header must be set.' ) end diff --git a/spec/grape/middleware/versioner/header_spec.rb b/spec/grape/middleware/versioner/header_spec.rb index 29fa056ca4..ec116a7419 100644 --- a/spec/grape/middleware/versioner/header_spec.rb +++ b/spec/grape/middleware/versioner/header_spec.rb @@ -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(Grape::Http::Headers::X_CASCADE => 'pass') expect(exception.status).to be 406 expect(exception.message).to include 'API vendor not found' end @@ -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(Grape::Http::Headers::X_CASCADE => 'pass') expect(exception.status).to be 406 expect(exception.message).to include('API vendor not found') end @@ -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(Grape::Http::Headers::X_CASCADE => 'pass') expect(exception.status).to be 406 expect(exception.message).to include('API version not found') end @@ -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(Grape::Http::Headers::X_CASCADE => 'pass') expect(exception.status).to be 406 expect(exception.message).to include('Accept header must be set.') end @@ -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(Grape::Http::Headers::X_CASCADE => 'pass') expect(exception.status).to be 406 expect(exception.message).to include('Accept header must be set.') end @@ -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(Grape::Http::Headers::X_CASCADE => 'pass') expect(exception.status).to be 406 expect(exception.message).to include('API version not found') end diff --git a/spec/grape/request_spec.rb b/spec/grape/request_spec.rb index 6a3f1948f0..3c0c9e2de3 100644 --- a/spec/grape/request_spec.rb +++ b/spec/grape/request_spec.rb @@ -89,9 +89,12 @@ module Grape 'HTTP_X_GRAPE_IS_COOL' => 'yeah' } end + let(:x_grape_is_cool_header) do + Grape.rack3? ? 'x-grape-is-cool' : 'X-Grape-Is-Cool' + end it 'cuts HTTP_ prefix and capitalizes header name words' do - expect(request.headers).to eq('X-Grape-Is-Cool' => 'yeah') + expect(request.headers).to eq(x_grape_is_cool_header => 'yeah') end end @@ -116,9 +119,12 @@ module Grape let(:env) do default_env.merge(request_headers) end + let(:grape_likes_symbolic_header) do + Grape.rack3? ? 'grape-likes-symbolic' : 'Grape-Likes-Symbolic' + end it 'converts them to string' do - expect(request.headers).to eq('Grape-Likes-Symbolic' => 'it is true') + expect(request.headers).to eq(grape_likes_symbolic_header => 'it is true') end end end diff --git a/spec/integration/rack/v2/headers_spec.rb b/spec/integration/rack/v2/headers_spec.rb new file mode 100644 index 0000000000..63c8e45727 --- /dev/null +++ b/spec/integration/rack/v2/headers_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +describe Grape::Http::Headers do + it { expect(described_class::ALLOW).to eq('Allow') } + it { expect(described_class::CACHE_CONTROL).to eq('Cache-Control') } + it { expect(described_class::CONTENT_LENGTH).to eq('Content-Length') } + it { expect(described_class::CONTENT_TYPE).to eq('Content-Type') } + it { expect(described_class::LOCATION).to eq('Location') } + it { expect(described_class::TRANSFER_ENCODING).to eq('Transfer-Encoding') } + it { expect(described_class::X_CASCADE).to eq('X-Cascade') } +end diff --git a/spec/integration/rack/v3/headers_spec.rb b/spec/integration/rack/v3/headers_spec.rb new file mode 100644 index 0000000000..3f5375e0c7 --- /dev/null +++ b/spec/integration/rack/v3/headers_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +describe Grape::Http::Headers do + it { expect(described_class::ALLOW).to eq('allow') } + it { expect(described_class::CACHE_CONTROL).to eq('cache-control') } + it { expect(described_class::CONTENT_LENGTH).to eq('content-length') } + it { expect(described_class::CONTENT_TYPE).to eq('content-type') } + it { expect(described_class::LOCATION).to eq('location') } + it { expect(described_class::TRANSFER_ENCODING).to eq('transfer-encoding') } + it { expect(described_class::X_CASCADE).to eq('x-cascade') } +end