Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error in (My)SQL Syntax for expire_on_login counter #924

Closed
grafgraffy opened this issue Mar 12, 2015 · 0 comments
Closed

Error in (My)SQL Syntax for expire_on_login counter #924

grafgraffy opened this issue Mar 12, 2015 · 0 comments
Labels
defect category: a defect or misbehaviour

Comments

@grafgraffy
Copy link

There is a bug (at least I think so) with the SQL Syntax for the expire_on_login counter. Currently (FR 3.0.7 (raddb/mods-config/sql/counter/mysql/expire_on_login_counter.conf) and 2.2.6 (raddb/modules/sqlcounter_expire_on_login) ) the syntax is:

query = "SELECT TIMESTAMPDIFF(SECOND, acctstarttime, NOW()) \
    FROM radacct \
    WHERE UserName='%{${key}}' \
    ORDER BY acctstarttime \
    LIMIT 1;" 

The problem is when the initial/first "lookup" in the database is done during authorization there is no entry in radacct yet (accounting after authorization) so the result of the query will fail with "no integer found in string ".... because of MySQL giving NULL as result.

I suggest to change the query to:

query = "SELECT IFNULL( MAX(TIME_TO_SEC(TIMEDIFF(NOW(), acctstarttime))),0) \
        FROM radacct \
        WHERE UserName='%{${key}}' \
        ORDER BY acctstarttime \
        LIMIT 1;"

so that if no entry is present (MySQL will reply with NULL Value) the query will then return 0 which is a number and can be used within the radius response instead of NULL.

The additional MAX Function around it makes sure that timechanges at the server will not result in negative results.

I'm not sure if it affects MySQL only, the important thing is that an integer value is responded by the query.

@arr2036 arr2036 added defect category: a defect or misbehaviour v2.x.x labels Mar 31, 2015
alandekok added a commit that referenced this issue Apr 10, 2015
We still need to audit / do something similar for the other DBs
alandekok added a commit that referenced this issue Apr 10, 2015
We still need to audit / do something similar for the other DBs
jpereira pushed a commit to jpereira/freeradius-server that referenced this issue Apr 30, 2015
We still need to audit / do something similar for the other DBs
jpereira pushed a commit to jpereira/freeradius-server that referenced this issue May 5, 2015
We still need to audit / do something similar for the other DBs
@arr2036 arr2036 closed this as completed Jun 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect category: a defect or misbehaviour
Projects
None yet
Development

No branches or pull requests

2 participants