Fix backtick character handling in sql queries (#1490312)

CHANGELOG

@@ -35,6 +35,7 @@ CHANGELOG Roundcube Webmail
 - Fix rows count when messages search fails (#1490266)
 - Fix bug where spellchecking in HTML editor do not work after switching editor type more than once (#1490311)
 - Fix bug where TinyMCE area height was too small on slow network connection (#1490310)
+- Fix backtick character handling in sql queries (#1490312)
 
 RELEASE 1.1.0
 -------------

program/lib/Roundcube/rcube_db.php

@@ -448,10 +448,15 @@ class rcube_db
             }
         }
 
-        // replace escaped '?' back to normal, see self::quote()
-        $query = str_replace('??', '?', $query);
         $query = rtrim($query, " \t\n\r\0\x0B;");
 
+        // replace escaped '?' and quotes back to normal, see self::quote()
+        $query = str_replace(
+            array('??', self::DEFAULT_QUOTE.self::DEFAULT_QUOTE),
+            array('?', self::DEFAULT_QUOTE),
+            $query
+        );
+
         // log query
         $this->debug($query);
 
@@ -516,9 +521,6 @@ class rcube_db
             }
         }
 
-        // replace escaped quote back to normal, see self::quote()
-        $query = str_replace($quote.$quote, $quote, $query);
-
         return $query;
     }
 

program/lib/Roundcube/rcube_db_oracle.php

@@ -155,10 +155,15 @@ class rcube_db_oracle extends rcube_db
             }
         }
 
-        // replace escaped '?' back to normal, see self::quote()
-        $query = str_replace('??', '?', $query);
         $query = rtrim($query, " \t\n\r\0\x0B;");
 
+        // replace escaped '?' and quotes back to normal, see self::quote()
+        $query = str_replace(
+            array('??', self::DEFAULT_QUOTE.self::DEFAULT_QUOTE),
+            array('?', self::DEFAULT_QUOTE),
+            $query
+        );
+
         // log query
         $this->debug($query);
 

tests/Framework/DB.php

@@ -25,6 +25,8 @@ class Framework_DB extends PHPUnit_Framework_TestCase
     {
         $db = new rcube_db_test_wrapper('test');
         $db->set_option('table_prefix', 'prefix_');
+        $db->set_option('identifier_start', '`');
+        $db->set_option('identifier_end', '`');
 
         $script = implode("\n", array(
             "CREATE TABLE `xxx` (test int, INDEX xxx (test));",
@@ -38,26 +40,88 @@ class Framework_DB extends PHPUnit_Framework_TestCase
             "SELECT test FROM xxx;",
         ));
         $output = implode("\n", array(
-            "CREATE TABLE `prefix_xxx` (test int, INDEX prefix_xxx (test));",
+            "CREATE TABLE `prefix_xxx` (test int, INDEX prefix_xxx (test))",
-            "ALTER TABLE `prefix_xxx` CHANGE test test int;",
+            "ALTER TABLE `prefix_xxx` CHANGE test test int",
-            "TRUNCATE prefix_xxx;",
+            "TRUNCATE prefix_xxx",
-            "DROP TABLE `prefix_vvv`;",
+            "DROP TABLE `prefix_vvv`",
             "CREATE TABLE `prefix_i` (test int CONSTRAINT `prefix_iii`
-                FOREIGN KEY (`test`) REFERENCES `prefix_xxx`(`test`) ON DELETE CASCADE ON UPDATE CASCADE);",
+                FOREIGN KEY (`test`) REFERENCES `prefix_xxx`(`test`) ON DELETE CASCADE ON UPDATE CASCADE)",
-            "INSERT INTO prefix_xxx test = 1;",
+            "INSERT INTO prefix_xxx test = 1",
-            "SELECT test FROM prefix_xxx;",
+            "SELECT test FROM prefix_xxx",
         ));
 
         $result = $db->exec_script($script);
-        $out    = '';
+        $out    = array();
 
         foreach ($db->queries as $q) {
-            $out[] = $q[0];
+            $out[] = $q;
         }
 
         $this->assertTrue($result, "Execute SQL script (result)");
         $this->assertSame(implode("\n", $out), $output, "Execute SQL script (content)");
     }
+
+    /**
+     * Test query parsing and arguments quoting
+     */
+    function test_query_parsing()
+    {
+        $db = new rcube_db_test_wrapper('test');
+        $db->set_option('identifier_start', '`');
+        $db->set_option('identifier_end', '`');
+
+        $db->query("SELECT ?", "test`test");
+        $db->query("SELECT ?", "test?test");
+        $db->query("SELECT ?", "test``test");
+        $db->query("SELECT ?", "test??test");
+        $db->query("SELECT `test` WHERE 'test``test'");
+        $db->query("SELECT `test` WHERE 'test??test'");
+        $db->query("SELECT `test` WHERE `test` = ?", "`te``st`");
+        $db->query("SELECT `test` WHERE `test` = ?", "?test?");
+        $db->query("SELECT `test` WHERE `test` = ?", "????");
+
+        $expected = implode("\n", array(
+            "SELECT 'test`test'",
+            "SELECT 'test?test'",
+            "SELECT 'test``test'",
+            "SELECT 'test??test'",
+            "SELECT `test` WHERE 'test`test'",
+            "SELECT `test` WHERE 'test?test'",
+            "SELECT `test` WHERE `test` = '`te``st`'",
+            "SELECT `test` WHERE `test` = '?test?'",
+            "SELECT `test` WHERE `test` = '????'",
+        ));
+
+       $this->assertSame($expected, implode("\n", $db->queries), "Query parsing [1]");
+
+        $db->set_option('identifier_start', '"');
+        $db->set_option('identifier_end', '"');
+        $db->queries = array();
+
+        $db->query("SELECT ?", "test`test");
+        $db->query("SELECT ?", "test?test");
+        $db->query("SELECT ?", "test``test");
+        $db->query("SELECT ?", "test??test");
+        $db->query("SELECT `test` WHERE 'test``test'");
+        $db->query("SELECT `test` WHERE 'test??test'");
+        $db->query("SELECT `test` WHERE `test` = ?", "`te``st`");
+        $db->query("SELECT `test` WHERE `test` = ?", "?test?");
+        $db->query("SELECT `test` WHERE `test` = ?", "????");
+
+        $expected = implode("\n", array(
+            "SELECT 'test`test'",
+            "SELECT 'test?test'",
+            "SELECT 'test``test'",
+            "SELECT 'test??test'",
+            "SELECT \"test\" WHERE 'test`test'",
+            "SELECT \"test\" WHERE 'test?test'",
+            "SELECT \"test\" WHERE \"test\" = '`te``st`'",
+            "SELECT \"test\" WHERE \"test\" = '?test?'",
+            "SELECT \"test\" WHERE \"test\" = '????'",
+        ));
+
+       $this->assertSame($expected, implode("\n", $db->queries), "Query parsing [2]");
+    }
 }
 
 /**
@@ -67,8 +131,30 @@ class rcube_db_test_wrapper extends rcube_db
 {
     public $queries = array();
 
-    protected function _query($query, $offset, $numrows, $params)
+    protected function query_execute($query)
+    {
+        $this->queries[] = $query;
+    }
+
+    public function db_connect($mode, $force = false)
+    {
+        $this->dbh = new rcube_db_test_dbh();
+    }
+
+    public function is_connected()
+    {
+        return true;
+    }
+
+    protected function debug($data)
+    {
+    }
+}
+
+class rcube_db_test_dbh
+{
+    public function quote($data, $type)
     {
-        $this->queries[] = array(trim($query), $offset, $numrows, $params);
+        return "'$data'";
     }
 }