[icinga-checkins] icinga.org: icingaweb2-module-director/fiddle/mfrosch: Detect and properly handle import loop during parameter resolving

git at icinga.org git at icinga.org
Tue May 24 16:46:45 CEST 2016


Module: icingaweb2-module-director
Branch: fiddle/mfrosch
Commit: bc87e27bbf495e628f615cc54b472182a058d4c0
URL:    https://git.icinga.org/?p=icingaweb2-module-director.git;a=commit;h=bc87e27bbf495e628f615cc54b472182a058d4c0

Author: Markus Frosch <lazyfrosch at icinga.org>
Date:   Thu May 19 10:43:26 2016 +0200

Detect and properly handle import loop during parameter resolving

Sensible resetting of the imports to a previous state.

---

 library/Director/Exception/NestingException.php  |    7 +++
 library/Director/Objects/IcingaObject.php        |   66 ++++++++++++++++++----
 library/Director/Web/Form/DirectorObjectForm.php |   11 +++-
 3 files changed, 71 insertions(+), 13 deletions(-)

diff --git a/library/Director/Exception/NestingException.php b/library/Director/Exception/NestingException.php
new file mode 100644
index 0000000..189e380
--- /dev/null
+++ b/library/Director/Exception/NestingException.php
@@ -0,0 +1,7 @@
+<?php
+
+namespace Icinga\Module\Director\Exception;
+
+use Icinga\Exception\IcingaException;
+
+class NestingException extends IcingaException {}
\ No newline at end of file
diff --git a/library/Director/Objects/IcingaObject.php b/library/Director/Objects/IcingaObject.php
index bee05d9..112568b 100644
--- a/library/Director/Objects/IcingaObject.php
+++ b/library/Director/Objects/IcingaObject.php
@@ -9,6 +9,8 @@ use Icinga\Module\Director\IcingaConfig\IcingaConfig;
 use Icinga\Module\Director\IcingaConfig\IcingaConfigRenderer;
 use Icinga\Module\Director\IcingaConfig\IcingaConfigHelper as c;
 use Icinga\Data\Filter\Filter;
+
+use Icinga\Module\Director\Exception\NestingException;
 use Icinga\Exception\ConfigurationError;
 use Icinga\Exception\ProgrammingError;
 use Exception;
@@ -596,11 +598,27 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
             $imports = array($imports);
         }
 
-        $this->imports()->set($imports);
-        if ($this->imports()->hasBeenModified()) {
+        $importObj = $this->imports();
+        $old_imports = $this->imports()->listImportNames();
+
+        // try to set imports and resolve properties
+        try {
+            $importObj->set($imports);
+            if ($importObj->hasBeenModified()) {
+                $this->clearImportedObjects();
+                $this->invalidateResolveCache();
+                // force resolve to "validate" imports
+                $this->resolveProperties();
+            }
+        }
+        catch (NestingException $e) {
+            // revert imports and throw error to form
+            $importObj->set($old_imports);
             $this->clearImportedObjects();
             $this->invalidateResolveCache();
+            throw $e;
         }
+
     }
 
     public function getImports()
@@ -764,6 +782,8 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
         return $db->fetchOne($query);
     }
 
+    protected static $importLoopDetection = array();
+
     protected function resolve($what)
     {
         if ($this->hasResolveCached($what)) {
@@ -780,18 +800,38 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
         $getInherited = 'getInherited' . $what;
         $getOrigins   = 'getOrigins'  . $what;
 
+        $type = $this->getType();
+
+        // add myself to loop detection stack
+        static::$importLoopDetection[$type][$this->getObjectName()] = 1;
+
         $blacklist = array('id', 'object_type', 'object_name', 'disabled');
         foreach ($objects as $name => $object) {
-            $origins = $object->$getOrigins();
+            if (
+                array_key_exists($type, static::$importLoopDetection)
+                && array_key_exists($name, static::$importLoopDetection[$type])
+            ) {
+                throw new NestingException(
+                    'Detected a loop while resolving %s \'%s\', involving: %s',
+                    $type,
+                    $name,
+                    join(', ', array_keys(static::$importLoopDetection[$type]))
+                );
+            }
+            else { // only deep resolve values when not in a loop
+                static::$importLoopDetection[$type][$name] = 1;
 
-            foreach ($object->$getInherited() as $key => $value) {
-                if (in_array($key, $blacklist)) {
-                    continue;
+                $origins = $object->$getOrigins();
+
+                foreach ($object->$getInherited() as $key => $value) {
+                    if (in_array($key, $blacklist)) {
+                        continue;
+                    }
+                    // $vals[$name]->$key = $value;
+                    $vals['_MERGED_']->$key = $value;
+                    $vals['_INHERITED_']->$key = $value;
+                    $vals['_ORIGINS_']->$key = $origins->$key;
                 }
-                // $vals[$name]->$key = $value;
-                $vals['_MERGED_']->$key = $value;
-                $vals['_INHERITED_']->$key = $value;
-                $vals['_ORIGINS_']->$key = $origins->$key;
             }
 
             foreach ($object->$get() as $key => $value) {
@@ -806,8 +846,14 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer
                 $vals['_INHERITED_']->$key = $value;
                 $vals['_ORIGINS_']->$key = $name;
             }
+
+            // leave object at loop detection
+            unset(static::$importLoopDetection[$type][$name]);
         }
 
+        // delete myself from loop detection
+        unset(static::$importLoopDetection[$type][$this->getObjectName()]);
+
         foreach ($this->$get() as $key => $value) {
             if ($value === null) {
                 continue;
diff --git a/library/Director/Web/Form/DirectorObjectForm.php b/library/Director/Web/Form/DirectorObjectForm.php
index 3d59de3..ee14f2e 100644
--- a/library/Director/Web/Form/DirectorObjectForm.php
+++ b/library/Director/Web/Form/DirectorObjectForm.php
@@ -3,6 +3,8 @@
 namespace Icinga\Module\Director\Web\Form;
 
 use Exception;
+use Icinga\Module\Director\Exception\NestingException;
+
 use Icinga\Module\Director\IcingaConfig\StateFilterSet;
 use Icinga\Module\Director\IcingaConfig\TypeFilterSet;
 use Icinga\Module\Director\Objects\IcingaObject;
@@ -68,11 +70,14 @@ abstract class DirectorObjectForm extends QuickForm
         }
         if ($this->hasBeenSent()) {
             if ($el = $this->getElement('imports')) {
-                $this->populate($this->getRequest()->getPost());
-                $object->imports = $el->getValue();
+                try {
+                    $object->setImports($el->getValue());
+                } catch (NestingException $e) {
+                    // ignore - setImports will reset the old value
+                    // the input field should show an error later
+                }
             }
         }
-
         $this->object->importedObjects();
         $object->resolveUnresolvedRelatedProperties();
         return $this;



More information about the icinga-checkins mailing list