Other changes needed to support both Rack 2 and 3 (#1)

schinery · Oct 23, 2023 · 5ecb7c7 · 5ecb7c7
1 parent ff4c6b5
commit 5ecb7c7
Show file tree

Hide file tree

Showing 14 changed files with 137 additions and 83 deletions.
diff --git a/UPGRADING.md b/UPGRADING.md
@@ -7,12 +7,22 @@ Upgrading Grape
 
 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, the following headers are now lowercase:
 
+* `allow`
+* `cache-control`
+* `content-length`
 * `content-type`
+* `location`
+* `transfer-encoding`
 * `x-cascade`
 
 For Rack < 3 the following response headers are returned using HTTP/1 semantics, like so:
 
+* `Allow`
+* `Cache-Control`
+* `Content-Length`
 * `Content-Type`
+* `Location`
+* `Transfer-Encoding`
 * `X-Cascade`
 
 See [#2355](https://github.com/ruby-grape/grape/pull/2355) for more information.

diff --git 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
@@ -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
@@ -12,11 +12,21 @@ module Headers
       QUERY_STRING    = 'QUERY_STRING'
 
       if Gem::Version.new(Rack.release) < Gem::Version.new('3')
-        CONTENT_TYPE = 'Content-Type'
-        X_CASCADE    = 'X-Cascade'
+        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
-        CONTENT_TYPE = 'content-type'
-        X_CASCADE    = 'x-cascade'
+        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'

diff --git a/lib/grape/request.rb b/lib/grape/request.rb
@@ -47,7 +47,11 @@ def build_headers
     end
 
     def transform_header(header)
-      -header[5..].split('_').each(&:capitalize!).join('-')
+      if Gem::Version.new(Rack.release) < Gem::Version.new('3')
+        -header[5..].split('_').map(&:capitalize).join('-')
+      else
+        -header[5..].tr('_', '-').downcase
+      end
     end
   end
 end
diff --git a/spec/grape/api/custom_validations_spec.rb b/spec/grape/api/custom_validations_spec.rb
@@ -162,9 +162,15 @@ 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'
+          header = if Gem::Version.new(Rack.release) < Gem::Version.new('3')
+                     'X-Access-Token'
+                   else
+                     'x-access-token'
+                   end
+          raise Grape::Exceptions::Validation.new(params: @attrs, message: 'Can not set Admin only field.') unless request.headers[header] == 'admin'
         end
       end
     end
@@ -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 rack_versioned_headers[:x_access_token], '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 rack_versioned_headers[:x_access_token], '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
@@ -689,7 +689,7 @@ class DummyFormatClass
         'example'
       end
       put '/example'
-      expect(last_response.headers[content_type_header]).to eql 'text/plain'
+      expect(last_response.headers[rack_versioned_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_header]).to eq('text/plain')
+      expect(last_response.headers[rack_versioned_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[rack_versioned_headers[:cache_control]]).to be_nil
     end
 
     it 'sets content type for xml' do
       get '/foo.xml'
-      expect(last_response.headers[content_type_header]).to eq('application/xml')
+      expect(last_response.headers[rack_versioned_headers[:content_type]]).to eq('application/xml')
     end
 
     it 'sets content type for json' do
       get '/foo.json'
-      expect(last_response.headers[content_type_header]).to eq('application/json')
+      expect(last_response.headers[rack_versioned_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_header]).to eq('application/json')
+      expect(last_response.headers[rack_versioned_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_header]).to eq('application/octet-stream')
+      expect(last_response.headers[rack_versioned_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_header]).to eq('application/octet-stream')
+      expect(last_response.headers[rack_versioned_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_header]).to eq('text/plain')
+      expect(last_response.headers[rack_versioned_headers[:content_length]]).to eq('25')
+      expect(last_response.headers[rack_versioned_headers[:content_type]]).to eq('text/plain')
       expect(last_response.body).to eq(file_content)
     end
 
@@ -1257,34 +1257,34 @@ 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_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')
+      expect(last_response.headers[rack_versioned_headers[:content_type]]).to eq('text/plain')
+      expect(last_response.headers[rack_versioned_headers[:content_length]]).to be_nil
+      expect(last_response.headers[rack_versioned_headers[:cache_control]]).to eq('no-cache')
+      expect(last_response.headers[rack_versioned_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
 
     it 'sets content type for error' do
       subject.get('/error') { error!('error in plain text', 500) }
       get '/error'
-      expect(last_response.headers[content_type_header]).to eql 'text/plain'
+      expect(last_response.headers[rack_versioned_headers[:content_type]]).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_header]).to eql 'application/json'
+      expect(last_response.headers[rack_versioned_headers[:content_type]]).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_header]).to eql 'application/xml'
+      expect(last_response.headers[rack_versioned_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_header]).to eql 'application/custom'
+        expect(last_response.headers[rack_versioned_headers[:content_type]]).to eql 'application/custom'
       end
 
       it 'sets content type for error' do
         get '/error.custom'
-        expect(last_response.headers[content_type_header]).to eql 'application/custom'
+        expect(last_response.headers[rack_versioned_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_header]).to eq('image/png')
+          expect(last_response.headers[rack_versioned_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_header]).to eq('application/x-ruby')
+        expect(last_response.headers[rack_versioned_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_header] == 'pass' if env['PATH_INFO'].exclude?('boo')
+          headers[rack_versioned_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_header]).to eq('pass')
+        expect(last_response.headers[rack_versioned_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_header
+        expect(last_response.headers.keys).not_to include rack_versioned_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_header]).to eq('pass')
+        expect(last_response.headers[rack_versioned_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_header
+        expect(last_response.headers.keys).not_to include rack_versioned_headers[:x_cascade]
       end
     end
   end