Encode/decode INI properties in IniFileChain since the encoding/decoding is...

Encode/decode INI properties in IniFileChain since the encoding/decoding is used to avoid issues w/ parse_ini_file() use.

core/Config.php

@@ -334,9 +334,6 @@ class Config extends Singleton
             }
         }
 
-        // decode section datas
-        $this->decodeValues($this->settings->getAll());
-
         // Check config.ini.php last
         if (!$inTrackerRequest) {
             $this->checkLocalConfigFound();
@@ -361,46 +358,6 @@ class Config extends Singleton
         }
     }
 
-    /**
-     * Decode HTML entities
-     *
-     * @param mixed $values
-     * @return mixed
-     */
-    protected function decodeValues(&$values)
-    {
-        if (is_array($values)) {
-            foreach ($values as &$value) {
-                $value = $this->decodeValues($value);
-            }
-            return $values;
-        } elseif (is_string($values)) {
-            return html_entity_decode($values, ENT_COMPAT, 'UTF-8');
-        }
-        return $values;
-    }
-
-    /**
-     * Encode HTML entities
-     *
-     * @param mixed $values
-     * @return mixed
-     */
-    protected function encodeValues(&$values)
-    {
-        if (is_array($values)) {
-            foreach ($values as &$value) {
-                $value = $this->encodeValues($value);
-            }
-        } elseif (is_float($values)) {
-            $values = Common::forceDotAsSeparatorForDecimalPoint($values);
-        } elseif (is_string($values)) {
-            $values = htmlentities($values, ENT_COMPAT, 'UTF-8');
-            $values = str_replace('$', '$', $values);
-        }
-        return $values;
-    }
-
     /**
      * Returns a configuration value or section by name.
      *
@@ -455,21 +412,9 @@ class Config extends Singleton
      */
     public function dumpConfig()
     {
-        $this->encodeValues($this->settings->getAll());
-
-        try {
-            $header = "; <?php exit; ?> DO NOT REMOVE THIS LINE\n";
-            $header .= "; file automatically generated or modified by Piwik; you can manually override the default values in global.ini.php by redefining them in this file.\n";
-            $dumpedString = $this->settings->dumpChanges($header);
-
-            $this->decodeValues($this->settings->getAll());
-        } catch (Exception $ex) {
-            $this->decodeValues($this->settings->getAll());
-
-            throw $ex;
-        }
-
-        return $dumpedString;
+        $header = "; <?php exit; ?> DO NOT REMOVE THIS LINE\n";
+        $header .= "; file automatically generated or modified by Piwik; you can manually override the default values in global.ini.php by redefining them in this file.\n";
+        return $this->settings->dumpChanges($header);
     }
 
     /**

core/Config/IniFileChain.php

@@ -7,6 +7,7 @@
  */
 namespace Piwik\Config;
 
+use Piwik\Common;
 use Piwik\Ini\IniReader;
 use Piwik\Ini\IniReadingException;
 use Piwik\Ini\IniWriter;
@@ -113,8 +114,7 @@ class IniFileChain
      */
     public function dump($header = '')
     {
-        $writer = new IniWriter();
-        return $writer->writeToString($this->mergedSettings, $header);
+        return $this->dumpSettings($this->mergedSettings, $header);
     }
 
     /**
@@ -184,8 +184,7 @@ class IniFileChain
                 }
             });
 
-            $writer = new IniWriter();
-            return $writer->writeToString($configToWrite, $header);
+            return $this->dumpSettings($configToWrite, $header);
         } else {
             return null;
         }
@@ -206,7 +205,8 @@ class IniFileChain
         foreach ($this->settingsChain as $file => $ignore) {
             if (is_readable($file)) {
                 try {
-                    $this->settingsChain[$file] = $reader->readFile($file);
+                    $contents = $reader->readFile($file);
+                    $this->settingsChain[$file] = $this->decodeValues($contents);
                 } catch (IniReadingException $ex) {
                     $message = Piwik::translate('General_ExceptionUnreadableFileDisabledMethod', array($file, "parse_ini_file()"));
                     throw new IniReadingException($message, $code = 0, $ex);
@@ -399,4 +399,52 @@ class IniFileChain
 
         return array_search($sectionName, $settingsDataSectionNames);
     }
+
+    /**
+     * Encode HTML entities
+     *
+     * @param mixed $values
+     * @return mixed
+     */
+    protected function encodeValues(&$values)
+    {
+        if (is_array($values)) {
+            foreach ($values as &$value) {
+                $value = $this->encodeValues($value);
+            }
+        } elseif (is_float($values)) {
+            $values = Common::forceDotAsSeparatorForDecimalPoint($values);
+        } elseif (is_string($values)) {
+            $values = htmlentities($values, ENT_COMPAT, 'UTF-8');
+            $values = str_replace('$', '$', $values);
+        }
+        return $values;
+    }
+
+    /**
+     * Decode HTML entities
+     *
+     * @param mixed $values
+     * @return mixed
+     */
+    protected function decodeValues(&$values)
+    {
+        if (is_array($values)) {
+            foreach ($values as &$value) {
+                $value = $this->decodeValues($value);
+            }
+            return $values;
+        } elseif (is_string($values)) {
+            return html_entity_decode($values, ENT_COMPAT, 'UTF-8');
+        }
+        return $values;
+    }
+
+    private function dumpSettings($values, $header)
+    {
+        $values = $this->encodeValues($values);
+
+        $writer = new IniWriter();
+        return $writer->writeToString($values, $header);
+    }
 }
\ No newline at end of file

tests/PHPUnit/Unit/Config/IniFileChainTest.php

@@ -141,7 +141,7 @@ class IniFileChainTest extends PHPUnit_Framework_TestCase
                         )
                     ),
                     'Section2' => array(
-                        'var4' => 'value5'
+                        'var4' => 'val$ue5'
                     )
                 )
             ),
@@ -160,7 +160,7 @@ class IniFileChainTest extends PHPUnit_Framework_TestCase
                         )
                     ),
                     'Section2' => array(
-                        'var4' => 'value5'
+                        'var4' => 'val$ue5'
                     )
                 )
             )
@@ -194,7 +194,7 @@ class IniFileChainTest extends PHPUnit_Framework_TestCase
 
         $data =& $fileChain->get('Section1');
 
-        $this->assertEquals(array('var1' => 'value2', 'var3' => array('value3', 'value4')), $data);
+        $this->assertEquals(array('var1' => 'val"ue2', 'var3' => array('value3', 'value4')), $data);
 
         $data['var1'] = 'changed';
         $data['var3'][] = 'newValue';
@@ -241,7 +241,17 @@ class IniFileChainTest extends PHPUnit_Framework_TestCase
             __DIR__ . '/test_files/default_settings_2.ini.php'
         );
 
-        $this->assertEquals(array('var1' => 'value2', 'var3' => array('value3', 'value4')), $fileChain->getFrom($defaultSettingsPath, 'Section1'));
+        $this->assertEquals(array('var1' => 'val"ue2', 'var3' => array('value3', 'value4')), $fileChain->getFrom($defaultSettingsPath, 'Section1'));
+    }
+
+    public function test_getFrom_CorrectlyReturnsUnencodedValue()
+    {
+        $userSettingsPath = __DIR__ . '/test_files/special_values.ini.php';
+        $fileChain = new IniFileChain(array(), $userSettingsPath);
+
+        $this->assertEquals(array(
+            'value1' => 'a"bc', 'value2' => array('<script>', '${@piwik(crash))}'
+        )), $fileChain->getFrom($userSettingsPath, 'Section'));
     }
 
     public function getTestDataForDumpTest()
@@ -253,7 +263,7 @@ class IniFileChainTest extends PHPUnit_Framework_TestCase
                 ),
                 __DIR__ . '/test_files/default_settings_2.ini.php', // user settings
                 "; some header\n",
-                "; some header\n[Section1]\nvar1 = \"overriddenValue1\"\nvar3[] = \"overriddenValue2\"\nvar3[] = \"overriddenValue3\"\n\n[Section2]\nvar4 = \"value5\"\n\n",
+                "; some header\n[Section1]\nvar1 = \"overriddenValue1\"\nvar3[] = \"overriddenValue2\"\nvar3[] = \"overriddenValue3\"\n\n[Section2]\nvar4 = \"val&#36;ue5\"\n\n",
                 "; some header\n[Section1]\nvar1 = \"overriddenValue1\"\nvar3[] = \"overriddenValue2\"\nvar3[] = \"overriddenValue3\"\n\n"
             )
         );

tests/PHPUnit/Unit/Config/test_files/default_settings_1.ini.php

 [Section1]
 
-var1 = "value2"
+var1 = "val"ue2"
 var3[] = "value3"
 var3[] = "value4"
 
 [Section2]
 
-var4 = "value5"
+var4 = "val$ue5"
\ No newline at end of file

tests/PHPUnit/Unit/ConfigTest.php

@@ -145,7 +145,7 @@ class ConfigTest extends PHPUnit_Framework_TestCase
 
     /**
      * Dataprovider for testDumpConfig
-     */
+$     */
     public function getDumpConfigData()
     {
         $header = "; <?php exit; ?> DO NOT REMOVE THIS LINE\n" .