logging/Manager: Fix crash for rotation format function not returning

While working on a rotation format function, ran into Zeek crashing
when not returning a value from it, fix and recover the same way as
for scripting errors.

scripts/base/frameworks/logging/main.zeek

@@ -163,9 +163,6 @@ export {
 	};
 
 	## A function that one may use to customize log file rotation paths.
-	## Note that the "fname" field of the *ri* argument is always an
-	## empty string for the purpose of this function call (i.e. the full
-	## file name is not determined yet).
 	const rotation_format_func: function(ri: RotationFmtInfo): RotationPath &redef;
 
 	## Default naming format for timestamps embedded into filenames.

src/logging/Manager.cc

@@ -1561,10 +1561,19 @@ std::string Manager::FormatRotationPath(EnumValPtr writer, std::string_view path
 	ri->Assign<FuncVal>(5, std::move(postprocessor));
 
 	std::string rval;
+	ValPtr res = Val::nil;
 
 	try
 		{
-		auto res = rotation_format_func->Invoke(ri);
+		res = rotation_format_func->Invoke(ri);
+		}
+	catch ( InterpreterException& e )
+		{
+		// Will have logged something, res continues to be nil
+		}
+
+	if ( res )
+		{
 		auto rp_val = res->AsRecordVal();
 		auto dir_val = rp_val->GetFieldOrDefault(0);
 		auto prefix = rp_val->GetFieldAs<StringVal>(1)->CheckString();
@@ -1590,7 +1599,7 @@ std::string Manager::FormatRotationPath(EnumValPtr writer, std::string_view path
 		else
 			rval = util::fmt("%s/%s", dir, prefix);
 		}
-	catch ( InterpreterException& e )
+	else
 		{
 		auto rot_str = format_rotation_time_fallback((time_t)open);
 		rval = util::fmt("%.*s-%s", static_cast<int>(path.size()), path.data(), rot_str.data());

testing/btest/Baseline/scripts.base.frameworks.logging.rotate-custom-fmt-func-bad/.stderr

@@ -0,0 +1,5 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+1299481205.000000 warning: non-void function returning without a value: Test::my_rotation_format_func
+1299481205.000000 error: Failed to call Log::rotation_format_func for path test continuing with rotation to: ./test-11-03-07_06.00.05
+1299495605.000000 expression error in <...>/rotate-custom-fmt-func-bad.zeek, line 34: division by zero (Test::c / 0)
+1299495605.000000 error: Failed to call Log::rotation_format_func for path test continuing with rotation to: ./test-11-03-07_10.00.05

testing/btest/Baseline/scripts.base.frameworks.logging.rotate-custom-fmt-func-bad/out

@@ -0,0 +1,11 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+test-1.log
+test-10.log
+test-11-03-07_06.00.05.log
+test-11-03-07_10.00.05.log
+test-2.log
+test-3.log
+test-5.log
+test-6.log
+test-7.log
+test-9.log

testing/btest/scripts/base/frameworks/logging/rotate-custom-fmt-func-bad.zeek

@@ -0,0 +1,50 @@
+# @TEST-DOC: A non-behaving rotation_format_func could cause segfaults by not returning a value. Cover this.
+# @TEST-EXEC: zeek -b -r ${TRACES}/rotation.trace %INPUT
+# @TEST-EXEC: ls test*log | sort >> out
+# @TEST-EXEC: btest-diff out
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr
+
+module Test;
+
+export {
+	redef enum Log::ID += { LOG };
+
+	type Log: record {
+		t: time;
+		id: conn_id;
+	} &log;
+}
+
+global rotation_count: table[string] of count &default = 0;
+
+# Custom Log::rotation_format_func that triggers two errors:
+# * Returning nil/no value
+# * Divide by zero
+#
+# The logging Manager should cover either case and fall back
+# to a fixed format.
+function my_rotation_format_func(ri: Log::RotationFmtInfo): Log::RotationPath
+	{
+	local c = rotation_count[ri$path] + 1;
+	rotation_count[ri$path] = c;
+
+	if ( ri$path == "test" && c == 4 )
+		print "do nothing";  # returns nil
+	else if ( ri$path == "test" && c == 8 )
+		return [$file_basename=fmt("%s-%s", ri$path, c / 0)];  # divide by zero
+	else
+		return [$file_basename=fmt("%s-%s", ri$path, c)];
+	}
+
+redef Log::default_rotation_interval = 1hr;
+redef Log::rotation_format_func = my_rotation_format_func;
+
+event zeek_init()
+	{
+	Log::create_stream(Test::LOG, [$columns=Log]);
+	}
+
+event new_connection(c: connection)
+	{
+	Log::write(Test::LOG, [$t=network_time(), $id=c$id]);
+	}