diff --git a/src/Attr.cc b/src/Attr.cc index 73685ec8fa..61e6767f15 100644 --- a/src/Attr.cc +++ b/src/Attr.cc @@ -19,7 +19,7 @@ const char* attr_name(attr_tag t) "&persistent", "&synchronized", "&postprocessor", "&encrypt", "&match", "&disable_print_hook", "&raw_output", "&mergeable", "&priority", - "&group", "&log", "(&tracked)", + "&group", "&log", "&error_handler", "(&tracked)", }; return attr_names[int(t)]; @@ -408,6 +408,12 @@ void Attributes::CheckAttr(Attr* a) Error("&group only applicable to events"); break; + case ATTR_ERROR_HANDLER: + if ( type->Tag() != TYPE_FUNC || + ! type->AsFuncType()->IsEvent() ) + Error("&group only applicable to events"); + break; + case ATTR_LOG: if ( ! LogVal::IsCompatibleType(type) ) Error("&log applied to a type that cannot be logged"); diff --git a/src/Attr.h b/src/Attr.h index bdde98b4a7..89a81428e5 100644 --- a/src/Attr.h +++ b/src/Attr.h @@ -36,6 +36,7 @@ typedef enum { ATTR_PRIORITY, ATTR_GROUP, ATTR_LOG, + ATTR_ERROR_HANDLER, ATTR_TRACKED, // hidden attribute, tracked by NotifierRegistry #define NUM_ATTRS (int(ATTR_TRACKED) + 1) } attr_tag; diff --git a/src/Event.h b/src/Event.h index 9028ca5fab..ce498ae9cc 100644 --- a/src/Event.h +++ b/src/Event.h @@ -40,10 +40,16 @@ protected: event_serializer->Serialize(&info, handler->Name(), args); } + if ( handler->ErrorHandler() ) + reporter->BeginErrorHandler(); + handler->Call(args, no_remote); if ( obj ) // obj->EventDone(); Unref(obj); + + if ( handler->ErrorHandler() ) + reporter->EndErrorHandler(); } EventHandlerPtr handler; diff --git a/src/EventHandler.cc b/src/EventHandler.cc index 9cc5306c9c..9a06763bc0 100644 --- a/src/EventHandler.cc +++ b/src/EventHandler.cc @@ -13,6 +13,7 @@ EventHandler::EventHandler(const char* arg_name) local = 0; type = 0; group = 0; + error_handler = false; enabled = true; } diff --git a/src/EventHandler.h b/src/EventHandler.h index af7931c3c2..1d8a265219 100644 --- a/src/EventHandler.h +++ b/src/EventHandler.h @@ -39,6 +39,11 @@ public: void SetUsed() { used = true; } bool Used() { return used; } + // Handlers marked as error handlers will not be called recursively to + // avoid infinite loops if they trigger a similar error themselves. + void SetErrorHandler() { error_handler = true; } + bool ErrorHandler() { return error_handler; } + const char* Group() { return group; } void SetGroup(const char* arg_group) { group = copy_string(arg_group); } @@ -57,6 +62,7 @@ private: FuncType* type; bool used; // this handler is indeed used somewhere bool enabled; + bool error_handler; // this handler reports error messages. declare(List, SourceID); typedef List(SourceID) receiver_list; diff --git a/src/EventRegistry.cc b/src/EventRegistry.cc index 109ad51a3f..f5691ab448 100644 --- a/src/EventRegistry.cc +++ b/src/EventRegistry.cc @@ -96,6 +96,15 @@ void EventRegistry::SetGroup(const char* name, const char* group) eh->SetGroup(group); } +void EventRegistry::SetErrorHandler(const char* name) + { + EventHandler* eh = Lookup(name); + if ( ! eh ) + reporter->InternalError("unknown event handler in SetErrorHandler()"); + + eh->SetErrorHandler(); + } + void EventRegistry::EnableGroup(const char* group, bool enable) { IterCookie* c = handlers.InitForIteration(); diff --git a/src/EventRegistry.h b/src/EventRegistry.h index 980b8ea83f..bd9e0cd185 100644 --- a/src/EventRegistry.h +++ b/src/EventRegistry.h @@ -31,6 +31,11 @@ public: // Associates a group with the given event. void SetGroup(const char* name, const char* group); + // Marks a handler as handling errors. Error handler will not be called + // recursively to avoid infinite loops in case they trigger an error + // themselves. + void SetErrorHandler(const char* name); + // Enable/disable all members of the group. void EnableGroup(const char* group, bool enable); diff --git a/src/ID.cc b/src/ID.cc index 09c5504f9d..2decef725f 100644 --- a/src/ID.cc +++ b/src/ID.cc @@ -223,6 +223,7 @@ void ID::UpdateValAttrs() if ( Type()->Tag() == TYPE_FUNC ) { Attr* attr = attrs->FindAttr(ATTR_GROUP); + if ( attr ) { Val* group = attr->AttrExpr()->ExprVal(); @@ -234,6 +235,11 @@ void ID::UpdateValAttrs() Error("&group attribute takes string"); } } + + attr = attrs->FindAttr(ATTR_ERROR_HANDLER); + + if ( attr ) + event_registry->SetErrorHandler(Name()); } if ( Type()->Tag() == TYPE_RECORD ) diff --git a/src/Reporter.cc b/src/Reporter.cc index 08e27ae048..fe2dee6592 100644 --- a/src/Reporter.cc +++ b/src/Reporter.cc @@ -25,6 +25,7 @@ Reporter::Reporter() { errors = 0; via_events = false; + in_error_handler = 0; openlog("bro", 0, LOG_LOCAL5); } @@ -246,7 +247,7 @@ void Reporter::DoLog(const char* prefix, EventHandlerPtr event, FILE* out, Conne FatalError("out of memory in Reporter"); } - if ( event && via_events ) + if ( event && via_events && ! in_error_handler ) { val_list* vl = new val_list; @@ -304,8 +305,6 @@ void Reporter::DoLog(const char* prefix, EventHandlerPtr event, FILE* out, Conne fprintf(out, s.c_str()); } - - if ( alloced ) free(alloced); } diff --git a/src/Reporter.h b/src/Reporter.h index 00c414808f..d3b728113a 100644 --- a/src/Reporter.h +++ b/src/Reporter.h @@ -79,6 +79,12 @@ public: void PopLocation() { locations.pop_back(); } + // Signals that we're entering processing an error handler event. + void BeginErrorHandler() { ++in_error_handler; } + + // Signals that we're done processing an error handler event. + void EndErrorHandler() { --in_error_handler; } + private: void DoLog(const char* prefix, EventHandlerPtr event, FILE* out, Connection* conn, val_list* addl, bool location, bool time, const char* fmt, va_list ap); @@ -87,6 +93,7 @@ private: int errors; bool via_events; + int in_error_handler; std::list > locations; }; diff --git a/src/builtin-func.y b/src/builtin-func.y index c842385627..fd40613236 100644 --- a/src/builtin-func.y +++ b/src/builtin-func.y @@ -460,7 +460,8 @@ const_def: TOK_CONST opt_ws TOK_ID opt_ws ':' opt_ws TOK_ID opt_ws ';' /* Currently support only boolean and string values */ -opt_attr_init: '=' opt_ws TOK_BOOL opt_ws +opt_attr_init: /* nothing */ + | '=' opt_ws TOK_BOOL opt_ws { fprintf(fp_bro_init, "=%s%c%s", $2, ($3) ? 'T' : 'F', $4); } diff --git a/src/event.bif b/src/event.bif index 8065a87530..c46a1ff50a 100644 --- a/src/event.bif +++ b/src/event.bif @@ -479,7 +479,8 @@ event rotate_size%(f: file%); event netflow_v5_header%(h: nf_v5_header%); event netflow_v5_record%(r: nf_v5_record%); -# Different types of reporter messages. -event reporter_message%(t: time, msg: string, location: string%); -event reporter_warning%(t: time, msg: string, location: string%); -event reporter_error%(t: time, msg: string, location: string%); +# Different types of reporter messages. These won't be called +# recursively. +event reporter_message%(t: time, msg: string, location: string%) &error_handler; +event reporter_warning%(t: time, msg: string, location: string%) &error_handler; +event reporter_error%(t: time, msg: string, location: string%) &error_handler; diff --git a/src/parse.y b/src/parse.y index 132af965ad..8ff5944901 100644 --- a/src/parse.y +++ b/src/parse.y @@ -3,7 +3,7 @@ // See the file "COPYING" in the main distribution directory for copyright. %} -%expect 85 +%expect 88 %token TOK_ADD TOK_ADD_TO TOK_ADDR TOK_ANY %token TOK_ATENDIF TOK_ATELSE TOK_ATIF TOK_ATIFDEF TOK_ATIFNDEF @@ -24,7 +24,7 @@ %token TOK_ATTR_EXPIRE_CREATE TOK_ATTR_EXPIRE_READ TOK_ATTR_EXPIRE_WRITE %token TOK_ATTR_PERSISTENT TOK_ATTR_SYNCHRONIZED %token TOK_ATTR_DISABLE_PRINT_HOOK TOK_ATTR_RAW_OUTPUT TOK_ATTR_MERGEABLE -%token TOK_ATTR_PRIORITY TOK_ATTR_GROUP TOK_ATTR_LOG +%token TOK_ATTR_PRIORITY TOK_ATTR_GROUP TOK_ATTR_LOG TOK_ATTR_ERROR_HANDLER %token TOK_DEBUG @@ -1320,6 +1320,8 @@ attr: { $$ = new Attr(ATTR_GROUP, $3); } | TOK_ATTR_LOG { $$ = new Attr(ATTR_LOG); } + | TOK_ATTR_ERROR_HANDLER + { $$ = new Attr(ATTR_ERROR_HANDLER); } ; stmt: diff --git a/src/scan.l b/src/scan.l index a5ea9afc88..862f4c729d 100644 --- a/src/scan.l +++ b/src/scan.l @@ -290,6 +290,7 @@ when return TOK_WHEN; &disable_print_hook return TOK_ATTR_DISABLE_PRINT_HOOK; &raw_output return TOK_ATTR_RAW_OUTPUT; &encrypt return TOK_ATTR_ENCRYPT; +&error_handler return TOK_ATTR_ERROR_HANDLER; &expire_func return TOK_ATTR_EXPIRE_FUNC; &group return TOK_ATTR_GROUP; &log return TOK_ATTR_LOG; diff --git a/testing/btest/Baseline/core.reporter-error-in-handler/output b/testing/btest/Baseline/core.reporter-error-in-handler/output new file mode 100644 index 0000000000..d155222577 --- /dev/null +++ b/testing/btest/Baseline/core.reporter-error-in-handler/output @@ -0,0 +1,3 @@ +[script] reporter_error: no such index (a[1]) [/da/home/robin/bro/topic/testing/btest/.tmp/core.reporter-error-in-handler/reporter-error-in-handler.bro, line 28] [0.000000] +1st error printed on script level +error in /da/home/robin/bro/topic/testing/btest/.tmp/core.reporter-error-in-handler/reporter-error-in-handler.bro, line 22: no such index (a[2]) diff --git a/testing/btest/core/reporter-error-in-handler.bro b/testing/btest/core/reporter-error-in-handler.bro new file mode 100644 index 0000000000..c4a21d5902 --- /dev/null +++ b/testing/btest/core/reporter-error-in-handler.bro @@ -0,0 +1,29 @@ +# +# This test procudes a recursive error: the error handler is itself broken. Rather +# than looping indefinitly, the error inside the handler should reported to stderr. +# +# @TEST-EXEC: bro %INPUT >output 2>&1 +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff output + +global a: table[count] of count; + +global c = 0; + +event reporter_error(t: time, msg: string, location: string) +{ + c += 1; + + if ( c > 1 ) + print "FAILED: 2nd error reported to script as well."; + + else + { + print "1st error printed on script level"; + print a[2]; + } +} + +event bro_init() +{ + print a[1]; +}