This allows us to have nicer hunk headers. --- .gitattributes | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..a365646 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,3 @@ +*.gemspec diff=ruby +*.rb diff=ruby +Rakefile diff=ruby
While rb_block_call appeared in Ruby 1.9, RB_BLOCK_CALL_FUNC_ARGLIST only appeared in 2.1 --- clogger.gemspec | 1 + ext/clogger_ext/clogger.c | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/clogger.gemspec b/clogger.gemspec index 7a08dc5..306ed3b 100644 --- a/clogger.gemspec +++ b/clogger.gemspec @@ -5,6 +5,7 @@ s.name = %q{clogger} s.version = (ENV['VERSION'] || '2.4.0').dup s.homepage = 'https://YHBT.net/clogger/' + s.required_ruby_version = ">= 2.1.0" s.authors = ["cloggers"] s.summary = 'configurable request logging for Rack' s.description = File.read('README').split("\n\n")[1] diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c index cea4072..29c23b4 100644 --- a/ext/clogger_ext/clogger.c +++ b/ext/clogger_ext/clogger.c @@ -134,6 +134,7 @@ static ID sq_brace_id; static ID new_id; static ID to_path_id; static ID respond_to_id; +static ID each_id; static VALUE cClogger; static VALUE mFormat; static VALUE cRackHeaders; @@ -840,7 +841,7 @@ static VALUE clogger_init(int argc, VALUE *argv, VALUE self) return self; } -static VALUE body_iter_i(VALUE str, VALUE self) +static VALUE body_iter_i(RB_BLOCK_CALL_FUNC_ARGLIST(str, self)) { struct clogger *c = clogger_get(self); @@ -873,7 +874,7 @@ static VALUE clogger_each(VALUE self) rb_need_block(); c->body_bytes_sent = 0; - rb_iterate(rb_each, c->body, body_iter_i, self); + rb_block_call(c->body, each_id, 0, NULL, body_iter_i, self); return self; } @@ -1098,6 +1099,7 @@ void Init_clogger_ext(void) new_id = rb_intern("new"); to_path_id = rb_intern("to_path"); respond_to_id = rb_intern("respond_to?"); + each_id = rb_intern("each"); cClogger = rb_define_class("Clogger", rb_cObject); mFormat = rb_define_module_under(cClogger, "Format"); rb_define_alloc_func(cClogger, clogger_alloc);
Just a minor problem I noticed on my other machine since I'm way behind on upgrades in `production'... Also 2 more minor patches being worked on (will post on https://yhbt.net/clogger-public/ w/o Cc in a bit) -----8<------ Subject: [PATCH] test_broken_header_response: allow NoMethodError w/ rack 2 Debian 11 (oldstable) packages rack 2.1.4 which fails with NoMethodError when passed an invalid response header. rack 2.2.6.4 on Debian 12 (stable) doesn't raise, however. --- test/test_clogger.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/test_clogger.rb b/test/test_clogger.rb index 5646025..955946b 100644 --- a/test/test_clogger.rb +++ b/test/test_clogger.rb @@ -485,7 +485,10 @@ def test_broken_header_response if IS_RACK3 then assert_raise(ArgumentError) { cl.call(@req) } else - assert_nothing_raised { cl.call(@req) } + begin + cl.call(@req) + rescue NoMethodError # rack 2.1.4 raises, but not 2.2.6.4 + end end end
Stefan Sundin <git@stefansundin.com> wrote: > Thank you for the review. I think I have addressed most, if > not all, of the issues. Thanks, pushed to clogger.git as commit 6b0768f8791c5d6dd2e8a5aea0da76d46549477e and 2991f00afa4e445214f3b997bb37cb746c01cd2d I've also pushed out TypedData memory leak workaround; might try to fix some deprecation warnings while testing some other stuff but personal things and storms + water leaks keep getting in the way :< > Still learning email based patch workflow, so please feel free > to let me know if I'm doing anything wrong. Seems fine :> If you have documentation improvements to git.git, I'm sure folks at git@vger.kernel.org would be happy to look at them :>
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
--- clogger.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clogger.gemspec b/clogger.gemspec index d461383..ab479c5 100644 --- a/clogger.gemspec +++ b/clogger.gemspec @@ -19,5 +19,5 @@ Gem::Specification.new do |s| s.add_dependency(%q<rack>, ['>= 1.0', '< 3.0']) s.add_development_dependency('test-unit', '~> 3.0') s.extensions = %w(ext/clogger_ext/extconf.rb) - s.licenses = %w(LGPL-2.1+) + s.licenses = %w(LGPL-2.1-or-later) end -- 2.43.0
Thank you for the review. I think I have addressed most, if not all, of the issues. Still learning email based patch workflow, so please feel free to let me know if I'm doing anything wrong. Regards, Stefan
This gives us memsize information and appears to fix a leak under Ruby 3.3. --- ext/clogger_ext/clogger.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c index 8cd40be..9861d70 100644 --- a/ext/clogger_ext/clogger.c +++ b/ext/clogger_ext/clogger.c @@ -225,18 +225,28 @@ static void clogger_mark(void *ptr) rb_gc_mark(c->body); } +static size_t memsize(const void *ptr) +{ + return sizeof(struct clogger); +} + +static const rb_data_type_t clogger_type = { + "clogger", + { clogger_mark, RUBY_TYPED_DEFAULT_FREE, memsize, /* reserved */ }, +}; + static VALUE clogger_alloc(VALUE klass) { struct clogger *c; - return Data_Make_Struct(klass, struct clogger, clogger_mark, -1, c); + return TypedData_Make_Struct(klass, struct clogger, &clogger_type, c); } static struct clogger *clogger_get(VALUE self) { struct clogger *c; - Data_Get_Struct(self, struct clogger, c); + TypedData_Get_Struct(self, struct clogger, &clogger_type, c); assert(c); return c; }
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.
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
--- clogger.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clogger.gemspec b/clogger.gemspec index d461383..ab479c5 100644 --- a/clogger.gemspec +++ b/clogger.gemspec @@ -19,5 +19,5 @@ Gem::Specification.new do |s| s.add_dependency(%q<rack>, ['>= 1.0', '< 3.0']) s.add_development_dependency('test-unit', '~> 3.0') s.extensions = %w(ext/clogger_ext/extconf.rb) - s.licenses = %w(LGPL-2.1+) + s.licenses = %w(LGPL-2.1-or-later) end -- 2.43.0
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!
Apparently, time(2) may go backwards in the first 1 - 2.5ms of every second since Linux + glibc 2.31+ (and some proprietary OSes). Use clock_gettime(2) with CLOCK_REALTIME to workaround the problem at the cost of a slight performance hit(*). While git will likely use gettimeofday(2) for compatibility with proprietary OSes, gettimeofday(2) was declared obsolete in POSIX.1-2008 and we don't support proprietary OSes. (*) https://inbox.sourceware.org/libc-alpha/87ttywq0je.fsf@oldenburg.str.redhat.com/ Link: https://lore.kernel.org/git/20230319064353.686226-3-eggert@cs.ucla.edu/T/ --- ext/clogger_ext/clogger.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c index 7989472..23aea39 100644 --- a/ext/clogger_ext/clogger.c +++ b/ext/clogger_ext/clogger.c @@ -475,9 +475,21 @@ static void append_request_length(struct clogger *c) } } +/* + * time(2) may slip backwards, so use CLOCK_REALTIME for accuracy + * https://lore.kernel.org/git/20230319064353.686226-3-eggert@cs.ucla.edu/T/ + */ +static time_t cur_time(void) +{ + struct timespec now; + + (void)clock_gettime(CLOCK_REALTIME, &now); + return now.tv_sec; +} + static long local_gmtoffset(struct tm *tm) { - time_t t = time(NULL); + time_t t = cur_time(); tzset(); localtime_r(&t, tm); @@ -537,7 +549,7 @@ static void append_time_utc(struct clogger *c) char buf[sizeof("01/Jan/1970:00:00:00 +0000")]; struct tm tm; int nr; - time_t t = time(NULL); + time_t t = cur_time(); gmtime_r(&t, &tm); nr = snprintf(buf, sizeof(buf), @@ -556,7 +568,7 @@ append_time(struct clogger *c, enum clogger_opcode op, VALUE fmt, VALUE buf) size_t buf_size = RSTRING_LEN(buf) + 1; /* "\0" */ size_t nr; struct tm tmp; - time_t t = time(NULL); + time_t t = cur_time(); if (op == CL_OP_TIME_LOCAL) localtime_r(&t, &tmp);
That's what _DEFAULT_SOURCE will set if it's respected, and we've had POSIX 2008 goodies for 15 years, now. --- ext/clogger_ext/clogger.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c index 8cd40be..7989472 100644 --- a/ext/clogger_ext/clogger.c +++ b/ext/clogger_ext/clogger.c @@ -17,7 +17,7 @@ # include <fcntl.h> #endif #ifndef _POSIX_C_SOURCE -# define _POSIX_C_SOURCE 200112L +# define _POSIX_C_SOURCE 200809L #endif #include <time.h> #include <stdlib.h>
`struct timespec' has 32-bit tv_sec and tv_nsec on 32-bit x86 GNU/Linux system, causing excessive overflow and test failures. --- ext/clogger_ext/clogger.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c index 2ec9510..8cd40be 100644 --- a/ext/clogger_ext/clogger.c +++ b/ext/clogger_ext/clogger.c @@ -386,30 +386,29 @@ static void append_request_time_fmt(struct clogger *c, VALUE op) clock_gettime(hopefully_CLOCK_MONOTONIC, &now); clock_diff(&now, &c->ts_start); if (ipow) { - struct timespec prev; unsigned long adj = 1; - /* - * n.b. timespec.tv_sec may not be time_t on some platforms, - * so we use a full timespec struct instead of time_t: - */ - prev.tv_sec = now.tv_sec; + int64_t now_sec = now.tv_sec, now_nsec = now.tv_nsec, + prev_sec = now.tv_sec; + do { adj *= 10; } while (--ipow); - now.tv_sec *= adj; - now.tv_nsec *= adj; - if (now.tv_nsec >= NANO_PER_SEC) { - int64_t add = now.tv_nsec / NANO_PER_SEC; - now.tv_sec += add; - now.tv_nsec %= NANO_PER_SEC; + now_sec *= adj; + now_nsec *= adj; + if (now_nsec >= NANO_PER_SEC) { + int64_t add = now_nsec / NANO_PER_SEC; + now_sec += add; + now_nsec %= NANO_PER_SEC; } - if (now.tv_sec < prev.tv_sec) { /* overflowed */ - now.tv_nsec = NANO_PER_SEC - 1; + if (now_sec < prev_sec) { /* overflowed */ + now_nsec = NANO_PER_SEC - 1; /* * some platforms may use unsigned .tv_sec, but * they're not worth supporting, so keep unsigned: */ - now.tv_sec = (time_t)(sizeof(now.tv_sec) == 4 ? + now_sec = (time_t)(sizeof(now.tv_sec) == 4 ? INT_MAX : LONG_MAX); } + now.tv_sec = now_sec; + now.tv_nsec = now_nsec; } append_ts(c, op, &now); }
Clogger is Rack middleware for logging HTTP requests. The log format is customizable so you can specify exactly which fields to log. Changes: Only 2 code changes, neither of which is really relevant for 99% of users using the C extension and strict HTTP parsers. escape env['REQUEST_METHOD'] for non-strict HTTP servers pure: fix time.rb incompatibility in Ruby 3.1+ doc: drop git:// URLs, use shorter domain for IMAP links * public mailbox: https://YHBT.net/clogger-public/ * mailto:clogger-public@YHBT.net (no HTML, don't top post) * homepage: https://YHBT.net/clogger/ * git clone https://YHBT.net/clogger.git * torsocks git clone http://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/clogger.git * Atom feed: https://YHBT.net/clogger/NEWS.atom.xml
git:// is rarely necessary these days, and IMAP (unlike NNTP) doesn't advertise hostnames in its protocol, so we can use a shorter one to avoid advertising other projects. And stylize our domain name since "YHBT" is an acronym. --- .olddoc.yml | 15 +++++++-------- GNUmakefile | 2 +- README | 16 ++++++++-------- clogger.gemspec | 4 ++-- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/.olddoc.yml b/.olddoc.yml index efcc429..2ed15c3 100644 --- a/.olddoc.yml +++ b/.olddoc.yml @@ -1,16 +1,15 @@ --- -rdoc_url: https://yhbt.net/clogger/ -cgit_url: https://yhbt.net/clogger.git -git_url: git://yhbt.net/clogger.git -public_email: clogger-public@yhbt.net +rdoc_url: https://YHBT.net/clogger/ +cgit_url: https://YHBT.net/clogger.git +git_url: https://YHBT.net/clogger.git +public_email: clogger-public@YHBT.net ml_url: -- https://yhbt.net/clogger-public/ +- https://YHBT.net/clogger-public/ - http://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/clogger-public/ -- imaps://news.public-inbox.org/inbox.comp.lang.ruby.clogger.0 +- imaps://YHBT.net/inbox.comp.lang.ruby.clogger.0 - imap://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.lang.ruby.clogger.0 - nntps://news.public-inbox.org/inbox.comp.lang.ruby.clogger - nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.lang.ruby.clogger source_code: -- git clone git://yhbt.net/clogger.git -- git clone https://yhbt.net/clogger.git +- git clone https://YHBT.net/clogger.git - torsocks git clone http://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/clogger.git diff --git a/GNUmakefile b/GNUmakefile index dec2f5a..b75e10c 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -1,5 +1,5 @@ all:: -RSYNC_DEST := yhbt.net:/srv/yhbt/clogger/ +RSYNC_DEST := YHBT.net:/srv/yhbt/clogger/ rfpackage := clogger include pkg.mk test-ext: diff --git a/README b/README index 3c5e419..2bbe9e7 100644 --- a/README +++ b/README @@ -100,13 +100,13 @@ that receives a "<<" method: The latest development happens in git and is published to the following: - git clone https://yhbt.net/clogger.git - git clone git://repo.or.cz/clogger.git + git clone https://YHBT.net/clogger.git + git clone https://repo.or.cz/clogger.git You may also browse and download snapshot tarballs: -* https://yhbt.net/clogger.git -* http://repo.or.cz/w/clogger.git (gitweb) +* https://YHBT.net/clogger.git +* https://repo.or.cz/w/clogger.git (gitweb) We use email for coordination and development, see below: @@ -115,8 +115,8 @@ We use email for coordination and development, see below: All feedback (bug reports, user/development discussion, patches, pull requests) is done via publicly-archived email: -* https://yhbt.net/clogger-public/ -* imaps://news.public-inbox.org/inbox.comp.lang.ruby.clogger.0 +* https://YHBT.net/clogger-public/ +* imaps://YHBT.net/inbox.comp.lang.ruby.clogger.0 * nntps://news.public-inbox.org/inbox.comp.lang.ruby.clogger Tor users may also access HTTP, IMAP, and NNTP archives via .onion: @@ -131,9 +131,9 @@ username + password will work. No subscription or real names will ever be required to email us. Do not send HTML email, do not top post. -* mailto:clogger-public@yhbt.net +* mailto:clogger-public@YHBT.net -Homepage: https://yhbt.net/clogger/ +Homepage: https://YHBT.net/clogger/ == INSTALL diff --git a/clogger.gemspec b/clogger.gemspec index d8d40d2..769b7ff 100644 --- a/clogger.gemspec +++ b/clogger.gemspec @@ -4,11 +4,11 @@ manifest = File.exist?('.manifest') ? Gem::Specification.new do |s| s.name = %q{clogger} s.version = (ENV['VERSION'] || '2.1.0').dup - s.homepage = 'https://yhbt.net/clogger/' + s.homepage = 'https://YHBT.net/clogger/' s.authors = ["cloggers"] s.summary = 'configurable request logging for Rack' s.description = File.read('README').split("\n\n")[1] - s.email = %q{clogger-public@yhbt.net} + s.email = %q{clogger-public@YHBT.net} s.extra_rdoc_files = IO.readlines('.document').map!(&:chomp!).keep_if do |f| File.exist?(f) end
The time.rb distributed with Ruby 3.1+ no longer has the RFC2822_MONTH_NAME constant, as Time#strftime in Ruby is locale-independent (unlike strftime(3) in C). This doesn't affect the default C extension, it only affects the pure Ruby code used for Ruby implementations without C extension support. cf. ruby.git commit 5307fab6619e26e05d791d68c35ceef2e923e8d5 --- lib/clogger/pure.rb | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/lib/clogger/pure.rb b/lib/clogger/pure.rb index 7f82992..4b38e90 100644 --- a/lib/clogger/pure.rb +++ b/lib/clogger/pure.rb @@ -133,17 +133,10 @@ private when :time_iso8601 Time.now.iso8601 when :time_local - t = Time.now - off = t.utc_offset - sign = off < 0 ? '-' : '+' - sprintf("%02d/%s/%d:%02d:%02d:%02d #{sign}%02d%02d", - t.mday, Time::RFC2822_MONTH_NAME[t.mon - 1], - t.year, t.hour, t.min, t.sec, *(off.abs / 60).divmod(60)) + # %b in Ruby is locale-independent, unlike strftime(3) in C + Time.now.strftime('%d/%b/%Y:%H:%M:%S %z') when :time_utc - t = Time.now.utc - sprintf("%02d/%s/%d:%02d:%02d:%02d +0000", - t.mday, Time::RFC2822_MONTH_NAME[t.mon - 1], - t.year, t.hour, t.min, t.sec) + Time.now.utc.strftime('%d/%b/%Y:%H:%M:%S %z') else raise "EDOOFUS #{special_nr}" end
This doesn't affect most Rack HTTP servers since they have strict parsers, but is safer in case one doesn't... Influenced by CVE-2022-30123. --- ext/clogger_ext/clogger.c | 5 +++-- lib/clogger/pure.rb | 3 +-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c index 079817c..2ec9510 100644 --- a/ext/clogger_ext/clogger.c +++ b/ext/clogger_ext/clogger.c @@ -447,10 +447,11 @@ static void append_request(struct clogger *c) { VALUE tmp; - /* REQUEST_METHOD doesn't need escaping, Rack::Lint governs it */ tmp = rb_hash_aref(c->env, g_REQUEST_METHOD); - if (!NIL_P(tmp)) + if (!NIL_P(tmp)) { + tmp = byte_xs(tmp); rb_str_buf_append(c->log_buf, tmp); + } rb_str_buf_append(c->log_buf, g_space); diff --git a/lib/clogger/pure.rb b/lib/clogger/pure.rb index 8f1f706..7f82992 100644 --- a/lib/clogger/pure.rb +++ b/lib/clogger/pure.rb @@ -118,8 +118,7 @@ private version = env['HTTP_VERSION'] and version = " #{byte_xs(version)}" qs = env['QUERY_STRING'] qs.empty? or qs = "?#{byte_xs(qs)}" - "#{env['REQUEST_METHOD']} " \ - "#{request_uri(env)}#{version}" + "#{byte_xs(env['REQUEST_METHOD'] || '')} #{request_uri(env)}#{version}" when :request_uri request_uri(env) when :request_length
Clogger is Rack middleware for logging HTTP requests. The log format is customizable so you can specify exactly which fields to log. Changes: This release fixes compatibility with GC.compact on Ruby 3.x. Thanks to Ngan Pham for the patch and Aaron Patterson for the feedback for the (obsolete) kgio RubyGem: https://yhbt.net/kgio-public/CAAvYYt5Z5f2rMuXO5DMpR1-6uRvu_gXKDvqcyoZ+oNcLiTH39g@mail.gmail.com/T/ * public mailbox: https://yhbt.net/clogger-public/ * mailto:clogger-public@yhbt.net (no HTML, don't top post) * homepage: https://yhbt.net/clogger/ * git clone https://yhbt.net/clogger.git * torsocks git clone http://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/clogger.git * Atom feed: https://yhbt.net/clogger/NEWS.atom.xml
Tor is dropping v2 .onion in favor of more secure (but less readable) v3 .onion URLs. Zooko's triangle once again :/ (no, we won't support planet-destroying proof-of-work schemes) --- .olddoc.yml | 8 ++++---- README | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.olddoc.yml b/.olddoc.yml index af28387..efcc429 100644 --- a/.olddoc.yml +++ b/.olddoc.yml @@ -5,12 +5,12 @@ git_url: git://yhbt.net/clogger.git public_email: clogger-public@yhbt.net ml_url: - https://yhbt.net/clogger-public/ -- http://ou63pmih66umazou.onion/clogger-public/ +- http://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/clogger-public/ - imaps://news.public-inbox.org/inbox.comp.lang.ruby.clogger.0 -- imap://ou63pmih66umazou.onion/inbox.comp.lang.ruby.clogger.0 +- imap://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.lang.ruby.clogger.0 - nntps://news.public-inbox.org/inbox.comp.lang.ruby.clogger -- nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.clogger +- nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.lang.ruby.clogger source_code: - git clone git://yhbt.net/clogger.git - git clone https://yhbt.net/clogger.git -- torsocks git clone http://ou63pmih66umazou.onion/clogger.git +- torsocks git clone http://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/clogger.git diff --git a/README b/README index cdb8fba..3c5e419 100644 --- a/README +++ b/README @@ -121,9 +121,9 @@ requests) is done via publicly-archived email: Tor users may also access HTTP, IMAP, and NNTP archives via .onion: -* http://ou63pmih66umazou.onion/clogger-public/ -* imap://ou63pmih66umazou.onion/inbox.comp.lang.ruby.clogger.0 -* nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.clogger +* http://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/clogger-public/ +* imap://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.lang.ruby.clogger.0 +* nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.lang.ruby.clogger AUTH=ANONYMOUS is supported for IMAP and IMAPS, and any username + password will work.
With GC.compact in Ruby 3.x, Ruby-defined constants need to be explicitly marked to prevent movement: Link: https://yhbt.net/kgio-public/CAAvYYt5Z5f2rMuXO5DMpR1-6uRvu_gXKDvqcyoZ+oNcLiTH39g@mail.gmail.com/T/ --- ext/clogger_ext/clogger.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c index a3de120..079817c 100644 --- a/ext/clogger_ext/clogger.c +++ b/ext/clogger_ext/clogger.c @@ -1104,6 +1104,7 @@ void Init_clogger_ext(void) 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); rb_obj_freeze(mark_ary); }
Clogger is Rack middleware for logging HTTP requests. The log format is customizable so you can specify exactly which fields to log. Changes: clogger 2.3.0 - $request_time{POWER,PRECISION} support The $request_time can now be multiplied by a power-of-10, allowing up to nanosecond resolution without decimals. Thanks to Josh Natanson for contributing this feature: https://yhbt.net/clogger-public/CAPdx2swO4eaOWaqL4-rMLq1H7pH6w-i760vPJTb92tyKfcc01Q@mail.gmail.com/ https://yhbt.net/clogger-public/CAPdx2szjqUuFjUtrgoeXXwmz0HzfdnWe+2h2Sp_ywDkTDVL0-g@mail.gmail.com/T/ There's also some minor fixes and doc updates: clogger: fix _BSD_SOURCE and _SVID_SOURCE deprecation warnings doc: update with IMAPS, NNTPS, and .onion mail archive URLs doc: document Fiber.current * public mail archives: https://yhbt.net/clogger-public/ * mailto:clogger-public@yhbt.net (no HTML, don't top post) * homepage: https://yhbt.net/clogger/ * git clone https://yhbt.net/clogger.git * Atom feed: https://yhbt.net/clogger/NEWS.atom.xml
It's always been supported (since it's just an eval), but make it explicit. We'll also drop the Actor.current example since both Rubinius and Revactor seem dead and Ractor (Guild) is probably coming... --- README | 2 +- test/test_clogger.rb | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/README b/README index c90dd1a..cdb8fba 100644 --- a/README +++ b/README @@ -89,7 +89,7 @@ that receives a "<<" method: * $ip - X-Forwarded-For request header if available, $remote_addr if not * $pid - process ID of the current process * $e{Thread.current} - Thread processing the request -* $e{Actor.current} - Actor processing the request (Revactor or Rubinius) +* $e{Fiber.current} - Fiber processing the request * $env{variable_name} - any Rack environment variable (e.g. rack.url_scheme) == REQUIREMENTS diff --git a/test/test_clogger.rb b/test/test_clogger.rb index b7dd4e9..1dee652 100644 --- a/test/test_clogger.rb +++ b/test/test_clogger.rb @@ -212,6 +212,22 @@ class TestClogger < Test::Unit::TestCase assert_equal "-#{current}-\n", str.string end + def test_fiber + begin + current = Fiber.current.to_s + rescue NameError => e + warn "your Ruby does not support fibers #{e}" + return + end + str = StringIO.new + app = lambda { |env| [ 302, {}, [] ] } + cl = Clogger.new(app, + :logger => str, + :format => "-$e{Fiber.current}-\n") + status, headers, body = cl.call(@req) + assert_equal "-#{current}-\n", str.string + end + def test_pid str = StringIO.new app = lambda { |env| [ 302, {}, [] ] }
public-inbox.org started running an IMAP + IMAPS server. --- .olddoc.yml | 5 ++++- README | 24 ++++++++++++++++++------ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/.olddoc.yml b/.olddoc.yml index f774210..af28387 100644 --- a/.olddoc.yml +++ b/.olddoc.yml @@ -6,7 +6,10 @@ public_email: clogger-public@yhbt.net ml_url: - https://yhbt.net/clogger-public/ - http://ou63pmih66umazou.onion/clogger-public/ -- nntp://news.public-inbox.org/inbox.comp.lang.ruby.clogger +- imaps://news.public-inbox.org/inbox.comp.lang.ruby.clogger.0 +- imap://ou63pmih66umazou.onion/inbox.comp.lang.ruby.clogger.0 +- nntps://news.public-inbox.org/inbox.comp.lang.ruby.clogger +- nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.clogger source_code: - git clone git://yhbt.net/clogger.git - git clone https://yhbt.net/clogger.git diff --git a/README b/README index a6a5db8..c90dd1a 100644 --- a/README +++ b/README @@ -108,18 +108,30 @@ You may also browse and download snapshot tarballs: * https://yhbt.net/clogger.git * http://repo.or.cz/w/clogger.git (gitweb) -The mailing list (see below) is central for coordination and -development. Patches should always be sent inline -(git format-patch -M + git send-email) so we can reply to them inline. +We use email for coordination and development, see below: == CONTACT All feedback (bug reports, user/development discussion, patches, pull -requests) go to the public mailing list. +requests) is done via publicly-archived email: -* mailto:clogger-public@yhbt.net +* https://yhbt.net/clogger-public/ +* imaps://news.public-inbox.org/inbox.comp.lang.ruby.clogger.0 +* nntps://news.public-inbox.org/inbox.comp.lang.ruby.clogger + +Tor users may also access HTTP, IMAP, and NNTP archives via .onion: + +* http://ou63pmih66umazou.onion/clogger-public/ +* imap://ou63pmih66umazou.onion/inbox.comp.lang.ruby.clogger.0 +* nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.clogger -Do not send HTML mail or attachments. Do not top post. +AUTH=ANONYMOUS is supported for IMAP and IMAPS, and any +username + password will work. + +No subscription or real names will ever be required to email us. +Do not send HTML email, do not top post. + +* mailto:clogger-public@yhbt.net Homepage: https://yhbt.net/clogger/