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

Accept iso timestamp with a space instead of t #3893

Merged

Conversation

bazsi
Copy link
Collaborator

@bazsi bazsi commented Jan 28, 2022

This patch implements accepting ISO8601 timestamps with a space instead of a 'T' in the middle, as this format happens a lot in the wild.

For instance ZScaler produces this syslog header:

<134>2022-01-27 10:32:35      reason=Allowed... # message continues

The patch causes syslog-ng to accept this timestamp both in RFC5424 and RFC3164 mode. I've decided this way as even if we permitted the incorrect formatting only in RFC3164 mode, I expect the same to happen in 5424 implementations too. And I know it's against the spec, but this seems to be an uphill battle.

@bazsi bazsi force-pushed the accept-iso-timestamp-with-a-space-instead-of-T branch from be8e5a5 to 9fea7c2 Compare January 28, 2022 16:49
@kira-syslogng
Copy link
Contributor

Build SUCCESS

This patch implements accepting ISO8601 timestamps with a space instead of
a 'T' in the middle, as this format happens a lot in the wild.

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
@bazsi bazsi force-pushed the accept-iso-timestamp-with-a-space-instead-of-T branch from 9fea7c2 to 3fcdfad Compare January 28, 2022 17:19
@MrAnno
Copy link
Collaborator

MrAnno commented Jan 28, 2022

LGTM.

Please add a news entry.

@bazsi
Copy link
Collaborator Author

bazsi commented Jan 28, 2022

[----] tests/unit/test_msgparse.c:163: Assertion failed: Unexpected timestamp, value=1162083600, expected=1162080000, msg=<7>2006-10-29 02:00:00

Probably MacOS has a different timezone and that test does not mock the timezone setting. I think the other testcase covers the parsing piece, so I've removed this failing testcase from test_msgparse. We still have one but that contains timezone
information and thus we validate that timestamps without 'T" are parsed end-to-end via the normal parser.

That test_msgparse unit test is a dire need of a refactor...

Please add a news entry.

I was about to. :)

@MrAnno
Copy link
Collaborator

MrAnno commented Jan 28, 2022

That test_msgparse unit test is a dire need of a refactor...

Yeah, that one is terrible. Sorry about that, I was lazy when I ported the test to Criterion :(

Signed-off-by: Balazs Scheidler <bazsi77@gmail.com>
@kira-syslogng
Copy link
Contributor

Build SUCCESS

@MrAnno MrAnno added this to the syslog-ng-3.36 milestone Jan 28, 2022
@alltilla alltilla merged commit d023177 into syslog-ng:master Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants