Make html::parse_attrib_string() more robust

Fixes PHP Error: Expression parse error on: ($app->config->get('preview_pane',rcube_utils::get_boolean('')) == true ? ' checked=checked' : ')
8 years ago · 94f8ce3334
parent 062d95503e
commit 94f8ce3334
2 changed files with 48 additions and 10 deletions
--- a/program/lib/Roundcube/html.php
+++ b/program/lib/Roundcube/html.php
@ -344,14 +344,14 @@ class html
    public static function parse_attrib_string($str)
    {
        $attrib = array();
-        $regexp = '/\s*([-_a-z]+)=(["\'])??(?(2)([^\2]*)\2|(\S+?))/Ui';
+        $html   = '<html><body><div ' . rtrim($str, '/ ') . ' /></body></html>';

-        preg_match_all($regexp, stripslashes($str), $regs, PREG_SET_ORDER);
+        $document = new DOMDocument('1.0', RCUBE_CHARSET);
+        @$document->loadHTML($html);

-        // convert attributes to an associative array (name => value)
-        if ($regs) {
-            foreach ($regs as $attr) {
-                $attrib[strtolower($attr[1])] = html_entity_decode($attr[3] . $attr[4]);
+        if ($node = $document->getElementsByTagName('div')->item(0)) {
+            foreach ($node->attributes as $name => $attr) {
+                $attrib[strtolower($name)] = $attr->nodeValue;
            }
        }

--- a/tests/Framework/Html.php
+++ b/tests/Framework/Html.php
@ -61,9 +61,9 @@ class Framework_Html extends PHPUnit_Framework_TestCase
     * Test for attrib_string()
     * @dataProvider data_attrib_string
     */
-    function test_attrib_string($arg1, $arg2, $result)
+    function test_attrib_string($arg1, $arg2, $expected)
    {
-        $this->assertEquals(html::attrib_string($arg1, $arg2), $result);
+        $this->assertEquals($expected, html::attrib_string($arg1, $arg2));
    }

    /**
@ -86,8 +86,46 @@ class Framework_Html extends PHPUnit_Framework_TestCase
     * Test for quote()
     * @dataProvider data_quote
     */
-    function test_quote($str, $result)
+    function test_quote($str, $expected)
    {
-        $this->assertEquals(html::quote($str), $result);
+        $this->assertEquals($expected, html::quote($str));
+    }
+
+    /**
+     * Data for test_parse_attrib_string()
+     */
+    function data_parse_attrib_string()
+    {
+        return array(
+            array(
+                '',
+                array(),
+            ),
+            array(
+                'test="test1-val"',
+                array('test' => 'test1-val'),
+            ),
+            array(
+                'test1="test1-val"    test2=test2-val',
+                array('test1' => 'test1-val', 'test2' => 'test2-val'),
+            ),
+            array(
+                '   test1="test1\'val"    test2=\'test2"val\'   ',
+                array('test1' => 'test1\'val', 'test2' => 'test2"val'),
+            ),
+            array(
+                'expression="test == true ? \' test\' : \'\'" ',
+                array('expression' => 'test == true ? \' test\' : \'\''),
+            ),
+        );
+    }
+
+    /**
+     * Test for parse_attrib_string()
+     * @dataProvider data_parse_attrib_string
+     */
+    function test_parse_attrib_string($arg1, $expected)
+    {
+        $this->assertEquals($expected, html::parse_attrib_string($arg1));
    }
 }