Fix off-by one for octet-counting code in in_sysylog.rb #2771
Conversation
| break | ||
| end | ||
| pos = idx + msg.size |
| break | ||
| end | ||
| pos = idx + msg.size | ||
| # syslog might append a \n as an extra frame delimiter | ||
| msg.chomp! |
There was a problem hiding this comment.
How can we handle if the user adds \n at the end of msg intentionally?
In the situation, it shouldn't trim \n from msg.
There was a problem hiding this comment.
I was limiting this change to preventing the removal of characters other than \n. I could add that as a configuration option to control if the chomp should be called.
There was a problem hiding this comment.
I looked into this more and currently in the default parser configuration chomping at the in_syslog level has no impact because parse_syslog strips off trailing \n.
However, that is only part of the story. Until #2767 lands if RFC5424 is selected as the format for parser_syslog messages it will fail to parse unless in_syslog does a chomp on the incoming messages.
There also is a second issue, without this change octet counted transport always strips off the last character of a message (incorrectly presumed to be a delimiter) while the delimited transport does not strip off the delimiter! So if chomp is off by default then it will be a behavior change for octect counted transport. If chomp is on by default then it will be a behavior change for delimited transport. In light of that I would personally would keep chomp off by default as it because it will impact fewer users and this change is supposed to address that fact that octect counted transport is currently incorrect.
| d = create_driver([CONFIG, "<transport tcp> \n</transport>", 'frame_type octet_count'].join("\n")) | ||
| tests = create_test_case | ||
|
|
||
| d.run(expect_emits: 2) do | ||
| d.run(expect_emits: 1) do |
There was a problem hiding this comment.
Is there a reason you changed this line? I think it shouldn't be changed since two messages are sent.
|
Sorry for the delay! I left some comments. BTW, Could you follow DCO? https://github.com/fluent/fluentd/pull/2771/checks?check_run_id=384067806 |
|
@onet-git friendly ping |
|
done in #2942. thanks! |
Which issue(s) this PR fixes:
Fixes #
What this PR does / why we need it:
There is an off-by-one counting issue in octet-transported syslog messages. In most case syslog transport clients will append a
\nto the end of each line, hiding this bug. The bug gets triggered whenever a syslog client doesn't append a\n.Docs Changes:
None
Release Note:
Support octet-counted syslog transport when the last character in each message is not a \n.