You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by hu...@apache.org on 2014/03/28 19:38:42 UTC

svn commit: r1582858 - in /httpd/httpd/trunk: docs/log-message-tags/next-number modules/lua/lua_apr.c modules/lua/lua_apr.h modules/lua/lua_request.c modules/lua/lua_request.h

Author: humbedooh
Date: Fri Mar 28 18:38:41 2014
New Revision: 1582858

URL: http://svn.apache.org/r1582858
Log:
mod_lua: Redesign the table construction/access mechanism, so we pass on a struct with the request_rec, the table pointer and the table name instead of just the table pointer. This allows us to use the request_rec for logging/editing purposes, as well as inform the user which exact table in the request_rec was modified.

Modified:
    httpd/httpd/trunk/docs/log-message-tags/next-number
    httpd/httpd/trunk/modules/lua/lua_apr.c
    httpd/httpd/trunk/modules/lua/lua_apr.h
    httpd/httpd/trunk/modules/lua/lua_request.c
    httpd/httpd/trunk/modules/lua/lua_request.h

Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1582858&r1=1582857&r2=1582858&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
+++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Mar 28 18:38:41 2014
@@ -1 +1 @@
-2614
+2615

Modified: httpd/httpd/trunk/modules/lua/lua_apr.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/lua/lua_apr.c?rev=1582858&r1=1582857&r2=1582858&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/lua/lua_apr.c (original)
+++ httpd/httpd/trunk/modules/lua/lua_apr.c Fri Mar 28 18:38:41 2014
@@ -17,17 +17,18 @@
 
 #include "mod_lua.h"
 #include "lua_apr.h"
+APLOG_USE_MODULE(lua);
 
-apr_table_t *ap_lua_check_apr_table(lua_State *L, int index)
+req_table_t *ap_lua_check_apr_table(lua_State *L, int index)
 {
-    apr_table_t *t;
+    req_table_t* t;
     luaL_checkudata(L, index, "Apr.Table");
     t = lua_unboxpointer(L, index);
     return t;
 }
 
 
-void ap_lua_push_apr_table(lua_State *L, apr_table_t *t)
+void ap_lua_push_apr_table(lua_State *L, req_table_t *t)
 {
     lua_boxpointer(L, t);
     luaL_getmetatable(L, "Apr.Table");
@@ -36,26 +37,35 @@ void ap_lua_push_apr_table(lua_State *L,
 
 static int lua_table_set(lua_State *L)
 {
-    apr_table_t    *t = ap_lua_check_apr_table(L, 1);
+    req_table_t    *t = ap_lua_check_apr_table(L, 1);
     const char     *key = luaL_checkstring(L, 2);
     const char     *val = luaL_checkstring(L, 3);
-    
-    /* Prevent response/header splitting by not allowing newlines in tables.
-     * At this stage, we don't have the request_rec handy, and we can't change
-     * a const char*, so we'll redirect to a standard error value instead.
-     */
-    if (ap_strchr_c(val, '\n')) {
-        val = "[ERROR: Value contains newline, ignored.]";
+    /* Unless it's the 'notes' table, check for newline chars */
+    if (strcmp(t->n, "notes") && ap_strchr_c(val, '\n')) {
+        char *badchar;
+        char *replacement = apr_pstrdup(t->r->pool, val);
+        badchar = replacement;
+        while ( (badchar = ap_strchr(badchar, '\n')) ) {
+            *badchar = ' ';
+        }
+        if (t->r != NULL) {
+            ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, t->r, 
+                APLOGNO(02614) "mod_lua: Value for '%s' in table '%s' contains newline!",
+                  key, t->n);
+        }
+        apr_table_set(t->t, key, replacement);
+    }
+    else {
+        apr_table_set(t->t, key, val);
     }
-    apr_table_set(t, key, val);
     return 0;
 }
 
 static int lua_table_get(lua_State *L)
 {
-    apr_table_t    *t = ap_lua_check_apr_table(L, 1);
+    req_table_t    *t = ap_lua_check_apr_table(L, 1);
     const char     *key = luaL_checkstring(L, 2);
-    const char     *val = apr_table_get(t, key);
+    const char     *val = apr_table_get(t->t, key);
     lua_pushstring(L, val);
     return 1;
 }

Modified: httpd/httpd/trunk/modules/lua/lua_apr.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/lua/lua_apr.h?rev=1582858&r1=1582857&r2=1582858&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/lua/lua_apr.h (original)
+++ httpd/httpd/trunk/modules/lua/lua_apr.h Fri Mar 28 18:38:41 2014
@@ -30,7 +30,7 @@
 
 
 int ap_lua_init(lua_State *L, apr_pool_t * p);
-apr_table_t *ap_lua_check_apr_table(lua_State *L, int index);
-void ap_lua_push_apr_table(lua_State *L, apr_table_t *t);
+req_table_t *ap_lua_check_apr_table(lua_State *L, int index);
+void ap_lua_push_apr_table(lua_State *L, req_table_t *t);
 
 #endif /* !_LUA_APR_H_ */

Modified: httpd/httpd/trunk/modules/lua/lua_request.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/lua/lua_request.c?rev=1582858&r1=1582857&r2=1582858&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/lua/lua_request.c (original)
+++ httpd/httpd/trunk/modules/lua/lua_request.c Fri Mar 28 18:38:41 2014
@@ -44,7 +44,7 @@ APLOG_USE_MODULE(lua);
 
 typedef char *(*req_field_string_f) (request_rec * r);
 typedef int (*req_field_int_f) (request_rec * r);
-typedef apr_table_t *(*req_field_apr_table_f) (request_rec * r);
+typedef req_table_t *(*req_field_apr_table_f) (request_rec * r);
 
 
 void ap_lua_rstack_dump(lua_State *L, request_rec *r, const char *msg)
@@ -644,29 +644,49 @@ static int req_assbackwards_field(reques
     return r->assbackwards;
 }
 
-static apr_table_t* req_headers_in(request_rec *r)
+static req_table_t* req_headers_in(request_rec *r)
 {
-    return r->headers_in;
+  req_table_t* t = apr_palloc(r->pool, sizeof(req_table_t));
+  t->r = r;
+  t->t = r->headers_in;
+  t->n = "headers_in";
+  return t;
 }
 
-static apr_table_t* req_headers_out(request_rec *r)
+static req_table_t* req_headers_out(request_rec *r)
 {
-    return r->headers_out;
+  req_table_t* t = apr_palloc(r->pool, sizeof(req_table_t));
+  t->r = r;
+  t->t = r->headers_out;
+  t->n = "headers_out";
+  return t;
 }
 
-static apr_table_t* req_err_headers_out(request_rec *r)
+static req_table_t* req_err_headers_out(request_rec *r)
 {
-  return r->err_headers_out;
+  req_table_t* t = apr_palloc(r->pool, sizeof(req_table_t));
+  t->r = r;
+  t->t = r->err_headers_out;
+  t->n = "err_headers_out";
+  return t;
 }
 
-static apr_table_t* req_subprocess_env(request_rec *r)
+static req_table_t* req_subprocess_env(request_rec *r)
 {
-  return r->subprocess_env;
+  req_table_t* t = apr_palloc(r->pool, sizeof(req_table_t));
+  t->r = r;
+  t->t = r->subprocess_env;
+  t->n = "subprocess_env";
+  return t;
 }
 
-static apr_table_t* req_notes(request_rec *r)
+static req_table_t* req_notes(request_rec *r)
 {
-  return r->notes;
+  req_table_t* t = apr_palloc(r->pool, sizeof(req_table_t));
+  t->r = r;
+  t->t = r->notes;
+  t->n = "notes";
+  return t;
 }
 
 static int req_ssl_is_https_field(request_rec *r)
@@ -1788,7 +1808,7 @@ static int req_dispatch(lua_State *L)
     if (rft) {
         switch (rft->type) {
         case APL_REQ_FUNTYPE_TABLE:{
-                apr_table_t *rs;
+                req_table_t *rs;
                 req_field_apr_table_f func = (req_field_apr_table_f)rft->fun;
                 ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01486)
                               "request_rec->dispatching %s -> apr table",
@@ -2858,12 +2878,17 @@ void ap_lua_load_request_lmodule(lua_Sta
 
 void ap_lua_push_connection(lua_State *L, conn_rec *c)
 {
+    req_table_t* t;
     lua_boxpointer(L, c);
     luaL_getmetatable(L, "Apache2.Connection");
     lua_setmetatable(L, -2);
     luaL_getmetatable(L, "Apache2.Connection");
 
-    ap_lua_push_apr_table(L, c->notes);
+    t = apr_pcalloc(c->pool, sizeof(req_table_t));
+    t->t = c->notes;
+    t->r = NULL;
+    t->n = "notes";
+    ap_lua_push_apr_table(L, t);
     lua_setfield(L, -2, "notes");
 
     lua_pushstring(L, c->client_ip);

Modified: httpd/httpd/trunk/modules/lua/lua_request.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/lua/lua_request.h?rev=1582858&r1=1582857&r2=1582858&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/lua/lua_request.h (original)
+++ httpd/httpd/trunk/modules/lua/lua_request.h Fri Mar 28 18:38:41 2014
@@ -38,6 +38,15 @@ typedef struct
     int type;
 } req_fun_t;
 
+
+/* Struct to use as userdata for request_rec tables */
+typedef struct
+{
+    request_rec *r; /* Request_rec */
+    apr_table_t *t; /* apr_table_t* */
+    const char  *n; /* name of table */
+} req_table_t;
+
 typedef struct {
     int type;
     size_t size;



AW: svn commit: r1582858 - in /httpd/httpd/trunk: docs/log-message-tags/next-number modules/lua/lua_apr.c modules/lua/lua_apr.h modules/lua/lua_request.c modules/lua/lua_request.h

Posted by Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>.

> -----Ursprüngliche Nachricht-----
> Von: Daniel Gruno 
> Gesendet: Freitag, 28. März 2014 21:34
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r1582858 - in /httpd/httpd/trunk: docs/log-
> message-tags/next-number modules/lua/lua_apr.c modules/lua/lua_apr.h
> modules/lua/lua_request.c modules/lua/lua_request.h
> 
> On 03/28/2014 09:29 PM, Ruediger Pluem wrote:
> 
> >
> > Why this check if we already use t->r->pool above :-)?
> >
> >> +            ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, t->r,
> >> +                APLOGNO(02614) "mod_lua: Value for '%s' in table
> '%s' contains newline!",
> >> +                  key, t->n);
> >> +        }
> >> +        apr_table_set(t->t, key, replacement);
> >> +    }
> >> +    else {
> >> +        apr_table_set(t->t, key, val);
> >>      }
> >> -    apr_table_set(t, key, val);
> >>      return 0;
> >>  }
> >
> > Regards
> >
> > Rüdiger
> >
> 
> Brain fart, apologies :)
> Since we're already NOT fixing up any table called 'notes', we have no
> need to check if t->r is set, since it will only be NULL in the (super
> secret, hidden) connection notes table.

Maybe you should put a comment regarding this above the block, why it is not needed to check for r != NULL.

Regards

Rüdiger


Re: svn commit: r1582858 - in /httpd/httpd/trunk: docs/log-message-tags/next-number modules/lua/lua_apr.c modules/lua/lua_apr.h modules/lua/lua_request.c modules/lua/lua_request.h

Posted by Daniel Gruno <ru...@cord.dk>.
On 03/28/2014 09:29 PM, Ruediger Pluem wrote:

> 
> Why this check if we already use t->r->pool above :-)?
> 
>> +            ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, t->r, 
>> +                APLOGNO(02614) "mod_lua: Value for '%s' in table '%s' contains newline!",
>> +                  key, t->n);
>> +        }
>> +        apr_table_set(t->t, key, replacement);
>> +    }
>> +    else {
>> +        apr_table_set(t->t, key, val);
>>      }
>> -    apr_table_set(t, key, val);
>>      return 0;
>>  }
> 
> Regards
> 
> Rüdiger
> 

Brain fart, apologies :)
Since we're already NOT fixing up any table called 'notes', we have no
need to check if t->r is set, since it will only be NULL in the (super
secret, hidden) connection notes table.

Thanks for spotting this!

With regards,
Daniel.

Re: svn commit: r1582858 - in /httpd/httpd/trunk: docs/log-message-tags/next-number modules/lua/lua_apr.c modules/lua/lua_apr.h modules/lua/lua_request.c modules/lua/lua_request.h

Posted by Ruediger Pluem <rp...@apache.org>.

humbedooh@apache.org wrote:
> Author: humbedooh
> Date: Fri Mar 28 18:38:41 2014
> New Revision: 1582858
> 
> URL: http://svn.apache.org/r1582858
> Log:
> mod_lua: Redesign the table construction/access mechanism, so we pass on a struct with the request_rec, the table pointer and the table name instead of just the table pointer. This allows us to use the request_rec for logging/editing purposes, as well as inform the user which exact table in the request_rec was modified.
> 
> Modified:
>     httpd/httpd/trunk/docs/log-message-tags/next-number
>     httpd/httpd/trunk/modules/lua/lua_apr.c
>     httpd/httpd/trunk/modules/lua/lua_apr.h
>     httpd/httpd/trunk/modules/lua/lua_request.c
>     httpd/httpd/trunk/modules/lua/lua_request.h
> 

> Modified: httpd/httpd/trunk/modules/lua/lua_apr.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/lua/lua_apr.c?rev=1582858&r1=1582857&r2=1582858&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/lua/lua_apr.c (original)
> +++ httpd/httpd/trunk/modules/lua/lua_apr.c Fri Mar 28 18:38:41 2014

> @@ -36,26 +37,35 @@ void ap_lua_push_apr_table(lua_State *L,
>  
>  static int lua_table_set(lua_State *L)
>  {
> -    apr_table_t    *t = ap_lua_check_apr_table(L, 1);
> +    req_table_t    *t = ap_lua_check_apr_table(L, 1);
>      const char     *key = luaL_checkstring(L, 2);
>      const char     *val = luaL_checkstring(L, 3);
> -    
> -    /* Prevent response/header splitting by not allowing newlines in tables.
> -     * At this stage, we don't have the request_rec handy, and we can't change
> -     * a const char*, so we'll redirect to a standard error value instead.
> -     */
> -    if (ap_strchr_c(val, '\n')) {
> -        val = "[ERROR: Value contains newline, ignored.]";
> +    /* Unless it's the 'notes' table, check for newline chars */
> +    if (strcmp(t->n, "notes") && ap_strchr_c(val, '\n')) {
> +        char *badchar;
> +        char *replacement = apr_pstrdup(t->r->pool, val);
> +        badchar = replacement;
> +        while ( (badchar = ap_strchr(badchar, '\n')) ) {
> +            *badchar = ' ';
> +        }
> +        if (t->r != NULL) {

Why this check if we already use t->r->pool above :-)?

> +            ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, t->r, 
> +                APLOGNO(02614) "mod_lua: Value for '%s' in table '%s' contains newline!",
> +                  key, t->n);
> +        }
> +        apr_table_set(t->t, key, replacement);
> +    }
> +    else {
> +        apr_table_set(t->t, key, val);
>      }
> -    apr_table_set(t, key, val);
>      return 0;
>  }

Regards

Rüdiger