From c8e1a397586f6e4414b639c50f415366aaf6613c Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Mon, 25 Jul 2016 15:35:46 -0700 Subject: [PATCH] Input: Further small changes to error handling Calling Error() in an input reader now automatically will disable the reader and return a failure in the Update/Heartbeat calls. Also adds more tests. Addresses BIT-1181 --- src/input/ReaderBackend.cc | 10 +++++- src/input/ReaderBackend.h | 10 ++++++ .../.stderr | 5 +++ .../.stderrwithoutfirstline | 2 ++ .../out | 7 +++-- .../scripts/base/frameworks/input/errors.bro | 31 +++++++++++++++++++ .../base/frameworks/input/invalidtext.bro | 31 ++++++++++++++++--- 7 files changed, 89 insertions(+), 7 deletions(-) diff --git a/src/input/ReaderBackend.cc b/src/input/ReaderBackend.cc index aa161f5d40..5f68333b3d 100644 --- a/src/input/ReaderBackend.cc +++ b/src/input/ReaderBackend.cc @@ -306,11 +306,16 @@ bool ReaderBackend::Update() if ( ! success ) DisableFrontend(); - return success; + return !disabled; // always return failure if we have been disabled in the meantime } void ReaderBackend::DisableFrontend() { + // We might already have been disabled - e.g., due to a call to error. In that + // case, ignore this... + if ( disabled ) + return; + // We also set disabled here, because there still may be other // messages queued and we will dutifully ignore these from now. disabled = true; @@ -341,6 +346,9 @@ void ReaderBackend::Error(const char* msg) { SendOut(new ReaderErrorMessage(frontend, ReaderErrorMessage::ERROR, msg)); MsgThread::Error(msg); + + // Force errors to be fatal. + DisableFrontend(); } } diff --git a/src/input/ReaderBackend.h b/src/input/ReaderBackend.h index 3f4cdee475..4897c35be0 100644 --- a/src/input/ReaderBackend.h +++ b/src/input/ReaderBackend.h @@ -191,6 +191,16 @@ public: void Info(const char* msg) override; void Warning(const char* msg) override; + + /** + * Reports an error in the child thread. For input readers, it is assumed, + * that Info and Warnings do not cause the read operation to fail (they might + * signal that, e.g., a single line was ignored). + * + * It is assumed that Errors are not recoverable. Calling the Error function + * will return the error back to scriptland and also *automatically* causes + * the current reader to be disabled and torn down. + */ void Error(const char* msg) override; protected: diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.errors/.stderr b/testing/btest/Baseline/scripts.base.frameworks.input.errors/.stderr index 02c1f56ef3..238c9a1495 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.errors/.stderr +++ b/testing/btest/Baseline/scripts.base.frameworks.input.errors/.stderr @@ -33,4 +33,9 @@ error: Input stream event6: Event has wrong number of arguments error: Input stream event7: Incompatible type for event in field 3. Need type 'int':int, got 'record':record { i:int; r:record { i:int; s:file of string; }; } error: Input stream event8: Incompatible type for event in field 5. Need type 'addr':addr, got 'string':string error: Input stream event9: Event has wrong number of arguments +error: Input stream error1: Error event's first attribute must be of type Input::EventDescription +error: Input stream error2: Error event's first attribute must be of type Input::TableDescription +error: Input stream error3: Error event's first attribute must be of type Input::EventDescription +error: Input stream error4: Error event's second attribute must be of type string +error: Input stream error5: Error event's third attribute must be of type Reporter::Level received termination signal diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.invalidtext/.stderrwithoutfirstline b/testing/btest/Baseline/scripts.base.frameworks.input.invalidtext/.stderrwithoutfirstline index 1266134c0c..04f43b38bb 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.invalidtext/.stderrwithoutfirstline +++ b/testing/btest/Baseline/scripts.base.frameworks.input.invalidtext/.stderrwithoutfirstline @@ -1,4 +1,6 @@ warning: ../input.log/Input::READER_ASCII: String 'l' contained no parseable number warning: ../input.log/Input::READER_ASCII: Could not convert line ' l' to Val. Ignoring line. +warning: ../input.log/Input::READER_ASCII: String 'l' contained no parseable number +warning: ../input.log/Input::READER_ASCII: Could not convert line ' l' to Val. Ignoring line. received termination signal >>> diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.invalidtext/out b/testing/btest/Baseline/scripts.base.frameworks.input.invalidtext/out index 83a9b5f6db..6bcb395d16 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.invalidtext/out +++ b/testing/btest/Baseline/scripts.base.frameworks.input.invalidtext/out @@ -1,5 +1,8 @@ -Event, String 'l' contained no parseable number, Reporter::WARNING -Event, Could not convert line '\x09l' to Val. Ignoring line., Reporter::WARNING +TableErrorEvent, String 'l' contained no parseable number, Reporter::WARNING +TableErrorEvent, Could not convert line '\x09l' to Val. Ignoring line., Reporter::WARNING { [] = [c=5] } +EventErrorEvent, String 'l' contained no parseable number, Reporter::WARNING +EventErrorEvent, Could not convert line '\x09l' to Val. Ignoring line., Reporter::WARNING +Event, [c=5] diff --git a/testing/btest/scripts/base/frameworks/input/errors.bro b/testing/btest/scripts/base/frameworks/input/errors.bro index 2e3dfb206f..0d0376694a 100644 --- a/testing/btest/scripts/base/frameworks/input/errors.bro +++ b/testing/btest/scripts/base/frameworks/input/errors.bro @@ -118,6 +118,31 @@ event event10(description: Input::TableDescription, tpe: Input::Event, i: Idx, c { } +# these are legit to test the error events +event event11(description: Input::EventDescription, tpe: Input::Event, v: Val) + { + } + +event errorhandler1(desc: Input::TableDescription, msg: string, level: Reporter::Level) + { + } + +event errorhandler2(desc: Input::EventDescription, msg: string, level: Reporter::Level) + { + } + +event errorhandler3(desc: string, msg: string, level: Reporter::Level) + { + } + +event errorhandler4(desc: Input::EventDescription, msg: count, level: Reporter::Level) + { + } + +event errorhandler5(desc: Input::EventDescription, msg: string, level: count) + { + } + event kill_me() { terminate(); @@ -157,5 +182,11 @@ event bro_init() Input::add_event([$source="input.log", $name="event8", $fields=Val, $ev=event8, $want_record=F]); Input::add_event([$source="input.log", $name="event9", $fields=Val, $ev=event9, $want_record=F]); + Input::add_event([$source="input.log", $name="error1", $fields=Val, $ev=event11, $want_record=T, $error_ev=errorhandler1]); + Input::add_table([$source="input.log", $name="error2", $idx=Idx, $val=Val, $destination=val_table, $error_ev=errorhandler2]); + Input::add_event([$source="input.log", $name="error3", $fields=Val, $ev=event11, $want_record=T, $error_ev=errorhandler3]); + Input::add_event([$source="input.log", $name="error4", $fields=Val, $ev=event11, $want_record=T, $error_ev=errorhandler4]); + Input::add_event([$source="input.log", $name="error5", $fields=Val, $ev=event11, $want_record=T, $error_ev=errorhandler5]); + schedule 3secs { kill_me() }; } diff --git a/testing/btest/scripts/base/frameworks/input/invalidtext.bro b/testing/btest/scripts/base/frameworks/input/invalidtext.bro index 8c55b2ddad..a2cebb304c 100644 --- a/testing/btest/scripts/base/frameworks/input/invalidtext.bro +++ b/testing/btest/scripts/base/frameworks/input/invalidtext.bro @@ -26,11 +26,23 @@ type Val: record { c: count; }; +global endcount: count = 0; + global servers: table[string] of Val = table(); event handle_our_errors(desc: Input::TableDescription, msg: string, level: Reporter::Level) { - print outfile, "Event", msg, level; + print outfile, "TableErrorEvent", msg, level; + } + +event handle_our_errors_event(desc: Input::EventDescription, msg: string, level: Reporter::Level) + { + print outfile, "EventErrorEvent", msg, level; + } + +event line(description: Input::EventDescription, tpe: Input::Event, v: Val) + { + print outfile, "Event", v; } event bro_init() @@ -38,11 +50,22 @@ event bro_init() outfile = open("../out"); # first read in the old stuff into the table... Input::add_table([$source="../input.log", $name="ssh", $error_ev=handle_our_errors, $idx=Idx, $val=Val, $destination=servers]); + Input::add_event([$source="../input.log", $name="sshevent", $error_ev=handle_our_errors_event, $fields=Val, $want_record=T, $ev=line]); } event Input::end_of_data(name: string, source:string) { - print outfile, servers; - Input::remove("ssh"); - terminate(); + ++endcount; + + if ( endcount == 1 ) + { + print outfile, servers; + Input::remove("ssh"); + } + + if ( endcount == 2 ) + { + Input::remove("sshevent"); + terminate(); + } }