Rack 3 Compatibility

[Jeremy Evans <code@jeremyevans.net>]

This takes Eric's patch from December 25, 2022, and includes all
necessary test fixes to allow Unicorn tests to pass with both
Rack 3 and Rack 2 (and probably Rack 1). It includes a test fix for
newer curl versions and an OpenBSD test fix.

Hopefully this is acceptable and Unicorn 6.2 can be released with Rack 3
support.  If further fixes are needed, I'm happy to work on them.

Thanks,
Jeremy

---

From 6ba4e7234af9d6ea3e85e646f798b1fbf6799234 Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@jeremyevans.net>
Date: Thu, 1 Jun 2023 08:55:14 -0700
Subject: [PATCH] Support Rack 3 and fix tests on Rack 3

Most changes are to the tests to avoid uppercase characters in header
keys, which are no longer allowed in rack 3 (to allow for O(1) access).

This also changes a few places where an array of headers was used to
switch to a hash, as a hash is requierd in Rack 3.

Newer versions of curl use a 000 http_code for invalid http codes,
so switch from "42 -eq" to "500 -ne" in the test, as Rack::Lint will
always raise a 500 error.

There is one test that fails on OpenBSD when opening a fifo.  This is
unrelated to unicorn as far as I can see, so skip the remaining part
of the test in that case on OpenBSD.

Tests still pass on Rack 2, and presumably Rack 1 as well, though
I didn't test Rack 1.

Co-authored-by: Eric Wong <bofh@yhbt.net>
---
 lib/unicorn/http_response.rb     | 19 +++++++++++++------
 lib/unicorn/http_server.rb       | 21 +++++----------------
 t/heartbeat-timeout.ru           |  2 +-
 t/rack-input-tests.ru            |  2 +-
 t/t0300-no-default-middleware.sh |  2 +-
 t/t0301.ru                       |  4 ++--
 t/write-on-close.ru              |  2 +-
 test/exec/test_exec.rb           | 20 +++++++++++++-------
 test/unit/test_ccc.rb            |  2 +-
 test/unit/test_request.rb        |  2 +-
 test/unit/test_server.rb         |  8 ++++----
 test/unit/test_signals.rb        | 14 +++++++-------
 12 files changed, 50 insertions(+), 48 deletions(-)

diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb
index b23e521..19469b4 100644
--- a/lib/unicorn/http_response.rb
+++ b/lib/unicorn/http_response.rb
@@ -19,6 +19,18 @@ def err_response(code, response_start_sent)
       "#{code} #{STATUS_CODES[code]}\r\n\r\n"
   end
 
+  def append_header(buf, key, value)
+    case value
+    when Array # Rack 3
+      value.each { |v| buf << "#{key}: #{v}\r\n" }
+    when /\n/ # Rack 2
+      # avoiding blank, key-only cookies with /\n+/
+      value.split(/\n+/).each { |v| buf << "#{key}: #{v}\r\n" }
+    else
+      buf << "#{key}: #{value}\r\n"
+    end
+  end
+
   # writes the rack_response to socket as an HTTP response
   def http_response_write(socket, status, headers, body,
                           req = Unicorn::HttpRequest.new)
@@ -40,12 +52,7 @@ def http_response_write(socket, status, headers, body,
           # key in Rack < 1.5
           hijack = value
         else
-          if value =~ /\n/
-            # avoiding blank, key-only cookies with /\n+/
-            value.split(/\n+/).each { |v| buf << "#{key}: #{v}\r\n" }
-          else
-            buf << "#{key}: #{value}\r\n"
-          end
+          append_header(buf, key, value)
         end
       end
       socket.write(buf << "\r\n".freeze)
diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index 3416808..cad515b 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -589,22 +589,11 @@ def handle_error(client, e)
   end
 
   def e103_response_write(client, headers)
-    response = if @request.response_start_sent
-      "103 Early Hints\r\n"
-    else
-      "HTTP/1.1 103 Early Hints\r\n"
-    end
-
-    headers.each_pair do |k, vs|
-      next if !vs || vs.empty?
-      values = vs.to_s.split("\n".freeze)
-      values.each do |v|
-        response << "#{k}: #{v}\r\n"
-      end
-    end
-    response << "\r\n".freeze
-    response << "HTTP/1.1 ".freeze if @request.response_start_sent
-    client.write(response)
+    rss = @request.response_start_sent
+    buf = rss ? "103 Early Hints\r\n" : "HTTP/1.1 103 Early Hints\r\n"
+    headers.each { |key, value| append_header(buf, key, value) }
+    buf << (rss ? "\r\nHTTP/1.1 ".freeze : "\r\n".freeze)
+    client.write(buf)
   end
 
   def e100_response_write(client, env)
diff --git a/t/heartbeat-timeout.ru b/t/heartbeat-timeout.ru
index d9904e8..20a7938 100644
--- a/t/heartbeat-timeout.ru
+++ b/t/heartbeat-timeout.ru
@@ -1,5 +1,5 @@
 use Rack::ContentLength
-headers = { 'Content-Type' => 'text/plain' }
+headers = { 'content-type' => 'text/plain' }
 run lambda { |env|
   case env['PATH_INFO']
   when "/block-forever"
diff --git a/t/rack-input-tests.ru b/t/rack-input-tests.ru
index 8c35630..5459e85 100644
--- a/t/rack-input-tests.ru
+++ b/t/rack-input-tests.ru
@@ -16,6 +16,6 @@
     end while input.read(rand(cap), buf)
   end
 
-  [ 200, {'Content-Type' => 'text/plain'}, [ digest.hexdigest << "\n" ] ]
+  [ 200, {'content-type' => 'text/plain'}, [ digest.hexdigest << "\n" ] ]
 end
 run app
diff --git a/t/t0300-no-default-middleware.sh b/t/t0300-no-default-middleware.sh
index 779dc02..00feacc 100644
--- a/t/t0300-no-default-middleware.sh
+++ b/t/t0300-no-default-middleware.sh
@@ -9,7 +9,7 @@ t_begin "setup and start" && {
 }
 
 t_begin "check exit status with Rack::Lint not present" && {
-	test 42 -eq "$(curl -sf -o/dev/null -w'%{http_code}' http://$listen/)"
+	test 500 -ne "$(curl -sf -o/dev/null -w'%{http_code}' http://$listen/)"
 }
 
 t_begin "killing succeeds" && {
diff --git a/t/t0301.ru b/t/t0301.ru
index 1ae8ea7..ce68213 100644
--- a/t/t0301.ru
+++ b/t/t0301.ru
@@ -6,8 +6,8 @@
         "lint=#{caller.grep(%r{rack/lint\.rb})[0].split(':')[0]}\n"
   end
   h = {
-    'Content-Length' => b.size.to_s,
-    'Content-Type' => 'text/plain',
+    'content-length' => b.size.to_s,
+    'content-type' => 'text/plain',
   }
   [ 200, h, [ b ] ]
 end)
diff --git a/t/write-on-close.ru b/t/write-on-close.ru
index 54a2f2e..725c4d6 100644
--- a/t/write-on-close.ru
+++ b/t/write-on-close.ru
@@ -8,4 +8,4 @@ def close
   end
 end
 use Rack::ContentType, "text/plain"
-run(lambda { |_| [ 200, [%w(Transfer-Encoding chunked)], WriteOnClose.new ] })
+run(lambda { |_| [ 200, { 'transfer-encoding' => 'chunked' }, WriteOnClose.new ] })
diff --git a/test/exec/test_exec.rb b/test/exec/test_exec.rb
index aacd917..2929b2e 100644
--- a/test/exec/test_exec.rb
+++ b/test/exec/test_exec.rb
@@ -25,20 +25,20 @@ class ExecTest < Test::Unit::TestCase
 
   HI = <<-EOS
 use Rack::ContentLength
-run proc { |env| [ 200, { 'Content-Type' => 'text/plain' }, [ "HI\\n" ] ] }
+run proc { |env| [ 200, { 'content-type' => 'text/plain' }, [ "HI\\n" ] ] }
   EOS
 
   SHOW_RACK_ENV = <<-EOS
 use Rack::ContentLength
 run proc { |env|
-  [ 200, { 'Content-Type' => 'text/plain' }, [ ENV['RACK_ENV'] ] ]
+  [ 200, { 'content-type' => 'text/plain' }, [ ENV['RACK_ENV'] ] ]
 }
   EOS
 
   HELLO = <<-EOS
 class Hello
   def call(env)
-    [ 200, { 'Content-Type' => 'text/plain' }, [ "HI\\n" ] ]
+    [ 200, { 'content-type' => 'text/plain' }, [ "HI\\n" ] ]
   end
 end
   EOS
@@ -62,9 +62,9 @@ def call(env)
   a = ::File.stat(pwd)
   b = ::File.stat(Dir.pwd)
   if (a.ino == b.ino && a.dev == b.dev)
-    [ 200, { 'Content-Type' => 'text/plain' }, [ pwd ] ]
+    [ 200, { 'content-type' => 'text/plain' }, [ pwd ] ]
   else
-    [ 404, { 'Content-Type' => 'text/plain' }, [] ]
+    [ 404, { 'content-type' => 'text/plain' }, [] ]
   end
 }
   EOS
@@ -255,7 +255,13 @@ def test_working_directory_controls_relative_paths
 end
 EOF
     pid = xfork { redirect_test_io { exec($unicorn_bin, "-c#{tmp.path}") } }
-    File.open("#{other.path}/fifo", "rb").close
+    begin
+      fifo = File.open("#{other.path}/fifo", "rb")
+    rescue Errno::EINTR
+      # OpenBSD raises Errno::EINTR when opening
+      return if RUBY_PLATFORM =~ /openbsd/
+    end
+    fifo.close
 
     assert ! File.exist?("stderr_log_here")
     assert ! File.exist?("stdout_log_here")
@@ -557,7 +563,7 @@ def test_unicorn_config_listen_with_options
   def test_unicorn_config_per_worker_listen
     port2 = unused_port
     pid_spit = 'use Rack::ContentLength;' \
-      'run proc { |e| [ 200, {"Content-Type"=>"text/plain"}, ["#$$\\n"] ] }'
+      'run proc { |e| [ 200, {"content-type"=>"text/plain"}, ["#$$\\n"] ] }'
     File.open("config.ru", "wb") { |fp| fp.syswrite(pid_spit) }
     tmp = Tempfile.new('test.socket')
     File.unlink(tmp.path)
diff --git a/test/unit/test_ccc.rb b/test/unit/test_ccc.rb
index 0dc72e8..f518230 100644
--- a/test/unit/test_ccc.rb
+++ b/test/unit/test_ccc.rb
@@ -29,7 +29,7 @@ def test_ccc_tcpi
         # will wake up when writer closes
         sleep_pipe[0].read if env['PATH_INFO'] == '/sleep'
 
-        [ 200, [ %w(Content-Length 0),  %w(Content-Type text/plain) ], [] ]
+        [ 200, {'content-length'=>'0', 'content-type'=>'text/plain'}, [] ]
       end
       ENV['UNICORN_FD'] = srv.fileno.to_s
       opts = {
diff --git a/test/unit/test_request.rb b/test/unit/test_request.rb
index 6cb0268..7f22b24 100644
--- a/test/unit/test_request.rb
+++ b/test/unit/test_request.rb
@@ -22,7 +22,7 @@ def kgio_addr
   def setup
     @request = HttpRequest.new
     @app = lambda do |env|
-      [ 200, { 'Content-Length' => '0', 'Content-Type' => 'text/plain' }, [] ]
+      [ 200, { 'content-length' => '0', 'content-type' => 'text/plain' }, [] ]
     end
     @lint = Rack::Lint.new(@app)
   end
diff --git a/test/unit/test_server.rb b/test/unit/test_server.rb
index bc9a222..98e85ab 100644
--- a/test/unit/test_server.rb
+++ b/test/unit/test_server.rb
@@ -16,7 +16,7 @@ class TestHandler
   def call(env)
     while env['rack.input'].read(4096)
     end
-    [200, { 'Content-Type' => 'text/plain' }, ['hello!\n']]
+    [200, { 'content-type' => 'text/plain' }, ['hello!\n']]
   rescue Unicorn::ClientShutdown, Unicorn::HttpParserError => e
     $stderr.syswrite("#{e.class}: #{e.message} #{e.backtrace.empty?}\n")
     raise e
@@ -30,7 +30,7 @@ def call(env)
     env['rack.early_hints'].call(
       "Link" => "</style.css>; rel=preload; as=style\n</script.js>; rel=preload"
     )
-    [200, { 'Content-Type' => 'text/plain' }, ['hello!\n']]
+    [200, { 'content-type' => 'text/plain' }, ['hello!\n']]
   end
 end
 
@@ -45,7 +45,7 @@ def call(env)
 
     env["rack.after_reply"] << -> { @called = true }
 
-    [200, { 'Content-Type' => 'text/plain' }, ["after_reply_called: #{@called}"]]
+    [200, { 'content-type' => 'text/plain' }, ["after_reply_called: #{@called}"]]
   rescue Unicorn::ClientShutdown, Unicorn::HttpParserError => e
     $stderr.syswrite("#{e.class}: #{e.message} #{e.backtrace.empty?}\n")
     raise e
@@ -81,7 +81,7 @@ def test_preload_app_config
       tmp.sysseek(0)
       tmp.truncate(0)
       tmp.syswrite($$)
-      lambda { |env| [ 200, { 'Content-Type' => 'text/plain' }, [ "#$$\n" ] ] }
+      lambda { |env| [ 200, { 'content-type' => 'text/plain' }, [ "#$$\n" ] ] }
     }
     redirect_test_io do
       @server = HttpServer.new(app, :listeners => [ "127.0.0.1:#@port"] )
diff --git a/test/unit/test_signals.rb b/test/unit/test_signals.rb
index 56a7dfc..6c48754 100644
--- a/test/unit/test_signals.rb
+++ b/test/unit/test_signals.rb
@@ -47,7 +47,7 @@ def teardown
 
   def test_worker_dies_on_dead_master
     pid = fork {
-      app = lambda { |env| [ 200, {'X-Pid' => "#$$" }, [] ] }
+      app = lambda { |env| [ 200, {'x-pid' => "#$$" }, [] ] }
       opts = @server_opts.merge(:timeout => 3)
       redirect_test_io { HttpServer.new(app, opts).start.join }
     }
@@ -56,7 +56,7 @@ def test_worker_dies_on_dead_master
     sock.syswrite("GET / HTTP/1.0\r\n\r\n")
     buf = sock.readpartial(4096)
     assert_nil sock.close
-    buf =~ /\bX-Pid: (\d+)\b/ or raise Exception
+    buf =~ /\bx-pid: (\d+)\b/ or raise Exception
     child = $1.to_i
     wait_master_ready("test_stderr.#{pid}.log")
     wait_workers_ready("test_stderr.#{pid}.log", 1)
@@ -120,7 +120,7 @@ def test_timeout_slow_response
 
   def test_response_write
     app = lambda { |env|
-      [ 200, { 'Content-Type' => 'text/plain', 'X-Pid' => Process.pid.to_s },
+      [ 200, { 'content-type' => 'text/plain', 'x-pid' => Process.pid.to_s },
         Dd.new(@bs, @count) ]
     }
     redirect_test_io { @server = HttpServer.new(app, @server_opts).start }
@@ -130,7 +130,7 @@ def test_response_write
     buf = ''
     header_len = pid = nil
     buf = sock.sysread(16384, buf)
-    pid = buf[/\r\nX-Pid: (\d+)\r\n/, 1].to_i
+    pid = buf[/\r\nx-pid: (\d+)\r\n/, 1].to_i
     header_len = buf[/\A(.+?\r\n\r\n)/m, 1].size
     assert pid > 0, "pid not positive: #{pid.inspect}"
     read = buf.size
@@ -158,14 +158,14 @@ def test_request_read
     app = lambda { |env|
       while env['rack.input'].read(4096)
       end
-      [ 200, {'Content-Type'=>'text/plain', 'X-Pid'=>Process.pid.to_s}, [] ]
+      [ 200, {'content-type'=>'text/plain', 'x-pid'=>Process.pid.to_s}, [] ]
     }
     redirect_test_io { @server = HttpServer.new(app, @server_opts).start }
 
     wait_workers_ready("test_stderr.#{$$}.log", 1)
     sock = tcp_socket('127.0.0.1', @port)
     sock.syswrite("GET / HTTP/1.0\r\n\r\n")
-    pid = sock.sysread(4096)[/\r\nX-Pid: (\d+)\r\n/, 1].to_i
+    pid = sock.sysread(4096)[/\r\nx-pid: (\d+)\r\n/, 1].to_i
     assert_nil sock.close
 
     assert pid > 0, "pid not positive: #{pid.inspect}"
@@ -182,7 +182,7 @@ def test_request_read
     redirect_test_io { @server.stop(true) }
     # can't check for == since pending signals get merged
     assert size_before < @tmp.stat.size
-    assert_equal pid, sock.sysread(4096)[/\r\nX-Pid: (\d+)\r\n/, 1].to_i
+    assert_equal pid, sock.sysread(4096)[/\r\nx-pid: (\d+)\r\n/, 1].to_i
     assert_nil sock.close
   end
 end
-- 
2.40.0