about summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <normalperson@yhbt.net>2013-12-16 20:38:10 +0000
committerEric Wong <normalperson@yhbt.net>2014-05-12 06:33:24 +0000
commit58f027b6c7bf6bb319e5601594219887770edcc7 (patch)
treeb3020ed3a5f21937792d1e2fc079234e3f37a21e
parent2b2971794c5deda8fd1b100041c24f06ae55337c (diff)
:to_io never was a Rack extension, and ends up breaking the case
where an SSL socket is proxied.  The role of :to_io in IO-like
objects is to aid IO.select and like methods.
-rw-r--r--ext/clogger_ext/clogger.c46
-rw-r--r--ext/clogger_ext/ruby_1_9_compat.h31
-rw-r--r--lib/clogger/pure.rb9
-rw-r--r--test/test_clogger_to_path.rb37
4 files changed, 11 insertions, 112 deletions
diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c
index 122e7fb..6531d87 100644
--- a/ext/clogger_ext/clogger.c
+++ b/ext/clogger_ext/clogger.c
@@ -129,7 +129,6 @@ static ID size_id;
 static ID sq_brace_id;
 static ID new_id;
 static ID to_path_id;
-static ID to_io_id;
 static ID respond_to_id;
 static VALUE cClogger;
 static VALUE mFormat;
@@ -985,22 +984,17 @@ static VALUE to_path(VALUE self)
         struct stat sb;
         int rv;
         VALUE path = rb_funcall(c->body, to_path_id, 0);
+        const char *cpath = StringValueCStr(path);
+        unsigned devfd;
 
-        /* try to avoid an extra path lookup  */
-        if (rb_respond_to(c->body, to_io_id)) {
-                rv = fstat(my_fileno(c->body), &sb);
-        } else {
-                const char *cpath = StringValueCStr(path);
-                unsigned devfd;
-                /*
-                 * Rainbows! can use "/dev/fd/%u" in to_path output to avoid
-                 * extra open() syscalls, too.
-                 */
-                if (sscanf(cpath, "/dev/fd/%u", &devfd) == 1)
-                        rv = fstat((int)devfd, &sb);
-                else
-                        rv = stat(cpath, &sb);
-        }
+        /*
+         * Rainbows! can use "/dev/fd/%u" in to_path output to avoid
+         * extra open() syscalls, too.
+         */
+        if (sscanf(cpath, "/dev/fd/%u", &devfd) == 1)
+                rv = fstat((int)devfd, &sb);
+        else
+                rv = stat(cpath, &sb);
 
         /*
          * calling this method implies the web server will bypass
@@ -1011,24 +1005,6 @@ static VALUE to_path(VALUE self)
         return path;
 }
 
-/*
- * call-seq:
- *   clogger.to_io
- *
- * used to proxy +:to_io+ method calls to the wrapped response body.
- */
-static VALUE to_io(VALUE self)
-{
-        struct clogger *c = clogger_get(self);
-        struct stat sb;
-        VALUE io = rb_convert_type(c->body, T_FILE, "IO", "to_io");
-
-        if (fstat(my_fileno(io), &sb) == 0)
-                c->body_bytes_sent = sb.st_size;
-
-        return io;
-}
-
 /* :nodoc: */
 static VALUE body(VALUE self)
 {
@@ -1051,7 +1027,6 @@ void Init_clogger_ext(void)
         sq_brace_id = rb_intern("[]");
         new_id = rb_intern("new");
         to_path_id = rb_intern("to_path");
-        to_io_id = rb_intern("to_io");
         respond_to_id = rb_intern("respond_to?");
         cClogger = rb_define_class("Clogger", rb_cObject);
         mFormat = rb_define_module_under(cClogger, "Format");
@@ -1065,7 +1040,6 @@ void Init_clogger_ext(void)
         rb_define_method(cClogger, "wrap_body?", clogger_wrap_body, 0);
         rb_define_method(cClogger, "reentrant?", clogger_reentrant, 0);
         rb_define_method(cClogger, "to_path", to_path, 0);
-        rb_define_method(cClogger, "to_io", to_io, 0);
         rb_define_method(cClogger, "respond_to?", respond_to, 1);
         rb_define_method(cClogger, "body", body, 0);
         CONST_GLOBAL_STR(REMOTE_ADDR);
diff --git a/ext/clogger_ext/ruby_1_9_compat.h b/ext/clogger_ext/ruby_1_9_compat.h
index b5653dc..de9f074 100644
--- a/ext/clogger_ext/ruby_1_9_compat.h
+++ b/ext/clogger_ext/ruby_1_9_compat.h
@@ -18,34 +18,3 @@ static void rb_18_str_set_len(VALUE str, long len)
 }
 #define rb_str_set_len(str,len) rb_18_str_set_len(str,len)
 #endif
-
-#if ! HAVE_RB_IO_T
-#  define rb_io_t OpenFile
-#endif
-
-#ifdef GetReadFile
-#  define FPTR_TO_FD(fptr) (fileno(GetReadFile(fptr)))
-#else
-#  if !HAVE_RB_IO_T || (RUBY_VERSION_MAJOR == 1 && RUBY_VERSION_MINOR == 8)
-#    define FPTR_TO_FD(fptr) fileno(fptr->f)
-#  else
-#    define FPTR_TO_FD(fptr) fptr->fd
-#  endif
-#endif
-
-static int my_fileno(VALUE io)
-{
-        rb_io_t *fptr;
-
-        for (;;) {
-                switch (TYPE(io)) {
-                case T_FILE: {
-                        GetOpenFile(io, fptr);
-                        return FPTR_TO_FD(fptr);
-                }
-                default:
-                        io = rb_convert_type(io, T_FILE, "IO", "to_io");
-                        /* retry */
-                }
-        }
-}
diff --git a/lib/clogger/pure.rb b/lib/clogger/pure.rb
index 44f4e62..bb3fc16 100644
--- a/lib/clogger/pure.rb
+++ b/lib/clogger/pure.rb
@@ -82,17 +82,10 @@ class Clogger
 
   def to_path
     rv = @body.to_path
-    # try to avoid unnecessary path lookups with to_io.stat instead of
-    # File.size
-    @body_bytes_sent =
-           @body.respond_to?(:to_io) ? @body.to_io.stat.size : File.size(rv)
+    @body_bytes_sent = File.size(rv)
     rv
   end
 
-  def to_io
-    @body.to_io
-  end
-
 private
 
   def byte_xs(s)
diff --git a/test/test_clogger_to_path.rb b/test/test_clogger_to_path.rb
index 4cc9027..b437695 100644
--- a/test/test_clogger_to_path.rb
+++ b/test/test_clogger_to_path.rb
@@ -93,43 +93,6 @@ class TestCloggerToPath < Test::Unit::TestCase
     assert_equal "365 200\n", logger.string
   end
 
-  def test_wraps_to_path_to_io
-    logger = StringIO.new
-    tmp = Tempfile.new('')
-    def tmp.to_io
-      @to_io_called = super
-    end
-    def tmp.to_path
-      path
-    end
-    app = Rack::Builder.new do
-      tmp.syswrite(' ' * 365)
-      tmp.sysseek(0)
-      h = {
-        'Content-Length' => '0',
-        'Content-Type' => 'text/plain',
-      }
-      use Clogger,
-        :logger => logger,
-        :reentrant => true,
-        :format => '$body_bytes_sent $status'
-      run lambda { |env| [ 200, h, tmp ] }
-    end.to_app
-
-    status, headers, body = app.call(@req)
-    assert_instance_of(Clogger, body)
-    check_body(body)
-
-    assert_equal tmp.path, body.to_path
-    assert_nothing_raised { body.to_io }
-    assert_kind_of IO, tmp.instance_variable_get(:@to_io_called)
-    assert logger.string.empty?
-    assert ! tmp.closed?
-    body.close
-    assert tmp.closed?
-    assert_equal "365 200\n", logger.string
-  end
-
   def test_does_not_wrap_to_path
     logger = StringIO.new
     app = Rack::Builder.new do