From: Jeremy Evans <code@jeremyevans.net>
To: unicorn-public@yhbt.net
Subject: Rack 3 Compatibility
Date: Thu, 01 Jun 2023 18:54:19 +0000 (UTC) [thread overview]
Message-ID: <ZHjpWbJOhHZBtYSH@jeremyevans.local> (raw)
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
next reply other threads:[~2023-06-01 18:54 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-01 18:54 Jeremy Evans [this message]
2023-06-02 0:00 ` Rack 3 Compatibility Eric Wong
2023-06-02 0:27 ` Eric Wong
2023-06-02 2:45 ` Jeremy Evans
2023-06-05 9:12 ` [PATCH v2] chunk unterminated HTTP/1.1 responses for Rack 3.1 Eric Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://yhbt.net/unicorn/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZHjpWbJOhHZBtYSH@jeremyevans.local \
--to=code@jeremyevans.net \
--cc=unicorn-public@yhbt.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
http://yhbt.net/unicorn.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).