Add test to prevent regression regarding excessive logging #2131
Conversation
| end | ||
|
|
||
| it "logs info about the detected spoof" do | ||
| Paperclip.expects(:log).with('Content Type Spoof: Filename empty.html (image/jpg from Headers, ["text/html"] from Extension), content type discovered from file command: text/html. See documentation to allow this combination.') |
There was a problem hiding this comment.
I think capture will read better. Also, by matching what we want to match we'll make the spec less brittle to log string changes. Do you think this would improve the spec?
output = capture(:stdout) do
run_arbitrary_code
end
assert output =~ /pattern/Thank you!
There was a problem hiding this comment.
I agree that capturing std_out is better, testing behaviour instead of implementation. The brittleness might not be so bad in this case. A pattern could match against those very long lines of mime-types data which is the regression I am trying to prevent with this test. I'll try and figure something out in the weekend.
|
@bdewater do you have availability to rebase and get this PR ready? Thank you! :) |
…our to prevent regressions. Excessive logging can fill up disk space and become a denial of service attack, see https://cwe.mitre.org/data/definitions/779.html
|
@tute I've rebased my original PR. I've tried your suggestion and the following to capture log output: it "logs info about the detected spoof" do
aggregate_failures do
expect { spoofed? }.to output(/image\/jpg from Headers/).to_stdout
expect { spoofed? }.to output(/\["text\/html"\] from Extension/).to_stdout
expect { spoofed? }.to output(/file command: text\/html/).to_stdout
end
endbut the output is always an empty string. I don't have time to work on this for at least the next two weeks unfortunately. |
|
Thank you! :) |
This is the test from #2126 for the master branch.