Escape semicolons in node names

fixes #312
Icinga · Aug 7, 2023 · 44ecdb5 · 44ecdb5
1 parent be1f56b
commit 44ecdb5
Show file tree

Hide file tree

Showing 20 changed files with 222 additions and 62 deletions.
diff --git a/application/forms/AddNodeForm.php b/application/forms/AddNodeForm.php
@@ -3,6 +3,7 @@
 namespace Icinga\Module\Businessprocess\Forms;
 
 use Exception;
+use Icinga\Module\Businessprocess\BpConfig;
 use Icinga\Module\Businessprocess\BpNode;
 use Icinga\Module\Businessprocess\Common\EnumList;
 use Icinga\Module\Businessprocess\Common\Sort;
@@ -498,10 +499,13 @@ public function onSuccess()
             case 'new-process':
                 $properties = $this->getValues();
                 unset($properties['name']);
+                if (! $properties['alias']) {
+                    unset($properties['alias']);
+                }
                 if ($this->hasParentNode()) {
                     $properties['parentName'] = $this->parent->getName();
                 }
-                $changes->createNode($this->getValue('name'), $properties);
+                $changes->createNode(BpConfig::escapeName($this->getValue('name')), $properties);
                 break;
         }
 

diff --git a/application/forms/EditNodeForm.php b/application/forms/EditNodeForm.php
@@ -29,9 +29,9 @@ class EditNodeForm extends BpConfigBaseForm
 
     public function setup()
     {
-        $this->host = substr($this->getNode()->getName(), 0, strpos($this->getNode()->getName(), ';'));
-        if ($this->isService()) {
-            $this->service = substr($this->getNode()->getName(), strpos($this->getNode()->getName(), ';') + 1);
+        [$this->host, $suffix] = BpConfig::splitNodeName($this->getNode()->getName());
+        if ($suffix !== 'Hoststatus') {
+            $this->service = $suffix;
         }
 
         $view = $this->getView();

diff --git a/library/Businessprocess/BpConfig.php b/library/Businessprocess/BpConfig.php
@@ -471,7 +471,7 @@ public function createService($host, $service)
             )
         );
         $node->setBpConfig($this);
-        $this->nodes[$host . ';' . $service] = $node;
+        $this->nodes[$node->getName()] = $node;
         $this->hosts[$host] = true;
         return $node;
     }
@@ -480,7 +480,7 @@ public function createHost($host)
     {
         $node = new HostNode((object) array('hostname' => $host));
         $node->setBpConfig($this);
-        $this->nodes[$host . ';Hoststatus'] = $node;
+        $this->nodes[$node->getName()] = $node;
         $this->hosts[$host] = true;
         return $node;
     }
@@ -642,15 +642,13 @@ public function getNode($name)
 
         // Fallback: if it is a service, create an empty one:
         $this->warn(sprintf('The node "%s" doesn\'t exist', $name));
-        $pos = strpos($name, ';');
-        if ($pos !== false) {
-            $host = substr($name, 0, $pos);
-            $service = substr($name, $pos + 1);
-            // TODO: deactivated, this scares me, test it
-            if ($service === 'Hoststatus') {
-                return $this->createHost($host);
+
+        [$name, $suffix] = self::splitNodeName($name);
+        if ($suffix !== null) {
+            if ($suffix === 'Hoststatus') {
+                return $this->createHost($name);
             } else {
-                return $this->createService($host, $service);
+                return $this->createService($name, $suffix);
             }
         }
 
@@ -1015,4 +1013,61 @@ public function toArray($flat = false)
 
         return $data;
     }
+
+    /**
+     * Escape the given node name
+     *
+     * @param string $name
+     *
+     * @return string
+     */
+    public static function escapeName(string $name): string
+    {
+        return preg_replace('/((?<!\\\\);)/', '\\\\$1', $name);
+    }
+
+    /**
+     * Unescape the given node name
+     *
+     * @param string $name
+     *
+     * @return string
+     */
+    public static function unescapeName(string $name): string
+    {
+        return str_replace('\\;', ';', $name);
+    }
+
+    /**
+     * Join the given two name parts together
+     *
+     * The used separator is the semicolon. If a semicolon exists in either part, it's escaped.
+     *
+     * @param string $name
+     * @param ?string $suffix
+     *
+     * @return string
+     */
+    public static function joinNodeName(string $name, ?string $suffix = null): string
+    {
+        return self::escapeName($name) . ($suffix ? ";$suffix" : '');
+    }
+
+    /**
+     * Split the given node name into two parts
+     *
+     * The first part is always a string, with any semicolons unescaped.
+     * The second part may be null or a string otherwise.
+     *
+     * @param string $nodeName
+     *
+     * @return array
+     */
+    public static function splitNodeName(string $nodeName): array
+    {
+        $parts = preg_split('/(?<!\\\\);/', $nodeName, 2);
+        $parts[0] = self::unescapeName($parts[0]);
+
+        return array_pad($parts, 2, null);
+    }
 }
diff --git a/library/Businessprocess/BpNode.php b/library/Businessprocess/BpNode.php
@@ -56,7 +56,8 @@ class BpNode extends Node
 
     public function __construct($object)
     {
-        $this->name = $object->name;
+        $this->name = BpConfig::escapeName($object->name);
+        $this->alias = BpConfig::unescapeName($object->name);
         $this->operator = $object->operator;
         $this->childNames = $object->child_names;
     }

diff --git a/library/Businessprocess/Common/EnumList.php b/library/Businessprocess/Common/EnumList.php
@@ -4,6 +4,7 @@
 
 use Icinga\Application\Modules\Module;
 use Icinga\Data\Filter\Filter;
+use Icinga\Module\Businessprocess\BpConfig;
 use Icinga\Module\Businessprocess\IcingaDbObject;
 use Icinga\Module\Businessprocess\MonitoringRestrictions;
 use Icinga\Module\Businessprocess\ProvidedHook\Icingadb\IcingadbSupport;
@@ -51,9 +52,8 @@ protected function enumHostList()
         // fetchPairs doesn't seem to work when using the same column with
         // different aliases twice
         $res = array();
-        $suffix = ';Hoststatus';
         foreach ($names as $name) {
-            $res[$name . $suffix] = $name;
+            $res[BpConfig::joinNodeName($name, 'Hoststatus')] = $name;
         }
 
         return $res;
@@ -76,7 +76,7 @@ protected function enumServiceList($host)
 
         $services = array();
         foreach ($names as $name) {
-            $services[$host . ';' . $name] = $name;
+            $services[BpConfig::joinNodeName($host, $name)] = $name;
         }
 
         return $services;
@@ -100,9 +100,8 @@ protected function enumHostListByFilter($filter)
         // fetchPairs doesn't seem to work when using the same column with
         // different aliases twice
         $res = array();
-        $suffix = ';Hoststatus';
         foreach ($names as $name) {
-            $res[$name . $suffix] = $name;
+            $res[BpConfig::joinNodeName($name, 'Hoststatus')] = $name;
         }
 
         return $res;
@@ -115,7 +114,8 @@ protected function enumServiceListByFilter($filter)
         if ($this->useIcingaDbBackend()) {
             $objects = (new IcingaDbObject())->fetchServices($filter);
             foreach ($objects as $object) {
-                $services[$object->host->name . ';' . $object->name] = $object->host->name . ':' . $object->name;
+                $services[BpConfig::joinNodeName($object->host->name, $object->name)]
+                    = $object->host->name . ':' . $object->name;
             }
         } else {
             $objects = $this->backend
@@ -127,7 +127,8 @@ protected function enumServiceListByFilter($filter)
                 ->getQuery()
                 ->fetchAll();
             foreach ($objects as $object) {
-                $services[$object->host . ';' . $object->service] = $object->host . ':' . $object->service;
+                $services[BpConfig::joinNodeName($object->host, $object->service)]
+                    = $object->host . ':' . $object->service;
             }
         }
 

diff --git a/library/Businessprocess/HostNode.php b/library/Businessprocess/HostNode.php
@@ -35,7 +35,7 @@ class HostNode extends MonitoredNode
 
     public function __construct($object)
     {
-        $this->name     = $object->hostname . ';Hoststatus';
+        $this->name = BpConfig::joinNodeName($object->hostname, 'Hoststatus');
         $this->hostname = $object->hostname;
         if (isset($object->state)) {
             $this->setState($object->state);

diff --git a/library/Businessprocess/ImportedNode.php b/library/Businessprocess/ImportedNode.php
@@ -28,7 +28,7 @@ public function __construct(BpConfig $parentBp, $object)
     {
         $this->parentBp = $parentBp;
         $this->configName = $object->configName;
-        $this->nodeName = $object->node;
+        $this->nodeName = BpConfig::escapeName($object->node);
 
         parent::__construct((object) [
             'name'          => '@' . $this->configName . ':' . $this->nodeName,
@@ -69,11 +69,7 @@ public function getBpConfig()
 
     public function getAlias()
     {
-        if ($this->alias === null) {
-            $this->alias = $this->importedNode()->getAlias();
-        }
-
-        return $this->alias;
+        return $this->importedNode()->getAlias();
     }
 
     public function getOperator()

diff --git a/library/Businessprocess/Modification/NodeAddChildrenAction.php b/library/Businessprocess/Modification/NodeAddChildrenAction.php
@@ -33,13 +33,12 @@ public function applyTo(BpConfig $config)
 
         foreach ($this->children as $name) {
             if (! $config->hasNode($name) || $config->getNode($name)->getBpConfig()->getName() !== $config->getName()) {
-                if (strpos($name, ';') !== false) {
-                    list($host, $service) = preg_split('/;/', $name, 2);
-
-                    if ($service === 'Hoststatus') {
-                        $config->createHost($host);
+                [$prefix, $suffix] = BpConfig::splitNodeName($name);
+                if ($suffix !== null) {
+                    if ($suffix === 'Hoststatus') {
+                        $config->createHost($prefix);
                     } else {
-                        $config->createService($host, $service);
+                        $config->createService($prefix, $suffix);
                     }
                 } elseif ($name[0] === '@' && strpos($name, ':') !== false) {
                     list($configName, $nodeName) = preg_split('~:\s*~', substr($name, 1), 2);

diff --git a/library/Businessprocess/ProvidedHook/Icingadb/HostActions.php b/library/Businessprocess/ProvidedHook/Icingadb/HostActions.php
@@ -2,6 +2,7 @@
 
 namespace Icinga\Module\Businessprocess\ProvidedHook\Icingadb;
 
+use Icinga\Module\Businessprocess\BpConfig;
 use Icinga\Module\Icingadb\Hook\HostActionsHook;
 use Icinga\Module\Icingadb\Model\Host;
 use ipl\Web\Widget\Link;
@@ -15,7 +16,7 @@ public function getActionsForObject(Host $host): array
             new Link(
                 $label,
                 'businessprocess/node/impact?name='
-                . rawurlencode($host->name . ';Hoststatus')
+                . rawurlencode(BpConfig::joinNodeName($host->name, 'Hoststatus'))
             )
         );
     }

diff --git a/library/Businessprocess/ProvidedHook/Icingadb/ServiceActions.php b/library/Businessprocess/ProvidedHook/Icingadb/ServiceActions.php
@@ -2,6 +2,7 @@
 
 namespace Icinga\Module\Businessprocess\ProvidedHook\Icingadb;
 
+use Icinga\Module\Businessprocess\BpConfig;
 use Icinga\Module\Icingadb\Hook\ServiceActionsHook;
 use Icinga\Module\Icingadb\Model\Service;
 use ipl\Web\Widget\Link;
@@ -16,9 +17,7 @@ public function getActionsForObject(Service $service): array
                 $label,
                 sprintf(
                     'businessprocess/node/impact?name=%s',
-                    rawurlencode(
-                        sprintf('%s;%s', $service->host->name, $service->name)
-                    )
+                    rawurlencode(BpConfig::joinNodeName($service->host->name, $service->name))
                 )
             )
         );

diff --git a/library/Businessprocess/ProvidedHook/Monitoring/HostActions.php b/library/Businessprocess/ProvidedHook/Monitoring/HostActions.php
@@ -2,6 +2,7 @@
 
 namespace Icinga\Module\Businessprocess\ProvidedHook\Monitoring;
 
+use Icinga\Module\Businessprocess\BpConfig;
 use Icinga\Module\Monitoring\Hook\HostActionsHook;
 use Icinga\Module\Monitoring\Object\Host;
 
@@ -12,7 +13,7 @@ public function getActionsForHost(Host $host)
         $label = mt('businessprocess', 'Business Impact');
         return array(
             $label => 'businessprocess/node/impact?name='
-                . rawurlencode($host->getName() . ';Hoststatus')
+                . rawurlencode(BpConfig::joinNodeName($host->getName(), 'Hoststatus'))
         );
     }
 }
diff --git a/library/Businessprocess/ProvidedHook/Monitoring/ServiceActions.php b/library/Businessprocess/ProvidedHook/Monitoring/ServiceActions.php
@@ -4,6 +4,7 @@
 
 use Exception;
 use Icinga\Application\Config;
+use Icinga\Module\Businessprocess\BpConfig;
 use Icinga\Module\Monitoring\Hook\ServiceActionsHook;
 use Icinga\Module\Monitoring\Object\Service;
 use Icinga\Web\Url;
@@ -16,9 +17,7 @@ public function getActionsForService(Service $service)
         return array(
             $label => sprintf(
                 'businessprocess/node/impact?name=%s',
-                rawurlencode(
-                    sprintf('%s;%s', $service->getHost()->getName(), $service->getName())
-                )
+                rawurlencode(BpConfig::joinNodeName($service->getHost()->getName(), $service->getName()))
             )
         );
     }

diff --git a/library/Businessprocess/ServiceNode.php b/library/Businessprocess/ServiceNode.php
@@ -19,7 +19,7 @@ class ServiceNode extends MonitoredNode
 
     public function __construct($object)
     {
-        $this->name = $object->hostname . ';' . $object->service;
+        $this->name = BpConfig::joinNodeName($object->hostname, $object->service);
         $this->hostname = $object->hostname;
         $this->service  = $object->service;
         if (isset($object->state)) {

diff --git a/library/Businessprocess/State/IcingaDbState.php b/library/Businessprocess/State/IcingaDbState.php
@@ -99,9 +99,9 @@ public function reallyRetrieveStatesFromBackend()
     protected function handleDbRow($row, BpConfig $config, $objectName)
     {
         if ($objectName === 'service') {
-            $key = $row->host->name . ';' . $row->name;
+            $key = BpConfig::joinNodeName($row->host->name, $row->name);
         } else {
-            $key = $row->name . ';Hoststatus';
+            $key = BpConfig::joinNodeName($row->name, 'Hoststatus');
         }
 
         // We fetch more states than we need, so skip unknown ones

diff --git a/library/Businessprocess/State/MonitoringState.php b/library/Businessprocess/State/MonitoringState.php
@@ -115,12 +115,12 @@ public function reallyRetrieveStatesFromBackend()
 
     protected function handleDbRow($row, BpConfig $config)
     {
-        $key = $row->hostname;
-        if (property_exists($row, 'service')) {
-            $key .= ';' . $row->service;
-        } else {
-            $key .= ';Hoststatus';
-        }
+        $key = BpConfig::joinNodeName(
+            $row->hostname,
+            property_exists($row, 'service')
+                ? $row->service
+                : 'Hoststatus'
+        );
 
         // We fetch more states than we need, so skip unknown ones
         if (! $config->hasNode($key)) {