-
Notifications
You must be signed in to change notification settings - Fork 488
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
ensure syslog parser gets an EOF-terminated reader on udp receive #5297
Conversation
eb1021d
to
92a23d5
Compare
cc @erikbaranowski do I correctly remember you'd investigated this issue in the past? Could you take a look here? |
This looks good. @joshuapare can we add a test to syslogtarget_test.go to demonstrate the scenario that is now working with this fix? |
@joshuapare Do you still have time to work on this? It looks like all we need is a test for the fix, and then this is good to be merged :)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked through the existing code, and I think there's enough tests to prove that this doesn't break anything, even if we don't have a test for testing this specific case, I think we can just merge this for now. If it breaks again we'll want a regression test for sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seconded @rfratto. let's get this fix out and give it a try.
PR Description
Simple fix to convert the incoming datagram into a bytes reader that the ParseStream can handle. currently it freezes up because the datagrams are sent as is without separation so the reader routine blocks until an interrupt.
Which issue(s) this PR fixes
Fixes #5197
PR Checklist