From c03045ecde0f3270d7458ba7ac0d76a25afc6fb2 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sat, 29 Aug 2009 13:35:22 -0700 Subject: support "$request_uri" as a log variable This was documented in the README but never implemented. Some popular web servers set REQUEST_URI even though it's not required by Rack, so allow this variable to be used if possible. As a side effect, it is also less likely to be modified by certain handlers (*cough*Rails::Rack::Static*cough*). --- ext/clogger_ext/clogger.c | 48 +++++++++++++++++++++++++++++++++-------------- lib/clogger.rb | 1 + lib/clogger/pure.rb | 11 ++++++++++- test/test_clogger.rb | 16 ++++++++++++++++ 4 files changed, 61 insertions(+), 15 deletions(-) diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c index 34927d3..c639567 100644 --- a/ext/clogger_ext/clogger.c +++ b/ext/clogger_ext/clogger.c @@ -54,7 +54,8 @@ enum clogger_special { CL_SP_request_length, CL_SP_response_length, CL_SP_ip, - CL_SP_pid + CL_SP_pid, + CL_SP_request_uri, }; struct clogger { @@ -92,6 +93,7 @@ static VALUE g_HTTP_X_FORWARDED_FOR; static VALUE g_REMOTE_ADDR; static VALUE g_REQUEST_METHOD; static VALUE g_PATH_INFO; +static VALUE g_REQUEST_URI; static VALUE g_QUERY_STRING; static VALUE g_HTTP_VERSION; static VALUE g_rack_errors; @@ -410,30 +412,44 @@ static void append_time_fmt(struct clogger *c, const VALUE *op) append_tv(c, op, &now); } +static void append_request_uri(struct clogger *c) +{ + VALUE tmp; + + tmp = rb_hash_aref(c->env, g_REQUEST_URI); + if (NIL_P(tmp)) { + tmp = rb_hash_aref(c->env, g_PATH_INFO); + if (!NIL_P(tmp)) + rb_str_buf_append(c->log_buf, byte_xs(tmp)); + tmp = rb_hash_aref(c->env, g_QUERY_STRING); + if (!NIL_P(tmp) && RSTRING_LEN(tmp) != 0) { + rb_str_buf_append(c->log_buf, g_question_mark); + rb_str_buf_append(c->log_buf, byte_xs(tmp)); + } + } else { + rb_str_buf_append(c->log_buf, byte_xs(tmp)); + } +} + static void append_request(struct clogger *c) { VALUE tmp; - VALUE env = c->env; /* REQUEST_METHOD doesn't need escaping, Rack::Lint governs it */ - tmp = rb_hash_aref(env, g_REQUEST_METHOD); - rb_str_buf_append(c->log_buf, NIL_P(tmp) ? g_empty : tmp); + tmp = rb_hash_aref(c->env, g_REQUEST_METHOD); + if (!NIL_P(tmp)) + rb_str_buf_append(c->log_buf, tmp); + rb_str_buf_append(c->log_buf, g_space); - /* broken clients can send " and other questionable URIs */ - tmp = rb_hash_aref(env, g_PATH_INFO); - rb_str_buf_append(c->log_buf, NIL_P(tmp) ? g_empty : byte_xs(tmp)); + append_request_uri(c); - tmp = rb_hash_aref(env, g_QUERY_STRING); - if (RSTRING_LEN(tmp) != 0) { - rb_str_buf_append(c->log_buf, g_question_mark); - rb_str_buf_append(c->log_buf, byte_xs(tmp)); - } rb_str_buf_append(c->log_buf, g_space); /* HTTP_VERSION can be injected by malicious clients */ - tmp = rb_hash_aref(env, g_HTTP_VERSION); - rb_str_buf_append(c->log_buf, NIL_P(tmp) ? g_empty : byte_xs(tmp)); + tmp = rb_hash_aref(c->env, g_HTTP_VERSION); + if (!NIL_P(tmp)) + rb_str_buf_append(c->log_buf, byte_xs(tmp)); } static void append_request_length(struct clogger *c) @@ -538,6 +554,9 @@ static void special_var(struct clogger *c, enum clogger_special var) break; case CL_SP_pid: append_pid(c); + break; + case CL_SP_request_uri: + append_request_uri(c); } } @@ -788,6 +807,7 @@ void Init_clogger_ext(void) CONST_GLOBAL_STR(REQUEST_METHOD); CONST_GLOBAL_STR(PATH_INFO); CONST_GLOBAL_STR(QUERY_STRING); + CONST_GLOBAL_STR(REQUEST_URI); CONST_GLOBAL_STR(HTTP_VERSION); CONST_GLOBAL_STR2(rack_errors, "rack.errors"); CONST_GLOBAL_STR2(rack_input, "rack.input"); diff --git a/lib/clogger.rb b/lib/clogger.rb index 0e4148e..8e881c2 100644 --- a/lib/clogger.rb +++ b/lib/clogger.rb @@ -29,6 +29,7 @@ class Clogger :response_length => 4, # like body_bytes_sent, except "-" instead of "0" :ip => 5, # HTTP_X_FORWARDED_FOR || REMOTE_ADDR || - :pid => 6, # getpid() + :request_uri => 7 } private diff --git a/lib/clogger/pure.rb b/lib/clogger/pure.rb index 11c03f4..85c6777 100644 --- a/lib/clogger/pure.rb +++ b/lib/clogger/pure.rb @@ -63,6 +63,13 @@ private SPECIAL_RMAP = SPECIAL_VARS.inject([]) { |ary, (k,v)| ary[v] = k; ary } + def request_uri(env) + ru = env['REQUEST_URI'] and return byte_xs(ru) + qs = env['QUERY_STRING'] + qs.empty? or qs = "?#{byte_xs(qs)}" + "#{byte_xs(env['PATH_INFO'])}#{qs}" + end + def special_var(special_nr, env, status, headers) case SPECIAL_RMAP[special_nr] when :body_bytes_sent @@ -74,8 +81,10 @@ private qs = env['QUERY_STRING'] qs.empty? or qs = "?#{byte_xs(qs)}" "#{env['REQUEST_METHOD']} " \ - "#{byte_xs(env['PATH_INFO'])}#{qs} " \ + "#{request_uri(env)} " \ "#{byte_xs(env['HTTP_VERSION'])}" + when :request_uri + request_uri(env) when :request_length env['rack.input'].size.to_s when :response_length diff --git a/test/test_clogger.rb b/test/test_clogger.rb index f79017b..c3ae93c 100644 --- a/test/test_clogger.rb +++ b/test/test_clogger.rb @@ -333,6 +333,22 @@ class TestClogger < Test::Unit::TestCase assert_equal expect, str.string end + def test_request_uri_fallback + str = StringIO.new + app = lambda { |env| [ 200, {}, [] ] } + cl = Clogger.new(app, :logger => str, :format => '$request_uri') + status, headers, body = cl.call(@req) + assert_equal "/hello?goodbye=true\n", str.string + end + + def test_request_uri_set + str = StringIO.new + app = lambda { |env| [ 200, {}, [] ] } + cl = Clogger.new(app, :logger => str, :format => '$request_uri') + status, headers, body = cl.call(@req.merge("REQUEST_URI" => '/zzz')) + assert_equal "/zzz\n", str.string + end + def test_cookies str = StringIO.new app = lambda { |env| -- cgit v1.2.3-24-ge0c7