Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rack 3 support #1857

Merged
merged 29 commits into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
eff32e1
Don't allow Rack 3 builds to fail
dentarg Dec 23, 2022
482b157
Add `rackup`, `rack-session` as dependencies
dentarg Dec 24, 2022
c5f9817
Remove Rack 2 jobs
dentarg Dec 24, 2022
a86a32b
Run tests for sinatra-contrib and rack-protection
dentarg Dec 24, 2022
5efc85e
Rack 3 requires all response headers to be lowercase
dentarg Dec 28, 2022
c83b7f2
`SERVER_PROTOCOL` -> `HTTP_VERSION`
dentarg Dec 28, 2022
0b4622f
Rack 3 does not allow newlines in headers
dentarg Dec 28, 2022
943cfaa
Find `RACK_SESSION_UNPACKED_COOKIE_DATA`
dentarg Dec 28, 2022
26aeec7
rack-protection tests now pass (on Ruby 2.7.7)
dentarg Dec 28, 2022
1396f08
Cookie attributes are lowercase in Rack 3
dentarg Dec 28, 2022
0a9b043
sinatra-contrib tests now pass
dentarg Dec 29, 2022
44377b2
Fix server registration
dentarg Dec 30, 2022
8e6d11c
Update routing test `handles params without a value`
dentarg Feb 9, 2023
217c5db
Restore `continue-on-error`
dentarg Feb 13, 2023
a4a786a
Depend on `rackup` and `rack-session` >=2.0.0
dentarg Feb 24, 2023
8da32a4
`Rack::File` -> `Rack::Files` again
dentarg Feb 24, 2023
c129d58
Support `text/javascript` as JavaScript MIME type
dentarg Feb 25, 2023
41d6063
`Rack::Handler` -> `Rackup::Handler`
dentarg Mar 4, 2023
5b28218
Revert "Update routing test `handles params without a value`"
dentarg May 15, 2023
d3dfc19
Update rack requirement in rack-protection gemspec
dentarg Aug 7, 2023
4ba8664
Puma 5 is not compatible with Rack 3
dentarg Aug 7, 2023
7a514e2
Test with rack head
dentarg Aug 7, 2023
a378414
Fix typo in spec description
dentarg Nov 26, 2023
e721af6
Require rack 3.0.0, not 3.0.0.beta1
dentarg Dec 23, 2023
7a16094
rack-protection depends on rack-session
dentarg Dec 23, 2023
1e37204
CI: use rack stable (Rack >= 3.0)
dentarg Jan 2, 2024
8a78d8b
Make `rackup` an optional dependency
dentarg Jan 2, 2024
1377751
Skip "without rackup" test on rack head branch
dentarg Jan 3, 2024
985625d
Fix `test_app_start_without_rackup` on JVM rubies
dentarg Jan 4, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 12 additions & 10 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
fail-fast: false
matrix:
rack:
- "~> 2"
- stable
ruby:
- "2.6"
- "2.7"
Expand Down Expand Up @@ -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: |
Expand Down
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*.swp
*.rbc
/pkg
/Gemfile.lock
*.lock
/coverage
.yardoc
/doc
Expand Down
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
70 changes: 49 additions & 21 deletions lib/sinatra/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

# external dependencies
require 'rack'
begin
require 'rackup'
rescue LoadError
end
require 'tilt'
require 'rack/protection'
require 'mustermann'
Expand Down Expand Up @@ -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?
Expand All @@ -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]
Expand All @@ -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?
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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}"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/sinatra/show_exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
]
Expand Down
1 change: 1 addition & 0 deletions rack-protection/lib/rack/protection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require 'rack/protection/version'
require 'rack'
require 'rack/session'

module Rack
module Protection
Expand Down
2 changes: 1 addition & 1 deletion rack-protection/lib/rack/protection/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion rack-protection/lib/rack/protection/cookie_tossing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion rack-protection/lib/rack/protection/encrypted_cookie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion rack-protection/lib/rack/protection/frame_options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion rack-protection/lib/rack/protection/json_csrf.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion rack-protection/lib/rack/protection/referrer_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion rack-protection/lib/rack/protection/strict_transport.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions rack-protection/lib/rack/protection/xss_header.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion rack-protection/rack-protection.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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