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

[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

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

[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

[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.