From b0d812812f7b8539bf2066683f8c4b6d4d827e6e Mon Sep 17 00:00:00 2001 From: Seth Hall Date: Tue, 21 Feb 2017 15:45:26 -0500 Subject: [PATCH 1/6] In progress on ascii writer behavior change. --- src/input/readers/ascii/Ascii.cc | 83 ++++++++++++++++++++++++-------- src/input/readers/ascii/Ascii.h | 5 ++ 2 files changed, 69 insertions(+), 19 deletions(-) diff --git a/src/input/readers/ascii/Ascii.cc b/src/input/readers/ascii/Ascii.cc index 8b609bda04..3d7d6415a5 100644 --- a/src/input/readers/ascii/Ascii.cc +++ b/src/input/readers/ascii/Ascii.cc @@ -61,6 +61,11 @@ void Ascii::DoClose() bool Ascii::DoInit(const ReaderInfo& info, int num_fields, const Field* const* fields) { + continue_on_failure = true; + is_failed = false; + + filename = info.source; + separator.assign( (const char*) BifConst::InputAscii::separator->Bytes(), BifConst::InputAscii::separator->Len()); @@ -98,18 +103,10 @@ bool Ascii::DoInit(const ReaderInfo& info, int num_fields, const Field* const* f formatter::Ascii::SeparatorInfo sep_info(separator, set_separator, unset_field, empty_field); formatter = unique_ptr(new formatter::Ascii(this, sep_info)); - file.open(info.source); - if ( ! file.is_open() ) + if ( ! OpenFile() ) { - Error(Fmt("Init: cannot open %s", info.source)); - return false; - } - - if ( ReadHeader(false) == false ) - { - Error(Fmt("Init: cannot open %s; headers are incorrect", info.source)); - file.close(); - return false; + is_failed = true; + return continue_on_failure; } DoUpdate(); @@ -117,6 +114,26 @@ bool Ascii::DoInit(const ReaderInfo& info, int num_fields, const Field* const* f return true; } +bool Ascii::OpenFile() + { + file.open(filename); + if ( ! file.is_open() ) + { + Error(Fmt("Init: cannot open %s", filename.c_str())); + is_failed = true; + return continue_on_failure; + } + + if ( ReadHeader(false) == false ) + { + Error(Fmt("Init: cannot open %s; headers are incorrect", filename.c_str())); + file.close(); + is_failed = true; + return continue_on_failure; + } + + return true; + } bool Ascii::ReadHeader(bool useCached) { @@ -129,7 +146,8 @@ bool Ascii::ReadHeader(bool useCached) if ( ! GetLine(line) ) { Error("could not read first line"); - return false; + is_failed = true; + return continue_on_failure; } headerline = line; @@ -172,7 +190,9 @@ bool Ascii::ReadHeader(bool useCached) Error(Fmt("Did not find requested field %s in input data file %s.", field->name, Info().source)); - return false; + + is_failed = true; + return continue_on_failure; } FieldMapping f(field->name, field->type, field->subtype, ifields[field->name]); @@ -184,7 +204,9 @@ bool Ascii::ReadHeader(bool useCached) { Error(Fmt("Could not find requested port type field %s in input data file.", field->secondary_name)); - return false; + + is_failed = true; + return continue_on_failure; } f.secondary_position = ifields[field->secondary_name]; @@ -224,6 +246,12 @@ bool Ascii::GetLine(string& str) // read the entire file and send appropriate thingies back to InputMgr bool Ascii::DoUpdate() { + if ( is_failed ) + if ( ! OpenFile() ) + { + printf("do updates after failure?!\n"); + } + switch ( Info().mode ) { case MODE_REREAD: { @@ -232,7 +260,8 @@ bool Ascii::DoUpdate() if ( stat(Info().source, &sb) == -1 ) { Error(Fmt("Could not get stat for %s", Info().source)); - return false; + is_failed = true; + return continue_on_failure; } if ( sb.st_mtime <= mtime ) // no change @@ -255,7 +284,10 @@ bool Ascii::DoUpdate() { file.clear(); // remove end of file evil bits if ( !ReadHeader(true) ) - return false; // header reading failed + { + is_failed = true; + return continue_on_failure; // header reading failed + } break; } @@ -267,12 +299,14 @@ bool Ascii::DoUpdate() if ( ! file.is_open() ) { Error(Fmt("cannot open %s", Info().source)); - return false; + is_failed = true; + return continue_on_failure; } if ( ReadHeader(false) == false ) { - return false; + is_failed = true; + return continue_on_failure; } break; @@ -334,7 +368,9 @@ bool Ascii::DoUpdate() delete fields[i]; delete [] fields; - return false; + + is_failed = true; + return continue_on_failure; } Value* val = formatter->ParseValue(stringfields[(*fit).position], (*fit).name, (*fit).type, (*fit).subtype); @@ -390,6 +426,15 @@ bool Ascii::DoUpdate() bool Ascii::DoHeartbeat(double network_time, double current_time) { + printf("heartbeat\n"); + is_failed = false; + + if ( ! file.is_open() ) + OpenFile(); + + //if ( is_failed ) + // return continue_on_failure; + switch ( Info().mode ) { case MODE_MANUAL: diff --git a/src/input/readers/ascii/Ascii.h b/src/input/readers/ascii/Ascii.h index 20a459968d..102736d24e 100644 --- a/src/input/readers/ascii/Ascii.h +++ b/src/input/readers/ascii/Ascii.h @@ -55,7 +55,9 @@ protected: private: bool ReadHeader(bool useCached); bool GetLine(string& str); + bool OpenFile(); + string filename; ifstream file; time_t mtime; @@ -71,6 +73,9 @@ private: string empty_field; string unset_field; + bool continue_on_failure; + bool is_failed; + std::unique_ptr formatter; }; From 2b15ec1069bf150ea29c2ff3b46edfba4acbb109 Mon Sep 17 00:00:00 2001 From: Seth Hall Date: Tue, 21 Feb 2017 23:35:29 -0500 Subject: [PATCH 2/6] Another resilient Ascii reader checkpoint. This works correctly now (as a prototype at least). If a file disappears, the thread complains once and once the file reappears the thread will once again begin watching it. --- src/input/readers/ascii/Ascii.cc | 63 ++++++++++---------------------- src/input/readers/ascii/Ascii.h | 1 - 2 files changed, 19 insertions(+), 45 deletions(-) diff --git a/src/input/readers/ascii/Ascii.cc b/src/input/readers/ascii/Ascii.cc index 3d7d6415a5..2f91ebe27a 100644 --- a/src/input/readers/ascii/Ascii.cc +++ b/src/input/readers/ascii/Ascii.cc @@ -64,8 +64,6 @@ bool Ascii::DoInit(const ReaderInfo& info, int num_fields, const Field* const* f continue_on_failure = true; is_failed = false; - filename = info.source; - separator.assign( (const char*) BifConst::InputAscii::separator->Bytes(), BifConst::InputAscii::separator->Len()); @@ -103,11 +101,10 @@ bool Ascii::DoInit(const ReaderInfo& info, int num_fields, const Field* const* f formatter::Ascii::SeparatorInfo sep_info(separator, set_separator, unset_field, empty_field); formatter = unique_ptr(new formatter::Ascii(this, sep_info)); - if ( ! OpenFile() ) - { - is_failed = true; + OpenFile(); + + if ( is_failed ) return continue_on_failure; - } DoUpdate(); @@ -116,22 +113,25 @@ bool Ascii::DoInit(const ReaderInfo& info, int num_fields, const Field* const* f bool Ascii::OpenFile() { - file.open(filename); + file.open(Info().source); if ( ! file.is_open() ) { - Error(Fmt("Init: cannot open %s", filename.c_str())); + if ( ! is_failed ) + Warning(Fmt("Init: cannot open %s", Info().source)); is_failed = true; return continue_on_failure; } if ( ReadHeader(false) == false ) { - Error(Fmt("Init: cannot open %s; headers are incorrect", filename.c_str())); + if ( ! is_failed ) + Warning(Fmt("Init: cannot open %s; headers are incorrect", Info().source)); file.close(); is_failed = true; return continue_on_failure; } + is_failed = false; return true; } @@ -141,14 +141,10 @@ bool Ascii::ReadHeader(bool useCached) string line; map ifields; - if ( ! useCached ) + if ( headerline == "" ) { if ( ! GetLine(line) ) - { - Error("could not read first line"); - is_failed = true; - return continue_on_failure; - } + return false; headerline = line; } @@ -188,7 +184,7 @@ bool Ascii::ReadHeader(bool useCached) continue; } - Error(Fmt("Did not find requested field %s in input data file %s.", + Warning(Fmt("Did not find requested field %s in input data file %s.", field->name, Info().source)); is_failed = true; @@ -202,7 +198,7 @@ bool Ascii::ReadHeader(bool useCached) map::iterator fit2 = ifields.find(field->secondary_name); if ( fit2 == ifields.end() ) { - Error(Fmt("Could not find requested port type field %s in input data file.", + Warning(Fmt("Could not find requested port type field %s in input data file.", field->secondary_name)); is_failed = true; @@ -215,7 +211,6 @@ bool Ascii::ReadHeader(bool useCached) columnMap.push_back(f); } - // well, that seems to have worked... return true; } @@ -246,12 +241,6 @@ bool Ascii::GetLine(string& str) // read the entire file and send appropriate thingies back to InputMgr bool Ascii::DoUpdate() { - if ( is_failed ) - if ( ! OpenFile() ) - { - printf("do updates after failure?!\n"); - } - switch ( Info().mode ) { case MODE_REREAD: { @@ -259,7 +248,8 @@ bool Ascii::DoUpdate() struct stat sb; if ( stat(Info().source, &sb) == -1 ) { - Error(Fmt("Could not get stat for %s", Info().source)); + Warning(Fmt("Could not get stat for %s", Info().source)); + file.close(); is_failed = true; return continue_on_failure; } @@ -295,19 +285,7 @@ bool Ascii::DoUpdate() file.close(); } - file.open(Info().source); - if ( ! file.is_open() ) - { - Error(Fmt("cannot open %s", Info().source)); - is_failed = true; - return continue_on_failure; - } - - if ( ReadHeader(false) == false ) - { - is_failed = true; - return continue_on_failure; - } + OpenFile(); break; } @@ -361,7 +339,7 @@ bool Ascii::DoUpdate() if ( (*fit).position > pos || (*fit).secondary_position > pos ) { - Error(Fmt("Not enough fields in line %s. Found %d fields, want positions %d and %d", + Warning(Fmt("Not enough fields in line %s. Found %d fields, want positions %d and %d", line.c_str(), pos, (*fit).position, (*fit).secondary_position)); for ( int i = 0; i < fpos; i++ ) @@ -426,14 +404,11 @@ bool Ascii::DoUpdate() bool Ascii::DoHeartbeat(double network_time, double current_time) { - printf("heartbeat\n"); - is_failed = false; - if ( ! file.is_open() ) OpenFile(); - //if ( is_failed ) - // return continue_on_failure; + if ( is_failed ) + return continue_on_failure; switch ( Info().mode ) { diff --git a/src/input/readers/ascii/Ascii.h b/src/input/readers/ascii/Ascii.h index 102736d24e..c9fd55f3d5 100644 --- a/src/input/readers/ascii/Ascii.h +++ b/src/input/readers/ascii/Ascii.h @@ -57,7 +57,6 @@ private: bool GetLine(string& str); bool OpenFile(); - string filename; ifstream file; time_t mtime; From 75744d22bc1d556c3c9a42cb458af8ce181f1536 Mon Sep 17 00:00:00 2001 From: Seth Hall Date: Thu, 23 Feb 2017 23:13:12 -0500 Subject: [PATCH 3/6] Input's ascii reader is now more resilient. By default, the ASCII reader does not fail on errors anymore. If there is a problem parsing a line, a reporter warning is written and parsing continues. If the file is missing or can't be read, the input thread just tries again on the next heartbeat. Options have been added to recreate the previous behavior... const InputAscii::fail_on_invalid_lines: bool; and const InputAscii::fail_on_file_problem: bool; They are both set to `F` by default which makes the input readers resilient to failure. --- .../base/frameworks/input/readers/ascii.bro | 13 +++ src/input/readers/ascii/Ascii.cc | 80 ++++++++++++------- src/input/readers/ascii/Ascii.h | 7 +- src/input/readers/ascii/ascii.bif | 2 + .../bro..stderr | 4 + .../bro..stdout | 2 + .../bro..stderr | 4 +- .../input/missing-file-initially.bro | 47 +++++++++++ 8 files changed, 127 insertions(+), 32 deletions(-) create mode 100644 testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stderr create mode 100644 testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stdout create mode 100644 testing/btest/scripts/base/frameworks/input/missing-file-initially.bro diff --git a/scripts/base/frameworks/input/readers/ascii.bro b/scripts/base/frameworks/input/readers/ascii.bro index 1b486ddba0..521e8d4ca3 100644 --- a/scripts/base/frameworks/input/readers/ascii.bro +++ b/scripts/base/frameworks/input/readers/ascii.bro @@ -18,4 +18,17 @@ export { ## String to use for an unset &optional field. const unset_field = Input::unset_field &redef; + + ## Choose if the ascii input reader should globally + ## fail on invalid lines and continue parsing afterward. + ## Individual readers can use a different value. + const fail_on_invalid_lines = F &redef; + + ## Set to true if you would like the old behavior of the + ## ascii reader where the reader thread would die if any file + ## errors occur (like permissions problems or file missing). + ## The default behavior is to continue attempting to open and read + ## the file even in light of problems. + ## Individual readers can use a different value. + const fail_on_file_problem = F &redef; } diff --git a/src/input/readers/ascii/Ascii.cc b/src/input/readers/ascii/Ascii.cc index 2f91ebe27a..5621a87eb4 100644 --- a/src/input/readers/ascii/Ascii.cc +++ b/src/input/readers/ascii/Ascii.cc @@ -61,7 +61,6 @@ void Ascii::DoClose() bool Ascii::DoInit(const ReaderInfo& info, int num_fields, const Field* const* fields) { - continue_on_failure = true; is_failed = false; separator.assign( (const char*) BifConst::InputAscii::separator->Bytes(), @@ -76,6 +75,9 @@ bool Ascii::DoInit(const ReaderInfo& info, int num_fields, const Field* const* f unset_field.assign( (const char*) BifConst::InputAscii::unset_field->Bytes(), BifConst::InputAscii::unset_field->Len()); + fail_on_invalid_lines = BifConst::InputAscii::fail_on_invalid_lines; + fail_on_file_problem = BifConst::InputAscii::fail_on_file_problem; + // Set per-filter configuration options. for ( ReaderInfo::config_map::const_iterator i = info.config.begin(); i != info.config.end(); i++ ) { @@ -90,6 +92,12 @@ bool Ascii::DoInit(const ReaderInfo& info, int num_fields, const Field* const* f else if ( strcmp(i->first, "unset_field") == 0 ) unset_field.assign(i->second); + + else if ( strcmp(i->first, "fail_on_invalid_lines") == 0 ) + fail_on_invalid_lines = (strncmp(i->second, "T", 1) == 0); + + else if ( strcmp(i->first, "fail_on_file_problem") == 0 ) + fail_on_file_problem = (strncmp(i->second, "T", 1) == 0); } if ( separator.size() != 1 ) @@ -101,34 +109,40 @@ bool Ascii::DoInit(const ReaderInfo& info, int num_fields, const Field* const* f formatter::Ascii::SeparatorInfo sep_info(separator, set_separator, unset_field, empty_field); formatter = unique_ptr(new formatter::Ascii(this, sep_info)); - OpenFile(); - - if ( is_failed ) - return continue_on_failure; - DoUpdate(); return true; } +void Ascii::FailWarn(bool is_error, const char *msg) + { + if ( is_error ) + Error(msg); + else + Warning(msg); + } + bool Ascii::OpenFile() { + if ( file.is_open() && ! is_failed ) + return true; + file.open(Info().source); if ( ! file.is_open() ) { if ( ! is_failed ) - Warning(Fmt("Init: cannot open %s", Info().source)); + FailWarn(fail_on_file_problem, Fmt("Init: cannot open %s", Info().source)); is_failed = true; - return continue_on_failure; + return !fail_on_file_problem; } if ( ReadHeader(false) == false ) { if ( ! is_failed ) - Warning(Fmt("Init: cannot open %s; headers are incorrect", Info().source)); + FailWarn(fail_on_file_problem, Fmt("Init: cannot open %s; headers are incorrect", Info().source)); file.close(); is_failed = true; - return continue_on_failure; + return !fail_on_file_problem; } is_failed = false; @@ -141,7 +155,7 @@ bool Ascii::ReadHeader(bool useCached) string line; map ifields; - if ( headerline == "" ) + if ( ! useCached ) { if ( ! GetLine(line) ) return false; @@ -184,11 +198,12 @@ bool Ascii::ReadHeader(bool useCached) continue; } - Warning(Fmt("Did not find requested field %s in input data file %s.", - field->name, Info().source)); + if ( ! is_failed ) + FailWarn(fail_on_file_problem, Fmt("Did not find requested field %s in input data file %s.", + field->name, Info().source)); is_failed = true; - return continue_on_failure; + return !fail_on_file_problem; } FieldMapping f(field->name, field->type, field->subtype, ifields[field->name]); @@ -198,11 +213,12 @@ bool Ascii::ReadHeader(bool useCached) map::iterator fit2 = ifields.find(field->secondary_name); if ( fit2 == ifields.end() ) { - Warning(Fmt("Could not find requested port type field %s in input data file.", - field->secondary_name)); + if ( ! is_failed ) + FailWarn(fail_on_file_problem, Fmt("Could not find requested port type field %s in input data file.", + field->secondary_name)); is_failed = true; - return continue_on_failure; + return !fail_on_file_problem; } f.secondary_position = ifields[field->secondary_name]; @@ -241,6 +257,9 @@ bool Ascii::GetLine(string& str) // read the entire file and send appropriate thingies back to InputMgr bool Ascii::DoUpdate() { + if ( ! OpenFile() ) + return !fail_on_file_problem; + switch ( Info().mode ) { case MODE_REREAD: { @@ -248,10 +267,11 @@ bool Ascii::DoUpdate() struct stat sb; if ( stat(Info().source, &sb) == -1 ) { - Warning(Fmt("Could not get stat for %s", Info().source)); + if ( ! is_failed ) + FailWarn(fail_on_file_problem, Fmt("Could not get stat for %s", Info().source)); file.close(); is_failed = true; - return continue_on_failure; + return !fail_on_file_problem; } if ( sb.st_mtime <= mtime ) // no change @@ -276,7 +296,7 @@ bool Ascii::DoUpdate() if ( !ReadHeader(true) ) { is_failed = true; - return continue_on_failure; // header reading failed + return !fail_on_file_problem; // header reading failed } break; @@ -339,7 +359,7 @@ bool Ascii::DoUpdate() if ( (*fit).position > pos || (*fit).secondary_position > pos ) { - Warning(Fmt("Not enough fields in line %s. Found %d fields, want positions %d and %d", + FailWarn(fail_on_invalid_lines, Fmt("Not enough fields in line %s. Found %d fields, want positions %d and %d", line.c_str(), pos, (*fit).position, (*fit).secondary_position)); for ( int i = 0; i < fpos; i++ ) @@ -347,8 +367,15 @@ bool Ascii::DoUpdate() delete [] fields; - is_failed = true; - return continue_on_failure; + if ( fail_on_invalid_lines ) + { + return false; + } + else + { + error = true; + break; + } } Value* val = formatter->ParseValue(stringfields[(*fit).position], (*fit).name, (*fit).type, (*fit).subtype); @@ -404,11 +431,8 @@ bool Ascii::DoUpdate() bool Ascii::DoHeartbeat(double network_time, double current_time) { - if ( ! file.is_open() ) - OpenFile(); - - if ( is_failed ) - return continue_on_failure; + if ( ! OpenFile() ) + return !fail_on_file_problem; switch ( Info().mode ) { diff --git a/src/input/readers/ascii/Ascii.h b/src/input/readers/ascii/Ascii.h index c9fd55f3d5..9300382ca2 100644 --- a/src/input/readers/ascii/Ascii.h +++ b/src/input/readers/ascii/Ascii.h @@ -56,6 +56,8 @@ private: bool ReadHeader(bool useCached); bool GetLine(string& str); bool OpenFile(); + void FailWarn(bool is_error, const char *msg); + ifstream file; time_t mtime; @@ -71,8 +73,11 @@ private: string set_separator; string empty_field; string unset_field; + bool fail_on_invalid_lines; + bool fail_on_file_problem; - bool continue_on_failure; + // this is an internal indicator in case the read is currently in a failed state + // it's used by the options for continuing instead of failing and killing the reader. bool is_failed; std::unique_ptr formatter; diff --git a/src/input/readers/ascii/ascii.bif b/src/input/readers/ascii/ascii.bif index 8bb3a96492..80ff4611e7 100644 --- a/src/input/readers/ascii/ascii.bif +++ b/src/input/readers/ascii/ascii.bif @@ -5,3 +5,5 @@ const separator: string; const set_separator: string; const empty_field: string; const unset_field: string; +const fail_on_invalid_lines: bool; +const fail_on_file_problem: bool; diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stderr b/testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stderr new file mode 100644 index 0000000000..6c4d0f0194 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stderr @@ -0,0 +1,4 @@ +warning: ../does-not-exist.dat/Input::READER_ASCII: Init: cannot open ../does-not-exist.dat +error: ../does-not-exist.dat/Input::READER_ASCII: Init: cannot open ../does-not-exist.dat +error: ../does-not-exist.dat/Input::READER_ASCII: terminating thread +received termination signal diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stdout b/testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stdout new file mode 100644 index 0000000000..d96c6d457d --- /dev/null +++ b/testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stdout @@ -0,0 +1,2 @@ +now it does +and more! diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.missing-file/bro..stderr b/testing/btest/Baseline/scripts.base.frameworks.input.missing-file/bro..stderr index 5093925d2d..f5d074fe7e 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.missing-file/bro..stderr +++ b/testing/btest/Baseline/scripts.base.frameworks.input.missing-file/bro..stderr @@ -1,4 +1,2 @@ -error: does-not-exist.dat/Input::READER_ASCII: Init: cannot open does-not-exist.dat -error: does-not-exist.dat/Input::READER_ASCII: Init failed -error: does-not-exist.dat/Input::READER_ASCII: terminating thread +warning: does-not-exist.dat/Input::READER_ASCII: Init: cannot open does-not-exist.dat received termination signal diff --git a/testing/btest/scripts/base/frameworks/input/missing-file-initially.bro b/testing/btest/scripts/base/frameworks/input/missing-file-initially.bro new file mode 100644 index 0000000000..a7128a4455 --- /dev/null +++ b/testing/btest/scripts/base/frameworks/input/missing-file-initially.bro @@ -0,0 +1,47 @@ +# This tests files that don't exist initially and then do later during +# runtime to make sure the ascii reader is resilient to files missing. +# It does a second test at the same time which configures the old +# failing behavior. + +# @TEST-EXEC: btest-bg-run bro bro %INPUT +# @TEST-EXEC: btest-bg-wait -k 5 +# @TEST-EXEC: btest-diff bro/.stdout +# @TEST-EXEC: btest-diff bro/.stderr + +@TEST-START-FILE does-exist.dat +#separator \x09 +#fields line +#types string +now it does +and more! +@TEST-END-FILE + +redef exit_only_after_terminate = T; + +@load base/frameworks/input + +module A; + +type Val: record { + line: string; +}; + +event line(description: Input::EventDescription, tpe: Input::Event, v: Val) + { + print v$line; + } + +event line2(description: Input::EventDescription, tpe: Input::Event, v: Val) + { + print "DONT PRINT THIS LINE"; + } + + +event bro_init() + { + Input::add_event([$source="../does-not-exist.dat", $name="input", $reader=Input::READER_ASCII, $mode=Input::REREAD, $fields=Val, $ev=line, $want_record=T]); + Input::add_event([$source="../does-not-exist.dat", $name="input2", $reader=Input::READER_ASCII, $mode=Input::REREAD, $fields=Val, $ev=line2, $want_record=T, + $config=table(["fail_on_file_problem"] = "T")]); + + system("sleep 2; mv ../does-exist.dat ../does-not-exist.dat;"); + } From 50781590804ee0ba2e3d54b186478f4613fa61a0 Mon Sep 17 00:00:00 2001 From: Seth Hall Date: Thu, 23 Feb 2017 23:13:48 -0500 Subject: [PATCH 4/6] Tiny fix to correct a warning message. --- src/input/Manager.cc | 2 +- .../scripts.base.frameworks.input.missing-enum/bro..stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/input/Manager.cc b/src/input/Manager.cc index b84d822101..d029e38092 100644 --- a/src/input/Manager.cc +++ b/src/input/Manager.cc @@ -2380,7 +2380,7 @@ Val* Manager::ValueToVal(const Stream* i, const Value* val, BroType* request_typ bro_int_t index = request_type->AsEnumType()->Lookup(module, var.c_str()); if ( index == -1 ) { - Warning(i, "Value not '%s' for stream '%s' is not a valid enum.", + Warning(i, "Value '%s' for stream '%s' is not a valid enum.", enum_string.c_str(), i->name.c_str()); have_error = true; diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.missing-enum/bro..stderr b/testing/btest/Baseline/scripts.base.frameworks.input.missing-enum/bro..stderr index 20207bcf94..8cd0c5ab6c 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.missing-enum/bro..stderr +++ b/testing/btest/Baseline/scripts.base.frameworks.input.missing-enum/bro..stderr @@ -1,2 +1,2 @@ -warning: Value not 'IdoNot::Exist' for stream 'enum' is not a valid enum. +warning: Value 'IdoNot::Exist' for stream 'enum' is not a valid enum. received termination signal From b6e6302b4031d36d6e297b25aba444b0dc26f1e8 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Fri, 3 Mar 2017 12:40:37 -0800 Subject: [PATCH 5/6] Ascii reader error changes - fix small bugs The changes are now a bit more succinct with less code changes required. Behavior is tested a little bit more thoroughly and a memory problem when reading incomplete lines was fixed. ReadHeader also always directly returns if header reading failed. Error messages now are back to what they were before the change, if the new behavior is not used. I also tweaked the documentation text a bit. --- .../base/frameworks/input/readers/ascii.bro | 34 ++++++-- src/input/readers/ascii/Ascii.cc | 85 ++++++++++--------- src/input/readers/ascii/Ascii.h | 10 ++- .../out | 26 ++++++ .../bro..stderr | 4 + .../bro..stdout | 3 + .../bro..stderr | 4 +- .../base/frameworks/input/invalid-lines.bro | 67 +++++++++++++++ .../base/frameworks/input/invalidtext.bro | 1 + .../input/missing-file-initially.bro | 12 +-- .../base/frameworks/input/missing-file.bro | 1 + 11 files changed, 187 insertions(+), 60 deletions(-) create mode 100644 testing/btest/Baseline/scripts.base.frameworks.input.invalid-lines/out create mode 100644 testing/btest/scripts/base/frameworks/input/invalid-lines.bro diff --git a/scripts/base/frameworks/input/readers/ascii.bro b/scripts/base/frameworks/input/readers/ascii.bro index 521e8d4ca3..83d8ced94b 100644 --- a/scripts/base/frameworks/input/readers/ascii.bro +++ b/scripts/base/frameworks/input/readers/ascii.bro @@ -19,16 +19,32 @@ export { ## String to use for an unset &optional field. const unset_field = Input::unset_field &redef; - ## Choose if the ascii input reader should globally - ## fail on invalid lines and continue parsing afterward. - ## Individual readers can use a different value. + ## Fail on invalid lines. If set to false, the ascii + ## input reader will jump over invalid lines, reporting + ## warnings in reporter.log. If set to true, errors in + ## input lines will be handled as fatal errors for the + ## reader thread; reading will abort immediately and + ## an error will be logged to reporter.log. + ## Invidivual readers can use a different value using + ## the $config table. + ## fail_on_invalid_lines = T was the default behavior + ## untill Bro 2.5. const fail_on_invalid_lines = F &redef; - ## Set to true if you would like the old behavior of the - ## ascii reader where the reader thread would die if any file - ## errors occur (like permissions problems or file missing). - ## The default behavior is to continue attempting to open and read - ## the file even in light of problems. - ## Individual readers can use a different value. + ## Fail on file read problems. If set to true, the ascii + ## input reader will fail when encountering any problems + ## while reading a file different from invalid lines. + ## Examples fur such problems are permission problems, or + ## missing files. + ## When set to false, these problems will be ignored. This + ## has an especially big effect for the REREAD mode, which will + ## seamlessly recover from read errors when a file is + ## only temporarily inaccessible. For MANUAL or STREAM files, + ## errors will most likely still be fatal since no automatic + ## re-reading of the file is attempted. + ## Invidivual readers can use a different value using + ## the $config table. + ## fail_on_file_problem = T was the default behavior + ## untill Bro 2.5. const fail_on_file_problem = F &redef; } diff --git a/src/input/readers/ascii/Ascii.cc b/src/input/readers/ascii/Ascii.cc index 5621a87eb4..bedddce075 100644 --- a/src/input/readers/ascii/Ascii.cc +++ b/src/input/readers/ascii/Ascii.cc @@ -61,7 +61,7 @@ void Ascii::DoClose() bool Ascii::DoInit(const ReaderInfo& info, int num_fields, const Field* const* fields) { - is_failed = false; + suppress_warnings = false; separator.assign( (const char*) BifConst::InputAscii::separator->Bytes(), BifConst::InputAscii::separator->Len()); @@ -109,43 +109,48 @@ bool Ascii::DoInit(const ReaderInfo& info, int num_fields, const Field* const* f formatter::Ascii::SeparatorInfo sep_info(separator, set_separator, unset_field, empty_field); formatter = unique_ptr(new formatter::Ascii(this, sep_info)); - DoUpdate(); - - return true; + return DoUpdate(); } -void Ascii::FailWarn(bool is_error, const char *msg) +void Ascii::FailWarn(bool is_error, const char *msg, bool suppress_future) { if ( is_error ) Error(msg); else - Warning(msg); + { + // suppress error message when we are already in error mode. + // There is no reason to repeat it every second. + if ( ! suppress_warnings ) + Warning(msg); + + if ( suppress_future ) + suppress_warnings = true; + } } bool Ascii::OpenFile() { - if ( file.is_open() && ! is_failed ) + if ( file.is_open() ) return true; file.open(Info().source); + if ( ! file.is_open() ) { - if ( ! is_failed ) - FailWarn(fail_on_file_problem, Fmt("Init: cannot open %s", Info().source)); - is_failed = true; - return !fail_on_file_problem; + FailWarn(fail_on_file_problem, Fmt("Init: cannot open %s", Info().source), true); + + return ! fail_on_file_problem; } if ( ReadHeader(false) == false ) { - if ( ! is_failed ) - FailWarn(fail_on_file_problem, Fmt("Init: cannot open %s; headers are incorrect", Info().source)); + FailWarn(fail_on_file_problem, Fmt("Init: cannot open %s; problem reading file header", Info().source), true); + file.close(); - is_failed = true; - return !fail_on_file_problem; + return ! fail_on_file_problem; } - is_failed = false; + suppress_warnings = false; return true; } @@ -158,7 +163,11 @@ bool Ascii::ReadHeader(bool useCached) if ( ! useCached ) { if ( ! GetLine(line) ) + { + FailWarn(fail_on_file_problem, Fmt("Could not read input data file %s; first line could not be read", + Info().source), true); return false; + } headerline = line; } @@ -198,12 +207,10 @@ bool Ascii::ReadHeader(bool useCached) continue; } - if ( ! is_failed ) - FailWarn(fail_on_file_problem, Fmt("Did not find requested field %s in input data file %s.", - field->name, Info().source)); + FailWarn(fail_on_file_problem, Fmt("Did not find requested field %s in input data file %s.", + field->name, Info().source), true); - is_failed = true; - return !fail_on_file_problem; + return false; } FieldMapping f(field->name, field->type, field->subtype, ifields[field->name]); @@ -213,12 +220,10 @@ bool Ascii::ReadHeader(bool useCached) map::iterator fit2 = ifields.find(field->secondary_name); if ( fit2 == ifields.end() ) { - if ( ! is_failed ) - FailWarn(fail_on_file_problem, Fmt("Could not find requested port type field %s in input data file.", - field->secondary_name)); + FailWarn(fail_on_file_problem, Fmt("Could not find requested port type field %s in input data file.", + field->secondary_name), true); - is_failed = true; - return !fail_on_file_problem; + return false; } f.secondary_position = ifields[field->secondary_name]; @@ -258,7 +263,7 @@ bool Ascii::GetLine(string& str) bool Ascii::DoUpdate() { if ( ! OpenFile() ) - return !fail_on_file_problem; + return ! fail_on_file_problem; switch ( Info().mode ) { case MODE_REREAD: @@ -267,11 +272,10 @@ bool Ascii::DoUpdate() struct stat sb; if ( stat(Info().source, &sb) == -1 ) { - if ( ! is_failed ) - FailWarn(fail_on_file_problem, Fmt("Could not get stat for %s", Info().source)); + FailWarn(fail_on_file_problem, Fmt("Could not get stat for %s", Info().source), true); + file.close(); - is_failed = true; - return !fail_on_file_problem; + return ! fail_on_file_problem; } if ( sb.st_mtime <= mtime ) // no change @@ -293,10 +297,9 @@ bool Ascii::DoUpdate() if ( Info().mode == MODE_STREAM ) { file.clear(); // remove end of file evil bits - if ( !ReadHeader(true) ) + if ( ! ReadHeader(true) ) { - is_failed = true; - return !fail_on_file_problem; // header reading failed + return ! fail_on_file_problem; // header reading failed } break; @@ -360,15 +363,15 @@ bool Ascii::DoUpdate() if ( (*fit).position > pos || (*fit).secondary_position > pos ) { FailWarn(fail_on_invalid_lines, Fmt("Not enough fields in line %s. Found %d fields, want positions %d and %d", - line.c_str(), pos, (*fit).position, (*fit).secondary_position)); - - for ( int i = 0; i < fpos; i++ ) - delete fields[i]; - - delete [] fields; + line.c_str(), pos, (*fit).position, (*fit).secondary_position)); if ( fail_on_invalid_lines ) { + for ( int i = 0; i < fpos; i++ ) + delete fields[i]; + + delete [] fields; + return false; } else @@ -432,7 +435,7 @@ bool Ascii::DoUpdate() bool Ascii::DoHeartbeat(double network_time, double current_time) { if ( ! OpenFile() ) - return !fail_on_file_problem; + return ! fail_on_file_problem; switch ( Info().mode ) { diff --git a/src/input/readers/ascii/Ascii.h b/src/input/readers/ascii/Ascii.h index 9300382ca2..7a7fa52590 100644 --- a/src/input/readers/ascii/Ascii.h +++ b/src/input/readers/ascii/Ascii.h @@ -56,8 +56,10 @@ private: bool ReadHeader(bool useCached); bool GetLine(string& str); bool OpenFile(); - void FailWarn(bool is_error, const char *msg); - + // Call Warning or Error, depending on the is_error boolean. + // In case of a warning, setting suppress_future to true will suppress all future warnings + // (by setting suppress_warnings to true, until suppress_warnings is set back to false) + void FailWarn(bool is_error, const char *msg, bool suppress_future = false); ifstream file; time_t mtime; @@ -77,8 +79,8 @@ private: bool fail_on_file_problem; // this is an internal indicator in case the read is currently in a failed state - // it's used by the options for continuing instead of failing and killing the reader. - bool is_failed; + // it's used to suppress duplicate error messages. + bool suppress_warnings; std::unique_ptr formatter; }; diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.invalid-lines/out b/testing/btest/Baseline/scripts.base.frameworks.input.invalid-lines/out new file mode 100644 index 0000000000..3406639d29 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.frameworks.input.invalid-lines/out @@ -0,0 +1,26 @@ +{ +[-43] = [b=T, e=SSH::LOG, c=21, p=123/unknown, sn=10.0.0.0/24, a=1.2.3.4, d=3.14, t=1315801931.273616, iv=100.0, s=hurz, ns=4242 HOHOHO, sc={ +2, +4, +1, +3 +}, ss={ +BB, +AA, +CC +}, se={ + +}, vc=[10, 20, 30], ve=[]], +[-42] = [b=T, e=SSH::LOG, c=21, p=123/unknown, sn=10.0.0.0/24, a=1.2.3.4, d=3.14, t=1315801931.273616, iv=100.0, s=hurz, ns=4242, sc={ +2, +4, +1, +3 +}, ss={ +BB, +AA, +CC +}, se={ + +}, vc=[10, 20, 30], ve=[]] +} diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stderr b/testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stderr index 6c4d0f0194..337cdcda87 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stderr +++ b/testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stderr @@ -1,4 +1,8 @@ warning: ../does-not-exist.dat/Input::READER_ASCII: Init: cannot open ../does-not-exist.dat +warning: ../does-not-exist.dat/Input::READER_ASCII: Init: cannot open ../does-not-exist.dat +warning: ../does-not-exist.dat/Input::READER_ASCII: Init: cannot open ../does-not-exist.dat error: ../does-not-exist.dat/Input::READER_ASCII: Init: cannot open ../does-not-exist.dat +error: ../does-not-exist.dat/Input::READER_ASCII: Init failed error: ../does-not-exist.dat/Input::READER_ASCII: terminating thread +warning: ../does-not-exist.dat/Input::READER_ASCII: Could not get stat for ../does-not-exist.dat received termination signal diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stdout b/testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stdout index d96c6d457d..39309bc8cf 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stdout +++ b/testing/btest/Baseline/scripts.base.frameworks.input.missing-file-initially/bro..stdout @@ -1,2 +1,5 @@ now it does and more! +now it does +and more! +Streaming still works diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.missing-file/bro..stderr b/testing/btest/Baseline/scripts.base.frameworks.input.missing-file/bro..stderr index f5d074fe7e..5093925d2d 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.missing-file/bro..stderr +++ b/testing/btest/Baseline/scripts.base.frameworks.input.missing-file/bro..stderr @@ -1,2 +1,4 @@ -warning: does-not-exist.dat/Input::READER_ASCII: Init: cannot open does-not-exist.dat +error: does-not-exist.dat/Input::READER_ASCII: Init: cannot open does-not-exist.dat +error: does-not-exist.dat/Input::READER_ASCII: Init failed +error: does-not-exist.dat/Input::READER_ASCII: terminating thread received termination signal diff --git a/testing/btest/scripts/base/frameworks/input/invalid-lines.bro b/testing/btest/scripts/base/frameworks/input/invalid-lines.bro new file mode 100644 index 0000000000..83be1efd09 --- /dev/null +++ b/testing/btest/scripts/base/frameworks/input/invalid-lines.bro @@ -0,0 +1,67 @@ +# @TEST-EXEC: btest-bg-run bro bro -b %INPUT +# @TEST-EXEC: btest-bg-wait 10 +# @TEST-EXEC: btest-diff out + +redef exit_only_after_terminate = T; +redef InputAscii::fail_on_invalid_lines = F; + +@TEST-START-FILE input.log +#separator \x09 +#path ssh +#fields b i e c p sn a d t iv s sc ss se vc ve ns +#types bool int enum count port subnet addr double time interval string table table table vector vector string +T -42 SSH::LOG 21 123 10.0.0.0/24 1.2.3.4 3.14 1315801931.273616 100.000000 hurz 2,4,1,3 CC,AA,BB EMPTY 10,20,30 +T -42 SSH::LOG 21 123 10.0.0.0/24 1.2.3.4 3.14 1315801931.273616 100.000000 hurz 2,4,1,3 CC,AA,BB EMPTY 10,20,30 EMPTY 4242 +T -43 SSH::LOG 21 123 10.0.0.0/24 1.2.3.4 3.14 1315801931.273616 100.000000 hurz 2,4,1,3 CC,AA,BB EMPTY 10,20,30 EMPTY 4242 HOHOHO +T -41 +@TEST-END-FILE + +@load base/protocols/ssh + +global outfile: file; + +redef InputAscii::empty_field = "EMPTY"; + +module A; + +type Idx: record { + i: int; +}; + +type Val: record { + b: bool; + e: Log::ID; + c: count; + p: port; + sn: subnet; + a: addr; + d: double; + t: time; + iv: interval; + s: string; + ns: string; + sc: set[count]; + ss: set[string]; + se: set[string]; + vc: vector of int; + ve: vector of int; +}; + +global servers: table[int] of Val = table(); +global servers2: table[int] of Val = table(); + +event bro_init() + { + outfile = open("../out"); + # first read in the old stuff into the table... + Input::add_table([$source="../input.log", $name="ssh", $idx=Idx, $val=Val, $destination=servers]); + Input::add_table([$source="../input.log", $name="ssh2", $idx=Idx, $val=Val, $destination=servers2, $config=table(["fail_on_invalid_lines"] = "T")]); + } + +event Input::end_of_data(name: string, source:string) + { + print outfile, servers; + Input::remove("ssh"); + close(outfile); + terminate(); + } diff --git a/testing/btest/scripts/base/frameworks/input/invalidtext.bro b/testing/btest/scripts/base/frameworks/input/invalidtext.bro index 1de4e96671..3f5b590dec 100644 --- a/testing/btest/scripts/base/frameworks/input/invalidtext.bro +++ b/testing/btest/scripts/base/frameworks/input/invalidtext.bro @@ -13,6 +13,7 @@ @TEST-END-FILE redef exit_only_after_terminate = T; +redef InputAscii::fail_on_invalid_lines = T; global outfile: file; diff --git a/testing/btest/scripts/base/frameworks/input/missing-file-initially.bro b/testing/btest/scripts/base/frameworks/input/missing-file-initially.bro index a7128a4455..73fd57284e 100644 --- a/testing/btest/scripts/base/frameworks/input/missing-file-initially.bro +++ b/testing/btest/scripts/base/frameworks/input/missing-file-initially.bro @@ -1,10 +1,12 @@ # This tests files that don't exist initially and then do later during # runtime to make sure the ascii reader is resilient to files missing. -# It does a second test at the same time which configures the old +# It does a second test at the same time which configures the old # failing behavior. # @TEST-EXEC: btest-bg-run bro bro %INPUT -# @TEST-EXEC: btest-bg-wait -k 5 +# @TEST-EXEC: sleep 2; cp does-exist.dat does-not-exist.dat +# @TEST-EXEC: sleep 2; mv does-not-exist.dat does-not-exist-again.dat; echo "Streaming still works" >> does-not-exist-again.dat +# @TEST-EXEC: btest-bg-wait -k 3 # @TEST-EXEC: btest-diff bro/.stdout # @TEST-EXEC: btest-diff bro/.stderr @@ -40,8 +42,8 @@ event line2(description: Input::EventDescription, tpe: Input::Event, v: Val) event bro_init() { Input::add_event([$source="../does-not-exist.dat", $name="input", $reader=Input::READER_ASCII, $mode=Input::REREAD, $fields=Val, $ev=line, $want_record=T]); - Input::add_event([$source="../does-not-exist.dat", $name="input2", $reader=Input::READER_ASCII, $mode=Input::REREAD, $fields=Val, $ev=line2, $want_record=T, + Input::add_event([$source="../does-not-exist.dat", $name="inputstream", $reader=Input::READER_ASCII, $mode=Input::STREAM, $fields=Val, $ev=line, $want_record=T]); + Input::add_event([$source="../does-not-exist.dat", $name="inputmanual", $reader=Input::READER_ASCII, $mode=Input::MANUAL, $fields=Val, $ev=line, $want_record=T]); + Input::add_event([$source="../does-not-exist.dat", $name="input2", $reader=Input::READER_ASCII, $mode=Input::REREAD, $fields=Val, $ev=line2, $want_record=T, $config=table(["fail_on_file_problem"] = "T")]); - - system("sleep 2; mv ../does-exist.dat ../does-not-exist.dat;"); } diff --git a/testing/btest/scripts/base/frameworks/input/missing-file.bro b/testing/btest/scripts/base/frameworks/input/missing-file.bro index 08adfe2150..2ec3bb937f 100644 --- a/testing/btest/scripts/base/frameworks/input/missing-file.bro +++ b/testing/btest/scripts/base/frameworks/input/missing-file.bro @@ -3,6 +3,7 @@ # @TEST-EXEC: btest-diff bro/.stderr redef exit_only_after_terminate = T; +redef InputAscii::fail_on_file_problem = T; global outfile: file; global try: count; From ff4d624ebe4bee14211ae49beb789b7221fe8e27 Mon Sep 17 00:00:00 2001 From: Seth Hall Date: Thu, 9 Mar 2017 12:18:35 -0500 Subject: [PATCH 6/6] Minor documentation fixes. --- scripts/base/frameworks/input/readers/ascii.bro | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/base/frameworks/input/readers/ascii.bro b/scripts/base/frameworks/input/readers/ascii.bro index 83d8ced94b..1d4072e118 100644 --- a/scripts/base/frameworks/input/readers/ascii.bro +++ b/scripts/base/frameworks/input/readers/ascii.bro @@ -25,16 +25,16 @@ export { ## input lines will be handled as fatal errors for the ## reader thread; reading will abort immediately and ## an error will be logged to reporter.log. - ## Invidivual readers can use a different value using + ## Individual readers can use a different value using ## the $config table. ## fail_on_invalid_lines = T was the default behavior - ## untill Bro 2.5. + ## until Bro 2.6. const fail_on_invalid_lines = F &redef; ## Fail on file read problems. If set to true, the ascii ## input reader will fail when encountering any problems ## while reading a file different from invalid lines. - ## Examples fur such problems are permission problems, or + ## Examples of such problems are permission problems, or ## missing files. ## When set to false, these problems will be ignored. This ## has an especially big effect for the REREAD mode, which will @@ -42,9 +42,9 @@ export { ## only temporarily inaccessible. For MANUAL or STREAM files, ## errors will most likely still be fatal since no automatic ## re-reading of the file is attempted. - ## Invidivual readers can use a different value using + ## Individual readers can use a different value using ## the $config table. ## fail_on_file_problem = T was the default behavior - ## untill Bro 2.5. + ## until Bro 2.6. const fail_on_file_problem = F &redef; }