about summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <normalperson@yhbt.net>2012-11-02 20:09:28 -0700
committerEric Wong <normalperson@yhbt.net>2012-11-02 20:09:28 -0700
commitaaf1279e3e73e301b8f65b651933ebe287b82b4d (patch)
treeaf080b511188bb86869a107831b350deafc2b71a
parent4bcd56a4f02b8a167d28505182cc6ff723fb027c (diff)
avoid calling "<<" on env["rack.errors"]
Rack::Lint::ErrorWrapper forbids the "<<" method.  This
fallback only comes into play when no log destination
(via :logger or :path) is specified and is rarely an
issue in real setups.
-rw-r--r--ext/clogger_ext/clogger.c9
-rw-r--r--lib/clogger/pure.rb10
-rw-r--r--test/test_clogger.rb17
3 files changed, 33 insertions, 3 deletions
diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c
index 857ed9a..04359f9 100644
--- a/ext/clogger_ext/clogger.c
+++ b/ext/clogger_ext/clogger.c
@@ -119,6 +119,7 @@ struct clogger {
         int reentrant; /* tri-state, -1:auto, 1/0 true/false */
 };
 
+static ID write_id;
 static ID ltlt_id;
 static ID call_id;
 static ID each_id;
@@ -687,9 +688,12 @@ static VALUE cwrite(struct clogger *c)
         } else {
                 VALUE logger = c->logger;
 
-                if (NIL_P(logger))
+                if (NIL_P(logger)) {
                         logger = rb_hash_aref(c->env, g_rack_errors);
-                rb_funcall(logger, ltlt_id, 1, dst);
+                        rb_funcall(logger, write_id, 1, dst);
+                } else {
+                        rb_funcall(logger, ltlt_id, 1, dst);
+                }
         }
 
         return Qnil;
@@ -1036,6 +1040,7 @@ void Init_clogger_ext(void)
 
         check_clock();
 
+        write_id = rb_intern("write");
         ltlt_id = rb_intern("<<");
         call_id = rb_intern("call");
         each_id = rb_intern("each");
diff --git a/lib/clogger/pure.rb b/lib/clogger/pure.rb
index 24392e7..44f4e62 100644
--- a/lib/clogger/pure.rb
+++ b/lib/clogger/pure.rb
@@ -161,7 +161,7 @@ private
   end
 
   def log(env, status, headers)
-    (@logger || env['rack.errors']) << @fmt_ops.map { |op|
+    str = @fmt_ops.map { |op|
       case op[0]
       when OP_LITERAL; op[1]
       when OP_REQUEST; byte_xs(env[op[1]] || "-")
@@ -182,5 +182,13 @@ private
         raise "EDOOFUS #{op.inspect}"
       end
     }.join('')
+
+    l = @logger
+    if l
+      l << str
+    else
+      env['rack.errors'].write(str)
+    end
+    nil
   end
 end
diff --git a/test/test_clogger.rb b/test/test_clogger.rb
index 10c587a..9bff918 100644
--- a/test/test_clogger.rb
+++ b/test/test_clogger.rb
@@ -837,4 +837,21 @@ class TestClogger < Test::Unit::TestCase
     expect = "\"GET http://example.com/hello?goodbye=true HTTP/1.0\"\n"
     assert_equal [ expect ], s
   end
+
+  def test_lint_error_wrapper
+    require 'rack/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))
+    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)
+  end
 end