From 5b5efab847e6af682f979b5d57c33db7ebc1474d Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Thu, 7 Jan 2021 17:23:29 +0100 Subject: [PATCH 1/2] Properly handle service downtime referencing a deleted host Only two out of three cases were handled properly by the code: host downtimes referencing a deleted host and service downtimes referencing a deleted service worked fine. However, if a service downtime references a deleted host, `Host::GetByName()` returns `nullptr` which isn't accounted for. Use `Service::GetByNamePair()` instead as this performs a check for the host being null internally. --- lib/icinga/downtime.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/icinga/downtime.cpp b/lib/icinga/downtime.cpp index 1364f722c..2bdec256c 100644 --- a/lib/icinga/downtime.cpp +++ b/lib/icinga/downtime.cpp @@ -75,12 +75,10 @@ void Downtime::OnAllConfigLoaded() { ObjectImpl::OnAllConfigLoaded(); - Host::Ptr host = Host::GetByName(GetHostName()); - if (GetServiceName().IsEmpty()) - m_Checkable = host; + m_Checkable = Host::GetByName(GetHostName()); else - m_Checkable = host->GetServiceByShortName(GetServiceName()); + m_Checkable = Service::GetByNamePair(GetHostName(), GetServiceName()); if (!m_Checkable) BOOST_THROW_EXCEPTION(ScriptError("Downtime '" + GetName() + "' references a host/service which doesn't exist.", GetDebugInfo())); From 77427bedae21e48aa252cd26df239f0b68f8b4d3 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Thu, 7 Jan 2021 17:48:01 +0100 Subject: [PATCH 2/2] AddDowntime: return Downtime::Ptr instead of String containing the name At numerous places in the code, something like this is performed: String name = Downtime::AddDowntime(...); Downtime::Ptr downtime = Downtime::GetByName(name); However, `downtime` can be a `nullptr` after this as it is possible that the downtime is deleted in between. This commit changes the return type of `Downtime::AddDowntime` to return a Downtime::Ptr instead of the full name of the downtime. `AddDowntime` performs the very same `GetByName()` operation internally, but handles the `nullptr` case correctly and throws an exception. --- lib/icinga/apiactions.cpp | 20 ++++++++------------ lib/icinga/downtime.cpp | 4 ++-- lib/icinga/downtime.hpp | 2 +- lib/icinga/externalcommandprocessor.cpp | 4 ++-- lib/icinga/scheduleddowntime.cpp | 9 ++++----- 5 files changed, 17 insertions(+), 22 deletions(-) diff --git a/lib/icinga/apiactions.cpp b/lib/icinga/apiactions.cpp index 757b37eca..85f57496c 100644 --- a/lib/icinga/apiactions.cpp +++ b/lib/icinga/apiactions.cpp @@ -351,10 +351,9 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object, } } - String downtimeName = Downtime::AddDowntime(checkable, author, comment, startTime, endTime, + Downtime::Ptr downtime = Downtime::AddDowntime(checkable, author, comment, startTime, endTime, fixed, triggerName, duration); - - Downtime::Ptr downtime = Downtime::GetByName(downtimeName); + String downtimeName = downtime->GetName(); Dictionary::Ptr additional = new Dictionary({ { "name", downtimeName }, @@ -374,10 +373,9 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object, Log(LogNotice, "ApiActions") << "Creating downtime for service " << hostService->GetName() << " on host " << host->GetName(); - String serviceDowntimeName = Downtime::AddDowntime(hostService, author, comment, startTime, endTime, + Downtime::Ptr serviceDowntime = Downtime::AddDowntime(hostService, author, comment, startTime, endTime, fixed, triggerName, duration); - - Downtime::Ptr serviceDowntime = Downtime::GetByName(serviceDowntimeName); + String serviceDowntimeName = serviceDowntime->GetName(); serviceDowntimes.push_back(new Dictionary({ { "name", serviceDowntimeName }, @@ -405,14 +403,13 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object, Log(LogNotice, "ApiActions") << "Scheduling downtime for child object " << child->GetName(); - String childDowntimeName = Downtime::AddDowntime(child, author, comment, startTime, endTime, + Downtime::Ptr childDowntime = Downtime::AddDowntime(child, author, comment, startTime, endTime, fixed, triggerName, duration); + String childDowntimeName = childDowntime->GetName(); Log(LogNotice, "ApiActions") << "Add child downtime '" << childDowntimeName << "'."; - Downtime::Ptr childDowntime = Downtime::GetByName(childDowntimeName); - Dictionary::Ptr childAdditional = new Dictionary({ { "name", childDowntimeName }, { "legacy_id", childDowntime->GetLegacyId() } @@ -430,10 +427,9 @@ Dictionary::Ptr ApiActions::ScheduleDowntime(const ConfigObject::Ptr& object, Log(LogNotice, "ApiActions") << "Creating downtime for service " << hostService->GetName() << " on child host " << host->GetName(); - String serviceDowntimeName = Downtime::AddDowntime(hostService, author, comment, startTime, endTime, + Downtime::Ptr serviceDowntime = Downtime::AddDowntime(hostService, author, comment, startTime, endTime, fixed, triggerName, duration); - - Downtime::Ptr serviceDowntime = Downtime::GetByName(serviceDowntimeName); + String serviceDowntimeName = serviceDowntime->GetName(); childServiceDowntimes.push_back(new Dictionary({ { "name", serviceDowntimeName }, diff --git a/lib/icinga/downtime.cpp b/lib/icinga/downtime.cpp index 2bdec256c..9c618b786 100644 --- a/lib/icinga/downtime.cpp +++ b/lib/icinga/downtime.cpp @@ -206,7 +206,7 @@ int Downtime::GetNextDowntimeID() return l_NextDowntimeID; } -String Downtime::AddDowntime(const Checkable::Ptr& checkable, const String& author, +Downtime::Ptr Downtime::AddDowntime(const Checkable::Ptr& checkable, const String& author, const String& comment, double startTime, double endTime, bool fixed, const String& triggeredBy, double duration, const String& scheduledDowntime, const String& scheduledBy, @@ -302,7 +302,7 @@ String Downtime::AddDowntime(const Checkable::Ptr& checkable, const String& auth << "' and '" << Utility::FormatDateTime("%Y-%m-%d %H:%M:%S", endTime) << "', author: '" << author << "', " << (fixed ? "fixed" : "flexible with " + Convert::ToString(duration) + "s duration"); - return fullName; + return downtime; } void Downtime::RemoveDowntime(const String& id, bool cancelled, bool expired, const MessageOrigin::Ptr& origin) diff --git a/lib/icinga/downtime.hpp b/lib/icinga/downtime.hpp index c33d9017d..6aac86148 100644 --- a/lib/icinga/downtime.hpp +++ b/lib/icinga/downtime.hpp @@ -45,7 +45,7 @@ public: static int GetNextDowntimeID(); - static String AddDowntime(const intrusive_ptr& checkable, const String& author, + static Ptr AddDowntime(const intrusive_ptr& checkable, const String& author, const String& comment, double startTime, double endTime, bool fixed, const String& triggeredBy, double duration, const String& scheduledDowntime = String(), const String& scheduledBy = String(), const String& id = String(), diff --git a/lib/icinga/externalcommandprocessor.cpp b/lib/icinga/externalcommandprocessor.cpp index 9f088ae67..e966794ef 100644 --- a/lib/icinga/externalcommandprocessor.cpp +++ b/lib/icinga/externalcommandprocessor.cpp @@ -1029,7 +1029,7 @@ void ExternalCommandProcessor::ScheduleAndPropagateTriggeredHostDowntime(double, Log(LogNotice, "ExternalCommandProcessor") << "Creating downtime for host " << host->GetName(); - String parentDowntime = Downtime::AddDowntime(host, arguments[6], arguments[7], + Downtime::Ptr parentDowntime = Downtime::AddDowntime(host, arguments[6], arguments[7], Convert::ToDouble(arguments[1]), Convert::ToDouble(arguments[2]), Convert::ToBool(is_fixed), triggeredBy, Convert::ToDouble(arguments[5])); @@ -1045,7 +1045,7 @@ void ExternalCommandProcessor::ScheduleAndPropagateTriggeredHostDowntime(double, (void) Downtime::AddDowntime(child, arguments[6], arguments[7], Convert::ToDouble(arguments[1]), Convert::ToDouble(arguments[2]), - Convert::ToBool(is_fixed), parentDowntime, Convert::ToDouble(arguments[5])); + Convert::ToBool(is_fixed), parentDowntime->GetName(), Convert::ToDouble(arguments[5])); } } diff --git a/lib/icinga/scheduleddowntime.cpp b/lib/icinga/scheduleddowntime.cpp index 6642841f4..c18f4f045 100644 --- a/lib/icinga/scheduleddowntime.cpp +++ b/lib/icinga/scheduleddowntime.cpp @@ -253,11 +253,10 @@ void ScheduledDowntime::CreateNextDowntime() return; } - String downtimeName = Downtime::AddDowntime(GetCheckable(), GetAuthor(), GetComment(), + Downtime::Ptr downtime = Downtime::AddDowntime(GetCheckable(), GetAuthor(), GetComment(), segment.first, segment.second, GetFixed(), String(), GetDuration(), GetName(), GetName()); - - Downtime::Ptr downtime = Downtime::GetByName(downtimeName); + String downtimeName = downtime->GetName(); int childOptions = Downtime::ChildOptionsFromValue(GetChildOptions()); if (childOptions > 0) { @@ -275,11 +274,11 @@ void ScheduledDowntime::CreateNextDowntime() Log(LogNotice, "ScheduledDowntime") << "Scheduling downtime for child object " << child->GetName(); - String childDowntimeName = Downtime::AddDowntime(child, GetAuthor(), GetComment(), + Downtime::Ptr childDowntime = Downtime::AddDowntime(child, GetAuthor(), GetComment(), segment.first, segment.second, GetFixed(), triggerName, GetDuration(), GetName(), GetName()); Log(LogNotice, "ScheduledDowntime") - << "Add child downtime '" << childDowntimeName << "'."; + << "Add child downtime '" << childDowntime->GetName() << "'."; } } }