rlm_rediswho with spaces

[Brian Candler]

On Thu, Oct 25, 2012 at 05:41:01PM +0100, Phil Mayers wrote:
> You can probably use the escape context stuff for this.

Ah, I had missed redis_escape_function, and I'm just trying to get my head
around this.

Given a module like

redis mydb {
  ...
}

and attributes

     Tmp-String-1 := "foo bar"
     Tmp-String-2 := "qux baz"

then I believe what happens in

     Tmp-String-3 := "%{mydb:SET %{Tmp-String-1} %{Tmp-String-2}}"

is that the expansions of %{Tmp-String-1} and %{Tmp-String-2} are passed via
redis_escape_function?  That means they would expand to

     %{mydb:SET foo=20bar qux=20baz}

I haven't tested rlm_redis, but I don't see this happening when I use
rlm_rediswho. e.g.

       start-insert = "LPUSH %{User-Name} %{Event-Timestamp},%{Acct-Status-Type},..."

just passes through bare spaces if the user-name contains spaces.

So that's the first problem to fix.  In rlm_rediswho.c I suspect this
line...

                if (!radius_xlat(query, sizeof (query), fmt, request, NULL)) {

...just needs the NULL changing to inst->redis_escape_func.

> Basically:
> 
>  1. Define a struct for holding the command and arguments
>  2. Modify the redis_escape function to save to the context:
> 
> int resdis_escape(REQUEST *in, char *out, size_t outlen,
>                   const char *in, void *arg) {
> 
>   struct redis_command *c = arg;
>   c->arg[c->argnum++] = strdup(in);
>   *out++ = '%';
>   *out++ = 's';
>   *out++ = '\0';
>   return 2;
> }

I think there may be a problem with that approach: given

       start-insert = "LPUSH %{User-Name} %{Event-Timestamp},%{Acct-Status-Type},..."

I don't want %{Event-Timestamp} and %{Acct-Status-Type} to be treated as
separate arguments.

   ("LPUSH %s %s,%s", "foo", "bar", "baz")    # wrong
   ("LUSH %s %s", "foo", "bar,baz")           # right

I think what's needed is an inverse of redis_escape when building the
command and args, which should be pretty straightforward.

I'll have a bash at coding that now.

Cheers,

Brian.