From 6e8d43a552d6687de52282dd3152e2811cbb3b4d Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Tue, 24 Sep 2024 11:03:20 -0400 Subject: [PATCH 1/2] Avoid duplicating warnings when reading table Invalid lines in a file was the one case that would not suppress future warnings. Just make it suppress warnings too, but clear that suppression if there is a field in between that doesn't error. Fixes #3692 --- src/input/readers/ascii/Ascii.cc | 11 ++-- .../.stderrwithoutfirstline | 5 ++ .../input/invalid-lines-duplicate.zeek | 50 +++++++++++++++++++ 3 files changed, 61 insertions(+), 5 deletions(-) create mode 100644 testing/btest/Baseline/scripts.base.frameworks.input.invalid-lines-duplicate/.stderrwithoutfirstline create mode 100644 testing/btest/scripts/base/frameworks/input/invalid-lines-duplicate.zeek diff --git a/src/input/readers/ascii/Ascii.cc b/src/input/readers/ascii/Ascii.cc index 365336f93d..6d862fd1ca 100644 --- a/src/input/readers/ascii/Ascii.cc +++ b/src/input/readers/ascii/Ascii.cc @@ -6,7 +6,6 @@ #include #include #include -#include #include "zeek/input/readers/ascii/ascii.bif.h" #include "zeek/threading/SerialTypes.h" @@ -345,7 +344,8 @@ bool Ascii::DoUpdate() { FailWarn(fail_on_invalid_lines, Fmt("Not enough fields in line '%s' of %s. Found " "%d fields, want positions %d and %d", - line.c_str(), fname.c_str(), pos, fit.position, fit.secondary_position)); + line.c_str(), fname.c_str(), pos, fit.position, fit.secondary_position), + ! fail_on_invalid_lines); if ( fail_on_invalid_lines ) { for ( int i = 0; i < fpos; i++ ) @@ -374,8 +374,6 @@ bool Ascii::DoUpdate() { if ( fit.secondary_position != -1 ) { // we have a port definition :) assert(val->type == TYPE_PORT); - // Error(Fmt("Got type %d != PORT with secondary position!", val->type)); - val->val.port_val.proto = formatter->ParseProto(stringfields[fit.secondary_position]); } @@ -395,8 +393,10 @@ bool Ascii::DoUpdate() { delete[] fields; continue; } + // If there's no error, then it makes sense to report the next error. + else + StopWarningSuppression(); - // printf("fpos: %d, second.num_fields: %d\n", fpos, (*it).second.num_fields); assert(fpos == NumFields()); if ( Info().mode == MODE_STREAM ) @@ -408,6 +408,7 @@ bool Ascii::DoUpdate() { if ( Info().mode != MODE_STREAM ) EndCurrentSend(); + StopWarningSuppression(); return true; } diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.invalid-lines-duplicate/.stderrwithoutfirstline b/testing/btest/Baseline/scripts.base.frameworks.input.invalid-lines-duplicate/.stderrwithoutfirstline new file mode 100644 index 0000000000..1b84bb3c2f --- /dev/null +++ b/testing/btest/Baseline/scripts.base.frameworks.input.invalid-lines-duplicate/.stderrwithoutfirstline @@ -0,0 +1,5 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +warning: ..<...>/Input::READER_ASCII: ../input.log, line 3: Not enough fields in line 'hello' of ../input.log. Found 0 fields, want positions 1 and -1 +warning: ..<...>/Input::READER_ASCII: ../input.log, line 10: Not enough fields in line 'hello' of ../input.log. Found 0 fields, want positions 1 and -1 +received termination signal +>>> diff --git a/testing/btest/scripts/base/frameworks/input/invalid-lines-duplicate.zeek b/testing/btest/scripts/base/frameworks/input/invalid-lines-duplicate.zeek new file mode 100644 index 0000000000..fd54f4c2e6 --- /dev/null +++ b/testing/btest/scripts/base/frameworks/input/invalid-lines-duplicate.zeek @@ -0,0 +1,50 @@ +# @TEST-EXEC: btest-bg-run zeek zeek -b %INPUT +# @TEST-EXEC: btest-bg-wait 10 +# @TEST-EXEC: sed 1d .stderr > .stderrwithoutfirstline +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderrwithoutfirstline + +redef exit_only_after_terminate = T; +redef InputAscii::fail_on_invalid_lines = F; + +@TEST-START-FILE input.log +#fields a b c +#types string bool bool +hello +hello +hello +hello +hello +hello +"hi" T F +hello +hello +hello +hello +hello +hello +hello +@TEST-END-FILE + +type Key: record { + a: string; +}; + +type Val: record { + b: bool &log; + c: bool &log; +}; + +global test_table: table[string] of Val = table(); + +event zeek_init() { + Input::add_table([ + $source="../input.log", $name="test_table", + $idx=Key, $val=Val, $destination=test_table, + $mode=Input::REREAD + ]); +} + + +event Input::end_of_data(name: string, source:string) { + terminate(); +} From ecabf882acaf1221cc5b2bcf7966f904dd1b388a Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Fri, 27 Sep 2024 13:20:17 -0400 Subject: [PATCH 2/2] Report suppressed warnings count This also triggers if there is one warning, which seems a little weird, but it seems mostly reasonable. --- src/input/ReaderBackend.cc | 16 +++++++++++----- src/input/ReaderBackend.h | 3 ++- src/input/readers/ascii/Ascii.cc | 4 ++-- .../.stderrwithoutfirstline | 2 ++ 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/input/ReaderBackend.cc b/src/input/ReaderBackend.cc index 15506fe11f..140e85c55d 100644 --- a/src/input/ReaderBackend.cc +++ b/src/input/ReaderBackend.cc @@ -250,22 +250,28 @@ void ReaderBackend::Info(const char* msg) { MsgThread::Info(msg); } +void ReaderBackend::StopWarningSuppression() { + suppress_warnings = false; + if ( warnings_suppressed > 0 ) + Warning(Fmt("Suppressed %zu warning(s)", warnings_suppressed)); + warnings_suppressed = 0; +} + void ReaderBackend::FailWarn(bool is_error, const char* msg, bool suppress_future) { if ( is_error ) Error(msg); else { - // suppress error message when we are already in error mode. - // There is no reason to repeat it every second. - if ( ! suppress_warnings ) - Warning(msg); + Warning(msg); if ( suppress_future ) suppress_warnings = true; } } void ReaderBackend::Warning(const char* msg) { - if ( suppress_warnings ) + if ( suppress_warnings ) { + warnings_suppressed++; return; + } SendOut(new ReaderErrorMessage(frontend, ReaderErrorMessage::WARNING, msg)); MsgThread::Warning(msg); diff --git a/src/input/ReaderBackend.h b/src/input/ReaderBackend.h index 2a2b5932a3..afc5f7de5c 100644 --- a/src/input/ReaderBackend.h +++ b/src/input/ReaderBackend.h @@ -203,7 +203,7 @@ public: */ void FailWarn(bool is_error, const char* msg, bool suppress_future = false); - inline void StopWarningSuppression() { suppress_warnings = false; }; + void StopWarningSuppression(); // Overridden from MsgThread. bool OnHeartbeat(double network_time, double current_time) override; @@ -365,6 +365,7 @@ private: // this is an internal indicator in case the read is currently in a failed state // it's used to suppress duplicate error messages. bool suppress_warnings = false; + size_t warnings_suppressed = 0; }; } // namespace zeek::input diff --git a/src/input/readers/ascii/Ascii.cc b/src/input/readers/ascii/Ascii.cc index 6d862fd1ca..0601af5b4a 100644 --- a/src/input/readers/ascii/Ascii.cc +++ b/src/input/readers/ascii/Ascii.cc @@ -139,14 +139,14 @@ bool Ascii::OpenFile() { if ( ! file.is_open() ) { FailWarn(fail_on_file_problem, Fmt("Init: cannot open %s", fname.c_str()), true); - return ! fail_on_file_problem; + return false; } if ( ReadHeader(false) == false ) { FailWarn(fail_on_file_problem, Fmt("Init: cannot open %s; problem reading file header", fname.c_str()), true); file.close(); - return ! fail_on_file_problem; + return false; } if ( ! read_location ) { diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.invalid-lines-duplicate/.stderrwithoutfirstline b/testing/btest/Baseline/scripts.base.frameworks.input.invalid-lines-duplicate/.stderrwithoutfirstline index 1b84bb3c2f..559a7eac60 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.invalid-lines-duplicate/.stderrwithoutfirstline +++ b/testing/btest/Baseline/scripts.base.frameworks.input.invalid-lines-duplicate/.stderrwithoutfirstline @@ -1,5 +1,7 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. warning: ..<...>/Input::READER_ASCII: ../input.log, line 3: Not enough fields in line 'hello' of ../input.log. Found 0 fields, want positions 1 and -1 +warning: ..<...>/Input::READER_ASCII: ../input.log, line 9: Suppressed 5 warning(s) warning: ..<...>/Input::READER_ASCII: ../input.log, line 10: Not enough fields in line 'hello' of ../input.log. Found 0 fields, want positions 1 and -1 +warning: ..<...>/Input::READER_ASCII: ../input.log, line 16: Suppressed 6 warning(s) received termination signal >>>