IniParser: Properly unescape special characters in section names and option values

refs #3648

Signed-off-by: Eric Lippmann <eric.lippmann@icinga.com>

library/Icinga/File/Ini/Dom/Section.php

@@ -41,13 +41,18 @@ class Section
     /**
      * @param   string  $name       The immutable name of this section
      *
-     * @throws  ConfigurationError  When the section name is empty
+     * @throws  ConfigurationError  When the section name is empty or contains brackets
      */
     public function __construct($name)
     {
         $this->name = trim($name);
         if (strlen($this->name) < 1) {
-            throw new ConfigurationError(sprintf('Ini file error: empty section identifier'));
+            throw new ConfigurationError('Ini file error: empty section identifier');
+        } elseif (strpos($name, '[') !== false || strpos($name, ']') !== false) {
+            throw new ConfigurationError(
+                'Ini file error: Section name "%s" must not contain any brackets ([, ])',
+                $name
+            );
         }
     }
 
@@ -165,7 +170,6 @@ class Section
         $str = trim($str);
         $str = str_replace('\\', '\\\\', $str);
         $str = str_replace('"', '\\"', $str);
-        $str = str_replace(']', '\\]', $str);
         $str = str_replace(';', '\\;', $str);
         return str_replace(PHP_EOL, ' ', $str);
     }

library/Icinga/File/Ini/IniParser.php

@@ -269,9 +269,38 @@ class IniParser
 
         $unescaped = array();
         foreach ($configArray as $section => $options) {
-            $unescaped[preg_replace('/\\\\(.)/', '\1', $section)] = $options;
+            $unescaped[self::unescapeSectionName($section)] = array_map([__CLASS__, 'unescapeOptionValue'], $options);
         }
 
         return Config::fromArray($unescaped)->setConfigFile($file);
     }
+
+    /**
+     * Unescape significant characters in the given section name
+     *
+     * @param   string  $str
+     *
+     * @return  string
+     */
+    protected static function unescapeSectionName($str)
+    {
+        $str = str_replace('\\"', '"', $str);
+        $str = str_replace('\\;', ';', $str);
+
+        return str_replace('\\\\', '\\', $str);
+    }
+
+    /**
+     * Unescape significant characters in the given option value
+     *
+     * @param   string  $str
+     *
+     * @return  string
+     */
+    protected static function unescapeOptionValue($str)
+    {
+        $str = str_replace('\\"', '"', $str);
+
+        return str_replace('\\\\', '\\', $str);
+    }
 }

test/php/library/Icinga/File/Ini/IniParserTest.php

@@ -3,7 +3,6 @@
 
 namespace Tests\Icinga\Config;
 
-use Icinga\File\Ini\Dom\Document;
 use Icinga\File\Ini\IniWriter;
 use Icinga\Test\BaseTestCase;
 use Icinga\Application\Config;
@@ -28,22 +27,45 @@ class IniParserTest extends BaseTestCase
     public function testSectionNameEscaping()
     {
         $config = <<<'EOD'
-[title with \]bracket]
-key1 = "1"
-key2 = "2"
-
 [title with \"quote]
-key1 = "1"
+
-key2 = "2"
+[title with \;semicolon]
+
+[title with \\backslash]
 EOD;
+
         $doc = IniParser::parseIni($config);
         $this->assertTrue(
-            $doc->hasSection('title with ]bracket'),
+            $doc->hasSection('title with "quote'),
-            'IniParser does not recognize escaped bracket in section'
+            'IniParser::parseIni does not recognize escaped quotes in section names'
         );
         $this->assertTrue(
-            $doc->hasSection('title with "quote'),
+            $doc->hasSection('title with ;semicolon'),
-            'IniParser does not recognize escaped quote in section'
+            'IniParser::parseIni does not recognize escaped semicolons in section names'
+        );
+        $this->assertTrue(
+            $doc->hasSection('title with \\backslash'),
+            'IniParser::parseIni does not recognize escaped backslashes in section names'
+        );
+
+        (new IniWriter(Config::fromArray([
+            'title with "quote'         => [],
+            'title with ;semicolon'     => [],
+            'title with \\backslash'    => []
+        ]), $this->tempFile))->write();
+
+        $configObject = IniParser::parseIniFile($this->tempFile);
+        $this->assertTrue(
+            $configObject->hasSection('title with "quote'),
+            'IniParser::parseIniFile does not recognize escaped quotes in section names'
+        );
+        $this->assertTrue(
+            $configObject->hasSection('title with ;semicolon'),
+            'IniParser::parseIniFile does not recognize escaped semicolons in section names'
+        );
+        $this->assertTrue(
+            $configObject->hasSection('title with \\backslash'),
+            'IniParser::parseIniFile does not recognize escaped backslashes in section names'
         );
     }
 
@@ -52,12 +74,50 @@ EOD;
         $config = <<<'EOD'
 [section]
 key1 = "key with escaped \"quote"
+key2 = "key with escaped backslash \\"
+key3 = "key with escaped backslash followed by quote \\\""
 EOD;
+
         $doc = IniParser::parseIni($config);
         $this->assertEquals(
             'key with escaped "quote',
             $doc->getSection('section')->getDirective('key1')->getValue(),
-            'IniParser does not recognize escaped bracket in section'
+            'IniParser::parseIni does not recognize escaped quotes in values'
+        );
+        $this->assertEquals(
+            'key with escaped backslash \\',
+            $doc->getSection('section')->getDirective('key2')->getValue(),
+            'IniParser::parseIni does not recognize escaped backslashes in values'
+        );
+        $this->assertEquals(
+            'key with escaped backslash followed by quote \\"',
+            $doc->getSection('section')->getDirective('key3')->getValue(),
+            'IniParser::parseIni does not recognize escaped backslashes followed by quotes in values'
+        );
+
+        (new IniWriter(Config::fromArray([
+            'section'   => [
+                'key1'  => 'key with escaped "quote',
+                'key2'  => 'key with escaped backslash \\',
+                'key3'  => 'key with escaped backslash followed by quote \\"'
+            ]
+        ]), $this->tempFile))->write();
+
+        $configObject = IniParser::parseIniFile($this->tempFile);
+        $this->assertEquals(
+            'key with escaped "quote',
+            $configObject->getSection('section')->get('key1'),
+            'IniParser::parseIniFile does not recognize escaped quotes in values'
+        );
+        $this->assertEquals(
+            'key with escaped backslash \\',
+            $configObject->getSection('section')->get('key2'),
+            'IniParser::parseIniFile does not recognize escaped backslashes in values'
+        );
+        $this->assertEquals(
+            'key with escaped backslash followed by quote \\"',
+            $configObject->getSection('section')->get('key3'),
+            'IniParser::parseIniFile does not recognize escaped backslashes followed by quotes in values'
         );
     }
 

test/php/library/Icinga/File/Ini/IniWriterTest.php

@@ -285,9 +285,6 @@ inkey' => 'blarg'
     public function testSectionNameEscaping()
     {
         $config = <<<'EOD'
-[section [brackets\]]
-foo = "bar"
-
 [section \;comment]
 foo = "bar"
 
@@ -304,7 +301,6 @@ EOD;
         $writer = new IniWriter(
             Config::fromArray(
                 array(
-                    'section [brackets]' => array('foo' => 'bar'),
                     'section ;comment' => array('foo' => 'bar'),
                     'section "quotes"' => array('foo' => 'bar'),
                     'section with \\' => array('foo' => 'bar'),
@@ -321,6 +317,15 @@ EOD;
         );
     }
 
+    /**
+     * @expectedException \Icinga\Exception\ConfigurationError
+     */
+    public function testWhetherBracketsAreIllegalInSectionNames()
+    {
+        $config = Config::fromArray(['section [brackets]' => []]);
+        (new IniWriter($config, $this->tempFile))->write();
+    }
+
     public function testDirectiveValueEscaping()
     {
         $config = <<<'EOD'