MDL-29894 forbid objects in DML parameters

Objects with __toString we never fully supported as parameters in DML layer, this finally brings consistent behaviour.

diff --git a/lib/dml/moodle_database.php b/lib/dml/moodle_database.php
index 2baf138..3c736ae 100644 (file)
--- a/lib/dml/moodle_database.php
+++ b/lib/dml/moodle_database.php
@@ -690,6 +690,17 @@ abstract class moodle_database {
         return "\$".$this->fix_sql_params_i;
     }
 
+    /**
+     * Detects object parameters and throws exception if found
+     * @param mixed $value
+     * @return void
+     */
+    protected function detect_objects($value) {
+        if (is_object($value)) {
+            throw new coding_exception('Invalid database query parameter value', 'Objects are are not allowed: '.get_class($value));
+        }
+    }
+
     /**
      * Normalizes sql query parameters and verifies parameters.
      * @param string $sql The query or part of it.
@@ -703,8 +714,9 @@ abstract class moodle_database {
         // convert table names
         $sql = $this->fix_table_names($sql);
 
-        // cast booleans to 1/0 int
+        // cast booleans to 1/0 int and detect forbidden objects
         foreach ($params as $key => $value) {
+            $this->detect_objects($value);
             $params[$key] = is_bool($value) ? (int)$value : $value;
         }
 

diff --git a/lib/dml/mssql_native_moodle_database.php b/lib/dml/mssql_native_moodle_database.php
index d5998a7..da600cf 100644 (file)
--- a/lib/dml/mssql_native_moodle_database.php
+++ b/lib/dml/mssql_native_moodle_database.php
@@ -517,6 +517,8 @@ class mssql_native_moodle_database extends moodle_database {
      * @return mixed the normalised value
      */
     protected function normalise_value($column, $value) {
+        $this->detect_objects($value);
+
         if (is_bool($value)) { /// Always, convert boolean to int
             $value = (int)$value;
         } // And continue processing because text columns with numeric info need special handling below

diff --git a/lib/dml/mysqli_native_moodle_database.php b/lib/dml/mysqli_native_moodle_database.php
index 242c5c4..2f3ab21 100644 (file)
--- a/lib/dml/mysqli_native_moodle_database.php
+++ b/lib/dml/mysqli_native_moodle_database.php
@@ -651,6 +651,8 @@ class mysqli_native_moodle_database extends moodle_database {
      * @return mixed the normalised value
      */
     protected function normalise_value($column, $value) {
+        $this->detect_objects($value);
+
         if (is_bool($value)) { // Always, convert boolean to int
             $value = (int)$value;
 

diff --git a/lib/dml/oci_native_moodle_database.php b/lib/dml/oci_native_moodle_database.php
index b780bd4..8e10ff1 100644 (file)
--- a/lib/dml/oci_native_moodle_database.php
+++ b/lib/dml/oci_native_moodle_database.php
@@ -682,6 +682,8 @@ class oci_native_moodle_database extends moodle_database {
      * @return mixed the normalised value
      */
     protected function normalise_value($column, $value) {
+        $this->detect_objects($value);
+
         if (is_bool($value)) { // Always, convert boolean to int
             $value = (int)$value;
 

diff --git a/lib/dml/pgsql_native_moodle_database.php b/lib/dml/pgsql_native_moodle_database.php
index d49803b..ed37e64 100644 (file)
--- a/lib/dml/pgsql_native_moodle_database.php
+++ b/lib/dml/pgsql_native_moodle_database.php
@@ -529,6 +529,8 @@ class pgsql_native_moodle_database extends moodle_database {
      * @return mixed the normalised value
      */
     protected function normalise_value($column, $value) {
+        $this->detect_objects($value);
+
         if (is_bool($value)) { // Always, convert boolean to int
             $value = (int)$value;
 
@@ -778,9 +780,10 @@ class pgsql_native_moodle_database extends moodle_database {
 
         $fields = implode(',', array_keys($params));
         $values = array();
-        $count = count($params);
-        for ($i=1; $i<=$count; $i++) {
-            $values[] = "\$".$i;
+        $i = 1;
+        foreach ($params as $value) {
+            $this->detect_objects($value);
+            $values[] = "\$".$i++;
         }
         $values = implode(',', $values);
 
@@ -876,6 +879,7 @@ class pgsql_native_moodle_database extends moodle_database {
         $blobs   = array();
 
         foreach ($dataobject as $field=>$value) {
+            $this->detect_objects($value);
             if (!isset($columns[$field])) {
                 continue;
             }
@@ -932,6 +936,7 @@ class pgsql_native_moodle_database extends moodle_database {
 
         $sets = array();
         foreach ($params as $field=>$value) {
+            $this->detect_objects($value);
             $sets[] = "$field = \$".$i++;
         }
 

diff --git a/lib/dml/simpletest/testdml.php b/lib/dml/simpletest/testdml.php
index f7c2152..39d7642 100644 (file)
--- a/lib/dml/simpletest/testdml.php
+++ b/lib/dml/simpletest/testdml.php
@@ -3062,6 +3062,117 @@ class dml_test extends UnitTestCase {
         $this->assertEqual(1, $DB->count_records($tablename));
     }
 
+    public function test_object_params() {
+        $DB = $this->tdb;
+        $dbman = $DB->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_key('primary', XMLDB_KEY_PRIMARY, array('id'));
+        $dbman->create_table($table);
+
+        $o = new stdClass(); // objects without __toString - never worked
+        try {
+            $DB->fix_sql_params("SELECT {{$tablename}} WHERE course = ? ", array($o));
+            $this->fail('coding_exception expected');
+        } catch (Exception $e) {
+            $this->assertTrue($e instanceof coding_exception);
+        }
+
+        // objects with __toString() forbidden everywhere since 2.3
+        $o = new dml_test_object_one();
+        try {
+            $DB->fix_sql_params("SELECT {{$tablename}} WHERE course = ? ", array($o));
+            $this->fail('coding_exception expected');
+        } catch (Exception $e) {
+            $this->assertTrue($e instanceof coding_exception);
+        }
+
+        try {
+            $DB->execute("SELECT {{$tablename}} WHERE course = ? ", array($o));
+            $this->fail('coding_exception expected');
+        } catch (Exception $e) {
+            $this->assertTrue($e instanceof coding_exception);
+        }
+
+        try {
+            $DB->get_recordset_sql("SELECT {{$tablename}} WHERE course = ? ", array($o));
+            $this->fail('coding_exception expected');
+        } catch (Exception $e) {
+            $this->assertTrue($e instanceof coding_exception);
+        }
+
+        try {
+            $DB->get_records_sql("SELECT {{$tablename}} WHERE course = ? ", array($o));
+            $this->fail('coding_exception expected');
+        } catch (Exception $e) {
+            $this->assertTrue($e instanceof coding_exception);
+        }
+
+        try {
+            $record = new stdClass();
+            $record->course = $o;
+            $DB->insert_record_raw($tablename, $record);
+            $this->fail('coding_exception expected');
+        } catch (Exception $e) {
+            $this->assertTrue($e instanceof coding_exception);
+        }
+
+        try {
+            $record = new stdClass();
+            $record->course = $o;
+            $DB->insert_record($tablename, $record);
+            $this->fail('coding_exception expected');
+        } catch (Exception $e) {
+            $this->assertTrue($e instanceof coding_exception);
+        }
+
+        try {
+            $record = new stdClass();
+            $record->course = $o;
+            $DB->import_record($tablename, $record);
+            $this->fail('coding_exception expected');
+        } catch (Exception $e) {
+            $this->assertTrue($e instanceof coding_exception);
+        }
+
+        try {
+            $record = new stdClass();
+            $record->id = 1;
+            $record->course = $o;
+            $DB->update_record_raw($tablename, $record);
+            $this->fail('coding_exception expected');
+        } catch (Exception $e) {
+            $this->assertTrue($e instanceof coding_exception);
+        }
+
+        try {
+            $record = new stdClass();
+            $record->id = 1;
+            $record->course = $o;
+            $DB->update_record($tablename, $record);
+            $this->fail('coding_exception expected');
+        } catch (Exception $e) {
+            $this->assertTrue($e instanceof coding_exception);
+        }
+
+        try {
+            $DB->set_field_select($tablename, 'course', 1, "course = ? ", array($o));
+            $this->fail('coding_exception expected');
+        } catch (Exception $e) {
+            $this->assertTrue($e instanceof coding_exception);
+        }
+
+        try {
+            $DB->delete_records_select($tablename, "course = ? ", array($o));
+            $this->fail('coding_exception expected');
+        } catch (Exception $e) {
+            $this->assertTrue($e instanceof coding_exception);
+        }
+    }
+
     function test_sql_null_from_clause() {
         $DB = $this->tdb;
         $sql = "SELECT 1 AS id ".$DB->sql_null_from_clause();
@@ -4506,3 +4617,13 @@ class moodle_database_for_testing extends moodle_database {
     public function commit_transaction() {}
     public function rollback_transaction() {}
 }
+
+
+/**
+ * Dumb test class with toString() returrning 1.
+ */
+class dml_test_object_one {
+    public function __toString() {
+        return 1;
+    }
+}
\ No newline at end of file

diff --git a/lib/dml/sqlsrv_native_moodle_database.php b/lib/dml/sqlsrv_native_moodle_database.php
index f8c799f..914c00e 100644 (file)
--- a/lib/dml/sqlsrv_native_moodle_database.php
+++ b/lib/dml/sqlsrv_native_moodle_database.php
@@ -579,6 +579,8 @@ class sqlsrv_native_moodle_database extends moodle_database {
      * @return mixed the normalised value
      */
     protected function normalise_value($column, $value) {
+        $this->detect_objects($value);
+
         if (is_bool($value)) {                               /// Always, convert boolean to int
             $value = (int)$value;
         }                                                    // And continue processing because text columns with numeric info need special handling below

diff --git a/lib/upgrade.txt b/lib/upgrade.txt
index f8f3af8..63c1c10 100644 (file)
--- a/lib/upgrade.txt
+++ b/lib/upgrade.txt
@@ -2,6 +2,12 @@ This files describes API changes in core lbraries and APIs,
 information provided here is intended especially for developers.
 
 
+=== 2.3 ===
+
+Database layer changes:
+* objects are not allowed in paramters of DML functions, use explicit casting to strings if necessary
+
+
 === 2.2 ===
 
 removed unused libraries: