Avoiding infinite loops when an error message handlers triggers errors

itself.

If an error is triggered inside one of the reporter_* handlers, the
message about that will now fall back to stderr.

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

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;

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;

src/EventHandler.cc

@@ -13,6 +13,7 @@ EventHandler::EventHandler(const char* arg_name)
 	local = 0;
 	type = 0;
 	group = 0;
+	error_handler = false;
 	enabled = true;
 	}
 

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;

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

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

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 )

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

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<std::pair<const Location*, const Location*> > locations;
 };

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

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;

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:

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;

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

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];
+}