MDL-29295 do not use strtok in dml and ddl layers

This prevents clobbering of internal pointer when code calling DML uses strtok too. Unit test added to prevent future uses.
moodle · Sep 13, 2011 · 2d1e165 · 2d1e165
1 parent f3f329d
commit 2d1e165
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 8 deletions.
diff --git a/lib/ddl/mssql_sql_generator.php b/lib/ddl/mssql_sql_generator.php
@@ -298,7 +298,7 @@ public function getAlterFieldSQL($xmldb_table, $xmldb_field, $skip_type_clause =
         $oldlength = $metac->max_length;
         $olddecimals = empty($metac->scale) ? null : $metac->scale;
         $oldnotnull = empty($metac->not_null) ? false : $metac->not_null;
-        $olddefault = empty($metac->has_default) ? null : strtok($metac->default_value, ':');
+        //$olddefault = empty($metac->has_default) ? null : strtok($metac->default_value, ':');
 
         $typechanged = true;  //By default, assume that the column type has changed
         $lengthchanged = true;  //By default, assume that the column length has changed

diff --git a/lib/dml/mssql_native_moodle_database.php b/lib/dml/mssql_native_moodle_database.php
@@ -615,7 +615,8 @@ protected function emulate_bound_params($sql, array $params=null) {
             return $sql;
         }
         /// ok, we have verified sql statement with ? and correct number of params
-        $return = strtok($sql, '?');
+        $parts = explode('?', $sql);
+        $return = array_shift($parts);
         foreach ($params as $param) {
             if (is_bool($param)) {
                 $return .= (int)$param;
@@ -640,7 +641,7 @@ protected function emulate_bound_params($sql, array $params=null) {
                 $return .= "N'$param'";
             }
 
-            $return .= strtok('?');
+            $return .= array_shift($parts);
         }
         return $return;
     }

diff --git a/lib/dml/mysqli_native_moodle_database.php b/lib/dml/mysqli_native_moodle_database.php
@@ -666,7 +666,8 @@ protected function emulate_bound_params($sql, array $params=null) {
             return $sql;
         }
         /// ok, we have verified sql statement with ? and correct number of params
-        $return = strtok($sql, '?');
+        $parts = explode('?', $sql);
+        $return = array_shift($parts);
         foreach ($params as $param) {
             if (is_bool($param)) {
                 $return .= (int)$param;
@@ -680,7 +681,7 @@ protected function emulate_bound_params($sql, array $params=null) {
                 $param = $this->mysqli->real_escape_string($param);
                 $return .= "'$param'";
             }
-            $return .= strtok('?');
+            $return .= array_shift($parts);
         }
         return $return;
     }

diff --git a/lib/dml/simpletest/testdml.php b/lib/dml/simpletest/testdml.php
@@ -423,6 +423,28 @@ function test_fix_sql_params() {
         $this->assertIdentical(array_values($params), array_values($inparams));
     }
 
+    public function test_strtok() {
+        // strtok was previously used by bound emulation, make sure it is not used any more
+        $DB = $this->tdb;
+        $dbman = $this->tdb->get_manager();
+
+        $table = $this->get_test_table();
+        $tablename = $table->getName();
+
+        $table->add_field('id', XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, XMLDB_NOTNULL, XMLDB_SEQUENCE, null);
+        $table->add_field('course', XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, XMLDB_NOTNULL, null, '0');
+        $table->add_field('name', XMLDB_TYPE_CHAR, '255', null, null, null, 'lala');
+        $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id'));
+        $dbman->create_table($table);
+
+        $str = 'a?b?c?d';
+        $this->assertIdentical(strtok($str, '?'), 'a');
+
+        $DB->get_records($tablename, array('id'=>1));
+
+        $this->assertIdentical(strtok('?'), 'b');
+    }
+
     public function test_tweak_param_names() {
         // Note the tweak_param_names() method is only available in the oracle driver,
         // hence we look for expected results indirectly, by testing various DML methods

diff --git a/lib/dml/sqlsrv_native_moodle_database.php b/lib/dml/sqlsrv_native_moodle_database.php
@@ -696,8 +696,8 @@ protected function emulate_bound_params($sql, array $params = null) {
             return $sql;
         }
         /// ok, we have verified sql statement with ? and correct number of params
-        $return = strtok($sql, '?');
-
+        $parts = explode('?', $sql);
+        $return = array_shift($parts);
         foreach ($params as $param) {
             if (is_bool($param)) {
                 $return .= (int)$param;
@@ -717,7 +717,7 @@ protected function emulate_bound_params($sql, array $params = null) {
                 $return .= "N'$param'";
             }
 
-            $return .= strtok('?');
+            $return .= array_shift($parts);
         }
         return $return;
     }