Merge remote-tracking branch 'origin/topic/etyp/remove-duplicate-table-warns'

* origin/topic/etyp/remove-duplicate-table-warns:
  Report suppressed warnings count
  Avoid duplicating warnings when reading table

CHANGES

@@ -1,3 +1,18 @@
+7.1.0-dev.375 | 2024-10-01 11:40:16 +0200
+
+  * Report suppressed warnings count (Evan Typanski, Corelight)
+
+    This also triggers if there is one warning, which seems a little weird,
+    but it seems mostly reasonable.
+
+  * GH-3692: Avoid duplicating warnings when reading table (Evan Typanski, Corelight)
+
+    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
+
 7.1.0-dev.371 | 2024-09-27 15:17:04 -0700
 
   * A round of ZAM/gen-C++-related updates and fixes (Vern Paxson, Corelight)

NEWS

@@ -90,6 +90,9 @@ Changed Functionality
   and the password is blanked with "<hidden>". Previously, the argument for the PASS
   command would be logged in clear.
 
+* The ASCII input reader now suppresses warnings for consecutive invalid lines,
+  producing a summary of total suppressions once a valid line is encountered.
+
 Removed Functionality
 ---------------------
 

VERSION

@@ -1 +1 @@
-7.1.0-dev.371
+7.1.0-dev.375

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);

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

src/input/readers/ascii/Ascii.cc

@@ -6,7 +6,6 @@
 #include <sys/types.h>
 #include <unistd.h>
 #include <cerrno>
-#include <sstream>
 
 #include "zeek/input/readers/ascii/ascii.bif.h"
 #include "zeek/threading/SerialTypes.h"
@@ -140,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 ) {
@@ -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;
 }
 

testing/btest/Baseline/scripts.base.frameworks.input.invalid-lines-duplicate/.stderrwithoutfirstline

@@ -0,0 +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
+>>>

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();
+}