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

Fix line skipping issue in receive_lines method #4530

Merged
merged 12 commits into from
Jul 14, 2024
Merged

Conversation

yugeeklab
Copy link
Contributor

@yugeeklab yugeeklab commented Jun 17, 2024

Which issue(s) this PR fixes:
Fixes #4494
recoverd from #4491

What this PR does / why we need it:
Before this patch, long lines could cause breakdowns in fluentd, potentially posing a vulnerability. With this patch, max_line_size will be integrated into the FIFO, enabling the system to skip lines exceeding the maximum size before executing receive_lines.

Docs Changes:

Release Note:

been-zino and others added 11 commits June 7, 2024 20:49
Before this patch, long lines could cause breakdowns in fluentd, potentially posing a vulnerability. With this patch, max_line_size will be integrated into the FIFO, enabling the system to skip lines exceeding the maximum size before executing receive_lines.

Co-authored-by: yugeeklab <[email protected]>
Co-authored-by: moggaa <[email protected]>
Signed-off-by: been.zino <[email protected]>
Co-authored-by: Daijiro Fukuda <[email protected]>
Signed-off-by: been.zino <[email protected]>
Co-authored-by: Daijiro Fukuda <[email protected]>
Signed-off-by: been.zino <[email protected]>
Co-authored-by: Daijiro Fukuda <[email protected]>
Signed-off-by: been.zino <[email protected]>
Co-authored-by: Daijiro Fukuda <[email protected]>
Signed-off-by: been.zino <[email protected]>
Co-authored-by: Daijiro Fukuda <[email protected]>
Signed-off-by: been.zino <[email protected]>
Co-authored-by: Daijiro Fukuda <[email protected]>
Signed-off-by: been.zino <[email protected]>
Signed-off-by: Daijiro Fukuda <[email protected]>
@daipom
Copy link
Contributor

daipom commented Jun 17, 2024

@yugeeklab I have pushed the commits for #4491 (comment).

Copy link
Contributor

@daipom daipom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM.
I will wait a bit to merge for other maintainers.

Copy link
Member

@ashie ashie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically it look good to me, thanks for your contribution!
I've added some nitpick comments about logging.

In addition, it would be happy if we can simplify FIFO#read_lines to make easy to read although I have no idea about it yet.
What do you think about these my comments? @daipom

lib/fluent/plugin/in_tail.rb Outdated Show resolved Hide resolved
lib/fluent/plugin/in_tail.rb Show resolved Hide resolved
Signed-off-by: Daijiro Fukuda <[email protected]>
Co-authored-by: Takuro Ashie <[email protected]>
Copy link
Member

@ashie ashie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Thanks!

@daipom
Copy link
Contributor

daipom commented Jul 8, 2024

In addition, it would be happy if we can simplify FIFO#read_lines to make easy to read although I have no idea about it yet. What do you think about these my comments? @daipom

Thanks for your comments!
About debug messages, I have added a fix: 955cee5.

About the simplification, I have the same opinion. I'll think about it for a bit.
If I come up with a good idea, I will add a fix.
Thanks!

@ashie ashie merged commit 64595e2 into fluent:master Jul 14, 2024
16 checks passed
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.

in_tail plugin can cause breakdowns in fluentd
4 participants