Merge branch 'w12_MDL-32094_integration_fix' of git://github.com/skodak/moodle
authorSam Hemelryk <sam@moodle.com>
Tue, 20 Mar 2012 20:20:29 +0000 (09:20 +1300)
committerSam Hemelryk <sam@moodle.com>
Tue, 20 Mar 2012 20:20:29 +0000 (09:20 +1300)
lib/dml/moodle_database.php
lib/dml/mssql_native_moodle_database.php
lib/dml/mysqli_native_moodle_database.php
lib/dml/oci_native_moodle_database.php
lib/dml/pgsql_native_moodle_database.php
lib/dml/simpletest/testdml.php
lib/dml/sqlsrv_native_moodle_database.php
lib/upgrade.txt
mod/glossary/lib.php
mod/lesson/pagetypes/multichoice.php

index 2baf138..3c736ae 100644 (file)
@@ -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;
         }
 
index d5998a7..da600cf 100644 (file)
@@ -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
index 242c5c4..2f3ab21 100644 (file)
@@ -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;
 
index b780bd4..8e10ff1 100644 (file)
@@ -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;
 
index d49803b..ed37e64 100644 (file)
@@ -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++;
         }
 
index f7c2152..39d7642 100644 (file)
@@ -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
index f8c799f..914c00e 100644 (file)
@@ -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
index f8f3af8..63c1c10 100644 (file)
@@ -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:
index 1242ea0..f8b0d1f 100644 (file)
@@ -2890,7 +2890,12 @@ function glossary_comment_validate($comment_param) {
     if (!$record = $DB->get_record('glossary_entries', array('id'=>$comment_param->itemid))) {
         throw new comment_exception('invalidcommentitemid');
     }
-    if (!$glossary = $DB->get_record('glossary', array('id'=>$record->glossaryid))) {
+    if ($record->sourceglossaryid && $record->sourceglossaryid == $comment_param->cm->instance) {
+        $glossary = $DB->get_record('glossary', array('id'=>$record->sourceglossaryid));
+    } else {
+        $glossary = $DB->get_record('glossary', array('id'=>$record->glossaryid));
+    }
+    if (!$glossary) {
         throw new comment_exception('invalidid', 'data');
     }
     if (!$course = $DB->get_record('course', array('id'=>$glossary->course))) {
index 7ab9d2c..f930756 100644 (file)
@@ -145,8 +145,7 @@ class lesson_page_type_multichoice extends lesson_page {
             $answers = $this->get_used_answers();
             $ncorrect = 0;
             $nhits = 0;
-            $correctresponse = '';
-            $wrongresponse = '';
+            $responses = array();
             $correctanswerid = 0;
             $wronganswerid = 0;
             // store student's answers for displaying on feedback page
@@ -155,6 +154,9 @@ class lesson_page_type_multichoice extends lesson_page {
                 foreach ($studentanswers as $answerid) {
                     if ($answerid == $answer->id) {
                         $result->studentanswer .= '<br />'.format_text($answer->answer, $answer->answerformat, $formattextdefoptions);
+                        if (trim(strip_tags($answer->response))) {
+                            $responses[$answerid] = format_text($answer->response, $answer->responseformat, $formattextdefoptions);
+                        }
                     }
                 }
             }
@@ -182,10 +184,6 @@ class lesson_page_type_multichoice extends lesson_page {
                         if ($correctanswerid == 0) {
                             $correctanswerid = $answer->id;
                         }
-                        // ...also save any response from the correct answers...
-                        if (trim(strip_tags($answer->response))) {
-                            $correctresponse = format_text($answer->response, $answer->responseformat, $formattextdefoptions);
-                        }
                     } else {
                         // save the first jumpto page id, may be needed!...
                         if (!isset($wrongpageid)) {
@@ -196,10 +194,6 @@ class lesson_page_type_multichoice extends lesson_page {
                         if ($wronganswerid == 0) {
                             $wronganswerid = $answer->id;
                         }
-                        // ...and from the incorrect ones, don't know which to use at this stage
-                        if (trim(strip_tags($answer->response))) {
-                            $wrongresponse = format_text($answer->response, $answer->responseformat, $formattextdefoptions);
-                        }
                     }
                 }
             } else {
@@ -220,10 +214,6 @@ class lesson_page_type_multichoice extends lesson_page {
                         if ($correctanswerid == 0) {
                             $correctanswerid = $answer->id;
                         }
-                        // ...also save any response from the correct answers...
-                        if (trim(strip_tags($answer->response))) {
-                            $correctresponse = format_text($answer->response, $answer->responseformat, $formattextdefoptions);
-                        }
                     } else {
                         // save the first jumpto page id, may be needed!...
                         if (!isset($wrongpageid)) {
@@ -234,20 +224,16 @@ class lesson_page_type_multichoice extends lesson_page {
                         if ($wronganswerid == 0) {
                             $wronganswerid = $answer->id;
                         }
-                        // ...and from the incorrect ones, don't know which to use at this stage
-                        if (trim(strip_tags($answer->response))) {
-                            $wrongresponse = format_text($answer->response, $answer->responseformat, $formattextdefoptions);
-                        }
                     }
                 }
             }
             if ((count($studentanswers) == $ncorrect) and ($nhits == $ncorrect)) {
                 $result->correctanswer = true;
-                $result->response  = $correctresponse;
+                $result->response  = implode('<br />', $responses);
                 $result->newpageid = $correctpageid;
                 $result->answerid  = $correctanswerid;
             } else {
-                $result->response  = $wrongresponse;
+                $result->response  = implode('<br />', $responses);
                 $result->newpageid = $wrongpageid;
                 $result->answerid  = $wronganswerid;
             }