* Rack 3 Compatibility
@ 2023-06-01 18:54 Jeremy Evans
2023-06-02 0:00 ` Eric Wong
0 siblings, 1 reply; 5+ messages in thread
From: Jeremy Evans @ 2023-06-01 18:54 UTC (permalink / raw)
To: unicorn-public
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: Rack 3 Compatibility
2023-06-01 18:54 Rack 3 Compatibility Jeremy Evans
@ 2023-06-02 0:00 ` Eric Wong
2023-06-02 0:27 ` Eric Wong
2023-06-02 2:45 ` Jeremy Evans
0 siblings, 2 replies; 5+ messages in thread
From: Eric Wong @ 2023-06-02 0:00 UTC (permalink / raw)
To: Jeremy Evans; +Cc: unicorn-public
Jeremy Evans <code@jeremyevans.net> wrote:
> 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.
Isn't a chunk replacement needed for Rack 3.1, also?
I dunno if I missed anything else in Rack 3.x; and don't want to
make too many releases if we can do 3.0 and 3.1 in one go.
-------8<-------
Subject: [PATCH] chunk unterminated HTTP/1.1 responses
Rack::Chunked will be gone in Rack 3.1, so provide a
non-middleware fallback which takes advantage of IO#write
supporting multiple arguments in Ruby 2.5+.
We still need to support Ruby 2.4, at least, since Rack 3.0
does. So a new (GC-unfriendly) Unicorn::WriteSplat module now
exists for Ruby <= 2.4 users.
---
ext/unicorn_http/unicorn_http.rl | 11 +++++++++++
lib/unicorn.rb | 4 ++--
lib/unicorn/http_response.rb | 21 ++++++++++++++++++++-
lib/unicorn/socket_helper.rb | 18 ++++++++++++++++--
lib/unicorn/write_splat.rb | 7 +++++++
test/unit/test_server.rb | 2 +-
6 files changed, 57 insertions(+), 6 deletions(-)
create mode 100644 lib/unicorn/write_splat.rb
diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl
index ba23438..c339024 100644
--- a/ext/unicorn_http/unicorn_http.rl
+++ b/ext/unicorn_http/unicorn_http.rl
@@ -28,6 +28,7 @@ void init_unicorn_httpdate(void);
#define UH_FL_TO_CLEAR 0x200
#define UH_FL_RESSTART 0x400 /* for check_client_connection */
#define UH_FL_HIJACK 0x800
+#define UH_FL_RES_CHUNK_OK (1U << 12)
/* all of these flags need to be set for keepalive to be supported */
#define UH_FL_KEEPALIVE (UH_FL_KAVERSION | UH_FL_REQEOF | UH_FL_HASHEADER)
@@ -158,6 +159,7 @@ http_version(struct http_parser *hp, const char *ptr, size_t len)
if (CONST_MEM_EQ("HTTP/1.1", ptr, len)) {
/* HTTP/1.1 implies keepalive unless "Connection: close" is set */
HP_FL_SET(hp, KAVERSION);
+ HP_FL_SET(hp, RES_CHUNK_OK);
v = g_http_11;
} else if (CONST_MEM_EQ("HTTP/1.0", ptr, len)) {
v = g_http_10;
@@ -801,6 +803,14 @@ static VALUE HttpParser_keepalive(VALUE self)
return HP_FL_ALL(hp, KEEPALIVE) ? Qtrue : Qfalse;
}
+/* :nodoc: */
+static VALUE chunkable_response_p(VALUE self)
+{
+ struct http_parser *hp = data_get(self);
+
+ return HP_FL_ALL(hp, RES_CHUNK_OK) ? Qtrue : Qfalse;
+}
+
/**
* call-seq:
* parser.next? => true or false
@@ -981,6 +991,7 @@ void Init_unicorn_http(void)
rb_define_method(cHttpParser, "content_length", HttpParser_content_length, 0);
rb_define_method(cHttpParser, "body_eof?", HttpParser_body_eof, 0);
rb_define_method(cHttpParser, "keepalive?", HttpParser_keepalive, 0);
+ rb_define_method(cHttpParser, "chunkable_response?", chunkable_response_p, 0);
rb_define_method(cHttpParser, "headers?", HttpParser_has_headers, 0);
rb_define_method(cHttpParser, "next?", HttpParser_next, 0);
rb_define_method(cHttpParser, "buf", HttpParser_buf, 0);
diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index 1a50631..8b1cda7 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -75,8 +75,8 @@ def self.builder(ru, op)
# return value, matches rackup defaults based on env
# Unicorn does not support persistent connections, but Rainbows!
- # and Zbatery both do. Users accustomed to the Rack::Server default
- # middlewares will need ContentLength/Chunked middlewares.
+ # does. Users accustomed to the Rack::Server default
+ # middlewares will need ContentLength middleware.
case ENV["RACK_ENV"]
when "development"
when "deployment"
diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb
index 19469b4..342dd0b 100644
--- a/lib/unicorn/http_response.rb
+++ b/lib/unicorn/http_response.rb
@@ -35,11 +35,12 @@ def append_header(buf, key, value)
def http_response_write(socket, status, headers, body,
req = Unicorn::HttpRequest.new)
hijack = nil
-
+ do_chunk = false
if headers
code = status.to_i
msg = STATUS_CODES[code]
start = req.response_start_sent ? ''.freeze : 'HTTP/1.1 '.freeze
+ term = false
buf = "#{start}#{msg ? %Q(#{code} #{msg}) : status}\r\n" \
"Date: #{httpdate}\r\n" \
"Connection: close\r\n"
@@ -47,6 +48,12 @@ def http_response_write(socket, status, headers, body,
case key
when %r{\A(?:Date|Connection)\z}i
next
+ when %r{\AContent-Length\z}i
+ append_header(buf, key, value)
+ term = true
+ when %r{\ATransfer-Encoding\z}i
+ append_header(buf, key, value)
+ term = true if /\bchunked\b/i === value # value may be Array :x
when "rack.hijack"
# This should only be hit under Rack >= 1.5, as this was an illegal
# key in Rack < 1.5
@@ -55,12 +62,24 @@ def http_response_write(socket, status, headers, body,
append_header(buf, key, value)
end
end
+ if !hijack && !term && req.chunkable_response?
+ do_chunk = true
+ buf << "Transfer-Encoding: chunked\r\n".freeze
+ end
socket.write(buf << "\r\n".freeze)
end
if hijack
req.hijacked!
hijack.call(socket)
+ elsif do_chunk
+ begin
+ body.each do |b|
+ socket.write("#{b.bytesize.to_s(16)}\r\n", b, "\r\n".freeze)
+ end
+ ensure
+ socket.write("0\r\n\r\n".freeze)
+ end
else
body.each { |chunk| socket.write(chunk) }
end
diff --git a/lib/unicorn/socket_helper.rb b/lib/unicorn/socket_helper.rb
index 8a6f6ee..4ae4c85 100644
--- a/lib/unicorn/socket_helper.rb
+++ b/lib/unicorn/socket_helper.rb
@@ -15,6 +15,20 @@ def kgio_tryaccept # :nodoc:
end
end
+ if IO.instance_method(:write).arity # Ruby <= 2.4
+ require 'unicorn/write_splat'
+ UNIXClient = Class.new(Kgio::Socket) # :nodoc:
+ class UNIXSrv < Kgio::UNIXServer # :nodoc:
+ include Unicorn::WriteSplat
+ def kgio_tryaccept # :nodoc:
+ super(UNIXClient)
+ end
+ end
+ TCPClient.__send__(:include, Unicorn::WriteSplat)
+ else # Ruby 2.5+
+ UNIXSrv = Kgio::UNIXServer
+ end
+
module SocketHelper
# internal interface
@@ -135,7 +149,7 @@ def bind_listen(address = '0.0.0.0:8080', opt = {})
end
old_umask = File.umask(opt[:umask] || 0)
begin
- Kgio::UNIXServer.new(address)
+ UNIXSrv.new(address)
ensure
File.umask(old_umask)
end
@@ -203,7 +217,7 @@ def server_cast(sock)
Socket.unpack_sockaddr_in(sock.getsockname)
TCPSrv.for_fd(sock.fileno)
rescue ArgumentError
- Kgio::UNIXServer.for_fd(sock.fileno)
+ UNIXSrv.for_fd(sock.fileno)
end
end
diff --git a/lib/unicorn/write_splat.rb b/lib/unicorn/write_splat.rb
new file mode 100644
index 0000000..7e6e363
--- /dev/null
+++ b/lib/unicorn/write_splat.rb
@@ -0,0 +1,7 @@
+# -*- encoding: binary -*-
+# compatibility module for Ruby <= 2.4, remove when we go Ruby 2.5+
+module Unicorn::WriteSplat # :nodoc:
+ def write(*arg) # :nodoc:
+ super(arg.join(''))
+ end
+end
diff --git a/test/unit/test_server.rb b/test/unit/test_server.rb
index 98e85ab..cea9791 100644
--- a/test/unit/test_server.rb
+++ b/test/unit/test_server.rb
@@ -196,7 +196,7 @@ def test_client_shutdown_writes
# continue to process our request and never hit EOFError on our sock
sock.shutdown(Socket::SHUT_WR)
buf = sock.read
- assert_equal 'hello!\n', buf.split(/\r\n\r\n/).last
+ assert_match %r{\bhello!\\n\b}, buf.split(/\r\n\r\n/).last
next_client = Net::HTTP.get(URI.parse("http://127.0.0.1:#@port/"))
assert_equal 'hello!\n', next_client
lines = File.readlines("test_stderr.#$$.log")
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: Rack 3 Compatibility
2023-06-02 0:00 ` Eric Wong
@ 2023-06-02 0:27 ` Eric Wong
2023-06-02 2:45 ` Jeremy Evans
1 sibling, 0 replies; 5+ messages in thread
From: Eric Wong @ 2023-06-02 0:27 UTC (permalink / raw)
To: Jeremy Evans; +Cc: unicorn-public
Eric Wong <bofh@yhbt.net> wrote:
> +++ b/lib/unicorn/socket_helper.rb
> @@ -15,6 +15,20 @@ def kgio_tryaccept # :nodoc:
> end
> end
>
> + if IO.instance_method(:write).arity # Ruby <= 2.4
Erm, that should be:
if IO.instance_method(:write).arity == 1 # Ruby <= 2.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Rack 3 Compatibility
2023-06-02 0:00 ` 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
1 sibling, 1 reply; 5+ messages in thread
From: Jeremy Evans @ 2023-06-02 2:45 UTC (permalink / raw)
To: Eric Wong; +Cc: unicorn-public
On 06/02 12:00, Eric Wong wrote:
> Jeremy Evans <code@jeremyevans.net> wrote:
> > 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.
>
> Isn't a chunk replacement needed for Rack 3.1, also?
> I dunno if I missed anything else in Rack 3.x; and don't want to
> make too many releases if we can do 3.0 and 3.1 in one go.
We deprecated Rack::Chunked in Rack 3.0 and plan to remove it in Rack
3.1. I agree it would be best to deal with this now, I just wasn't sure
how you wanted to handle it. Your patch below to deal with it at the
server level looks good, though it doesn't appear to remove the Chunked
usage at line 69 of unicorn.rb. I recommend that also be removed.
Thanks,
Jeremy
> -------8<-------
> Subject: [PATCH] chunk unterminated HTTP/1.1 responses
>
> Rack::Chunked will be gone in Rack 3.1, so provide a
> non-middleware fallback which takes advantage of IO#write
> supporting multiple arguments in Ruby 2.5+.
>
> We still need to support Ruby 2.4, at least, since Rack 3.0
> does. So a new (GC-unfriendly) Unicorn::WriteSplat module now
> exists for Ruby <= 2.4 users.
> ---
> ext/unicorn_http/unicorn_http.rl | 11 +++++++++++
> lib/unicorn.rb | 4 ++--
> lib/unicorn/http_response.rb | 21 ++++++++++++++++++++-
> lib/unicorn/socket_helper.rb | 18 ++++++++++++++++--
> lib/unicorn/write_splat.rb | 7 +++++++
> test/unit/test_server.rb | 2 +-
> 6 files changed, 57 insertions(+), 6 deletions(-)
> create mode 100644 lib/unicorn/write_splat.rb
>
> diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl
> index ba23438..c339024 100644
> --- a/ext/unicorn_http/unicorn_http.rl
> +++ b/ext/unicorn_http/unicorn_http.rl
> @@ -28,6 +28,7 @@ void init_unicorn_httpdate(void);
> #define UH_FL_TO_CLEAR 0x200
> #define UH_FL_RESSTART 0x400 /* for check_client_connection */
> #define UH_FL_HIJACK 0x800
> +#define UH_FL_RES_CHUNK_OK (1U << 12)
>
> /* all of these flags need to be set for keepalive to be supported */
> #define UH_FL_KEEPALIVE (UH_FL_KAVERSION | UH_FL_REQEOF | UH_FL_HASHEADER)
> @@ -158,6 +159,7 @@ http_version(struct http_parser *hp, const char *ptr, size_t len)
> if (CONST_MEM_EQ("HTTP/1.1", ptr, len)) {
> /* HTTP/1.1 implies keepalive unless "Connection: close" is set */
> HP_FL_SET(hp, KAVERSION);
> + HP_FL_SET(hp, RES_CHUNK_OK);
> v = g_http_11;
> } else if (CONST_MEM_EQ("HTTP/1.0", ptr, len)) {
> v = g_http_10;
> @@ -801,6 +803,14 @@ static VALUE HttpParser_keepalive(VALUE self)
> return HP_FL_ALL(hp, KEEPALIVE) ? Qtrue : Qfalse;
> }
>
> +/* :nodoc: */
> +static VALUE chunkable_response_p(VALUE self)
> +{
> + struct http_parser *hp = data_get(self);
> +
> + return HP_FL_ALL(hp, RES_CHUNK_OK) ? Qtrue : Qfalse;
> +}
> +
> /**
> * call-seq:
> * parser.next? => true or false
> @@ -981,6 +991,7 @@ void Init_unicorn_http(void)
> rb_define_method(cHttpParser, "content_length", HttpParser_content_length, 0);
> rb_define_method(cHttpParser, "body_eof?", HttpParser_body_eof, 0);
> rb_define_method(cHttpParser, "keepalive?", HttpParser_keepalive, 0);
> + rb_define_method(cHttpParser, "chunkable_response?", chunkable_response_p, 0);
> rb_define_method(cHttpParser, "headers?", HttpParser_has_headers, 0);
> rb_define_method(cHttpParser, "next?", HttpParser_next, 0);
> rb_define_method(cHttpParser, "buf", HttpParser_buf, 0);
> diff --git a/lib/unicorn.rb b/lib/unicorn.rb
> index 1a50631..8b1cda7 100644
> --- a/lib/unicorn.rb
> +++ b/lib/unicorn.rb
> @@ -75,8 +75,8 @@ def self.builder(ru, op)
>
> # return value, matches rackup defaults based on env
> # Unicorn does not support persistent connections, but Rainbows!
> - # and Zbatery both do. Users accustomed to the Rack::Server default
> - # middlewares will need ContentLength/Chunked middlewares.
> + # does. Users accustomed to the Rack::Server default
> + # middlewares will need ContentLength middleware.
> case ENV["RACK_ENV"]
> when "development"
> when "deployment"
> diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb
> index 19469b4..342dd0b 100644
> --- a/lib/unicorn/http_response.rb
> +++ b/lib/unicorn/http_response.rb
> @@ -35,11 +35,12 @@ def append_header(buf, key, value)
> def http_response_write(socket, status, headers, body,
> req = Unicorn::HttpRequest.new)
> hijack = nil
> -
> + do_chunk = false
> if headers
> code = status.to_i
> msg = STATUS_CODES[code]
> start = req.response_start_sent ? ''.freeze : 'HTTP/1.1 '.freeze
> + term = false
> buf = "#{start}#{msg ? %Q(#{code} #{msg}) : status}\r\n" \
> "Date: #{httpdate}\r\n" \
> "Connection: close\r\n"
> @@ -47,6 +48,12 @@ def http_response_write(socket, status, headers, body,
> case key
> when %r{\A(?:Date|Connection)\z}i
> next
> + when %r{\AContent-Length\z}i
> + append_header(buf, key, value)
> + term = true
> + when %r{\ATransfer-Encoding\z}i
> + append_header(buf, key, value)
> + term = true if /\bchunked\b/i === value # value may be Array :x
> when "rack.hijack"
> # This should only be hit under Rack >= 1.5, as this was an illegal
> # key in Rack < 1.5
> @@ -55,12 +62,24 @@ def http_response_write(socket, status, headers, body,
> append_header(buf, key, value)
> end
> end
> + if !hijack && !term && req.chunkable_response?
> + do_chunk = true
> + buf << "Transfer-Encoding: chunked\r\n".freeze
> + end
> socket.write(buf << "\r\n".freeze)
> end
>
> if hijack
> req.hijacked!
> hijack.call(socket)
> + elsif do_chunk
> + begin
> + body.each do |b|
> + socket.write("#{b.bytesize.to_s(16)}\r\n", b, "\r\n".freeze)
> + end
> + ensure
> + socket.write("0\r\n\r\n".freeze)
> + end
> else
> body.each { |chunk| socket.write(chunk) }
> end
> diff --git a/lib/unicorn/socket_helper.rb b/lib/unicorn/socket_helper.rb
> index 8a6f6ee..4ae4c85 100644
> --- a/lib/unicorn/socket_helper.rb
> +++ b/lib/unicorn/socket_helper.rb
> @@ -15,6 +15,20 @@ def kgio_tryaccept # :nodoc:
> end
> end
>
> + if IO.instance_method(:write).arity # Ruby <= 2.4
> + require 'unicorn/write_splat'
> + UNIXClient = Class.new(Kgio::Socket) # :nodoc:
> + class UNIXSrv < Kgio::UNIXServer # :nodoc:
> + include Unicorn::WriteSplat
> + def kgio_tryaccept # :nodoc:
> + super(UNIXClient)
> + end
> + end
> + TCPClient.__send__(:include, Unicorn::WriteSplat)
> + else # Ruby 2.5+
> + UNIXSrv = Kgio::UNIXServer
> + end
> +
> module SocketHelper
>
> # internal interface
> @@ -135,7 +149,7 @@ def bind_listen(address = '0.0.0.0:8080', opt = {})
> end
> old_umask = File.umask(opt[:umask] || 0)
> begin
> - Kgio::UNIXServer.new(address)
> + UNIXSrv.new(address)
> ensure
> File.umask(old_umask)
> end
> @@ -203,7 +217,7 @@ def server_cast(sock)
> Socket.unpack_sockaddr_in(sock.getsockname)
> TCPSrv.for_fd(sock.fileno)
> rescue ArgumentError
> - Kgio::UNIXServer.for_fd(sock.fileno)
> + UNIXSrv.for_fd(sock.fileno)
> end
> end
>
> diff --git a/lib/unicorn/write_splat.rb b/lib/unicorn/write_splat.rb
> new file mode 100644
> index 0000000..7e6e363
> --- /dev/null
> +++ b/lib/unicorn/write_splat.rb
> @@ -0,0 +1,7 @@
> +# -*- encoding: binary -*-
> +# compatibility module for Ruby <= 2.4, remove when we go Ruby 2.5+
> +module Unicorn::WriteSplat # :nodoc:
> + def write(*arg) # :nodoc:
> + super(arg.join(''))
> + end
> +end
> diff --git a/test/unit/test_server.rb b/test/unit/test_server.rb
> index 98e85ab..cea9791 100644
> --- a/test/unit/test_server.rb
> +++ b/test/unit/test_server.rb
> @@ -196,7 +196,7 @@ def test_client_shutdown_writes
> # continue to process our request and never hit EOFError on our sock
> sock.shutdown(Socket::SHUT_WR)
> buf = sock.read
> - assert_equal 'hello!\n', buf.split(/\r\n\r\n/).last
> + assert_match %r{\bhello!\\n\b}, buf.split(/\r\n\r\n/).last
> next_client = Net::HTTP.get(URI.parse("http://127.0.0.1:#@port/"))
> assert_equal 'hello!\n', next_client
> lines = File.readlines("test_stderr.#$$.log")
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] chunk unterminated HTTP/1.1 responses for Rack 3.1
2023-06-02 2:45 ` Jeremy Evans
@ 2023-06-05 9:12 ` Eric Wong
0 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2023-06-05 9:12 UTC (permalink / raw)
To: Jeremy Evans; +Cc: unicorn-public
Jeremy Evans <code@jeremyevans.net> wrote:
> We deprecated Rack::Chunked in Rack 3.0 and plan to remove it in Rack
> 3.1. I agree it would be best to deal with this now, I just wasn't sure
> how you wanted to handle it. Your patch below to deal with it at the
> server level looks good, though it doesn't appear to remove the Chunked
> usage at line 69 of unicorn.rb. I recommend that also be removed.
OK. Also added HEAD and STATUS_WITH_NO_ENTITY_BODY checks...
No tests, yet; they'll be in Perl 5. (I started rewriting a bunch
of tests in Perl5 last year since tests are where Ruby's yearly
breaking changes are most unacceptable to me).
No trailers for responses yet, either; I didn't realize Rack::Chunked
added special support for that in 2020.
I care deeply about trailers in requests, but never used them
for responses.
--------8<-------
Subject: [PATCH v2] chunk unterminated HTTP/1.1 responses for Rack 3.1
Rack::Chunked will be gone in Rack 3.1, so provide a
non-middleware fallback which takes advantage of IO#write
supporting multiple arguments in Ruby 2.5+.
We still need to support Ruby 2.4, at least, since Rack 3.0
does. So a new (GC-unfriendly) Unicorn::WriteSplat module now
exists for Ruby <= 2.4 users.
---
v2: remove Rack::Chunk load attempt
fix arity check for Ruby <= 2.4
update docs + examples
Interdiff:
diff --git a/Documentation/unicorn.1 b/Documentation/unicorn.1
index d76d40f..b2c5e70 100644
--- a/Documentation/unicorn.1
+++ b/Documentation/unicorn.1
@@ -176,7 +176,7 @@ As of Unicorn 0.94.0, RACK_ENV is exported as a process\-wide environment
variable as well. While not current a part of the Rack specification as
of Rack 1.0.1, this has become a de facto standard in the Rack world.
.PP
-Note the Rack::ContentLength and Rack::Chunked middlewares are also
+Note the Rack::ContentLength middleware is also
loaded by "deployment" and "development", but no other values of
RACK_ENV. If needed, they must be individually specified in the
RACKUP_FILE, some frameworks do not require them.
diff --git a/examples/echo.ru b/examples/echo.ru
index 14908c5..e982180 100644
--- a/examples/echo.ru
+++ b/examples/echo.ru
@@ -19,7 +19,6 @@ def each(&block)
end
-use Rack::Chunked
run lambda { |env|
/\A100-continue\z/i =~ env['HTTP_EXPECT'] and return [100, {}, []]
[ 200, { 'Content-Type' => 'application/octet-stream' },
diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl
index c339024..afdf680 100644
--- a/ext/unicorn_http/unicorn_http.rl
+++ b/ext/unicorn_http/unicorn_http.rl
@@ -28,11 +28,15 @@ void init_unicorn_httpdate(void);
#define UH_FL_TO_CLEAR 0x200
#define UH_FL_RESSTART 0x400 /* for check_client_connection */
#define UH_FL_HIJACK 0x800
-#define UH_FL_RES_CHUNK_OK (1U << 12)
+#define UH_FL_RES_CHUNK_VER (1U << 12)
+#define UH_FL_RES_CHUNK_METHOD (1U << 13)
/* all of these flags need to be set for keepalive to be supported */
#define UH_FL_KEEPALIVE (UH_FL_KAVERSION | UH_FL_REQEOF | UH_FL_HASHEADER)
+/* we can only chunk responses for non-HEAD HTTP/1.1 requests */
+#define UH_FL_RES_CHUNKABLE (UH_FL_RES_CHUNK_VER | UH_FL_RES_CHUNK_METHOD)
+
static unsigned int MAX_HEADER_LEN = 1024 * (80 + 32); /* same as Mongrel */
/* this is only intended for use with Rainbows! */
@@ -146,6 +150,9 @@ request_method(struct http_parser *hp, const char *ptr, size_t len)
{
VALUE v = rb_str_new(ptr, len);
+ if (len != 4 || memcmp(ptr, "HEAD", 4))
+ HP_FL_SET(hp, RES_CHUNK_METHOD);
+
rb_hash_aset(hp->env, g_request_method, v);
}
@@ -159,7 +166,7 @@ http_version(struct http_parser *hp, const char *ptr, size_t len)
if (CONST_MEM_EQ("HTTP/1.1", ptr, len)) {
/* HTTP/1.1 implies keepalive unless "Connection: close" is set */
HP_FL_SET(hp, KAVERSION);
- HP_FL_SET(hp, RES_CHUNK_OK);
+ HP_FL_SET(hp, RES_CHUNK_VER);
v = g_http_11;
} else if (CONST_MEM_EQ("HTTP/1.0", ptr, len)) {
v = g_http_10;
@@ -806,9 +813,9 @@ static VALUE HttpParser_keepalive(VALUE self)
/* :nodoc: */
static VALUE chunkable_response_p(VALUE self)
{
- struct http_parser *hp = data_get(self);
+ const struct http_parser *hp = data_get(self);
- return HP_FL_ALL(hp, RES_CHUNK_OK) ? Qtrue : Qfalse;
+ return HP_FL_ALL(hp, RES_CHUNKABLE) ? Qtrue : Qfalse;
}
/**
diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index 8b1cda7..b817b77 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -66,7 +66,6 @@ def self.builder(ru, op)
middleware = { # order matters
ContentLength: nil,
- Chunked: nil,
CommonLogger: [ $stderr ],
ShowExceptions: nil,
Lint: nil,
diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb
index 342dd0b..0ed0ae3 100644
--- a/lib/unicorn/http_response.rb
+++ b/lib/unicorn/http_response.rb
@@ -12,6 +12,12 @@ module Unicorn::HttpResponse
STATUS_CODES = defined?(Rack::Utils::HTTP_STATUS_CODES) ?
Rack::Utils::HTTP_STATUS_CODES : {}
+ STATUS_WITH_NO_ENTITY_BODY = defined?(
+ Rack::Utils::STATUS_WITH_NO_ENTITY_BODY) ?
+ Rack::Utils::STATUS_WITH_NO_ENTITY_BODY : begin
+ warn 'Rack::Utils::STATUS_WITH_NO_ENTITY_BODY missing'
+ {}
+ end
# internal API, code will always be common-enough-for-even-old-Rack
def err_response(code, response_start_sent)
@@ -40,7 +46,7 @@ def http_response_write(socket, status, headers, body,
code = status.to_i
msg = STATUS_CODES[code]
start = req.response_start_sent ? ''.freeze : 'HTTP/1.1 '.freeze
- term = false
+ term = STATUS_WITH_NO_ENTITY_BODY.include?(code) || false
buf = "#{start}#{msg ? %Q(#{code} #{msg}) : status}\r\n" \
"Date: #{httpdate}\r\n" \
"Connection: close\r\n"
diff --git a/lib/unicorn/socket_helper.rb b/lib/unicorn/socket_helper.rb
index 4ae4c85..c2ba75e 100644
--- a/lib/unicorn/socket_helper.rb
+++ b/lib/unicorn/socket_helper.rb
@@ -15,7 +15,7 @@ def kgio_tryaccept # :nodoc:
end
end
- if IO.instance_method(:write).arity # Ruby <= 2.4
+ if IO.instance_method(:write).arity == 1 # Ruby <= 2.4
require 'unicorn/write_splat'
UNIXClient = Class.new(Kgio::Socket) # :nodoc:
class UNIXSrv < Kgio::UNIXServer # :nodoc:
diff --git a/test/unit/test_server.rb b/test/unit/test_server.rb
index cea9791..fe98fcc 100644
--- a/test/unit/test_server.rb
+++ b/test/unit/test_server.rb
@@ -196,7 +196,7 @@ def test_client_shutdown_writes
# continue to process our request and never hit EOFError on our sock
sock.shutdown(Socket::SHUT_WR)
buf = sock.read
- assert_match %r{\bhello!\\n\b}, buf.split(/\r\n\r\n/).last
+ assert_match %r{\bhello!\\n\b}, buf.split(/\r\n\r\n/, 2).last
next_client = Net::HTTP.get(URI.parse("http://127.0.0.1:#@port/"))
assert_equal 'hello!\n', next_client
lines = File.readlines("test_stderr.#$$.log")
Documentation/unicorn.1 | 2 +-
examples/echo.ru | 1 -
ext/unicorn_http/unicorn_http.rl | 18 ++++++++++++++++++
lib/unicorn.rb | 5 ++---
lib/unicorn/http_response.rb | 27 ++++++++++++++++++++++++++-
lib/unicorn/socket_helper.rb | 18 ++++++++++++++++--
lib/unicorn/write_splat.rb | 7 +++++++
test/unit/test_server.rb | 2 +-
8 files changed, 71 insertions(+), 9 deletions(-)
create mode 100644 lib/unicorn/write_splat.rb
diff --git a/Documentation/unicorn.1 b/Documentation/unicorn.1
index d76d40f..b2c5e70 100644
--- a/Documentation/unicorn.1
+++ b/Documentation/unicorn.1
@@ -176,7 +176,7 @@ As of Unicorn 0.94.0, RACK_ENV is exported as a process\-wide environment
variable as well. While not current a part of the Rack specification as
of Rack 1.0.1, this has become a de facto standard in the Rack world.
.PP
-Note the Rack::ContentLength and Rack::Chunked middlewares are also
+Note the Rack::ContentLength middleware is also
loaded by "deployment" and "development", but no other values of
RACK_ENV. If needed, they must be individually specified in the
RACKUP_FILE, some frameworks do not require them.
diff --git a/examples/echo.ru b/examples/echo.ru
index 14908c5..e982180 100644
--- a/examples/echo.ru
+++ b/examples/echo.ru
@@ -19,7 +19,6 @@ def each(&block)
end
-use Rack::Chunked
run lambda { |env|
/\A100-continue\z/i =~ env['HTTP_EXPECT'] and return [100, {}, []]
[ 200, { 'Content-Type' => 'application/octet-stream' },
diff --git a/ext/unicorn_http/unicorn_http.rl b/ext/unicorn_http/unicorn_http.rl
index ba23438..afdf680 100644
--- a/ext/unicorn_http/unicorn_http.rl
+++ b/ext/unicorn_http/unicorn_http.rl
@@ -28,10 +28,15 @@ void init_unicorn_httpdate(void);
#define UH_FL_TO_CLEAR 0x200
#define UH_FL_RESSTART 0x400 /* for check_client_connection */
#define UH_FL_HIJACK 0x800
+#define UH_FL_RES_CHUNK_VER (1U << 12)
+#define UH_FL_RES_CHUNK_METHOD (1U << 13)
/* all of these flags need to be set for keepalive to be supported */
#define UH_FL_KEEPALIVE (UH_FL_KAVERSION | UH_FL_REQEOF | UH_FL_HASHEADER)
+/* we can only chunk responses for non-HEAD HTTP/1.1 requests */
+#define UH_FL_RES_CHUNKABLE (UH_FL_RES_CHUNK_VER | UH_FL_RES_CHUNK_METHOD)
+
static unsigned int MAX_HEADER_LEN = 1024 * (80 + 32); /* same as Mongrel */
/* this is only intended for use with Rainbows! */
@@ -145,6 +150,9 @@ request_method(struct http_parser *hp, const char *ptr, size_t len)
{
VALUE v = rb_str_new(ptr, len);
+ if (len != 4 || memcmp(ptr, "HEAD", 4))
+ HP_FL_SET(hp, RES_CHUNK_METHOD);
+
rb_hash_aset(hp->env, g_request_method, v);
}
@@ -158,6 +166,7 @@ http_version(struct http_parser *hp, const char *ptr, size_t len)
if (CONST_MEM_EQ("HTTP/1.1", ptr, len)) {
/* HTTP/1.1 implies keepalive unless "Connection: close" is set */
HP_FL_SET(hp, KAVERSION);
+ HP_FL_SET(hp, RES_CHUNK_VER);
v = g_http_11;
} else if (CONST_MEM_EQ("HTTP/1.0", ptr, len)) {
v = g_http_10;
@@ -801,6 +810,14 @@ static VALUE HttpParser_keepalive(VALUE self)
return HP_FL_ALL(hp, KEEPALIVE) ? Qtrue : Qfalse;
}
+/* :nodoc: */
+static VALUE chunkable_response_p(VALUE self)
+{
+ const struct http_parser *hp = data_get(self);
+
+ return HP_FL_ALL(hp, RES_CHUNKABLE) ? Qtrue : Qfalse;
+}
+
/**
* call-seq:
* parser.next? => true or false
@@ -981,6 +998,7 @@ void Init_unicorn_http(void)
rb_define_method(cHttpParser, "content_length", HttpParser_content_length, 0);
rb_define_method(cHttpParser, "body_eof?", HttpParser_body_eof, 0);
rb_define_method(cHttpParser, "keepalive?", HttpParser_keepalive, 0);
+ rb_define_method(cHttpParser, "chunkable_response?", chunkable_response_p, 0);
rb_define_method(cHttpParser, "headers?", HttpParser_has_headers, 0);
rb_define_method(cHttpParser, "next?", HttpParser_next, 0);
rb_define_method(cHttpParser, "buf", HttpParser_buf, 0);
diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index 1a50631..b817b77 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -66,7 +66,6 @@ def self.builder(ru, op)
middleware = { # order matters
ContentLength: nil,
- Chunked: nil,
CommonLogger: [ $stderr ],
ShowExceptions: nil,
Lint: nil,
@@ -75,8 +74,8 @@ def self.builder(ru, op)
# return value, matches rackup defaults based on env
# Unicorn does not support persistent connections, but Rainbows!
- # and Zbatery both do. Users accustomed to the Rack::Server default
- # middlewares will need ContentLength/Chunked middlewares.
+ # does. Users accustomed to the Rack::Server default
+ # middlewares will need ContentLength middleware.
case ENV["RACK_ENV"]
when "development"
when "deployment"
diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb
index 19469b4..0ed0ae3 100644
--- a/lib/unicorn/http_response.rb
+++ b/lib/unicorn/http_response.rb
@@ -12,6 +12,12 @@ module Unicorn::HttpResponse
STATUS_CODES = defined?(Rack::Utils::HTTP_STATUS_CODES) ?
Rack::Utils::HTTP_STATUS_CODES : {}
+ STATUS_WITH_NO_ENTITY_BODY = defined?(
+ Rack::Utils::STATUS_WITH_NO_ENTITY_BODY) ?
+ Rack::Utils::STATUS_WITH_NO_ENTITY_BODY : begin
+ warn 'Rack::Utils::STATUS_WITH_NO_ENTITY_BODY missing'
+ {}
+ end
# internal API, code will always be common-enough-for-even-old-Rack
def err_response(code, response_start_sent)
@@ -35,11 +41,12 @@ def append_header(buf, key, value)
def http_response_write(socket, status, headers, body,
req = Unicorn::HttpRequest.new)
hijack = nil
-
+ do_chunk = false
if headers
code = status.to_i
msg = STATUS_CODES[code]
start = req.response_start_sent ? ''.freeze : 'HTTP/1.1 '.freeze
+ term = STATUS_WITH_NO_ENTITY_BODY.include?(code) || false
buf = "#{start}#{msg ? %Q(#{code} #{msg}) : status}\r\n" \
"Date: #{httpdate}\r\n" \
"Connection: close\r\n"
@@ -47,6 +54,12 @@ def http_response_write(socket, status, headers, body,
case key
when %r{\A(?:Date|Connection)\z}i
next
+ when %r{\AContent-Length\z}i
+ append_header(buf, key, value)
+ term = true
+ when %r{\ATransfer-Encoding\z}i
+ append_header(buf, key, value)
+ term = true if /\bchunked\b/i === value # value may be Array :x
when "rack.hijack"
# This should only be hit under Rack >= 1.5, as this was an illegal
# key in Rack < 1.5
@@ -55,12 +68,24 @@ def http_response_write(socket, status, headers, body,
append_header(buf, key, value)
end
end
+ if !hijack && !term && req.chunkable_response?
+ do_chunk = true
+ buf << "Transfer-Encoding: chunked\r\n".freeze
+ end
socket.write(buf << "\r\n".freeze)
end
if hijack
req.hijacked!
hijack.call(socket)
+ elsif do_chunk
+ begin
+ body.each do |b|
+ socket.write("#{b.bytesize.to_s(16)}\r\n", b, "\r\n".freeze)
+ end
+ ensure
+ socket.write("0\r\n\r\n".freeze)
+ end
else
body.each { |chunk| socket.write(chunk) }
end
diff --git a/lib/unicorn/socket_helper.rb b/lib/unicorn/socket_helper.rb
index 8a6f6ee..c2ba75e 100644
--- a/lib/unicorn/socket_helper.rb
+++ b/lib/unicorn/socket_helper.rb
@@ -15,6 +15,20 @@ def kgio_tryaccept # :nodoc:
end
end
+ if IO.instance_method(:write).arity == 1 # Ruby <= 2.4
+ require 'unicorn/write_splat'
+ UNIXClient = Class.new(Kgio::Socket) # :nodoc:
+ class UNIXSrv < Kgio::UNIXServer # :nodoc:
+ include Unicorn::WriteSplat
+ def kgio_tryaccept # :nodoc:
+ super(UNIXClient)
+ end
+ end
+ TCPClient.__send__(:include, Unicorn::WriteSplat)
+ else # Ruby 2.5+
+ UNIXSrv = Kgio::UNIXServer
+ end
+
module SocketHelper
# internal interface
@@ -135,7 +149,7 @@ def bind_listen(address = '0.0.0.0:8080', opt = {})
end
old_umask = File.umask(opt[:umask] || 0)
begin
- Kgio::UNIXServer.new(address)
+ UNIXSrv.new(address)
ensure
File.umask(old_umask)
end
@@ -203,7 +217,7 @@ def server_cast(sock)
Socket.unpack_sockaddr_in(sock.getsockname)
TCPSrv.for_fd(sock.fileno)
rescue ArgumentError
- Kgio::UNIXServer.for_fd(sock.fileno)
+ UNIXSrv.for_fd(sock.fileno)
end
end
diff --git a/lib/unicorn/write_splat.rb b/lib/unicorn/write_splat.rb
new file mode 100644
index 0000000..7e6e363
--- /dev/null
+++ b/lib/unicorn/write_splat.rb
@@ -0,0 +1,7 @@
+# -*- encoding: binary -*-
+# compatibility module for Ruby <= 2.4, remove when we go Ruby 2.5+
+module Unicorn::WriteSplat # :nodoc:
+ def write(*arg) # :nodoc:
+ super(arg.join(''))
+ end
+end
diff --git a/test/unit/test_server.rb b/test/unit/test_server.rb
index 98e85ab..fe98fcc 100644
--- a/test/unit/test_server.rb
+++ b/test/unit/test_server.rb
@@ -196,7 +196,7 @@ def test_client_shutdown_writes
# continue to process our request and never hit EOFError on our sock
sock.shutdown(Socket::SHUT_WR)
buf = sock.read
- assert_equal 'hello!\n', buf.split(/\r\n\r\n/).last
+ assert_match %r{\bhello!\\n\b}, buf.split(/\r\n\r\n/, 2).last
next_client = Net::HTTP.get(URI.parse("http://127.0.0.1:#@port/"))
assert_equal 'hello!\n', next_client
lines = File.readlines("test_stderr.#$$.log")
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-05 9:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-01 18:54 Rack 3 Compatibility Jeremy Evans
2023-06-02 0:00 ` 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
Code repositories for project(s) associated with this public inbox
https://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).