xlat expansion of absent VPs

[Arran Cudbard-Bell]

On 17 Jun 2013, at 16:27, Stelian Ionescu <sionescu at cddr.org> wrote:

> On Mon, 2013-06-17 at 10:53 -0400, Alan DeKok wrote:
>> Stelian Ionescu wrote:
>>> In processing a request that does not contain Called-Station-Id, I
>>> noticed that absent VPs now get expanded to an underscore instead of the
>>> usual empty string:
>> 
>>  That's how it was in 2.x.  Or at least how the code was *supposed* to
>> work.
> 
> That's definitely not how it worked in 2.x, and as for what was supposed
> to happen, it's too late to argue about it now.
> 
> 
>>  The idea is that a place-holder is better than silently expanding to
>> an empty string.  An empty string means that data which is supposed to
>> be 4 columns (for example) is now suddenly 3.  That's bad.

No, because all values should be quoted (even integers and timestamps)
and a "" or a '' is perfectly acceptable in the SQL world.

If users have an issue with it they can do %{%{FOO}:-_}, inserting
_ by default is magical, inconsistent and unexpected.

Besides, for SQL the proper behaviour would be %{%{FOO}:-NULL}, NULL
being a value which is not known, anything else is arguably wrong.

> That's not true, at least not in a SQL statement.
> SELECT %{FOO}, %{BAR}, %{BAZ} that may expand to «SELECT a, , c» will
> raise a syntax error. I can't think of a case where the deletion of a
> token will result in a valid SQL expression.

I have to agree. If an attribute doesn't exist then it should expand to "",
that's the behaviour i'm used to too.

Previously the xlat code wrongly treated zero length expansions as errors,
this might of been the original reason for returning a one char expansion.

I think some signatures/functions still need updating (namely those of actual
xlat functions), but other xlat operations should now return -1 to indicate an error. 

I certainly updated all calls to radius_xlat and radius_axlat to check for < 0
return codes.

Arran Cudbard-Bell <a.cudbardb at freeradius.org>
FreeRADIUS Development Team