librelist archives

[PATCH 0/3] ext: ensure we work with relaxed Rack::Lint

From:

Eric Wong

Date:

2009-10-06 @ 19:03

The next version of Rack will relax Lint to allow subclasses
of String and Hash objects.  Ensure our C extension continues
to work with those.

  [1/3] ext: convert non-Hashes #to_hash if possible
  [2/3] ext: ensure all objects we byte_xs are Strings
  [3/3] tests for subclassing

-- 
Eric Wong

Rack::Lint will be relaxed in the next version to allow
subclasses of String and Hash objects, so ensure we're
good to go when the next version of Rack hits.
---
 test/test_clogger.rb |   40 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/test/test_clogger.rb b/test/test_clogger.rb
index 23d6e58..215256c 100644
--- a/test/test_clogger.rb
+++ b/test/test_clogger.rb
@@ -7,6 +7,11 @@ require "stringio"
 require "rack"
 
 require "clogger"
+
+# used to test subclasses
+class FooString < String
+end
+
 class TestClogger < Test::Unit::TestCase
   include Clogger::Format
 
@@ -392,6 +397,41 @@ class TestClogger < Test::Unit::TestCase
     assert_nothing_raised { cl.call(@req) }
   end
 
+  def test_subclass_hash
+    str = StringIO.new
+    req = Rack::Utils::HeaderHash.new(@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 {} }
+    assert str.size > 0
+  end
+
+  def test_subclassed_string_req
+    str = StringIO.new
+    req = {}
+    @req.each { |key,value|
+      req[FooString.new(key)] = value.kind_of?(String) ?
+                                FooString.new(value) : value
+    }
+    app = lambda { |env| [302, [ %w(a) ], []] }
+    cl = Clogger.new(app, :logger => str, :format => Rack_1_0)
+    assert_nothing_raised { cl.call(req).last.each {} }
+    assert str.size > 0
+  end
+
+  def test_subclassed_string_in_body
+    str = StringIO.new
+    body = "hello"
+    r = nil
+    app = lambda { |env| [302, [ %w(a) ], [FooString.new(body)]] }
+    cl = Clogger.new(app, :logger => str, :format => '$body_bytes_sent')
+    assert_nothing_raised { cl.call(@req).last.each { |x| r = x } }
+    assert str.size > 0
+    assert_equal body.size.to_s << "\n", str.string
+    assert_equal r, body
+    assert r.object_id != body.object_id
+  end
+
   def test_http_09_request
     str = StringIO.new
     app = lambda { |env| [302, [ %w(a) ], []] }
-- 
1.6.5.rc2.18.gcbf7e

Rack will be relaxing the spec to allow subclasses
of String objects.  Just in case they're not compatible,
we'll convert them to strings.
---
 ext/clogger_ext/clogger.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c
index f935c11..26d1ec6 100644
--- a/ext/clogger_ext/clogger.c
+++ b/ext/clogger_ext/clogger.c
@@ -124,7 +124,7 @@ static inline int need_escape(unsigned c)
 }
 
 /* we are encoding-agnostic, clients can send us all sorts of junk */
-static VALUE byte_xs(VALUE from)
+static VALUE byte_xs_str(VALUE from)
 {
 	static const char esc[] = "0123456789ABCDEF";
 	unsigned char *new_ptr;
@@ -164,6 +164,11 @@ static VALUE byte_xs(VALUE from)
 	return rv;
 }
 
+static VALUE byte_xs(VALUE from)
+{
+	return byte_xs_str(rb_obj_as_string(from));
+}
+
 static void clogger_mark(void *ptr)
 {
 	struct clogger *c = ptr;
@@ -459,7 +464,7 @@ static void append_request_env(struct clogger *c, VALUE key)
 {
 	VALUE tmp = rb_hash_aref(c->env, key);
 
-	tmp = NIL_P(tmp) ? g_dash : byte_xs(rb_obj_as_string(tmp));
+	tmp = NIL_P(tmp) ? g_dash : byte_xs(tmp);
 	rb_str_buf_append(c->log_buf, tmp);
 }
 
@@ -470,7 +475,7 @@ static void append_response(struct clogger *c, VALUE key)
 	assert(rb_obj_class(c->headers) == cHeaderHash);
 
 	v = rb_funcall(c->headers, sq_brace_id, 1, key);
-	v = NIL_P(v) ? g_dash : byte_xs(rb_obj_as_string(v));
+	v = NIL_P(v) ? g_dash : byte_xs(v);
 	rb_str_buf_append(c->log_buf, v);
 }
 
-- 
1.6.5.rc2.18.gcbf7e

This is to remain compatible with Rack if it relaxes
Rack::Lint to allow subclasses.
---
 ext/clogger_ext/clogger.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c
index 7813c8c..f935c11 100644
--- a/ext/clogger_ext/clogger.c
+++ b/ext/clogger_ext/clogger.c
@@ -714,6 +714,8 @@ static VALUE clogger_call(VALUE self, VALUE env)
 	struct clogger *c = clogger_get(self);
 	VALUE rv;
 
+	env = rb_check_convert_type(env, T_HASH, "Hash", "to_hash");
+
 	if (c->wrap_body) {
 		if (c->reentrant < 0) {
 			VALUE tmp = rb_hash_aref(env, g_rack_multithread);
-- 
1.6.5.rc2.18.gcbf7e