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

Restore rack 1.6 compatibility #3156

Merged
merged 5 commits into from May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 7 additions & 9 deletions .github/workflows/tests.yaml
Expand Up @@ -23,7 +23,7 @@ jobs:

test_mri:
name: >-
MRI: ${{ matrix.os }} ${{ matrix.ruby }}${{ matrix.no-ssl }}${{ matrix.yjit }}${{ matrix.rack-2 }}
MRI: ${{ matrix.os }} ${{ matrix.ruby }}${{ matrix.no-ssl }}${{ matrix.yjit }}${{ matrix.rack-v }}
needs: [rubocop, skip_duplicate_runs]
env:
CI: true
Expand All @@ -41,16 +41,17 @@ jobs:
os: [ ubuntu-20.04, ubuntu-22.04, macos-11, macos-12, macos-13, windows-2022 ]
ruby: [ 2.4, 2.5, 2.6, 2.7, '3.0', 3.1, 3.2, head ]
no-ssl: ['']
rack-2: ['']
rack-v: ['']
yjit: ['']
include:
- { os: windows-2022 , ruby: ucrt }
- { os: windows-2022 , ruby: mswin }
- { os: windows-2022 , ruby: 2.7 , no-ssl: ' no SSL' }
- { os: ubuntu-20.04 , ruby: 2.7 , no-ssl: ' no SSL' }
- { os: ubuntu-22.04 , ruby: head , yjit: ' yjit' }
- { os: ubuntu-22.04 , ruby: 2.4 , rack-2: ' rack2' }
- { os: ubuntu-22.04 , ruby: 3.2 , rack-2: ' rack2' }
- { os: ubuntu-22.04 , ruby: 2.4 , rack-v: ' rack2' }
- { os: ubuntu-22.04 , ruby: 3.2 , rack-v: ' rack2' }
- { os: ubuntu-22.04 , ruby: 2.4 , rack-v: ' rack1' }

exclude:
- { os: ubuntu-22.04 , ruby: 2.4 }
Expand Down Expand Up @@ -83,12 +84,9 @@ jobs:
shell: bash
run: echo 'PUMA_DISABLE_SSL=true' >> $GITHUB_ENV

- name: Test with Rack 2
if: |
(matrix.rack-2 == ' rack2') &&
(needs.skip_duplicate_runs.outputs.should_skip != 'true')
- name: Set Rack version, see Gemfile
shell: bash
run: echo 'PUMA_CI_RACK_2=true' >> $GITHUB_ENV
run: echo 'PUMA_CI_RACK=${{ matrix.rack-v }}' >> $GITHUB_ENV

- name: load ruby
if: ${{ needs.skip_duplicate_runs.outputs.should_skip != 'true' }}
Expand Down
16 changes: 14 additions & 2 deletions Gemfile
Expand Up @@ -11,8 +11,20 @@ gem "minitest-retry"
gem "minitest-proveit"
gem "minitest-stub-const"

gem "rack", (ENV['PUMA_CI_RACK_2'] ? "~> 2.2" : ">= 2.2")
gem "rackup" unless ENV['PUMA_CI_RACK_2']
use_rackup = false
rack_vers =
case ENV['PUMA_CI_RACK']&.strip
when 'rack2'
'~> 2.2'
when 'rack1'
'~> 1.6'
else
use_rackup = true
'>= 2.2'
end

gem "rack", rack_vers
gem "rackup" if use_rackup

gem "jruby-openssl", :platform => "jruby"

Expand Down
2 changes: 1 addition & 1 deletion lib/puma/rack_default.rb
Expand Up @@ -11,7 +11,7 @@ def self.default(options = {})
end
end
end
elsif Object.const_defined?(:Rack) && Rack::RELEASE < '3'
elsif Object.const_defined?(:Rack) && Rack.release < '3'
module Rack
module Handler
def self.default(options = {})
Expand Down
8 changes: 6 additions & 2 deletions lib/rack/handler/puma.rb
Expand Up @@ -31,7 +31,11 @@ def config(app, options = {})

conf = ::Puma::Configuration.new(options, default_options.merge({events: @events})) do |user_config, file_config, default_config|
if options.delete(:Verbose)
require 'rack/common_logger'
begin
require 'rack/commonlogger' # Rack 1.x
rescue LoadError
require 'rack/common_logger' # Rack 2 and later
end
app = ::Rack::CommonLogger.new(app, STDOUT)
end

Expand Down Expand Up @@ -123,7 +127,7 @@ class << self
end
end
else
do_register = Object.const_defined?(:Rack) && Rack::RELEASE < '3'
do_register = Object.const_defined?(:Rack) && Rack.release < '3'
module Rack
module Handler
module Puma
Expand Down
4 changes: 2 additions & 2 deletions test/helper.rb
Expand Up @@ -182,7 +182,7 @@ def skip_if(*engs, suffix: '', bt: caller)
when :fork then "Skipped if Kernel.fork exists" if HAS_FORK
when :unix then "Skipped if UNIXSocket exists" if Puma::HAS_UNIX_SOCKET
when :aunix then "Skipped if abstract UNIXSocket" if Puma.abstract_unix_socket?
when :rack3 then "Skipped if Rack 3.x" if Rack::RELEASE >= '3'
when :rack3 then "Skipped if Rack 3.x" if Rack.release >= '3'
else false
end
skip skip_msg, bt if skip_msg
Expand All @@ -201,7 +201,7 @@ def skip_unless(eng, bt: caller)
when :fork then MSG_FORK unless HAS_FORK
when :unix then MSG_UNIX unless Puma::HAS_UNIX_SOCKET
when :aunix then MSG_AUNIX unless Puma.abstract_unix_socket?
when :rack3 then "Skipped unless Rack >= 3.x" unless ::Rack::RELEASE >= '3'
when :rack3 then "Skipped unless Rack >= 3.x" unless ::Rack.release >= '3'
else false
end
skip skip_msg, bt if skip_msg
Expand Down
2 changes: 1 addition & 1 deletion test/test_config.rb
Expand Up @@ -17,7 +17,7 @@ def test_default_max_threads
end

def test_app_from_rackup
if Rack::RELEASE >= '3'
if Rack.release >= '3'
fn = "test/rackup/hello-bind_rack3.ru"
bind = "tcp://0.0.0.0:9292"
else
Expand Down
3 changes: 3 additions & 0 deletions test/test_puma_server_hijack.rb
Expand Up @@ -29,6 +29,7 @@ def setup
end

def teardown
return if skipped?
@server.stop(true)
assert_empty @log_writer.stdout.string
assert_empty @log_writer.stderr.string
Expand Down Expand Up @@ -172,13 +173,15 @@ def test_http_10_header_with_content_length
end

def test_partial_hijack_body_closes_body
skip 'Not supported with Rack 1.x' if Rack.release.start_with? '1.'
@available = true
hdrs = { 'Content-Type' => 'text/plain' }
body = ::Rack::BodyProxy.new(HIJACK_LAMBDA) { @available = true }
partial_hijack_closes_body(hdrs, body)
end

def test_partial_hijack_header_closes_body_correct_precedence
skip 'Not supported with Rack 1.x' if Rack.release.start_with? '1.'
@available = true
incorrect_lambda = ->(io) {
io.syswrite 'incorrect body.call'
Expand Down
28 changes: 17 additions & 11 deletions test/test_rack_server.rb
Expand Up @@ -3,14 +3,18 @@
require "net/http"

# don't load Rack, as it autoloads everything
require "rack/body_proxy"
require "rack/lint"
require "rack/version"
require "rack/common_logger"
begin
require "rack/body_proxy"
require "rack/lint"
require "rack/version"
require "rack/common_logger"
rescue LoadError # Rack 1.6
require "rack"
end

# Rack::Chunked is loaded by Rack v2, needs to be required by Rack 3.0,
# and is removed in Rack 3.1
require "rack/chunked" if Rack::RELEASE.start_with? '3.0'
require "rack/chunked" if Rack.release.start_with? '3.0'

require "nio"

Expand Down Expand Up @@ -41,7 +45,7 @@ def call(env)

class ServerLint < Rack::Lint
def call(env)
if Rack::RELEASE < '3'
if Rack.release < '3'
check_env env
else
Wrapper.new(@app, env).check_environment env
Expand Down Expand Up @@ -206,13 +210,15 @@ def test_rack_body_proxy_content_length

headers = header_hash socket

content_length = headers["Content-Length"].to_i

socket.close

stop

assert_equal str_ary_bytes, content_length
if Rack.release.start_with? '1.'
assert_equal "chunked", headers["Transfer-Encoding"]
else
assert_equal str_ary_bytes, headers["Content-Length"].to_i
end
end

def test_common_logger
Expand Down Expand Up @@ -241,7 +247,7 @@ def test_rack_chunked_array1
resp = Net::HTTP.get_response URI(@tcp)
assert_equal 'chunked', resp['transfer-encoding']
assert_equal STR_1KB, resp.body.force_encoding(Encoding::UTF_8)
end if Rack::RELEASE < '3.1'
end if Rack.release < '3.1'

def test_rack_chunked_array10
body = Array.new 10, STR_1KB
Expand All @@ -253,7 +259,7 @@ def test_rack_chunked_array10
resp = Net::HTTP.get_response URI(@tcp)
assert_equal 'chunked', resp['transfer-encoding']
assert_equal STR_1KB * 10, resp.body.force_encoding(Encoding::UTF_8)
end if Rack::RELEASE < '3.1'
end if Rack.release < '3.1'

def test_puma_enum
body = Array.new(10, STR_1KB).to_enum
Expand Down