Add support for Rack 3, drop support for Rack 2 (#1857)

Close #1797 This work was sponsored by 84codes (https://84.codes/). Co-authored-by: Samuel Williams <samuel.williams@oriontransfer.co.nz> Co-authored-by: Eloy Pérez <ej.perezgomez@gmail.com>
sinatra · Jan 5, 2024 · 8a17d4b · 8a17d4b
1 parent 509a5a2
commit 8a17d4b
Show file tree

Hide file tree

Showing 40 changed files with 172 additions and 112 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -18,7 +18,7 @@ jobs:
       fail-fast: false
       matrix:
         rack:
-          - "~> 2"
+          - stable
         ruby:
           - "2.6"
           - "2.7"
@@ -67,29 +67,31 @@ jobs:
         puma:
           - stable
         rack:
-          - '~> 2'
+          - stable
         tilt:
           - stable
         # Due to https://github.com/actions/runner/issues/849, we have to use quotes for '3.0'
         ruby: [2.6, 2.7, '3.0', 3.1, 3.2, 3.3, truffleruby]
         include:
+          # Rack
+          - { ruby: 3.2, rack: head,   puma: stable,   tilt: stable, allow-failure: true }
           # Puma
-          - { ruby: 3.1, rack: '~> 2', puma: '~> 5',   tilt: stable }
-          - { ruby: 3.2, rack: '~> 2', puma: head,     tilt: stable, allow-failure: true }
+          - { ruby: 3.2, rack: stable, puma: head,     tilt: stable, allow-failure: true }
           # Tilt
-          - { ruby: 3.2, rack: '~> 2', puma: stable,   tilt: head, allow-failure: true }
+          - { ruby: 3.2, rack: stable, puma: stable,   tilt: head, allow-failure: true }
           # Due to flaky tests, see https://github.com/sinatra/sinatra/pull/1870
-          - { ruby: jruby-9.3, rack: '~> 2', puma: stable, tilt: stable, allow-failure: true }
+          - { ruby: jruby-9.3, rack: stable, puma: stable, tilt: stable, allow-failure: true }
           # Due to https://github.com/jruby/jruby/issues/7647
-          - { ruby: jruby-9.4, rack: '~> 2', puma: stable, tilt: stable, allow-failure: true }
+          - { ruby: jruby-9.4, rack: stable, puma: stable, tilt: stable, allow-failure: true }
           # Never fail our build due to problems with head
-          - { ruby: ruby-head,        rack: '~> 2', puma: stable, tilt: stable, allow-failure: true }
-          - { ruby: jruby-head,       rack: '~> 2', puma: stable, tilt: stable, allow-failure: true }
-          - { ruby: truffleruby-head, rack: '~> 2', puma: stable, tilt: stable, allow-failure: true }
+          - { ruby: ruby-head,        rack: stable, puma: stable, tilt: stable, allow-failure: true }
+          - { ruby: jruby-head,       rack: stable, puma: stable, tilt: stable, allow-failure: true }
+          - { ruby: truffleruby-head, rack: stable, puma: stable, tilt: stable, allow-failure: true }
     env:
       rack: ${{ matrix.rack }}
       puma: ${{ matrix.puma }}
       tilt: ${{ matrix.tilt }}
+
     steps:
     - name: Install dependencies
       run: |

diff --git a/.gitignore b/.gitignore
@@ -4,7 +4,7 @@
 *.swp
 *.rbc
 /pkg
-/Gemfile.lock
+*.lock
 /coverage
 .yardoc
 /doc

diff --git a/Gemfile b/Gemfile
@@ -17,6 +17,7 @@ rack_version = ENV['rack'].to_s
 rack_version = nil if rack_version.empty? || (rack_version == 'stable')
 rack_version = { github: 'rack/rack' } if rack_version == 'head'
 gem 'rack', rack_version
+gem 'rackup'
 
 puma_version = ENV['puma'].to_s
 puma_version = nil if puma_version.empty? || (puma_version == 'stable')

diff --git a/lib/sinatra/base.rb b/lib/sinatra/base.rb
@@ -2,6 +2,10 @@
 
 # external dependencies
 require 'rack'
+begin
+  require 'rackup'
+rescue LoadError
+end
 require 'tilt'
 require 'rack/protection'
 require 'mustermann'
@@ -176,8 +180,8 @@ def finish
       result = body
 
       if drop_content_info?
-        headers.delete 'Content-Length'
-        headers.delete 'Content-Type'
+        headers.delete 'content-length'
+        headers.delete 'content-type'
       end
 
       if drop_body?
@@ -186,9 +190,9 @@ def finish
       end
 
       if calculate_content_length?
-        # if some other code has already set Content-Length, don't muck with it
+        # if some other code has already set content-length, don't muck with it
         # currently, this would be the static file-handler
-        headers['Content-Length'] = body.map(&:bytesize).reduce(0, :+).to_s
+        headers['content-length'] = body.map(&:bytesize).reduce(0, :+).to_s
       end
 
       [status, headers, result]
@@ -197,7 +201,7 @@ def finish
     private
 
     def calculate_content_length?
-      headers['Content-Type'] && !headers['Content-Length'] && (Array === body)
+      headers['content-type'] && !headers['content-length'] && (Array === body)
     end
 
     def drop_content_info?
@@ -289,10 +293,8 @@ def body(value = nil, &block)
         def block.each; yield(call) end
         response.body = block
       elsif value
-        # Rack 2.0 returns a Rack::File::Iterator here instead of
-        # Rack::File as it was in the previous API.
         unless request.head? || value.is_a?(Rack::Files::Iterator) || value.is_a?(Stream)
-          headers.delete 'Content-Length'
+          headers.delete 'content-length'
         end
         response.body = value
       else
@@ -302,7 +304,10 @@ def block.each; yield(call) end
 
     # Halt processing and redirect to the URI provided.
     def redirect(uri, *args)
-      if (env['HTTP_VERSION'] == 'HTTP/1.1') && (env['REQUEST_METHOD'] != 'GET')
+      # SERVER_PROTOCOL is required in Rack 3, fall back to HTTP_VERSION
+      # for servers not updated for Rack 3 (like Puma 5)
+      http_version = env['SERVER_PROTOCOL'] || env['HTTP_VERSION']
+      if (http_version == 'HTTP/1.1') && (env['REQUEST_METHOD'] != 'GET')
         status 303
       else
         status 302
@@ -372,10 +377,10 @@ def mime_type(type)
       Base.mime_type(type)
     end
 
-    # Set the Content-Type of the response body given a media type or file
+    # Set the content-type of the response body given a media type or file
     # extension.
     def content_type(type = nil, params = {})
-      return response['Content-Type'] unless type
+      return response['content-type'] unless type
 
       default = params.delete :default
       mime_type = mime_type(type) || default
@@ -393,7 +398,7 @@ def content_type(type = nil, params = {})
           "#{key}=#{val}"
         end.join(', ')
       end
-      response['Content-Type'] = mime_type
+      response['content-type'] = mime_type
     end
 
     # https://html.spec.whatwg.org/#multipart-form-data
@@ -412,12 +417,12 @@ def attachment(filename = nil, disposition = :attachment)
       params = format('; filename="%s"', File.basename(filename).gsub(/["\r\n]/, MULTIPART_FORM_DATA_REPLACEMENT_TABLE))
       response['Content-Disposition'] << params
       ext = File.extname(filename)
-      content_type(ext) unless response['Content-Type'] || ext.empty?
+      content_type(ext) unless response['content-type'] || ext.empty?
     end
 
     # Use the contents of the file at +path+ as the response body.
     def send_file(path, opts = {})
-      if opts[:type] || !response['Content-Type']
+      if opts[:type] || !response['content-type']
         content_type opts[:type] || File.extname(path), default: 'application/octet-stream'
       end
 
@@ -433,7 +438,7 @@ def send_file(path, opts = {})
       result = file.serving(request, path)
 
       result[1].each { |k, v| headers[k] ||= v }
-      headers['Content-Length'] = result[1]['Content-Length']
+      headers['content-length'] = result[1]['content-length']
       opts[:status] &&= Integer(opts[:status])
       halt (opts[:status] || result[0]), result[2]
     rescue Errno::ENOENT
@@ -995,7 +1000,7 @@ def call!(env) # :nodoc:
       invoke { dispatch! }
       invoke { error_block!(response.status) } unless @env['sinatra.error']
 
-      unless @response['Content-Type']
+      unless @response['content-type']
         if Array === body && body[0].respond_to?(:content_type)
           content_type body[0].content_type
         elsif (default = settings.default_content_type)
@@ -1058,7 +1063,7 @@ def route!(base = settings, pass_block = nil)
       routes = base.routes[@request.request_method]
 
       routes&.each do |pattern, conditions, block|
-        response.delete_header('Content-Type') unless @pinned_response
+        response.delete_header('content-type') unless @pinned_response
 
         returned_pass_block = process_route(pattern, conditions) do |*args|
           env['sinatra.route'] = "#{@request.request_method} #{pattern}"
@@ -1179,7 +1184,7 @@ def dispatch!
       invoke do
         static! if settings.static? && (request.get? || request.head?)
         filter! :before do
-          @pinned_response = !response['Content-Type'].nil?
+          @pinned_response = !response['content-type'].nil?
         end
         route!
       end
@@ -1460,7 +1465,13 @@ def mime_type(type, value = nil)
       #   mime_types :js   # => ['application/javascript', 'text/javascript']
       def mime_types(type)
         type = mime_type type
-        type =~ %r{^application/(xml|javascript)$} ? [type, "text/#{$1}"] : [type]
+        if type =~ %r{^application/(xml|javascript)$}
+          [type, "text/#{$1}"]
+        elsif type =~ %r{^text/(xml|javascript)$}
+          [type, "application/#{$1}"]
+        else
+          [type]
+        end
       end
 
       # Define a before filter; runs before all requests within the same
@@ -1589,10 +1600,27 @@ def quit!
       # Puma, Falcon, or WEBrick (in that order). If given a block, will call
       # with the constructed handler once we have taken the stage.
       def run!(options = {}, &block)
+        unless defined?(Rackup::Handler)
+          rackup_warning = <<~MISSING_RACKUP
+            Sinatra could not start, the "rackup" gem was not found!
+
+            Add it to your bundle with:
+
+                bundle add rackup
+
+            or install it with:
+
+                gem install rackup
+
+          MISSING_RACKUP
+          warn rackup_warning
+          exit 1
+        end
+
         return if running?
 
         set options
-        handler         = Rack::Handler.pick(server)
+        handler         = Rackup::Handler.pick(server)
         handler_name    = handler.name.gsub(/.*::/, '')
         server_settings = settings.respond_to?(:server_settings) ? settings.server_settings : {}
         server_settings.merge!(Port: port, Host: bind)
@@ -1724,7 +1752,7 @@ def provides(*types)
         types.map! { |t| mime_types(t) }
         types.flatten!
         condition do
-          response_content_type = response['Content-Type']
+          response_content_type = response['content-type']
           preferred_type = request.preferred_type(types)
 
           if response_content_type

diff --git a/lib/sinatra/show_exceptions.rb b/lib/sinatra/show_exceptions.rb
@@ -38,8 +38,8 @@ def call(env)
       [
         500,
         {
-          'Content-Type' => content_type,
-          'Content-Length' => body.bytesize.to_s
+          'content-type' => content_type,
+          'content-length' => body.bytesize.to_s
         },
         [body]
       ]

diff --git a/rack-protection/lib/rack/protection.rb b/rack-protection/lib/rack/protection.rb
@@ -2,6 +2,7 @@
 
 require 'rack/protection/version'
 require 'rack'
+require 'rack/session'
 
 module Rack
   module Protection

diff --git a/rack-protection/lib/rack/protection/base.rb b/rack-protection/lib/rack/protection/base.rb
@@ -74,7 +74,7 @@ def instrument(env)
 
       def deny(env)
         warn env, "attack prevented by #{self.class}"
-        [options[:status], { 'Content-Type' => 'text/plain' }, [options[:message]]]
+        [options[:status], { 'content-type' => 'text/plain' }, [options[:message]]]
       end
 
       def report(env)

diff --git a/rack-protection/lib/rack/protection/content_security_policy.rb b/rack-protection/lib/rack/protection/content_security_policy.rb
@@ -26,7 +26,7 @@ module Protection
     #               https://scotthelme.co.uk/csp-cheat-sheet/
     #               http://www.html5rocks.com/en/tutorials/security/content-security-policy/
     #
-    # Sets the 'Content-Security-Policy[-Report-Only]' header.
+    # Sets the 'content-security-policy[-report-only]' header.
     #
     # Options: ContentSecurityPolicy configuration is a complex topic with
     #          several levels of support that has evolved over time.
@@ -71,7 +71,7 @@ def csp_policy
 
       def call(env)
         status, headers, body = @app.call(env)
-        header = options[:report_only] ? 'Content-Security-Policy-Report-Only' : 'Content-Security-Policy'
+        header = options[:report_only] ? 'content-security-policy-report-only' : 'content-security-policy'
         headers[header] ||= csp_policy if html? headers
         [status, headers, body]
       end

diff --git a/rack-protection/lib/rack/protection/cookie_tossing.rb b/rack-protection/lib/rack/protection/cookie_tossing.rb
@@ -51,7 +51,7 @@ def remove_bad_cookies(request, response)
       def redirect(env)
         request = Request.new(env)
         warn env, "attack prevented by #{self.class}"
-        [302, { 'Content-Type' => 'text/html', 'Location' => request.path }, []]
+        [302, { 'content-type' => 'text/html', 'location' => request.path }, []]
       end
 
       def bad_cookies

diff --git a/rack-protection/lib/rack/protection/encrypted_cookie.rb b/rack-protection/lib/rack/protection/encrypted_cookie.rb
@@ -202,7 +202,7 @@ def extract_session_id(request)
       end
 
       def unpacked_cookie_data(request)
-        request.fetch_header(RACK_SESSION_UNPACKED_COOKIE_DATA) do |k|
+        request.fetch_header(Session::RACK_SESSION_UNPACKED_COOKIE_DATA) do |k|
           session_data = cookie_data = request.cookies[@key]
 
           # Try to decrypt with the first secret, if that returns nil, try

diff --git a/rack-protection/lib/rack/protection/frame_options.rb b/rack-protection/lib/rack/protection/frame_options.rb
@@ -31,7 +31,7 @@ def frame_options
 
       def call(env)
         status, headers, body        = @app.call(env)
-        headers['X-Frame-Options'] ||= frame_options if html? headers
+        headers['x-frame-options'] ||= frame_options if html? headers
         [status, headers, body]
       end
     end

diff --git a/rack-protection/lib/rack/protection/json_csrf.rb b/rack-protection/lib/rack/protection/json_csrf.rb
@@ -39,7 +39,7 @@ def call(env)
       def has_vector?(request, headers)
         return false if request.xhr?
         return false if options[:allow_if]&.call(request.env)
-        return false unless headers['Content-Type'].to_s.split(';', 2).first =~ %r{^\s*application/json\s*$}
+        return false unless headers['content-type'].to_s.split(';', 2).first =~ %r{^\s*application/json\s*$}
 
         origin(request.env).nil? and referrer(request.env) != request.host
       end

diff --git a/rack-protection/lib/rack/protection/referrer_policy.rb b/rack-protection/lib/rack/protection/referrer_policy.rb
@@ -19,7 +19,7 @@ class ReferrerPolicy < Base
 
       def call(env)
         status, headers, body = @app.call(env)
-        headers['Referrer-Policy'] ||= options[:referrer_policy]
+        headers['referrer-policy'] ||= options[:referrer_policy]
         [status, headers, body]
       end
     end

diff --git a/rack-protection/lib/rack/protection/strict_transport.rb b/rack-protection/lib/rack/protection/strict_transport.rb
@@ -33,7 +33,7 @@ def strict_transport
 
       def call(env)
         status, headers, body = @app.call(env)
-        headers['Strict-Transport-Security'] ||= strict_transport
+        headers['strict-transport-security'] ||= strict_transport
         [status, headers, body]
       end
     end

diff --git a/rack-protection/lib/rack/protection/xss_header.rb b/rack-protection/lib/rack/protection/xss_header.rb
@@ -18,8 +18,8 @@ class XSSHeader < Base
 
       def call(env)
         status, headers, body = @app.call(env)
-        headers['X-XSS-Protection']       ||= "1; mode=#{options[:xss_mode]}" if html? headers
-        headers['X-Content-Type-Options'] ||= 'nosniff'                       if options[:nosniff]
+        headers['x-xss-protection']       ||= "1; mode=#{options[:xss_mode]}" if html? headers
+        headers['x-content-type-options'] ||= 'nosniff'                       if options[:nosniff]
         [status, headers, body]
       end
     end

diff --git a/rack-protection/rack-protection.gemspec b/rack-protection/rack-protection.gemspec
@@ -40,5 +40,6 @@ RubyGems 2.0 or newer is required to protect against public gem pushes. You can
 
   # dependencies
   s.add_dependency 'base64', '>= 0.1.0'
-  s.add_dependency 'rack', '~> 2.2', '>= 2.2.4'
+  s.add_dependency 'rack', '>= 3.0.0', '< 4'
+  s.add_dependency 'rack-session', '>= 2.0.0', '< 3'
 end
diff --git a/rack-protection/spec/lib/rack/protection/authenticity_token_spec.rb b/rack-protection/spec/lib/rack/protection/authenticity_token_spec.rb
@@ -61,7 +61,7 @@
   it 'allows for a custom authenticity token param' do
     mock_app do
       use Rack::Protection::AuthenticityToken, authenticity_param: 'csrf_param'
-      run proc { |_e| [200, { 'Content-Type' => 'text/plain' }, ['hi']] }
+      run proc { |_e| [200, { 'content-type' => 'text/plain' }, ['hi']] }
     end
 
     post('/', { 'csrf_param' => token }, 'rack.session' => { csrf: token })

diff --git a/rack-protection/spec/lib/rack/protection/content_security_policy_spec.rb b/rack-protection/spec/lib/rack/protection/content_security_policy_spec.rb
@@ -62,7 +62,7 @@
   end
 
   it 'should not override the header if already set' do
-    mock_app with_headers('Content-Security-Policy' => 'default-src: none')
+    mock_app with_headers('content-security-policy' => 'default-src: none')
     expect(get('/', {}, 'wants' => 'text/html').headers['Content-Security-Policy']).to eq('default-src: none')
   end
 end
-Original file line number
+Diff line change
@@ Expand Up / @@ -4,7 +4,7 @@ @@
     *.swp
     *.rbc
     /pkg
-    /Gemfile.lock
+    *.lock
     /coverage
     .yardoc
     /doc
@@ Expand Down @@