From: Stefan Sundin <git@stefansundin.com>
To: bofh@yhbt.net
Cc: clogger-public@yhbt.net, Stefan Sundin <git@stefansundin.com>
Subject: [PATCH v2 2/2] rack 3.x compatibility
Date: Sat, 27 Jan 2024 17:23:00 -0800 [thread overview]
Message-ID: <20240128012812.76997-3-git@stefansundin.com> (raw)
In-Reply-To: <20240128012812.76997-1-git@stefansundin.com>
Rack::Utils::HeaderHash will be removed in rack 3.1 so these changes mostly
address that. The initializer in Rack::Headers inherits from Hash, so
switching to the ::[] class method to achieve the same result.
---
clogger.gemspec | 3 +-
ext/clogger_ext/clogger.c | 28 +++++++++++------
lib/clogger/pure.rb | 14 +++++++--
test/test_clogger.rb | 61 +++++++++++++++++++++++++-----------
test/test_clogger_to_path.rb | 14 +++++----
5 files changed, 83 insertions(+), 37 deletions(-)
diff --git a/clogger.gemspec b/clogger.gemspec
index ab479c5..7a08dc5 100644
--- a/clogger.gemspec
+++ b/clogger.gemspec
@@ -16,8 +16,9 @@ Gem::Specification.new do |s|
s.test_files = %w(test/test_clogger.rb test/test_clogger_to_path.rb)
# HeaderHash wasn't case-insensitive in old versions
- s.add_dependency(%q<rack>, ['>= 1.0', '< 3.0'])
+ s.add_dependency(%q<rack>, ['>= 1.0', '< 4.0'])
s.add_development_dependency('test-unit', '~> 3.0')
+ s.add_development_dependency('rackup', '~> 2.0')
s.extensions = %w(ext/clogger_ext/extconf.rb)
s.licenses = %w(LGPL-2.1-or-later)
end
diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c
index 23aea39..1993ae8 100644
--- a/ext/clogger_ext/clogger.c
+++ b/ext/clogger_ext/clogger.c
@@ -136,7 +136,7 @@ static ID to_path_id;
static ID respond_to_id;
static VALUE cClogger;
static VALUE mFormat;
-static VALUE cHeaderHash;
+static VALUE cRackHeaders;
/* common hash lookup keys */
static VALUE g_HTTP_X_FORWARDED_FOR;
@@ -172,6 +172,10 @@ static VALUE byte_xs(VALUE obj)
{
static const char esc[] = "0123456789ABCDEF";
unsigned char *new_ptr;
+ if (rb_obj_is_kind_of(obj, rb_cArray)) {
+ // Rack 3
+ obj = rb_ary_join(obj, rb_str_new2("\n"));
+ }
VALUE from = rb_obj_as_string(obj);
const unsigned char *ptr = (const unsigned char *)RSTRING_PTR(from);
long len = RSTRING_LEN(from);
@@ -628,7 +632,8 @@ static void append_response(struct clogger *c, VALUE key)
{
VALUE v;
- assert(rb_obj_is_kind_of(c->headers, cHeaderHash) && "not HeaderHash");
+ assert(rb_obj_is_kind_of(c->headers, cRackHeaders)
+ && "not Rack::Headers");
v = rb_funcall(c->headers, sq_brace_id, 1, key);
v = NIL_P(v) ? g_dash : byte_xs(v);
@@ -900,10 +905,9 @@ static VALUE ccall(struct clogger *c, VALUE env)
rv = rb_ary_dup(rv);
if (c->need_resp &&
- ! rb_obj_is_kind_of(c->headers, cHeaderHash)) {
- c->headers = rb_funcall(cHeaderHash, new_id, 1,
- c->headers);
- rb_ary_store(rv, 1, c->headers);
+ ! rb_obj_is_kind_of(c->headers, cRackHeaders)) {
+ c->headers = rb_funcall(cRackHeaders, sq_brace_id,
+ 1, c->headers);
}
} else {
VALUE tmp = rb_inspect(rv);
@@ -1114,9 +1118,15 @@ void Init_clogger_ext(void)
CONST_GLOBAL_STR2(rack_request_cookie_hash, "rack.request.cookie_hash");
tmp = rb_const_get(rb_cObject, rb_intern("Rack"));
- tmp = rb_const_get(tmp, rb_intern("Utils"));
- cHeaderHash = rb_const_get(tmp, rb_intern("HeaderHash"));
- rb_ary_push(mark_ary, cHeaderHash);
+ if (rb_const_defined(tmp, rb_intern("Headers"))) {
+ // Rack >= 3.0
+ cRackHeaders = rb_const_get(tmp, rb_intern("Headers"));
+ } else {
+ // Rack < 3.0
+ tmp = rb_const_get(tmp, rb_intern("Utils"));
+ cRackHeaders = rb_const_get(tmp, rb_intern("HeaderHash"));
+ }
+ rb_ary_push(mark_ary, cRackHeaders);
rb_obj_freeze(mark_ary);
}
diff --git a/lib/clogger/pure.rb b/lib/clogger/pure.rb
index 4b38e90..156f11e 100644
--- a/lib/clogger/pure.rb
+++ b/lib/clogger/pure.rb
@@ -8,10 +8,15 @@ class Clogger
attr_accessor :env, :status, :headers, :body
attr_writer :body_bytes_sent, :start
- def initialize(app, opts = {})
- # trigger autoload to avoid thread-safety issues later on
+ RackHeaders = if Object.const_defined?("Rack::Headers")
+ # Rack >= 3.0
+ Rack::Headers
+ else
+ # Rack < 3.0
Rack::Utils::HeaderHash
+ end
+ def initialize(app, opts = {})
@app = app
@logger = opts[:logger]
path = opts[:path]
@@ -35,7 +40,7 @@ class Clogger
raise TypeError, "app response not a 3 element Array: #{resp.inspect}"
end
status, headers, body = resp
- headers = Rack::Utils::HeaderHash.new(headers) if @need_resp
+ headers = RackHeaders[headers] if @need_resp
if @wrap_body
@reentrant = env['rack.multithread'] if @reentrant.nil?
wbody = @reentrant ? self.dup : self
@@ -91,6 +96,9 @@ private
def byte_xs(s)
s = s.dup
+ if s.is_a?(Array)
+ s = s.join("\n")
+ end
s.force_encoding(Encoding::BINARY) if defined?(Encoding::BINARY)
s.gsub!(/(['"\x00-\x1f\x7f-\xff])/) do |x|
"\\x#{$1.unpack('H2').first.upcase}"
diff --git a/test/test_clogger.rb b/test/test_clogger.rb
index 1dee652..5646025 100644
--- a/test/test_clogger.rb
+++ b/test/test_clogger.rb
@@ -10,6 +10,16 @@ require "rack"
require "clogger"
+IS_RACK3 = Gem::Version.new(Rack.release) >= Gem::Version.new('3.0.0')
+
+RackHeaders = if Object.const_defined?("Rack::Headers")
+ # Rack >= 3.0
+ Rack::Headers
+else
+ # Rack < 3.0
+ Rack::Utils::HeaderHash
+end
+
# used to test subclasses
class FooString < String
end
@@ -22,6 +32,7 @@ class TestClogger < Test::Unit::TestCase
@nginx_fmt = "%d/%b/%Y:%H:%M:%S %z"
@req = {
"REQUEST_METHOD" => "GET",
+ "SERVER_PROTOCOL" => "HTTP/1.0",
"HTTP_VERSION" => "HTTP/1.0",
"HTTP_USER_AGENT" => 'echo and socat \o/',
"PATH_INFO" => "/hello",
@@ -85,13 +96,13 @@ class TestClogger < Test::Unit::TestCase
def test_clen_stringio
start = DateTime.now - 1
str = StringIO.new
- app = lambda { |env| [ 301, {'Content-Length' => '5'}, ['abcde'] ] }
+ app = lambda { |env| [ 301, {'content-length' => '5'}, ['abcde'] ] }
format = Common.dup
assert format.gsub!(/response_length/, 'sent_http_content_length')
cl = Clogger.new(app, :logger => str, :format => format)
status, headers, body = cl.call(@req)
assert_equal(301, status)
- assert_equal({'Content-Length' => '5'}, headers)
+ assert_equal({'content-length' => '5'}, headers)
body.each { |part| assert_equal('abcde', part) }
str = str.string
r = %r{\Ahome - - \[[^\]]+\] "GET /hello\?goodbye=true HTTP/1.0" 301 5\n\z}
@@ -253,7 +264,7 @@ class TestClogger < Test::Unit::TestCase
def test_rack_1_0
start = DateTime.now - 1
str = StringIO.new
- app = lambda { |env| [ 200, {'Content-Length'=>'0'}, %w(a b c)] }
+ app = lambda { |env| [ 200, {'content-length'=>'0'}, %w(a b c)] }
cl = Clogger.new(app, :logger => str, :format => Rack_1_0)
status, headers, body = cl.call(@req)
tmp = []
@@ -344,7 +355,7 @@ class TestClogger < Test::Unit::TestCase
def test_combined
start = DateTime.now - 1
str = StringIO.new
- app = lambda { |env| [ 200, {'Content-Length'=>'3'}, %w(a b c)] }
+ app = lambda { |env| [ 200, {'content-length'=>'3'}, %w(a b c)] }
cl = Clogger.new(app, :logger => str, :format => Combined)
status, headers, body = cl.call(@req)
tmp = []
@@ -365,7 +376,7 @@ class TestClogger < Test::Unit::TestCase
def test_rack_errors_fallback
err = StringIO.new
- app = lambda { |env| [ 200, {'Content-Length'=>'3'}, %w(a b c)] }
+ app = lambda { |env| [ 200, {'content-length'=>'3'}, %w(a b c)] }
cl = Clogger.new(app, :format => '$pid')
req = @req.merge('rack.errors' => err)
status, headers, body = cl.call(req)
@@ -374,7 +385,7 @@ class TestClogger < Test::Unit::TestCase
def test_body_close
s_body = StringIO.new(%w(a b c).join("\n"))
- app = lambda { |env| [ 200, {'Content-Length'=>'5'}, s_body] }
+ app = lambda { |env| [ 200, {'content-length'=>'5'}, s_body] }
cl = Clogger.new(app, :logger => [], :format => '$pid')
status, headers, body = cl.call(@req)
assert ! s_body.closed?
@@ -384,7 +395,7 @@ class TestClogger < Test::Unit::TestCase
def test_escape
str = StringIO.new
- app = lambda { |env| [ 200, {'Content-Length'=>'5'}, [] ] }
+ app = lambda { |env| [ 200, {'content-length'=>'5'}, [] ] }
cl = Clogger.new(app,
:logger => str,
:format => '$http_user_agent "$request"')
@@ -398,13 +409,21 @@ class TestClogger < Test::Unit::TestCase
assert_equal expect, str.string
end
- # rack allows repeated headers with "\n":
- # { 'Set-Cookie' => "a\nb" } =>
- # Set-Cookie: a
- # Set-Cookie: b
- def test_escape_header_newlines
- str = StringIO.new
- app = lambda { |env| [302, { 'Set-Cookie' => "a\nb" }, [] ] }
+ # rack >= 3 allows repeated headers with array:
+ # { 'set-cookie' => ["a","b"] } =>
+ # set-cookie: a
+ # set-cookie: b
+ # rack < 3 allows repeated headers with "\n":
+ # { 'set-cookie' => "a\nb" } =>
+ # set-cookie: a
+ # set-cookie: b
+ def test_multiheader
+ str = StringIO.new
+ if IS_RACK3 then
+ app = lambda { |env| [302, { 'set-cookie' => ["a","b"] }, [] ] }
+ else
+ app = lambda { |env| [302, { 'set-cookie' => "a\nb" }, [] ] }
+ end
cl = Clogger.new(app, :logger => str, :format => '$sent_http_set_cookie')
cl.call(@req)
assert_equal "a\\x0Ab\n", str.string
@@ -463,12 +482,16 @@ class TestClogger < Test::Unit::TestCase
str = StringIO.new
app = lambda { |env| [302, [ %w(a) ], []] }
cl = Clogger.new(app, :logger => str, :format => '$sent_http_set_cookie')
- assert_nothing_raised { cl.call(@req) }
+ if IS_RACK3 then
+ assert_raise(ArgumentError) { cl.call(@req) }
+ else
+ assert_nothing_raised { cl.call(@req) }
+ end
end
def test_subclass_hash
str = StringIO.new
- req = Rack::Utils::HeaderHash.new(@req)
+ req = RackHeaders[@req]
app = lambda { |env| [302, [ %w(a) ], []] }
cl = Clogger.new(app, :logger => str, :format => Rack_1_0)
assert_nothing_raised { cl.call(req).last.each {}.close }
@@ -877,19 +900,21 @@ class TestClogger < Test::Unit::TestCase
end
def test_lint_error_wrapper
- require 'rack/lobster'
+ require 'rackup/lobster'
@req["SERVER_NAME"] = "FOO"
@req["SERVER_PORT"] = "666"
@req["rack.version"] = [1,1]
@req["rack.multithread"] = true
@req["rack.multiprocess"] = true
@req["rack.run_once"] = false
- app = Rack::ContentLength.new(Rack::ContentType.new(Rack::Lobster.new))
+ app = Rack::ContentLength.new(Rack::ContentType.new(Rackup::Lobster.new))
cl = Clogger.new(app, :format => :Combined)
@req["rack.errors"] = err = StringIO.new
status, headers, body = r = Rack::Lint.new(cl).call(@req)
body.each { |x| assert_kind_of String, x.to_str }
body.close # might raise here
assert_match(%r{GET /hello}, err.string)
+ rescue LoadError, Gem::ConflictError
+ # This test only works on Rack >= 3.0
end
end
diff --git a/test/test_clogger_to_path.rb b/test/test_clogger_to_path.rb
index f74b991..4cc1738 100644
--- a/test/test_clogger_to_path.rb
+++ b/test/test_clogger_to_path.rb
@@ -3,6 +3,7 @@ $stderr.sync = $stdout.sync = true
require "test/unit"
require "date"
require "stringio"
+require "tempfile"
require "rack"
require "clogger"
@@ -26,6 +27,7 @@ class TestCloggerToPath < Test::Unit::TestCase
@req = {
"REQUEST_METHOD" => "GET",
"HTTP_VERSION" => "HTTP/1.0",
+ "SERVER_PROTOCOL" => "HTTP/1.0",
"HTTP_USER_AGENT" => 'echo and socat \o/',
"PATH_INFO" => "/",
"QUERY_STRING" => "",
@@ -50,8 +52,8 @@ class TestCloggerToPath < Test::Unit::TestCase
app = Rack::Builder.new do
tmp.syswrite(' ' * 365)
h = {
- 'Content-Length' => '0',
- 'Content-Type' => 'text/plain',
+ 'content-length' => '0',
+ 'content-type' => 'text/plain',
}
use Clogger,
:logger => logger,
@@ -82,8 +84,8 @@ class TestCloggerToPath < Test::Unit::TestCase
app = Rack::Builder.new do
tmp.syswrite(' ' * 365)
h = {
- 'Content-Length' => '0',
- 'Content-Type' => 'text/plain',
+ 'content-length' => '0',
+ 'content-type' => 'text/plain',
}
use Clogger,
:logger => logger,
@@ -106,8 +108,8 @@ class TestCloggerToPath < Test::Unit::TestCase
logger = StringIO.new
app = Rack::Builder.new do
h = {
- 'Content-Length' => '3',
- 'Content-Type' => 'text/plain',
+ 'content-length' => '3',
+ 'content-type' => 'text/plain',
}
use Clogger,
:logger => logger,
--
2.43.0
next prev parent reply other threads:[~2024-01-28 1:28 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-23 3:08 rack 3.x compatibility Stefan Sundin
2024-01-23 3:08 ` [PATCH 1/2] update deprecated license identifier Stefan Sundin
2024-01-23 3:08 ` [PATCH 2/2] rack 3.x compatibility Stefan Sundin
2024-01-23 12:47 ` Eric Wong
2024-01-28 1:22 ` v2 Stefan Sundin
2024-01-28 1:22 ` [PATCH v2 1/2] update deprecated license identifier Stefan Sundin
2024-01-28 1:23 ` Stefan Sundin [this message]
2024-02-02 7:45 ` [PATCH] test_broken_header_response: allow NoMethodError w/ rack 2 Eric Wong
2024-02-01 7:58 ` v2 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: https://yhbt.net/clogger/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240128012812.76997-3-git@stefansundin.com \
--to=git@stefansundin.com \
--cc=bofh@yhbt.net \
--cc=clogger-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
https://yhbt.net/clogger.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).