From d55227f77e32d812c9af24e6c61914069eff1b48 Mon Sep 17 00:00:00 2001 From: Severin Schoepke Date: Mon, 15 May 2023 09:06:07 +0200 Subject: [PATCH 1/5] Use Rack.release over Rack::RELEASE The constant Rack::RELEASE was introduced only in rack version 2 (specifically: https://github.com/rack/rack/commit/a2fe30a5e70371c89c1b29fdc2dc5f8027bc5fe6) Earlier versions provided the class method Rack.release which still exists. https://github.com/puma/puma/pull/3061 has introduced usage of Rack::RELEASE and therefore broke compatibility with rack 1.6.X. --- lib/puma/rack_default.rb | 2 +- lib/rack/handler/puma.rb | 2 +- test/helper.rb | 4 ++-- test/test_config.rb | 2 +- test/test_rack_server.rb | 8 ++++---- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/puma/rack_default.rb b/lib/puma/rack_default.rb index 16c3b0762e..fa7ea2edb4 100644 --- a/lib/puma/rack_default.rb +++ b/lib/puma/rack_default.rb @@ -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 = {}) diff --git a/lib/rack/handler/puma.rb b/lib/rack/handler/puma.rb index e572ec6098..ab75b5bf6d 100644 --- a/lib/rack/handler/puma.rb +++ b/lib/rack/handler/puma.rb @@ -123,7 +123,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 diff --git a/test/helper.rb b/test/helper.rb index 0690093f8d..b8530c27d6 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -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 @@ -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 diff --git a/test/test_config.rb b/test/test_config.rb index a0e479c88d..118316eacf 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -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 diff --git a/test/test_rack_server.rb b/test/test_rack_server.rb index b6c53023b3..729cafe2d6 100644 --- a/test/test_rack_server.rb +++ b/test/test_rack_server.rb @@ -10,7 +10,7 @@ # 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" @@ -41,7 +41,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 @@ -241,7 +241,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 @@ -253,7 +253,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 From 318b4f6d24440fd1f1c61f2b2e3eb7f48eb591f6 Mon Sep 17 00:00:00 2001 From: MSP-Greg Date: Mon, 15 May 2023 08:18:25 -0500 Subject: [PATCH 2/5] test.yaml, Gemfile - allow Rack 1.6.x CI --- .github/workflows/tests.yaml | 16 +++++++--------- Gemfile | 16 ++++++++++++++-- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index f4bd576759..2a13c09725 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -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 @@ -41,7 +41,7 @@ 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 } @@ -49,8 +49,9 @@ jobs: - { 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 } @@ -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' }} diff --git a/Gemfile b/Gemfile index f0f30f0536..98ab9a7c36 100644 --- a/Gemfile +++ b/Gemfile @@ -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" From 1af282f2fb0aab32b200c93fea5395b4c1d78fb3 Mon Sep 17 00:00:00 2001 From: MSP-Greg Date: Mon, 15 May 2023 09:14:24 -0500 Subject: [PATCH 3/5] lib/rack/handler/puma.rb - allow Rack 1.6.x --- lib/rack/handler/puma.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/rack/handler/puma.rb b/lib/rack/handler/puma.rb index ab75b5bf6d..f70d3d8e96 100644 --- a/lib/rack/handler/puma.rb +++ b/lib/rack/handler/puma.rb @@ -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 From 6514997add225e41aacbe180e938e9da4be70cc5 Mon Sep 17 00:00:00 2001 From: MSP-Greg Date: Mon, 15 May 2023 08:47:44 -0500 Subject: [PATCH 4/5] test_rack_server.rb - allow testing with Rack 1.6.x --- test/test_rack_server.rb | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/test/test_rack_server.rb b/test/test_rack_server.rb index 729cafe2d6..b4540e122c 100644 --- a/test/test_rack_server.rb +++ b/test/test_rack_server.rb @@ -3,10 +3,14 @@ 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 @@ -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 From f2472fecab5a2be62a9075c5f440d834642680f1 Mon Sep 17 00:00:00 2001 From: MSP-Greg Date: Mon, 15 May 2023 10:26:00 -0500 Subject: [PATCH 5/5] test_puma_server_hijack.rb - allow testing with Rack 1.6.x --- test/test_puma_server_hijack.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/test_puma_server_hijack.rb b/test/test_puma_server_hijack.rb index 4b5b0374cf..30f48b4354 100644 --- a/test/test_puma_server_hijack.rb +++ b/test/test_puma_server_hijack.rb @@ -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 @@ -172,6 +173,7 @@ 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 } @@ -179,6 +181,7 @@ def test_partial_hijack_body_closes_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'