SQL escaping

[Phil Mayers]

On 09/20/2012 05:45 PM, Arran Cudbard-Bell wrote:
>
> On 20 Sep 2012, at 17:30, Phil Mayers <p.mayers at imperial.ac.uk>
> wrote:
>
>> On 20/09/12 07:21, Alan DeKok wrote:
>>
>>> add REQUEST and context (void*) to the RADIUS_ESCAPE_FUNC.
>>>
>>> Add it to the prototypes, to all modules (as UNUSED), and have
>>> xlat.c store the context, and pass it and REQUEST to the calling
>>> function
>>>
>>> then, add the proper pass of the context in LDAP, SQL, etc.
>>> individually.  Have it pass the right context, and then use it in
>>> the escaping function.
>>
>> Actually I've started to have a doubt about this having spent some
>> time looking at it.
>>
>> The xlat stuff is a bit more complex than I first appreciated.
>> There are quite a few places where the escape func is just ignored
>> when passed into *_xlat handlers
>
> Hmm ok, yes we should probably pass it into the radius_xlat call in
> those functions. I'm not sure why it's not currently... I was just
> following what  was there already.

Actually, further thought suggests passing the escape func around is 
probably not necessary *at all*. It could be removed from the signature 
of valuepair2str and decode_attribute along with regsitered *_xlat funcs.

The only place it needs to be is in radius_xlat, right after 
decode_attribute:

decode_attrbute(tmp_buf, ...)
func(tmp_buf, ..., ctx);

i.e. do the escaping after all the xlat/expansion has been done.

All calls to radius_xlat from *within* that code path can pass NULL; the 
final result is all that needs to be escaped.

Unless I'm missing something, this should be the "right" thing to do. It 
has the virtue of avoiding passing func and ctx around when it's only 
used in a few places. It also eliminates the need for the dummy xlat_copy.

Thoughts?