Merge pull request #9947 from Icinga/2141morebackport

Truncate too big notification command lines, fix GelfWriter deadlock and return 503 in /v1/console/* during reload

lib/methods/pluginnotificationtask.cpp

@@ -12,6 +12,20 @@
 #include "base/process.hpp"
 #include "base/convert.hpp"
 
+#ifdef __linux__
+#	include <linux/binfmts.h>
+#	include <unistd.h>
+
+#	ifndef PAGE_SIZE
+//		MAX_ARG_STRLEN is a multiple of PAGE_SIZE which is missing
+#		define PAGE_SIZE getpagesize()
+#	endif /* PAGE_SIZE */
+
+// Make e.g. the $host.output$ itself even 10% shorter to leave enough room
+// for e.g. --host-output= as in --host-output=$host.output$, but without int overflow
+const static auto l_MaxOutLen = MAX_ARG_STRLEN - MAX_ARG_STRLEN / 10u;
+#endif /* __linux__ */
+
 using namespace icinga;
 
 REGISTER_FUNCTION_NONCONST(Internal, PluginNotification, &PluginNotificationTask::ScriptFunc, "notification:user:cr:itype:author:comment:resolvedMacros:useResolvedMacros");
@@ -33,7 +47,11 @@ void PluginNotificationTask::ScriptFunc(const Notification::Ptr& notification,
 	Dictionary::Ptr notificationExtra = new Dictionary({
 		{ "type", Notification::NotificationTypeToStringCompat(type) }, //TODO: Change that to our types.
 		{ "author", author },
+#ifdef __linux__
+		{ "comment", comment.SubStr(0, l_MaxOutLen) }
+#else /* __linux__ */
 		{ "comment", comment }
+#endif /* __linux__ */
 	});
 
 	Host::Ptr host;
@@ -48,8 +66,35 @@ void PluginNotificationTask::ScriptFunc(const Notification::Ptr& notification,
 	resolvers.emplace_back("user", user);
 	resolvers.emplace_back("notification", notificationExtra);
 	resolvers.emplace_back("notification", notification);
-	if (service)
+
+	if (service) {
+#ifdef __linux__
+		auto cr (service->GetLastCheckResult());
+
+		if (cr) {
+			auto output (cr->GetOutput());
+
+			if (output.GetLength() > l_MaxOutLen) {
+				resolvers.emplace_back("service", new Dictionary({{"output", output.SubStr(0, l_MaxOutLen)}}));
+			}
+		}
+#endif /* __linux__ */
+
 		resolvers.emplace_back("service", service);
+	}
+
+#ifdef __linux__
+	auto hcr (host->GetLastCheckResult());
+
+	if (hcr) {
+		auto output (hcr->GetOutput());
+
+		if (output.GetLength() > l_MaxOutLen) {
+			resolvers.emplace_back("host", new Dictionary({{"output", output.SubStr(0, l_MaxOutLen)}}));
+		}
+	}
+#endif /* __linux__ */
+
 	resolvers.emplace_back("host", host);
 	resolvers.emplace_back("command", commandObj);
 

lib/perfdata/gelfwriter.cpp

@@ -114,18 +114,17 @@ void GelfWriter::Pause()
 
 	m_ReconnectTimer->Stop(true);
 
-	try {
-		ReconnectInternal();
-	} catch (const std::exception&) {
-		Log(LogInformation, "GelfWriter")
-			<< "'" << GetName() << "' paused. Unable to connect, not flushing buffers. Data may be lost on reload.";
-
-		ObjectImpl<GelfWriter>::Pause();
-		return;
-	}
+	m_WorkQueue.Enqueue([this]() {
+		try {
+			ReconnectInternal();
+		} catch (const std::exception&) {
+			Log(LogInformation, "GelfWriter")
+				<< "Unable to connect, not flushing buffers. Data may be lost.";
+		}
+	}, PriorityImmediate);
 
+	m_WorkQueue.Enqueue([this]() { DisconnectInternal(); }, PriorityLow);
 	m_WorkQueue.Join();
-	DisconnectInternal();
 
 	Log(LogInformation, "GelfWriter")
 		<< "'" << GetName() << "' paused.";
@@ -513,8 +512,6 @@ void GelfWriter::SendLogMessage(const Checkable::Ptr& checkable, const String& g
 
 	String log = msgbuf.str();
 
-	ObjectLock olock(this);
-
 	if (!GetConnected())
 		return;
 

lib/remote/consolehandler.cpp

@@ -1,5 +1,6 @@
 /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */
 
+#include "remote/configobjectslock.hpp"
 #include "remote/consolehandler.hpp"
 #include "remote/httputility.hpp"
 #include "remote/filterutility.hpp"
@@ -88,6 +89,13 @@ bool ConsoleHandler::HandleRequest(
 
 	bool sandboxed = HttpUtility::GetLastParameter(params, "sandboxed");
 
+	ConfigObjectsSharedLock lock (std::try_to_lock);
+
+	if (!lock) {
+		HttpUtility::SendJsonError(response, params, 503, "Icinga is reloading.");
+		return true;
+	}
+
 	if (methodName == "execute-script")
 		return ExecuteScriptHelper(request, response, params, command, session, sandboxed);
 	else if (methodName == "auto-complete-script")

test/CMakeLists.txt

@@ -32,12 +32,14 @@ set(base_test_SOURCES
   icinga-macros.cpp
   icinga-notification.cpp
   icinga-perfdata.cpp
+  methods-pluginnotificationtask.cpp
   remote-configpackageutility.cpp
   remote-url.cpp
   ${base_OBJS}
   $<TARGET_OBJECTS:config>
   $<TARGET_OBJECTS:remote>
   $<TARGET_OBJECTS:icinga>
+  $<TARGET_OBJECTS:methods>
 )
 
 if(ICINGA2_UNITY_BUILD)
@@ -199,6 +201,7 @@ add_boost_test(base
     icinga_perfdata/multi
     icinga_perfdata/scientificnotation
     icinga_perfdata/parse_edgecases
+    methods_pluginnotificationtask/truncate_long_output
     remote_configpackageutility/ValidateName
     remote_url/id_and_path
     remote_url/parameters

test/methods-pluginnotificationtask.cpp

@@ -0,0 +1,88 @@
+/* Icinga 2 | (c) 2023 Icinga GmbH | GPLv2+ */
+
+#include "base/array.hpp"
+#include "icinga/checkresult.hpp"
+#include "icinga/host.hpp"
+#include "icinga/notification.hpp"
+#include "icinga/notificationcommand.hpp"
+#include "icinga/service.hpp"
+#include "icinga/user.hpp"
+#include "methods/pluginnotificationtask.hpp"
+#include <BoostTestTargetConfig.h>
+#include <future>
+
+using namespace icinga;
+
+BOOST_AUTO_TEST_SUITE(methods_pluginnotificationtask)
+
+BOOST_AUTO_TEST_CASE(truncate_long_output)
+{
+#ifdef __linux__
+	Host::Ptr h = new Host();
+	CheckResult::Ptr hcr = new CheckResult();
+	CheckResult::Ptr scr = new CheckResult();
+	Service::Ptr s = new Service();
+	User::Ptr u = new User();
+	NotificationCommand::Ptr nc = new NotificationCommand();
+	Notification::Ptr n = new Notification();
+	String placeHolder (1024 * 1024, 'x');
+	std::promise<String> promise;
+	auto future (promise.get_future());
+
+	hcr->SetOutput("H" + placeHolder + "h", true);
+	scr->SetOutput("S" + placeHolder + "s", true);
+
+	h->SetName("example.com", true);
+	h->SetLastCheckResult(hcr, true);
+	h->Register();
+
+	s->SetHostName("example.com", true);
+	s->SetShortName("disk", true);
+	s->SetLastCheckResult(scr, true);
+	s->OnAllConfigLoaded(); // link Host
+
+	nc->SetCommandLine(
+		new Array({
+			"echo",
+			"host_output=$host.output$",
+			"service_output=$service.output$",
+			"notification_comment=$notification.comment$",
+			"output=$output$",
+			"comment=$comment$"
+		}),
+		true
+	);
+
+	nc->SetName("mail", true);
+	nc->Register();
+
+	n->SetFieldByName("host_name", "example.com", false, DebugInfo());
+	n->SetFieldByName("service_name", "disk", false, DebugInfo());
+	n->SetFieldByName("command", "mail", false, DebugInfo());
+	n->OnAllConfigLoaded(); // link Service
+
+	Checkable::ExecuteCommandProcessFinishedHandler = [&promise](const Value&, const ProcessResult& pr) {
+		promise.set_value(pr.Output);
+	};
+
+	PluginNotificationTask::ScriptFunc(n, u, nullptr, NotificationCustom, "jdoe", "C" + placeHolder + "c", nullptr, false);
+	future.wait();
+
+	Checkable::ExecuteCommandProcessFinishedHandler = nullptr;
+	h->Unregister();
+	nc->Unregister();
+
+	auto output (future.get());
+
+	BOOST_CHECK(output.Contains("host_output=Hx"));
+	BOOST_CHECK(!output.Contains("xh"));
+	BOOST_CHECK(output.Contains("x service_output=Sx"));
+	BOOST_CHECK(!output.Contains("xs"));
+	BOOST_CHECK(output.Contains("x notification_comment=Cx"));
+	BOOST_CHECK(!output.Contains("xc"));
+	BOOST_CHECK(output.Contains("x output=Sx"));
+	BOOST_CHECK(output.Contains("x comment=Cx"));
+#endif /* __linux__ */
+}
+
+BOOST_AUTO_TEST_SUITE_END()