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

Replace MySQL user_name and computer_name column '=5C' with '\' #3510

Merged
merged 3 commits into from
Aug 30, 2018

Conversation

satkunas
Copy link
Contributor

Description

Replaces '=5C' with '' in reports for top authentication successes and failures user_name and computer_name.

Impacts

none

Issue

fixes #3508

Delete branch after merge

YES

Bug Fixes

@satkunas
Copy link
Contributor Author

@julsemaan not tested.

@jrouzierinverse
Copy link
Member

jrouzierinverse commented Aug 27, 2018

freeradius encodes the sql changing special characters to the following format '=6F' where 6F is the hexadecimal representation of the character.

So either this can be done in perl or MySQL

$value =~ s/=([a-fA-F0-9]{2})/chr(hex($1))/ge;
DELIMITER $$

DROP FUNCTION IF EXISTS FREERADIUS_DECODE $$

CREATE FUNCTION FREERADIUS_DECODE (str text) 
RETURNS text
DETERMINISTIC
BEGIN 
    DECLARE result text;
    DECLARE ind INT DEFAULT 0;

    SET result = str;
    WHILE ind <= 255 DO
       SET result = REPLACE(result, CONCAT('=', LPAD(LOWER(HEX(ind)), 2, 0)), CHAR(ind));
       SET result = REPLACE(result, CONCAT('=', LPAD(HEX(ind), 2, 0)), CHAR(ind));
       SET ind = ind + 1;
    END WHILE;

    RETURN result;
END$$

DELIMITER ;
select FREERADIUS_DECODE('=5C') as decoded_value;
+---------------+
| decoded_value |
+---------------+
| \             |
+---------------+

@julsemaan
Copy link
Collaborator

I like the second option @jrouzierinverse has suggested since this allows us to integrate it easily in the custom reports

@satkunas
Copy link
Contributor Author

@julsemaan reworked

Note that db/pf-schema-X.Y.Z.sql does not currently contain any of our stored procedures. This is something we need to discuss before closing this PR.

@julsemaan
Copy link
Collaborator

PR looks good to me

@satkunas were you able to test te new code or should this be tested prior to merge?

@julsemaan
Copy link
Collaborator

We do put the stored procs in the schema file:
https://github.com/inverse-inc/packetfence/blob/devel/db/pf-schema-X.Y.Z.sql#L647

@satkunas
Copy link
Contributor Author

@julsemaan added to schema in 988d1b8

@satkunas
Copy link
Contributor Author

@satkunas were you able to test te new code or should this be tested prior to merge?

@julsemaan not tested fully.

@julsemaan julsemaan merged commit cf17793 into devel Aug 30, 2018
@julsemaan
Copy link
Collaborator

Won't be cherry-picking because this contains schema changes

@julsemaan julsemaan deleted the fix/escape-5c-names branch August 30, 2018 12:25
julsemaan added a commit that referenced this pull request Aug 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backslash in usernames in Reports section is shown as "=5C"
3 participants