[Bug 90] New: csv-parser's flags(drop-invalid) doesn' t work with escape-{backslash, double-char}
https://bugzilla.balabit.com/show_bug.cgi?id=90 Summary: csv-parser's flags(drop-invalid) doesn't work with escape-{backslash,double-char} Product: syslog-ng Version: 3.0.x Platform: PC OS/Version: Linux Status: NEW Severity: normal Priority: unspecified Component: syslog-ng AssignedTo: bazsi@balabit.hu ReportedBy: andrewn@locus.net Type of the Report: bug Estimated Hours: 0.0 Created an attachment (id=19) --> (https://bugzilla.balabit.com/attachment.cgi?id=19) Possible fix The example parser p_apache in syslog-ng-v3.0-guide-admin-en.pdf on numbered page 63 does not work when drop-invalid is added. The 2 main loops of log_csv_parser_process seem to treat cur_column and src differently upon termination. The patch attached attempts to fix that. It also checks to see that the whole string was parsed. I don't know if this is the correct fix, but so far it seems to Work For Me (tm). -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=90 Balazs Scheidler <bazsi@balabit.hu> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |NEEDINFO --- Comment #1 from Balazs Scheidler <bazsi@balabit.hu> 2010-06-29 17:27:49 --- why do you think it is handled differently? as I see there's only one branch based on LOG_CSV_PARSER_DROP_INVALID and it is taken, if there are still unprocessed columns. the definition of the "drop-invalid" flag from the documentation: "When the drop-invalid option is set, the parser does not process messages that have less columns than defined in the parser. Using this option practically turns the parser into a special filter, that matches messages that have the predifined number of columns (using the specified delimiters)." This means that if there are less columns in the input, parsing will fail, but otherwise it'll be successful. Can you please show a concrete example on what input you give to the parser and what you'd expect it to do? Thanks. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=90 --- Comment #2 from andrewn@locus.net 2010-06-29 20:08:38 --- Created an attachment (id=20) --> (https://bugzilla.balabit.com/attachment.cgi?id=20) Shell script that demonstrates Bug #90 This shell script demonstrates bug #90. It creates a temporary config file, executes the specified syslog-ng binary, and sends a single log entry. It assumes that /tmp is writable, ./syslog-ng-3.0.7-unpatched is the syslog-ng binary, and /bin/logger exists from util-linux. On syslog-ng 3.0.7 the parser in the first test works (flags(escape-none,drop-invalid)) while the in the second test it falls through (flags(escape-backslash,drop-invalid)). I would expect both sets of flags to behave identically on input which doesn't contain backslashes. The current code returns FALSE if (!cur_column && (self->flags & LOG_CSV_PARSER_DROP_INVALID)). Since cur_column is set to the next element after assigning a match cur_column should be NULL if all columns have been assigned. This is the case for escape-backslash and escape-double-char, but at line 251 escape-none breaks before setting cur_column if it hits the end of the string. The test should be (cur_column && (self->flags & LOG_CSV_PARSER_DROP_INVALID)). I added in the *src test because it makes sense based on the name of the flag. Based on the documentation you quoted it would have to be a separate flag. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=90 Balazs Scheidler <bazsi@balabit.hu> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEEDINFO |REOPENED --- Comment #3 from Balazs Scheidler <bazsi@balabit.hu> 2010-07-09 09:31:03 --- info provided, reopening. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=90 --- Comment #4 from Balazs Scheidler <bazsi@balabit.hu> 2010-07-09 09:32:57 --- the script is indeed nice, it must have been a lot of work. thanks for that, however it might have taken less time for you just to copy-paste information here :) -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=90 Balazs Scheidler <bazsi@balabit.hu> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution| |FIXED Status|REOPENED |RESOLVED --- Comment #5 from Balazs Scheidler <bazsi@balabit.hu> 2010-07-09 10:58:11 --- I've taken your patch, adapted it to the latest drop-invalid+greedy patch that was already on mainline and commited it to OSE 3.0 I've also added ac couple of testcases commit 6d1ce4e5bdb517941235f059b556fb6dc9d45363 Author: Balazs Scheidler <bazsi@balabit.hu> Date: Fri Jul 9 10:25:31 2010 +0200 csv-parser: fixed drop-invalid handling drop-invalid was not handled properly and caused valid messages to be dropped. The parser drops messages if: * not all columns are processed * there are leftover characters at the end of the parsing Greedy columns can be empty though. Testcases were extended to hopefully cover all cases now. Reported-By: andrewn at locus.net -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
participants (1)
-
bugzilla＠bugzilla.balabit.com