Detail file reader header detection sucks

[Arran Cudbard-Bell]

Alan DeKok wrote:
> Arran Cudbard-Bell wrote:
>   
>> src/modules/frs_detail/frs_detail.c
>>     
> ...
>   
>> This'd be fine, except the format of the detail file header is user
>> definable!
>>     
>
>   Sure... for use *other* than with the detail file reader.
>
>   
Yeah, but the configuration option still exists. I got caught out
changing the %t to a %T, as I was assuming the detail reader used the
header for Packet-Original-Timestamp (when in fact it's reformatting the
Unix timestamp included in the packet).

I need database formatted or unix timestamps for inserting command logs
into a database... I can convert c timestamps using unlang, but it's not
pretty.
>> Wouldn't it be safer to just assume that the first line that
>> doesn't start with a \t is the header?
>>     
>
>   Maybe.  Unless the detail file is corrupted somehow.
>
>   
I guess, but what do you want to do if that happens? Because at the
moment if the headers not detected correctly, it just skips on down and
uses the first attribute with a timestamp as its value, missing out half
the attributes in the process.

You then get a request coming in with no 'Acct-Status-Type', and if you
haven't got your configuration setup correctly, it'll just loop.

>> if (data->state == STATE_HEADER &&
>>     (buffer[0] != '\t')) {
>>     continue;
>> }
>>
>> It's not like the detail file listener actually does anything with the
>> data from the record header.
>>     
>
>   Well, yes.
>   
I guess this isn't that much of a problem, but I still think just
checking for a non-indented line gives more predictable behaviour, even
with corruption.

Arran

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 258 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freeradius.org/pipermail/freeradius-devel/attachments/20090809/825b0b77/attachment.pgp>