From 09cae70d5509530ed3abff9046b1dc0fe448b3b3 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Fri, 4 Sep 2009 19:19:28 -0700 Subject: use Rack::Utils::HeaderHash for $sent_http_* lookups No point in having extra code to do case-insensitive lookups, especially since the HeaderHash implementation is already in wide use and will only get faster as time goes by. --- Rakefile | 1 + ext/clogger_ext/clogger.c | 130 +++++++++++++--------------------------------- lib/clogger/pure.rb | 17 ++---- test/test_clogger.rb | 2 +- 4 files changed, 42 insertions(+), 108 deletions(-) diff --git a/Rakefile b/Rakefile index d1dcf85..0cc50a7 100644 --- a/Rakefile +++ b/Rakefile @@ -18,6 +18,7 @@ common = lambda do |hoe| hoe.email = 'clogger@librelist.com' hoe.spec_extras.merge!('rdoc_options' => [ "--title", title ]) hoe.remote_rdoc_dir = '' + hoe.extra_deps << [ 'rack', '> 0.9' ] end if ENV['CLOGGER_EXT'] diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c index 3bd374c..bec4c08 100644 --- a/ext/clogger_ext/clogger.c +++ b/ext/clogger_ext/clogger.c @@ -87,8 +87,11 @@ static ID close_id; static ID to_i_id; static ID to_s_id; static ID size_id; +static ID sq_brace_id; +static ID new_id; static VALUE cClogger; static VALUE mFormat; +static VALUE cHeaderHash; /* common hash lookup keys */ static VALUE g_HTTP_X_FORWARDED_FOR; @@ -161,98 +164,6 @@ static VALUE byte_xs(VALUE from) return rv; } -/* strcasecmp isn't locale independent, so we roll our own */ -static int str_case_eq(VALUE a, VALUE b) -{ - long alen = RSTRING_LEN(a); - long blen = RSTRING_LEN(b); - - if (alen == blen) { - const char *aptr = RSTRING_PTR(a); - const char *bptr = RSTRING_PTR(b); - - for (; alen--; ++aptr, ++bptr) { - if ((*bptr == *aptr) - || (*aptr >= 'A' && *aptr <= 'Z' && - (*aptr | 0x20) == *bptr)) - continue; - return 0; - } - return 1; - } - return 0; -} - -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_unsafe(VALUE kv, VALUE memo) -{ - struct response_ops *tmp = (struct response_ops *)memo; - VALUE key = rb_obj_as_string(RARRAY_PTR(kv)[0]); - long i = RARRAY_LEN(tmp->ops); - VALUE *ary = RARRAY_PTR(tmp->ops); - VALUE value; - - for (; --i >= 0; ary++) { - VALUE *op = RARRAY_PTR(*ary); - enum clogger_opcode opcode = NUM2INT(op[0]); - - if (opcode != CL_OP_RESPONSE) - continue; - assert(RARRAY_LEN(*ary) == 2); - if (!str_case_eq(key, op[1])) - continue; - - value = RARRAY_PTR(kv)[1]; - op[0] = INT2NUM(CL_OP_LITERAL); - op[1] = byte_xs(rb_obj_as_string(value)); - - if (!--tmp->nr) - rb_iter_break(); - return Qnil; - } - 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; - long i, len; - VALUE *ary; - - if (!c->need_resp) - return c->fmt_ops; - - tmp.nr = 0; - tmp.ops = rb_ary_dup(c->fmt_ops); - len = RARRAY_LEN(tmp.ops); - ary = RARRAY_PTR(tmp.ops); - - for (i = 0; i < len; ++i) { - VALUE *op = RARRAY_PTR(ary[i]); - - if (NUM2INT(op[0]) == CL_OP_RESPONSE) { - assert(RARRAY_LEN(ary[i]) == 2); - ary[i] = rb_ary_dup(ary[i]); - ++tmp.nr; - } - } - - rb_iterate(rb_each, c->headers, swap_sent_headers, (VALUE)&tmp); - - return tmp.ops; -} - static void clogger_mark(void *ptr) { struct clogger *c = ptr; @@ -552,6 +463,17 @@ static void append_request_env(struct clogger *c, VALUE key) rb_str_buf_append(c->log_buf, tmp); } +static void append_response(struct clogger *c, VALUE key) +{ + VALUE v; + + 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)); + rb_str_buf_append(c->log_buf, v); +} + static void special_var(struct clogger *c, enum clogger_special var) { switch (var) { @@ -586,7 +508,7 @@ static void special_var(struct clogger *c, enum clogger_special var) static VALUE cwrite(struct clogger *c) { - const VALUE ops = sent_headers_ops(c); + const VALUE ops = c->fmt_ops; const VALUE *ary = RARRAY_PTR(ops); long i = RARRAY_LEN(ops); VALUE dst = c->log_buf; @@ -605,8 +527,7 @@ static VALUE cwrite(struct clogger *c) append_request_env(c, op[1]); break; case CL_OP_RESPONSE: - /* headers we found already got swapped for literals */ - rb_str_buf_append(dst, g_dash); + append_response(c, op[1]); break; case CL_OP_SPECIAL: special_var(c, NUM2INT(op[1])); @@ -760,6 +681,12 @@ static VALUE ccall(struct clogger *c, VALUE env) c->status = tmp[0]; c->headers = tmp[1]; c->body = tmp[2]; + + if (cHeaderHash != rb_obj_class(c->headers)) { + c->headers = rb_funcall(cHeaderHash, new_id, 1, tmp[1]); + rv = rb_ary_dup(rv); + rb_ary_store(rv, 1, c->headers); + } } else { c->status = INT2NUM(500); c->headers = c->body = rb_ary_new(); @@ -828,6 +755,16 @@ static VALUE clogger_init_copy(VALUE clone, VALUE orig) #define CONST_GLOBAL_STR(val) CONST_GLOBAL_STR2(val, #val) +static void init_rack_utils_header_hash(void) +{ + VALUE mRack, mUtils; + + rb_require("rack"); + mRack = rb_define_module("Rack"); + mUtils = rb_define_module_under(mRack, "Utils"); + cHeaderHash = rb_define_class_under(mUtils, "HeaderHash", rb_cHash); +} + void Init_clogger_ext(void) { ltlt_id = rb_intern("<<"); @@ -837,6 +774,8 @@ void Init_clogger_ext(void) to_i_id = rb_intern("to_i"); to_s_id = rb_intern("to_s"); size_id = rb_intern("size"); + sq_brace_id = rb_intern("[]"); + new_id = rb_intern("new"); cClogger = rb_define_class("Clogger", rb_cObject); mFormat = rb_define_module_under(cClogger, "Format"); rb_define_alloc_func(cClogger, clogger_alloc); @@ -863,4 +802,5 @@ void Init_clogger_ext(void) CONST_GLOBAL_STR2(space, " "); CONST_GLOBAL_STR2(question_mark, "?"); CONST_GLOBAL_STR2(rack_request_cookie_hash, "rack.request.cookie_hash"); + init_rack_utils_header_hash(); } diff --git a/lib/clogger/pure.rb b/lib/clogger/pure.rb index 108d036..718db9a 100644 --- a/lib/clogger/pure.rb +++ b/lib/clogger/pure.rb @@ -1,6 +1,8 @@ # -*- encoding: binary -*- # :stopdoc: -# + +require 'rack' + # Not at all optimized for performance, this was written based on # the original C extension code so it's not very Ruby-ish... class Clogger @@ -23,6 +25,7 @@ class Clogger raise TypeError, "app response not a 3 element Array: #{resp.inspect}" end status, headers, body = resp + headers = Rack::Utils::HeaderHash.new(headers) if wrap_body? @reentrant = env['rack.multithread'] @env, @status, @headers, @body = env, status, headers, body @@ -114,7 +117,7 @@ private case op[0] when OP_LITERAL; op[1] when OP_REQUEST; byte_xs(env[op[1]] || "-") - when OP_RESPONSE; byte_xs(get_sent_header(headers, op[1])) + when OP_RESPONSE; byte_xs(headers[op[1]] || "-") when OP_SPECIAL; special_var(op[1], env, status, headers) when OP_EVAL; eval(op[1]).to_s rescue "-" when OP_TIME_LOCAL; Time.now.strftime(op[1]) @@ -133,14 +136,4 @@ private }.join('') end - def get_sent_header(headers, match) - 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 - end diff --git a/test/test_clogger.rb b/test/test_clogger.rb index 71dbad8..e65311f 100644 --- a/test/test_clogger.rb +++ b/test/test_clogger.rb @@ -389,7 +389,7 @@ class TestClogger < Test::Unit::TestCase 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) } + assert_nothing_raised { cl.call(@req) } end def test_http_09_request -- cgit v1.2.3-24-ge0c7