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

Prevent garbage from coming out of the LTSV parser #2748

Merged
merged 1 commit into from Dec 25, 2019

Conversation

threewordphrase
Copy link
Contributor

@threewordphrase threewordphrase commented Dec 25, 2019

Which issue(s) this PR fixes:
I have not opened one yet.

What this PR does / why we need it:
The LTSV parser does not check whether the delimiter is in the pair before it tries to parse it. With delimiter_pattern /\s+/ and label_delimiter =,If I give it the value:

"Sylvanus_007               : ok=45   changed=6"

it parses it like this:

{ "Sylvanus_007":null,":":null,"ok":"45","changed":"6" }

Both Sylvanus_007: null and ":": null were never specified by key=value. This PR prevents this drastically wrong behavior and parses it like this:

{ "ok":"45","changed":"6" }

Docs Changes:
None

Release Note:
None

Signed-off-by: Aaron Smith <projects@aaronsmith.co>
@threewordphrase
Copy link
Contributor Author

Not sure what the CI failure is about - I ran that test module per the README, but locally for me bundle exec rake test TEST=test/plugin/test_in_tail.rb passes fine. I cloned the repo fresh just to work on this PR. Ideas?

@repeatedly repeatedly merged commit 03750d6 into fluent:master Dec 25, 2019
@repeatedly
Copy link
Member

Thanks for the patch!

Not sure what the CI failure is about

This random failure is caused by travis-ci performance/machine. No problem.

@threewordphrase
Copy link
Contributor Author

Oh wow - I was not expecting this to be merged to master within 24 hours of starting on it. Thank you, fluentd maintainers! When will this version be available on rubygems?

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.

None yet

3 participants