about summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <normalperson@yhbt.net>2009-08-29 13:35:22 -0700
committerEric Wong <normalperson@yhbt.net>2009-08-29 13:35:22 -0700
commitc03045ecde0f3270d7458ba7ac0d76a25afc6fb2 (patch)
tree7ce6c2d567fc013f0bcbbebef4b663c851b293d7
parent46a176a741ad4d19d81946b4232c0c26fb8bdbc8 (diff)
downloadclogger-c03045ecde0f3270d7458ba7ac0d76a25afc6fb2.tar.gz
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*).
-rw-r--r--ext/clogger_ext/clogger.c48
-rw-r--r--lib/clogger.rb1
-rw-r--r--lib/clogger/pure.rb11
-rw-r--r--test/test_clogger.rb16
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|