diff --git a/library/Icinga/File/Ini/Dom/Directive.php b/library/Icinga/File/Ini/Dom/Directive.php index 9d090ee63..97bc6d37c 100644 --- a/library/Icinga/File/Ini/Dom/Directive.php +++ b/library/Icinga/File/Ini/Dom/Directive.php @@ -32,7 +32,7 @@ class Directive */ public function __construct($key) { - $this->key = trim(str_replace("\n", ' ', $key)); + $this->key = trim($key); if (strlen($this->key) < 1) { throw new Exception(sprintf('Ini parser error: empty key.')); } @@ -46,12 +46,20 @@ class Directive return $this->key; } + /** + * @return string + */ + public function getValue() + { + return $this->value; + } + /** * @param string $value */ public function setValue($value) { - $this->value = trim(str_replace("\n", ' ', $value)); + $this->value = trim($value); } /** @@ -67,10 +75,23 @@ class Directive } $str = implode(PHP_EOL, $comments) . PHP_EOL; } - $str .= sprintf('%s = "%s"', $this->key, $this->value); + $str .= sprintf('%s = "%s"', $this->sanitizeKey($this->key), $this->sanitizeValue($this->value)); if (isset ($this->commentPost)) { $str .= ' ' . $this->commentPost->render(); } return $str; } + + protected function sanitizeKey($str) + { + return trim(str_replace(PHP_EOL, ' ', $str)); + } + + protected function sanitizeValue($str) + { + $str = trim($str); + $str = str_replace('\\', '\\\\', $str); + $str = str_replace('"', '\\"', $str); + return str_replace(PHP_EOL, ' ', $str); + } } diff --git a/library/Icinga/File/Ini/Dom/Section.php b/library/Icinga/File/Ini/Dom/Section.php index 64779cc62..e46d4f8ca 100644 --- a/library/Icinga/File/Ini/Dom/Section.php +++ b/library/Icinga/File/Ini/Dom/Section.php @@ -29,15 +29,10 @@ class Section /** * @param string $name - * - * @throws Exception */ public function __construct($name) { - $this->name = trim(str_replace("\n", ' ', $name)); - if (false !== strpos($name, ';') || false !== strpos($name, ']')) { - throw new ConfigurationError(sprintf('Ini file error: invalid char in title: %s', $name)); - } + $this->name = trim($name); if (strlen($this->name) < 1) { throw new ConfigurationError(sprintf('Ini file error: empty section identifier')); } @@ -108,6 +103,16 @@ class Section if (isset($this->commentPost)) { $post = ' ' . $this->commentPost->render(); } - return $cms . sprintf('[%s]', $this->name) . $post . PHP_EOL . $dirs; + return $cms . sprintf('[%s]', $this->sanitize($this->name)) . $post . PHP_EOL . $dirs; + } + + protected function sanitize($str) + { + $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); } } diff --git a/library/Icinga/File/Ini/IniParser.php b/library/Icinga/File/Ini/IniParser.php index 4f9813689..20ff6271d 100644 --- a/library/Icinga/File/Ini/IniParser.php +++ b/library/Icinga/File/Ini/IniParser.php @@ -14,6 +14,7 @@ class IniParser { const LINE_START = 0; const SECTION = 1; + const ESCAPE = 2; const DIRECTIVE_KEY = 4; const DIRECTIVE_VALUE_START = 5; const DIRECTIVE_VALUE = 6; @@ -43,6 +44,7 @@ class IniParser $dir = null; $coms = array(); $state = self::LINE_START; + $escaping = null; $token = ''; $line = 0; @@ -67,10 +69,19 @@ class IniParser } break; + case self::ESCAPE: + $token .= $s; + $state = $escaping; + $escaping = null; + break; + case self::SECTION: if ($s === "\n") { self::throwParseError('Unterminated SECTION', $line); - } if ($s !== ']') { + } elseif ($s === '\\') { + $state = self::ESCAPE; + $escaping = self::SECTION; + } elseif ($s !== ']') { $token .= $s; } else { $sec = new Section($token); @@ -119,6 +130,11 @@ class IniParser break; case self::DIRECTIVE_VALUE: + /* + Escaping non-quoted values is not supported by php_parse_ini, it might + be reasonable to include in case we are switching completely our own + parser implementation + */ if ($s === "\n" || $s === ";") { $dir->setValue($token); $token = ''; @@ -137,6 +153,9 @@ class IniParser case self::DIRECTIVE_VALUE_QUOTED: if ($s === "\n") { self::throwParseError('Unterminated DIRECTIVE_VALUE_QUOTED', $line); + } elseif ($s === '\\') { + $state = self::ESCAPE; + $escaping = self::DIRECTIVE_VALUE_QUOTED; } elseif ($s !== '"') { $token .= $s; } else { @@ -204,6 +223,7 @@ class IniParser $sec->addDirective($dir); break; + case self::ESCAPE: case self::DIRECTIVE_VALUE_QUOTED: case self::DIRECTIVE_KEY: case self::SECTION: diff --git a/test/php/library/Icinga/File/Ini/IniParserTest.php b/test/php/library/Icinga/File/Ini/IniParserTest.php new file mode 100644 index 000000000..bf9143e86 --- /dev/null +++ b/test/php/library/Icinga/File/Ini/IniParserTest.php @@ -0,0 +1,63 @@ +tempFile = tempnam(sys_get_temp_dir(), 'icinga-ini-parser-test'); + } + + public function tearDown() + { + parent::tearDown(); + unlink($this->tempFile); + } + + public function testSectionNameEscaping() + { + $config = <<<'EOD' +[title with \]bracket] +key1 = "1" +key2 = "2" + +[title with \"quote] +key1 = "1" +key2 = "2" +EOD; + $doc = IniParser::parseIni($config); + $this->assertTrue( + $doc->hasSection('title with ]bracket'), + 'IniParser does not recognize escaped bracket in section' + ); + $this->assertTrue( + $doc->hasSection('title with "quote'), + 'IniParser does not recognize escaped quote in section' + ); + } + + public function testDirectiveValueEscaping() + { + $config = <<<'EOD' +[section] +key1 = "key with escaped \"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' + ); + } +} diff --git a/test/php/library/Icinga/File/Ini/IniWriterTest.php b/test/php/library/Icinga/File/Ini/IniWriterTest.php index a45e33e7b..18ca34046 100644 --- a/test/php/library/Icinga/File/Ini/IniWriterTest.php +++ b/test/php/library/Icinga/File/Ini/IniWriterTest.php @@ -285,26 +285,30 @@ inkey' => 'blarg' public function testSectionNameEscaping() { $config = <<<'EOD' -[section 1] -foo = "1337" +[section [brackets\]] +foo = "bar" -[section (with special chars)] -foo = "baz" +[section \;comment] +foo = "bar" -[section/as/arbitrary/path] -foo = "nope" +[section \"quotes\"] +foo = "bar" -[section.with.dots.in.it] +[section with \\] +foo = "bar" + +[section with newline] foo = "bar" EOD; $target = $this->writeConfigToTemporaryFile($config); $writer = new IniWriter( Config::fromArray( array( - 'section 1' => array('foo' => 1337), - 'section (with special chars)' => array('foo' => 'baz'), - 'section/as/arbitrary/path' => array('foo' => 'nope'), - 'section.with.dots.in.it' => array('foo' => 'bar') + 'section [brackets]' => array('foo' => 'bar'), + 'section ;comment' => array('foo' => 'bar'), + 'section "quotes"' => array('foo' => 'bar'), + 'section with \\' => array('foo' => 'bar'), + 'section with' . PHP_EOL . 'newline' => array('foo' => 'bar') ) ), $target @@ -317,6 +321,36 @@ EOD; ); } + public function testDirectiveValueEscaping() + { + $config = <<<'EOD' +[section] +key1 = "value with \"quotes\"" +key2 = "value with \\" +key3 = "value with newline" + +EOD; + $target = $this->writeConfigToTemporaryFile($config); + $writer = new IniWriter( + Config::fromArray( + array( + 'section' => array( + 'key1' => 'value with "quotes"', + 'key2' => 'value with \\', + 'key3' => 'value with' . PHP_EOL . 'newline' + ) + ) + ), + $target + ); + + $this->assertEquals( + trim($config), + trim($writer->render()), + 'IniWriter does not handle special chars in directives properly.' + ); + } + public function testSectionDeleted() { $config = <<<'EOD' @@ -356,7 +390,6 @@ EOD; ); } - /** * Write a INI-configuration string to a temporary file and return its path *