* [PATCH v2 2/2] rack 3.x compatibility
@ 2024-01-28 1:23 2% ` Stefan Sundin
0 siblings, 0 replies; 4+ results
From: Stefan Sundin @ 2024-01-28 1:23 UTC (permalink / raw)
To: bofh; +Cc: clogger-public, Stefan Sundin
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
^ permalink raw reply related [relevance 2%]
* Re: [PATCH 2/2] rack 3.x compatibility
2024-01-23 3:08 2% ` [PATCH 2/2] " Stefan Sundin
@ 2024-01-23 12:47 6% ` Eric Wong
0 siblings, 1 reply; 4+ results
From: Eric Wong @ 2024-01-23 12:47 UTC (permalink / raw)
To: Stefan Sundin; +Cc: clogger-public
Stefan Sundin <git@stefansundin.com> wrote:
> 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.
OK. Fwiw, I don't do much Ruby nowadays and will be keeping my
legacy projects Rack 2 for the forseeable future.
Patch 1 for the LGPL wording change is fine and I was able to
apply your patches without problems.
> The test test_broken_header_response was removed as Rack::Headers now throws
> an error in that case. I don't know if that necessarily is a good test case?
It's not a great test case, but I prefer we test malformed
responses in some way (more inline..)
> Since the rackup functionality was split out to its own gem, the test case
> test_lint_error_wrapper now only works with rack 3.x.
OK (more inline...)
> Other than that exception, all tests are passing with rack 1.6.13, 2.2.8,
> and 3.0.8.
Good to know.
> --- 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')
I think rackup can be optional and we can rescue NameError
(below) if missing.
> static VALUE g_HTTP_X_FORWARDED_FOR;
> @@ -628,7 +628,7 @@ 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");
Please wrap at <80 columns (I need gigantic fonts to see) Thanks.
> + RackHeaders = if Object.const_defined?("Rack::Headers")
> + # Rack >= 3.0
> + Rack::Headers
> + else
> + # Rack < 3.0
> + Rack::Utils::HeaderHash
> + end
> +
> def initialize(app, opts = {})
> # trigger autoload to avoid thread-safety issues later on
> - Rack::Utils::HeaderHash
> + RackHeaders
Minor: the explicit autoload trigger is no longer needed since the
above if/else already triggered it.
<snip>
> @@ -399,12 +408,12 @@ class TestClogger < Test::Unit::TestCase
> end
>
> # rack allows repeated headers with "\n":
> - # { 'Set-Cookie' => "a\nb" } =>
> - # Set-Cookie: a
> - # Set-Cookie: b
> + # { '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" }, [] ] }
> + app = lambda { |env| [302, { 'set-cookie' => "a\nb" }, [] ] }
Important:
Rack 3 uses arrays instead of "\n" to denote multi-values.
So I think the test (and clogger itself) need to to account
for array values (but continue to support Rack 2 "\n"-delimited
multi-values)
> - def test_broken_header_response
> - 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) }
> - end
It's fine if the broken header raises w/ Rack 3.
I mainly want to make sure a malformed response doesn't
trigger a segfault or memory corruption on our end.
So whether it raises or silently ignores the invalid response is
fine.
> @@ -877,14 +879,15 @@ class TestClogger < Test::Unit::TestCase
> end
>
> def test_lint_error_wrapper
> - require 'rack/lobster'
> + # This test only works on Rack >= 3.0
> + require 'rackup/lobster'
I prefer we rescue NameError here and skip tests if rackup or
lobster is missing. It's not important to run the odd tests if
we can save some bandwidth and storage.
I also noticed some problems with Ruby 3.3, will post followups
in a bit.
Thanks.
^ permalink raw reply [relevance 6%]
* [PATCH 2/2] rack 3.x compatibility
2024-01-23 3:08 7% rack 3.x compatibility Stefan Sundin
@ 2024-01-23 3:08 2% ` Stefan Sundin
2024-01-23 12:47 6% ` Eric Wong
0 siblings, 1 reply; 4+ results
From: Stefan Sundin @ 2024-01-23 3:08 UTC (permalink / raw)
To: clogger-public; +Cc: Stefan Sundin
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.
The test test_broken_header_response was removed as Rack::Headers now throws
an error in that case. I don't know if that necessarily is a good test case?
Since the rackup functionality was split out to its own gem, the test case
test_lint_error_wrapper now only works with rack 3.x.
Other than that exception, all tests are passing with rack 1.6.13, 2.2.8,
and 3.0.8.
---
clogger.gemspec | 3 ++-
ext/clogger_ext/clogger.c | 23 ++++++++++--------
lib/clogger/pure.rb | 12 ++++++++--
test/test_clogger.rb | 45 +++++++++++++++++++-----------------
test/test_clogger_to_path.rb | 14 ++++++-----
5 files changed, 58 insertions(+), 39 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..9ccafb5 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;
@@ -628,7 +628,7 @@ 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 +900,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 +1113,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..1ddd7f2 100644
--- a/lib/clogger/pure.rb
+++ b/lib/clogger/pure.rb
@@ -8,9 +8,17 @@ class Clogger
attr_accessor :env, :status, :headers, :body
attr_writer :body_bytes_sent, :start
+ RackHeaders = if Object.const_defined?("Rack::Headers")
+ # Rack >= 3.0
+ Rack::Headers
+ else
+ # Rack < 3.0
+ Rack::Utils::HeaderHash
+ end
+
def initialize(app, opts = {})
# trigger autoload to avoid thread-safety issues later on
- Rack::Utils::HeaderHash
+ RackHeaders
@app = app
@logger = opts[:logger]
@@ -35,7 +43,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
diff --git a/test/test_clogger.rb b/test/test_clogger.rb
index 1dee652..bbb5791 100644
--- a/test/test_clogger.rb
+++ b/test/test_clogger.rb
@@ -10,6 +10,14 @@ require "rack"
require "clogger"
+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 +30,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 +94,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 +262,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 +353,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 +374,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 +383,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 +393,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"')
@@ -399,12 +408,12 @@ class TestClogger < Test::Unit::TestCase
end
# rack allows repeated headers with "\n":
- # { 'Set-Cookie' => "a\nb" } =>
- # Set-Cookie: a
- # Set-Cookie: b
+ # { '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" }, [] ] }
+ app = lambda { |env| [302, { 'set-cookie' => "a\nb" }, [] ] }
cl = Clogger.new(app, :logger => str, :format => '$sent_http_set_cookie')
cl.call(@req)
assert_equal "a\\x0Ab\n", str.string
@@ -459,16 +468,9 @@ class TestClogger < Test::Unit::TestCase
assert_match %r{#{e}$}m, str
end
- def test_broken_header_response
- 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) }
- 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,14 +879,15 @@ class TestClogger < Test::Unit::TestCase
end
def test_lint_error_wrapper
- require 'rack/lobster'
+ # This test only works on Rack >= 3.0
+ 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)
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
^ permalink raw reply related [relevance 2%]
* rack 3.x compatibility
@ 2024-01-23 3:08 7% Stefan Sundin
2024-01-23 3:08 2% ` [PATCH 2/2] " Stefan Sundin
0 siblings, 1 reply; 4+ results
From: Stefan Sundin @ 2024-01-23 3:08 UTC (permalink / raw)
To: clogger-public
Hello. I did some work to get clogger up to spec with rack 3.x.
This is my first time using git to send patches, so hopefully
I did everything right. Feels good to not use GitHub for a change.
The only "breakage" here is that malformed headers will now
throw an error instead of work. Let me know if you prefer
another solution other than deleting the test
test_broken_header_response.
Thanks!
^ permalink raw reply [relevance 7%]
Results 1-4 of 4 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2024-01-23 3:08 7% rack 3.x compatibility Stefan Sundin
2024-01-23 3:08 2% ` [PATCH 2/2] " Stefan Sundin
2024-01-23 12:47 6% ` Eric Wong
2024-01-28 1:22 ` v2 Stefan Sundin
2024-01-28 1:23 2% ` [PATCH v2 2/2] rack 3.x compatibility Stefan Sundin
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).