librelist archives

[PATCH 1/3] ext: compiler compatibility fix

From:

Eric Wong

Date:

2009-08-31 @ 05:27

Remove trailing "," for enum
---
 ext/clogger_ext/clogger.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c
index de38ba2..369baf6 100644
--- a/ext/clogger_ext/clogger.c
+++ b/ext/clogger_ext/clogger.c
@@ -55,7 +55,7 @@ enum clogger_special {
 	CL_SP_response_length,
 	CL_SP_ip,
 	CL_SP_pid,
-	CL_SP_request_uri,
+	CL_SP_request_uri
 };
 
 struct clogger {
-- 
1.6.4.rc3

We're not Rack::Lint, but we still need to take steps to
avoid segfaulting if we host non-Rack::Lint-compliant
applications.

This also updates the pure variant to fail on bad applications,
too.
---
 ext/clogger_ext/clogger.c |   11 ++++++++++-
 lib/clogger/pure.rb       |    7 ++++++-
 test/test_clogger.rb      |    7 +++++++
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c
index 369baf6..19386e5 100644
--- a/ext/clogger_ext/clogger.c
+++ b/ext/clogger_ext/clogger.c
@@ -185,7 +185,7 @@ static int str_case_eq(VALUE a, VALUE b)
 struct response_ops { long nr; VALUE ops; };
 
 /* this can be worse than O(M*N) :<... but C loops are fast ... */
-static VALUE swap_sent_headers(VALUE kv, VALUE memo)
+static VALUE swap_sent_headers_unsafe(VALUE kv, VALUE memo)
 {
 	struct response_ops *tmp = (struct response_ops *)memo;
 	VALUE key = rb_obj_as_string(RARRAY_PTR(kv)[0]);
@@ -214,6 +214,15 @@ static VALUE swap_sent_headers(VALUE kv, VALUE memo)
 	return Qnil;
 }
 
+static VALUE swap_sent_headers(VALUE kv, VALUE memo)
+{
+	if (TYPE(kv) != T_ARRAY)
+		rb_raise(rb_eTypeError, "headers not returning pairs");
+	if (RARRAY_LEN(kv) < 2)
+		rb_raise(rb_eTypeError, "headers not returning pairs");
+	return swap_sent_headers_unsafe(kv, memo);
+}
+
 static VALUE sent_headers_ops(struct clogger *c)
 {
 	struct response_ops tmp;
diff --git a/lib/clogger/pure.rb b/lib/clogger/pure.rb
index f593af0..0a7aa24 100644
--- a/lib/clogger/pure.rb
+++ b/lib/clogger/pure.rb
@@ -133,7 +133,12 @@ private
   end
 
   def get_sent_header(headers, match)
-    headers.each { |key, value| match == key.downcase and return value }
+    headers.each do |pair|
+      Array === pair && pair.size >= 2 or
+        raise TypeError, "headers not returning pairs"
+      key, value = pair
+      match == key.downcase and return value
+    end
     "-"
   end
 
diff --git a/test/test_clogger.rb b/test/test_clogger.rb
index d2121da..9cb494a 100644
--- a/test/test_clogger.rb
+++ b/test/test_clogger.rb
@@ -373,4 +373,11 @@ 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_raise(TypeError) { cl.call(@req) }
+  end
+
 end
-- 
1.6.4.rc3

The pure variant was using lower-case output instead
of upper case, the ext variant was actually fine in this
case.  This is for nginx output format compatibility.
---
 lib/clogger/pure.rb  |    2 +-
 test/test_clogger.rb |   12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/lib/clogger/pure.rb b/lib/clogger/pure.rb
index 0a7aa24..ebbb1d6 100644
--- a/lib/clogger/pure.rb
+++ b/lib/clogger/pure.rb
@@ -62,7 +62,7 @@ private
   def byte_xs(s)
     s = s.dup
     s.force_encoding(Encoding::BINARY) if defined?(Encoding::BINARY)
-    s.gsub!(/(['"\x00-\x1f])/) { |x| "\\x#{$1.unpack('H2').first}" }
+    s.gsub!(/(['"\x00-\x1f])/) { |x| "\\x#{$1.unpack('H2').first.upcase}" }
     s
   end
 
diff --git a/test/test_clogger.rb b/test/test_clogger.rb
index 9cb494a..5e85e15 100644
--- a/test/test_clogger.rb
+++ b/test/test_clogger.rb
@@ -333,6 +333,18 @@ 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" }, [] ] }
+    cl = Clogger.new(app, :logger => str, :format => '$sent_http_set_cookie')
+    cl.call(@req)
+    assert_equal "a\\x0Ab\n", str.string
+  end
+
   def test_request_uri_fallback
     str = StringIO.new
     app = lambda { |env| [ 200, {}, [] ] }
-- 
1.6.4.rc3