From 58f027b6c7bf6bb319e5601594219887770edcc7 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Mon, 16 Dec 2013 20:38:10 +0000 Subject: remove :to_io support :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. --- ext/clogger_ext/clogger.c | 46 +++++++++------------------------------ ext/clogger_ext/ruby_1_9_compat.h | 31 -------------------------- lib/clogger/pure.rb | 9 +------- test/test_clogger_to_path.rb | 37 ------------------------------- 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 -- cgit v1.2.3-24-ge0c7