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

in_exec: Ensure to sleep for prevending fork bomb #1348

Merged
merged 3 commits into from
Dec 16, 2016

Conversation

autopp
Copy link

@autopp autopp commented Dec 5, 2016

Since 0.12.18, in_exec dose not sleep run_interval when command or output parsing failed.
This causes the fork bomb.
This is prevended by moving sleep @run_interval to ensure clause.

(Sorry, the added test case may not be good...)

autopp added 2 commits December 2, 2016 19:42
For testing, add new mode to exec_script.rb.
In this mode, exec_script.rb output a charactor to stderr and then
output a invalid json string to stdout.

The test case checks that the total length of command's stderr (this
means the count of command execution) is appropriate.
@tagomoris tagomoris added the v0.12 label Dec 6, 2016
@tagomoris tagomoris added the bug Something isn't working label Dec 6, 2016
@tagomoris
Copy link
Member

ping @repeatedly ?

@repeatedly
Copy link
Member

I'm waiting patch update.

@repeatedly
Copy link
Member

Ahh, wait. My comment isn't appear...

end

assert_equal true, d.emits.empty?
assert_equal true, File.read(@count_file).length.between?(1, 4)
Copy link
Member

Choose a reason for hiding this comment

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

For counting command execution, checking log lines seems enough like below:

assert_equal 2, logs.count { |line| line =~ /got invalid event and drop it/ }

@autopp
Copy link
Author

autopp commented Dec 15, 2016

@repeatedly Thanks for your review and advice! I updated the test code.
Is it necessary to rebase commits?

@repeatedly repeatedly merged commit 93b8edb into fluent:v0.12 Dec 16, 2016
@repeatedly
Copy link
Member

Is it necessary to rebase commits?

Not needed.

Thanks!

@autopp
Copy link
Author

autopp commented Dec 16, 2016

👍
Thanks you!

@autopp autopp deleted the ensure_sleep_in_exec branch December 16, 2016 07:54
@autopp autopp restored the ensure_sleep_in_exec branch December 18, 2016 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v0.12
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants