From 0d5a27b1b155dfcf782e6da2f4fd2c83300163f7 Mon Sep 17 00:00:00 2001
From: Dan Poltawski <dan@moodle.com>
Date: Thu, 3 Jul 2014 15:52:31 +0100
Subject: [PATCH 01/22] MDL-43848 messages: do not display message content in
 popups

The user is not in control of when these popups show up and it
may not be appropiate to have part of this content display whilst
in the presence of other users.
---
 lib/moodlelib.php | 38 +-------------------------------------
 1 file changed, 1 insertion(+), 37 deletions(-)

diff --git a/lib/moodlelib.php b/lib/moodlelib.php
index 857084bd4c7..020b3685db9 100644
--- a/lib/moodlelib.php
+++ b/lib/moodlelib.php
@@ -10500,43 +10500,7 @@ function message_popup_window() {
     //if we have new messages to notify the user about
     if (!empty($message_users)) {
 
-        $strmessages = '';
-        if (count($message_users)>1) {
-            $strmessages = get_string('unreadnewmessages', 'message', count($message_users));
-        } else {
-            $message_users = reset($message_users);
-
-            //show who the message is from if its not a notification
-            if (!$message_users->notification) {
-                $strmessages = get_string('unreadnewmessage', 'message', fullname($message_users) );
-            }
-
-            //try to display the small version of the message
-            $smallmessage = null;
-            if (!empty($message_users->smallmessage)) {
-                //display the first 200 chars of the message in the popup
-                $smallmessage = null;
-                if (textlib::strlen($message_users->smallmessage) > 200) {
-                    $smallmessage = textlib::substr($message_users->smallmessage,0,200).'...';
-                } else {
-                    $smallmessage = $message_users->smallmessage;
-                }
-
-                //prevent html symbols being displayed
-                if ($message_users->fullmessageformat == FORMAT_HTML) {
-                    $smallmessage = html_to_text($smallmessage);
-                } else {
-                    $smallmessage = s($smallmessage);
-                }
-            } else if ($message_users->notification) {
-                //its a notification with no smallmessage so just say they have a notification
-                $smallmessage = get_string('unreadnewnotification', 'message');
-            }
-            if (!empty($smallmessage)) {
-                $strmessages .= '<div id="usermessage">'.s($smallmessage).'</div>';
-            }
-        }
-
+        $strmessages = get_string('unreadnewmessages', 'message', count($message_users));
         $strgomessage = get_string('gotomessages', 'message');
         $strstaymessage = get_string('ignore','admin');
 

From f0ab42f9858ff157225a8c7cb50ac2504a1e3f97 Mon Sep 17 00:00:00 2001
From: Rex Lorenzo <rex@oid.ucla.edu>
Date: Fri, 16 May 2014 15:26:25 -0700
Subject: [PATCH 02/22] MDL-45485 auth_shibboleth: User taking over other
 user's session

Replacing code with call to complete_user_login() since it calls session_regenerate_id().
---
 auth/shibboleth/index.php | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/auth/shibboleth/index.php b/auth/shibboleth/index.php
index cf9695899ad..9cc3af1b789 100644
--- a/auth/shibboleth/index.php
+++ b/auth/shibboleth/index.php
@@ -49,21 +49,7 @@
         if ($shibbolethauth->user_login($frm->username, $frm->password)
                 && $user = authenticate_user_login($frm->username, $frm->password)) {
 
-            enrol_check_plugins($user);
-            session_set_user($user);
-
-            $USER->loggedin = true;
-            $USER->site     = $CFG->wwwroot; // for added security, store the site in the
-
-            update_user_login_times();
-
-            // Don't show previous shibboleth username on login page
-
-            set_login_session_preferences();
-
-            unset($SESSION->lang);
-            $SESSION->justloggedin = true;
-
+            complete_user_login($user);
             add_to_log(SITEID, 'user', 'login', "view.php?id=$USER->id&course=".SITEID, $USER->id, 0, $USER->id);
 
             if (user_not_fully_set_up($USER)) {

From 3fe105953d14766393e24372806fcf0a2b77c96d Mon Sep 17 00:00:00 2001
From: Marina Glancy <marina@moodle.com>
Date: Wed, 25 Jun 2014 14:40:27 +0800
Subject: [PATCH 03/22] MDL-45616 repositories: more clearly distinguish when
 we use source and when reference

Function repository::get_moodle_file() should always be called on packed reference and not on the source received from user.
Also added phpdocs to some other methods that were confusing source and reference
---
 repository/filepicker.php      |  2 +-
 repository/lib.php             | 23 +++++++++++++----------
 repository/repository_ajax.php |  2 +-
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/repository/filepicker.php b/repository/filepicker.php
index bd19ef7f73b..20d5b872bb6 100644
--- a/repository/filepicker.php
+++ b/repository/filepicker.php
@@ -293,7 +293,7 @@ case 'download':
     // note that in this case user may not have permission to access the source file directly
     // so no file_browser/file_info can be used below
     if ($repo->has_moodle_files()) {
-        $file = repository::get_moodle_file($fileurl);
+        $file = repository::get_moodle_file($reference);
         if ($file && $file->is_external_file()) {
             $sourcefield = $file->get_source(); // remember the original source
             $record->source = $repo::build_source_field($sourcefield);
diff --git a/repository/lib.php b/repository/lib.php
index ecbcb4fc149..bebb88157ff 100644
--- a/repository/lib.php
+++ b/repository/lib.php
@@ -799,13 +799,14 @@ abstract class repository implements cacheable_object {
     }
 
     /**
-     * Parses the 'source' returned by moodle repositories and returns an instance of stored_file
+     * Parses the moodle file reference and returns an instance of stored_file
      *
-     * @param string $source
+     * @param string $reference reference to the moodle internal file as retruned by
+     *        {@link repository::get_file_reference()} or {@link file_storage::pack_reference()}
      * @return stored_file|null
      */
-    public static function get_moodle_file($source) {
-        $params = file_storage::unpack_reference($source, true);
+    public static function get_moodle_file($reference) {
+        $params = file_storage::unpack_reference($reference, true);
         $fs = get_file_storage();
         return $fs->get_file($params['contextid'], $params['component'], $params['filearea'],
                     $params['itemid'], $params['filepath'], $params['filename']);
@@ -817,13 +818,14 @@ abstract class repository implements cacheable_object {
      * This is checked when user tries to pick the file from repository to deal with
      * potential parameter substitutions is request
      *
-     * @param string $source
+     * @param string $source source of the file, returned by repository as 'source' and received back from user (not cleaned)
      * @return bool whether the file is accessible by current user
      */
     public function file_is_accessible($source) {
         if ($this->has_moodle_files()) {
+            $reference = $this->get_file_reference($source);
             try {
-                $params = file_storage::unpack_reference($source, true);
+                $params = file_storage::unpack_reference($reference, true);
             } catch (file_reference_exception $e) {
                 return false;
             }
@@ -1401,12 +1403,13 @@ abstract class repository implements cacheable_object {
      * again to another file area (also as a copy or as a reference), the value of
      * files.source is copied.
      *
-     * @param string $source the value that repository returned in listing as 'source'
+     * @param string $source source of the file, returned by repository as 'source' and received back from user (not cleaned)
      * @return string|null
      */
     public function get_file_source_info($source) {
         if ($this->has_moodle_files()) {
-            return $this->get_reference_details($source, 0);
+            $reference = $this->get_file_reference($source);
+            return $this->get_reference_details($reference, 0);
         }
         return $source;
     }
@@ -1687,11 +1690,11 @@ abstract class repository implements cacheable_object {
     /**
      * Prepare file reference information
      *
-     * @param string $source
+     * @param string $source source of the file, returned by repository as 'source' and received back from user (not cleaned)
      * @return string file referece
      */
     public function get_file_reference($source) {
-        if ($this->has_moodle_files() && ($this->supported_returntypes() & FILE_REFERENCE)) {
+        if ($source && $this->has_moodle_files()) {
             $params = file_storage::unpack_reference($source);
             if (!is_array($params)) {
                 throw new repository_exception('invalidparams', 'repository');
diff --git a/repository/repository_ajax.php b/repository/repository_ajax.php
index 993f383a8be..d591a7c3a57 100644
--- a/repository/repository_ajax.php
+++ b/repository/repository_ajax.php
@@ -238,7 +238,7 @@ switch ($action) {
             // note that in this case user may not have permission to access the source file directly
             // so no file_browser/file_info can be used below
             if ($repo->has_moodle_files()) {
-                $file = repository::get_moodle_file($source);
+                $file = repository::get_moodle_file($reference);
                 if ($file && $file->is_external_file()) {
                     $sourcefield = $file->get_source(); // remember the original source
                     $record->source = $repo::build_source_field($sourcefield);

From 7bcf9b1e2cbdd1e877b828da75b17e3f8318fafc Mon Sep 17 00:00:00 2001
From: Marina Glancy <marina@moodle.com>
Date: Thu, 26 Jun 2014 08:53:25 +0800
Subject: [PATCH 04/22] MDL-45616 repositories: use json encoding instead of
 serialization

---
 repository/coursefiles/lib.php  | 10 +++++-----
 repository/equella/callback.php |  2 +-
 repository/equella/lib.php      | 15 ++++++++++-----
 repository/lib.php              | 26 +++++++++++++++++++++++---
 repository/local/lib.php        |  6 +++---
 repository/recent/lib.php       |  5 +++--
 repository/user/lib.php         |  8 ++++----
 7 files changed, 49 insertions(+), 23 deletions(-)

diff --git a/repository/coursefiles/lib.php b/repository/coursefiles/lib.php
index 30a111de3f6..ccb5dad603e 100644
--- a/repository/coursefiles/lib.php
+++ b/repository/coursefiles/lib.php
@@ -63,7 +63,7 @@ class repository_coursefiles extends repository {
         $browser = get_file_browser();
 
         if (!empty($encodedpath)) {
-            $params = unserialize(base64_decode($encodedpath));
+            $params = json_decode(base64_decode($encodedpath), true);
             if (is_array($params)) {
                 $filepath  = is_null($params['filepath']) ? NULL : clean_param($params['filepath'], PARAM_PATH);
                 $filename  = is_null($params['filename']) ? NULL : clean_param($params['filename'], PARAM_FILE);
@@ -80,12 +80,12 @@ class repository_coursefiles extends repository {
         if ($fileinfo = $browser->get_file_info($context, $component, $filearea, $itemid, $filepath, $filename)) {
             // build path navigation
             $pathnodes = array();
-            $encodedpath = base64_encode(serialize($fileinfo->get_params()));
+            $encodedpath = base64_encode(json_encode($fileinfo->get_params()));
             $pathnodes[] = array('name'=>$fileinfo->get_visible_name(), 'path'=>$encodedpath);
             $level = $fileinfo->get_parent();
             while ($level) {
                 $params = $level->get_params();
-                $encodedpath = base64_encode(serialize($params));
+                $encodedpath = base64_encode(json_encode($params));
                 if ($params['contextid'] != $context->id) {
                     break;
                 }
@@ -102,7 +102,7 @@ class repository_coursefiles extends repository {
                 if ($child->is_directory()) {
                     $params = $child->get_params();
                     $subdir_children = $child->get_children();
-                    $encodedpath = base64_encode(serialize($params));
+                    $encodedpath = base64_encode(json_encode($params));
                     $node = array(
                         'title' => $child->get_visible_name(),
                         'datemodified' => $child->get_timemodified(),
@@ -113,7 +113,7 @@ class repository_coursefiles extends repository {
                     );
                     $list[] = $node;
                 } else {
-                    $encodedpath = base64_encode(serialize($child->get_params()));
+                    $encodedpath = base64_encode(json_encode($child->get_params()));
                     $node = array(
                         'title' => $child->get_visible_name(),
                         'size' => $child->get_filesize(),
diff --git a/repository/equella/callback.php b/repository/equella/callback.php
index 01fc27dc7f3..f1c044d4001 100644
--- a/repository/equella/callback.php
+++ b/repository/equella/callback.php
@@ -55,7 +55,7 @@ if (isset($info->license)) {
     $license = s(clean_param($info->license, PARAM_ALPHAEXT));
 }
 
-$source = base64_encode(serialize((object)array('url'=>$url,'filename'=>$filename)));
+$source = base64_encode(json_encode(array('url'=>$url,'filename'=>$filename)));
 
 $js =<<<EOD
 <html>
diff --git a/repository/equella/lib.php b/repository/equella/lib.php
index 94b575b58b6..35178943816 100644
--- a/repository/equella/lib.php
+++ b/repository/equella/lib.php
@@ -125,7 +125,11 @@ class repository_equella extends repository {
      * @return string file referece
      */
     public function get_file_reference($source) {
-        return $source;
+        // Internally we store serialized value but user input is json-encoded for security reasons.
+        $ref = json_decode(base64_decode($source));
+        $filename  = clean_param($ref->filename, PARAM_FILE);
+        $url = clean_param($ref->url, PARAM_URL);
+        return base64_encode(serialize((object)array('url' => $url, 'filename' => $filename)));
     }
 
     /**
@@ -414,12 +418,13 @@ class repository_equella extends repository {
     /**
      * Return the source information
      *
-     * @param stdClass $url
+     * @param string $source
      * @return string|null
      */
-    public function get_file_source_info($url) {
-        $ref = unserialize(base64_decode($url));
-        return 'EQUELLA: ' . $ref->filename;
+    public function get_file_source_info($source) {
+        $ref = json_decode(base64_decode($source));
+        $filename  = clean_param($ref->filename, PARAM_FILE);
+        return 'EQUELLA: ' . $filename;
     }
 
     /**
diff --git a/repository/lib.php b/repository/lib.php
index bebb88157ff..fbaf167eff6 100644
--- a/repository/lib.php
+++ b/repository/lib.php
@@ -1691,12 +1691,32 @@ abstract class repository implements cacheable_object {
      * Prepare file reference information
      *
      * @param string $source source of the file, returned by repository as 'source' and received back from user (not cleaned)
-     * @return string file referece
+     * @return string file reference, ready to be stored
      */
     public function get_file_reference($source) {
         if ($source && $this->has_moodle_files()) {
-            $params = file_storage::unpack_reference($source);
-            if (!is_array($params)) {
+            $params = @json_decode(base64_decode($source), true);
+            if (!$params && !in_array($this->get_typename(), array('recent', 'user', 'local', 'coursefiles'))) {
+                // IMPORTANT! Since default format for moodle files was changed in the minor release as a security fix
+                // we maintain an old code here in order not to break 3rd party repositories that deal
+                // with moodle files. Repositories are strongly encouraged to be upgraded, see MDL-45616.
+                // In Moodle 2.8 this fallback will be removed.
+                $params = file_storage::unpack_reference($source, true);
+                return file_storage::pack_reference($params);
+            }
+            if (!is_array($params) || empty($params['contextid'])) {
+                throw new repository_exception('invalidparams', 'repository');
+            }
+            $params = array(
+                'component' => empty($params['component']) ? ''   : clean_param($params['component'], PARAM_COMPONENT),
+                'filearea'  => empty($params['filearea'])  ? ''   : clean_param($params['filearea'], PARAM_AREA),
+                'itemid'    => empty($params['itemid'])    ? 0    : clean_param($params['itemid'], PARAM_INT),
+                'filename'  => empty($params['filename'])  ? null : clean_param($params['filename'], PARAM_FILE),
+                'filepath'  => empty($params['filepath'])  ? null : clean_param($params['filepath'], PARAM_PATH),
+                'contextid' => clean_param($params['contextid'], PARAM_INT)
+            );
+            // Check if context exists.
+            if (!context::instance_by_id($params['contextid'], IGNORE_MISSING)) {
                 throw new repository_exception('invalidparams', 'repository');
             }
             return file_storage::pack_reference($params);
diff --git a/repository/local/lib.php b/repository/local/lib.php
index 93bdd2062ec..400392a63aa 100644
--- a/repository/local/lib.php
+++ b/repository/local/lib.php
@@ -56,7 +56,7 @@ class repository_local extends repository {
         $component = null;
 
         if (!empty($encodedpath)) {
-            $params = unserialize(base64_decode($encodedpath));
+            $params = json_decode(base64_decode($encodedpath), true);
             if (is_array($params) && isset($params['contextid'])) {
                 $component = is_null($params['component']) ? NULL : clean_param($params['component'], PARAM_COMPONENT);
                 $filearea  = is_null($params['filearea']) ? NULL : clean_param($params['filearea'], PARAM_AREA);
@@ -216,7 +216,7 @@ class repository_local extends repository {
      */
     private function get_node(file_info $fileinfo) {
         global $OUTPUT;
-        $encodedpath = base64_encode(serialize($fileinfo->get_params()));
+        $encodedpath = base64_encode(json_encode($fileinfo->get_params()));
         $node = array(
             'title' => $fileinfo->get_visible_name(),
             'datemodified' => $fileinfo->get_timemodified(),
@@ -256,7 +256,7 @@ class repository_local extends repository {
      * @return array
      */
     private function get_node_path(file_info $fileinfo) {
-        $encodedpath = base64_encode(serialize($fileinfo->get_params()));
+        $encodedpath = base64_encode(json_encode($fileinfo->get_params()));
         return array(
             'path' => $encodedpath,
             'name' => $fileinfo->get_visible_name()
diff --git a/repository/recent/lib.php b/repository/recent/lib.php
index 2471aa6093c..75b64d2932b 100644
--- a/repository/recent/lib.php
+++ b/repository/recent/lib.php
@@ -126,7 +126,7 @@ class repository_recent extends repository {
                 $fileinfo = $browser->get_file_info($context, $file['component'],
                         $file['filearea'], $file['itemid'], $file['filepath'], $file['filename']);
                 if ($fileinfo) {
-                    $params = base64_encode(serialize($file));
+                    $params = base64_encode(json_encode($file));
                     $node = array(
                         'title' => $fileinfo->get_visible_name(),
                         'size' => $fileinfo->get_filesize(),
@@ -192,7 +192,8 @@ class repository_recent extends repository {
      */
     public function file_is_accessible($source) {
         global $USER;
-        $file = self::get_moodle_file($source);
+        $reference = $this->get_file_reference($source);
+        $file = self::get_moodle_file($reference);
         return (!empty($file) && $file->get_userid() == $USER->id);
     }
 
diff --git a/repository/user/lib.php b/repository/user/lib.php
index 4e536d56ab6..1c4be73742d 100644
--- a/repository/user/lib.php
+++ b/repository/user/lib.php
@@ -61,7 +61,7 @@ class repository_user extends repository {
         $list = array();
 
         if (!empty($encodedpath)) {
-            $params = unserialize(base64_decode($encodedpath));
+            $params = json_decode(base64_decode($encodedpath), true);
             if (is_array($params)) {
                 $filepath = clean_param($params['filepath'], PARAM_PATH);
                 $filename = clean_param($params['filename'], PARAM_FILE);
@@ -84,7 +84,7 @@ class repository_user extends repository {
                 $level = $fileinfo;
                 $params = $fileinfo->get_params();
                 while ($level && $params['component'] == 'user' && $params['filearea'] == 'private') {
-                    $encodedpath = base64_encode(serialize($level->get_params()));
+                    $encodedpath = base64_encode(json_encode($level->get_params()));
                     $pathnodes[] = array('name'=>$level->get_visible_name(), 'path'=>$encodedpath);
                     $level = $level->get_parent();
                     $params = $level->get_params();
@@ -95,7 +95,7 @@ class repository_user extends repository {
                 $children = $fileinfo->get_children();
                 foreach ($children as $child) {
                     if ($child->is_directory()) {
-                        $encodedpath = base64_encode(serialize($child->get_params()));
+                        $encodedpath = base64_encode(json_encode($child->get_params()));
                         $node = array(
                             'title' => $child->get_visible_name(),
                             'datemodified' => $child->get_timemodified(),
@@ -106,7 +106,7 @@ class repository_user extends repository {
                         );
                         $list[] = $node;
                     } else {
-                        $encodedpath = base64_encode(serialize($child->get_params()));
+                        $encodedpath = base64_encode(json_encode($child->get_params()));
                         $node = array(
                             'title' => $child->get_visible_name(),
                             'size' => $child->get_filesize(),

From cadfc081976564e90fccf692f0b954ae36ed87dc Mon Sep 17 00:00:00 2001
From: Frederic Massart <fred@moodle.com>
Date: Tue, 10 Jun 2014 12:53:43 +0800
Subject: [PATCH 05/22] MDL-45463 mod_lti: Prevent XML entity injections from
 provider

---
 mod/lti/service.php | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/mod/lti/service.php b/mod/lti/service.php
index 60bcfa2caf8..d3645b4140c 100644
--- a/mod/lti/service.php
+++ b/mod/lti/service.php
@@ -53,7 +53,14 @@ if ($sharedsecret === false) {
     throw new Exception('Message signature not valid');
 }
 
-$xml = new SimpleXMLElement($rawbody);
+// TODO MDL-46023 Replace this code with a call to the new library.
+$origentity = libxml_disable_entity_loader(true);
+$xml = simplexml_load_string($rawbody);
+if (!$xml) {
+    libxml_disable_entity_loader($origentity);
+    throw new Exception('Invalid XML content');
+}
+libxml_disable_entity_loader($origentity);
 
 $body = $xml->imsx_POXBody;
 foreach ($body->children() as $child) {

From 01cf8fdd9cb75091f1688243012c7a236c110bee Mon Sep 17 00:00:00 2001
From: Frederic Massart <fred@moodle.com>
Date: Fri, 13 Jun 2014 15:40:15 +0800
Subject: [PATCH 06/22] MDL-45417 mod_imscp: Prevent entity injections from
 package content

---
 mod/imscp/locallib.php | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mod/imscp/locallib.php b/mod/imscp/locallib.php
index b3dbafbf9a8..888802a648a 100644
--- a/mod/imscp/locallib.php
+++ b/mod/imscp/locallib.php
@@ -105,9 +105,11 @@ function imscp_parse_structure($imscp, $context) {
  */
 function imscp_parse_manifestfile($manifestfilecontents, $imscp, $context) {
     $doc = new DOMDocument();
+    $oldentities = libxml_disable_entity_loader(true);
     if (!$doc->loadXML($manifestfilecontents, LIBXML_NONET)) {
         return null;
     }
+    libxml_disable_entity_loader($oldentities);
 
     // we put this fake URL as base in order to detect path changes caused by xml:base attributes
     $doc->documentURI = 'http://grrr/';
@@ -205,10 +207,14 @@ function imscp_recursive_href($manifestfilename, $imscp, $context) {
     if (!$manifestfile = $fs->get_file($context->id, 'mod_imscp', 'content', $imscp->revision, $dirname, $filename)) {
         return null;
     }
+
     $doc = new DOMDocument();
+    $oldentities = libxml_disable_entity_loader(true);
     if (!$doc->loadXML($manifestfile->get_content(), LIBXML_NONET)) {
         return null;
     }
+    libxml_disable_entity_loader($oldentities);
+
     $xmlresources = $doc->getElementsByTagName('resource');
     foreach ($xmlresources as $res) {
         if (!$href = $res->attributes->getNamedItem('href')) {

From bee1c88151e1814804df037803bbefbef343eaf5 Mon Sep 17 00:00:00 2001
From: Tim Hunt <T.J.Hunt@open.ac.uk>
Date: Thu, 26 Jun 2014 10:48:02 +0100
Subject: [PATCH 07/22] MDL-46148 qtype_calculated: fix formula validation.

---
 .../type/calculated/edit_calculated_form.php  |  45 ++++----
 question/type/calculated/question.php         | 104 +----------------
 question/type/calculated/questiontype.php     |  83 +++++++++++++-
 .../tests/formula_validation_test.php         | 105 ++++++++++++++++++
 .../edit_calculatedmulti_form.php             |  64 +++++------
 .../type/calculatedmulti/questiontype.php     |   7 ++
 6 files changed, 245 insertions(+), 163 deletions(-)
 create mode 100644 question/type/calculated/tests/formula_validation_test.php

diff --git a/question/type/calculated/edit_calculated_form.php b/question/type/calculated/edit_calculated_form.php
index d6d9f450e90..c3c02a90533 100644
--- a/question/type/calculated/edit_calculated_form.php
+++ b/question/type/calculated/edit_calculated_form.php
@@ -212,36 +212,39 @@ class qtype_calculated_edit_form extends qtype_numerical_edit_form {
         return 'calculated';
     }
 
+    /**
+     * Validate the equations in the some question content.
+     * @param array $errors where errors are being accumulated.
+     * @param string $field the field being validated.
+     * @param string $text the content of that field.
+     * @return array the updated $errors array.
+     */
+    protected function validate_text($errors, $field, $text) {
+        $problems = qtype_calculated_find_formula_errors_in_text($text);
+        if ($problems) {
+            $errors[$field] = $problems;
+        }
+        return $errors;
+    }
+
     public function validation($data, $files) {
+        $errors = parent::validation($data, $files);
 
         // Verifying for errors in {=...} in question text.
-        $qtext = "";
-        $qtextremaining = $data['questiontext']['text'];
-        $possibledatasets = $this->qtypeobj->find_dataset_names($data['questiontext']['text']);
-        foreach ($possibledatasets as $name => $value) {
-            $qtextremaining = str_replace('{'.$name.'}', '1', $qtextremaining);
-        }
-        while (preg_match('~\{=([^[:space:]}]*)}~', $qtextremaining, $regs1)) {
-            $qtextsplits = explode($regs1[0], $qtextremaining, 2);
-            $qtext = $qtext.$qtextsplits[0];
-            $qtextremaining = $qtextsplits[1];
-            if (!empty($regs1[1]) && $formulaerrors =
-                    qtype_calculated_find_formula_errors($regs1[1])) {
-                if (!isset($errors['questiontext'])) {
-                    $errors['questiontext'] = $formulaerrors.':'.$regs1[1];
-                } else {
-                    $errors['questiontext'] .= '<br/>'.$formulaerrors.':'.$regs1[1];
-                }
-            }
-        }
-
-        $errors = parent::validation($data, $files);
+        $errors = $this->validate_text($errors, 'questiontext', $data['questiontext']['text']);
+        $errors = $this->validate_text($errors, 'generalfeedback', $data['generalfeedback']['text']);
 
         // Check that the answers use datasets.
         $answers = $data['answer'];
         $mandatorydatasets = array();
         foreach ($answers as $key => $answer) {
+            $problems = qtype_calculated_find_formula_errors($answer);
+            if ($problems) {
+                $errors['answeroptions['.$key.']'] = $problems;
+            }
             $mandatorydatasets += $this->qtypeobj->find_dataset_names($answer);
+            $errors = $this->validate_text($errors, 'feedback[' . $key . ']',
+                    $data['feedback'][$key]['text']);
         }
         if (empty($mandatorydatasets)) {
             foreach ($answers as $key => $answer) {
diff --git a/question/type/calculated/question.php b/question/type/calculated/question.php
index b3246855d70..43f92b6647c 100644
--- a/question/type/calculated/question.php
+++ b/question/type/calculated/question.php
@@ -259,6 +259,7 @@ class qtype_calculated_dataset_loader {
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class qtype_calculated_variable_substituter {
+
     /** @var array variable name => value */
     protected $values;
 
@@ -388,108 +389,11 @@ class qtype_calculated_variable_substituter {
      * @return string the text with values substituted.
      */
     public function replace_expressions_in_text($text, $length = null, $format = null) {
-        $vs = $this; // Can't see to use $this in a PHP closure.
-        $text = preg_replace_callback('~\{=([^{}]*(?:\{[^{}]+}[^{}]*)*)}~',
+        $vs = $this; // Can't use $this in a PHP closure.
+        $text = preg_replace_callback(qtype_calculated::FORMULAS_IN_TEXT_REGEX,
                 function ($matches) use ($vs, $format, $length) {
                     return $vs->format_float($vs->calculate($matches[1]), $length, $format);
                 }, $text);
         return $this->substitute_values_pretty($text);
     }
-
-    /**
-     * Return an array describing any problems there are with an expression.
-     * Returns false if the expression is fine.
-     * @param string $formula an expression.
-     * @return array|false list of problems, or false if the exression is OK.
-     */
-    public function get_formula_errors($formula) {
-        // Validates the formula submitted from the question edit page.
-        // Returns false if everything is alright
-        // otherwise it constructs an error message.
-        // Strip away dataset names.
-        while (preg_match('~\\{[[:alpha:]][^>} <{"\']*\\}~', $formula, $regs)) {
-            $formula = str_replace($regs[0], '1', $formula);
-        }
-
-        // Strip away empty space and lowercase it.
-        $formula = strtolower(str_replace(' ', '', $formula));
-
-        $safeoperatorchar = '-+/*%>:^\~<?=&|!'; /* */
-        $operatorornumber = "[$safeoperatorchar.0-9eE]";
-
-        while (preg_match("~(^|[$safeoperatorchar,(])([a-z0-9_]*)" .
-                "\\(($operatorornumber+(,$operatorornumber+((,$operatorornumber+)+)?)?)?\\)~",
-            $formula, $regs)) {
-            switch ($regs[2]) {
-                // Simple parenthesis.
-                case '':
-                    if ((isset($regs[4]) && $regs[4]) || strlen($regs[3]) == 0) {
-                        return get_string('illegalformulasyntax', 'qtype_calculated', $regs[0]);
-                    }
-                    break;
-
-                    // Zero argument functions.
-                case 'pi':
-                    if ($regs[3]) {
-                        return get_string('functiontakesnoargs', 'qtype_calculated', $regs[2]);
-                    }
-                    break;
-
-                    // Single argument functions (the most common case).
-                case 'abs': case 'acos': case 'acosh': case 'asin': case 'asinh':
-                case 'atan': case 'atanh': case 'bindec': case 'ceil': case 'cos':
-                case 'cosh': case 'decbin': case 'decoct': case 'deg2rad':
-                case 'exp': case 'expm1': case 'floor': case 'is_finite':
-                case 'is_infinite': case 'is_nan': case 'log10': case 'log1p':
-                case 'octdec': case 'rad2deg': case 'sin': case 'sinh': case 'sqrt':
-                case 'tan': case 'tanh':
-                    if (!empty($regs[4]) || empty($regs[3])) {
-                        return get_string('functiontakesonearg', 'qtype_calculated', $regs[2]);
-                    }
-                    break;
-
-                    // Functions that take one or two arguments.
-                case 'log': case 'round':
-                    if (!empty($regs[5]) || empty($regs[3])) {
-                        return get_string('functiontakesoneortwoargs', 'qtype_calculated',
-                                $regs[2]);
-                    }
-                    break;
-
-                    // Functions that must have two arguments.
-                case 'atan2': case 'fmod': case 'pow':
-                    if (!empty($regs[5]) || empty($regs[4])) {
-                        return get_string('functiontakestwoargs', 'qtype_calculated', $regs[2]);
-                    }
-                    break;
-
-                    // Functions that take two or more arguments.
-                case 'min': case 'max':
-                    if (empty($regs[4])) {
-                        return get_string('functiontakesatleasttwo', 'qtype_calculated', $regs[2]);
-                    }
-                    break;
-
-                default:
-                    return get_string('unsupportedformulafunction', 'qtype_calculated', $regs[2]);
-            }
-
-            // Exchange the function call with '1' and then check for another function call.
-
-            if ($regs[1]) {
-                // The function call is proceeded by an operator.
-                $formula = str_replace($regs[0], $regs[1] . '1', $formula);
-            } else {
-                // The function call starts the formula.
-                $formula = preg_replace("~^$regs[2]\\([^)]*\\)~", '1', $formula);
-            }
-        }
-
-        if (preg_match("~[^$safeoperatorchar.0-9eE]+~", $formula, $regs)) {
-            return get_string('illegalformulasyntax', 'qtype_calculated', $regs[0]);
-        } else {
-            // Formula just might be valid.
-            return false;
-        }
-    }
-}
\ No newline at end of file
+}
diff --git a/question/type/calculated/questiontype.php b/question/type/calculated/questiontype.php
index 3e9143ebe94..fb0ed991c78 100644
--- a/question/type/calculated/questiontype.php
+++ b/question/type/calculated/questiontype.php
@@ -37,6 +37,9 @@ require_once($CFG->dirroot . '/question/type/numerical/question.php');
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class qtype_calculated extends question_type {
+    /** Regular expression that finds the formulas in content. */
+    const FORMULAS_IN_TEXT_REGEX = '~\{=([^{}]*(?:\{[^{}]+}[^{}]*)*)\}~';
+
     const MAX_DATASET_ITEMS = 100;
 
     public $wizardpagesnumber = 3;
@@ -481,6 +484,36 @@ class qtype_calculated extends question_type {
         $mform->display();
     }
 
+    /**
+     * Verify that the equations in part of the question are OK.
+     * We throw an exception here because this should have already been validated
+     * by the form. This is just a last line of defence to prevent a question
+     * being stored in the database if it has bad formulas. This saves us from,
+     * for example, malicious imports.
+     * @param string $text containing equations.
+     */
+    protected function validate_text($text) {
+        $error = qtype_calculated_find_formula_errors_in_text($text);
+        if ($error) {
+            throw new coding_exception($error);
+        }
+    }
+
+    /**
+     * Verify that an answer is OK.
+     * We throw an exception here because this should have already been validated
+     * by the form. This is just a last line of defence to prevent a question
+     * being stored in the database if it has bad formulas. This saves us from,
+     * for example, malicious imports.
+     * @param string $text containing equations.
+     */
+    protected function validate_answer($answer) {
+        $error = qtype_calculated_find_formula_errors($answer);
+        if ($error) {
+            throw new coding_exception($error);
+        }
+    }
+
     /**
      * This method prepare the $datasets in a format similar to dadatesetdefinitions_form.php
      * so that they can be saved
@@ -494,11 +527,12 @@ class qtype_calculated extends question_type {
      * @param int $questionfromid default = '0'
      */
     public function preparedatasets($form , $questionfromid = '0') {
+
         // The dataset names present in the edit_question_form and edit_calculated_form
         // are retrieved.
         $possibledatasets = $this->find_dataset_names($form->questiontext);
         $mandatorydatasets = array();
-        foreach ($form->answers as $answer) {
+        foreach ($form->answers as $key => $answer) {
             $mandatorydatasets += $this->find_dataset_names($answer);
         }
         // If there are identical datasetdefs already saved in the original question
@@ -587,8 +621,15 @@ class qtype_calculated extends question_type {
      */
     public function save_question($question, $form) {
         global $DB;
+
+        if (isset($form->correctfeedback)) {
+            $this->validate_text($form->correctfeedback['text']);
+            $this->validate_text($form->partiallycorrectfeedback['text']);
+            $this->validate_text($form->incorrectfeedback['text']);
+        }
+
         if ($this->wizardpagesnumber() == 1 || $question->qtype == 'calculatedsimple') {
-                $question = parent::save_question($question, $form);
+            $question = parent::save_question($question, $form);
             return $question;
         }
 
@@ -607,6 +648,14 @@ class qtype_calculated extends question_type {
             case '' :
             case 'question': // Coming from the first page, creating the second.
                 if (empty($form->id)) { // or a new question $form->id is empty.
+                    // Make it impossible to save bad formulas anywhere.
+                    $this->validate_text($form->questiontext['text']);
+                    $this->validate_text($form->generalfeedback['text']);
+                    foreach ($form->answer as $key => $answer) {
+                        $this->validate_answer($answer);
+                        $this->validate_text($form->feedback[$key]['text']);
+                    }
+
                     $question = parent::save_question($question, $form);
                     // Prepare the datasets using default $questionfromid.
                     $this->preparedatasets($form);
@@ -1972,6 +2021,11 @@ function qtype_calculated_calculate_answer($formula, $individualdata,
 }
 
 
+/**
+ * Validate a forumula.
+ * @param string $formula the formula to validate.
+ * @return string|boolean false if there are no problems. Otherwise a string error message.
+ */
 function qtype_calculated_find_formula_errors($formula) {
     // Validates the formula submitted from the question edit page.
     // Returns false if everything is alright
@@ -2000,7 +2054,7 @@ function qtype_calculated_find_formula_errors($formula) {
 
                 // Zero argument functions.
             case 'pi':
-                if ($regs[3]) {
+                if (array_key_exists(3, $regs)) {
                     return get_string('functiontakesnoargs', 'qtype_calculated', $regs[2]);
                 }
                 break;
@@ -2061,3 +2115,26 @@ function qtype_calculated_find_formula_errors($formula) {
         return false;
     }
 }
+
+/**
+ * Validate all the forumulas in a bit of text.
+ * @param string $text the text in which to validate the formulas.
+ * @return string|boolean false if there are no problems. Otherwise a string error message.
+ */
+function qtype_calculated_find_formula_errors_in_text($text) {
+    preg_match_all(qtype_calculated::FORMULAS_IN_TEXT_REGEX, $text, $matches);
+
+    $errors = array();
+    foreach ($matches[1] as $match) {
+        $error = qtype_calculated_find_formula_errors($match);
+        if ($error) {
+            $errors[] = $error;
+        }
+    }
+
+    if ($errors) {
+        return implode(' ', $errors);
+    }
+
+    return false;
+}
diff --git a/question/type/calculated/tests/formula_validation_test.php b/question/type/calculated/tests/formula_validation_test.php
new file mode 100644
index 00000000000..d7ff76ed18e
--- /dev/null
+++ b/question/type/calculated/tests/formula_validation_test.php
@@ -0,0 +1,105 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Unit tests for formula validation code.
+ *
+ * @package    qtype_calculated
+ * @copyright  2014 The Open University
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+require_once($CFG->dirroot . '/question/type/calculated/questiontype.php');
+
+
+/**
+ * Unit tests for formula validation code.
+ *
+ * @copyright  2014 The Open University
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class qtype_calculated_formula_validation_testcase extends basic_testcase {
+    protected function assert_nonempty_string($actual) {
+        $this->assertInternalType('string', $actual);
+        $this->assertNotEquals('', $actual);
+    }
+
+    public function test_simple_equations_ok() {
+        $this->assertFalse(qtype_calculated_find_formula_errors(1));
+        $this->assertFalse(qtype_calculated_find_formula_errors('1 + 1'));
+        $this->assertFalse(qtype_calculated_find_formula_errors('{x} + {y}'));
+        $this->assertFalse(qtype_calculated_find_formula_errors('{x}*{y}'));
+    }
+
+    public function test_safe_functions_ok() {
+        $this->assertFalse(qtype_calculated_find_formula_errors('abs(-1)'));
+        $this->assertFalse(qtype_calculated_find_formula_errors('tan(pi())'));
+        $this->assertFalse(qtype_calculated_find_formula_errors('log(10)'));
+        $this->assertFalse(qtype_calculated_find_formula_errors('log(64, 2)'));
+        $this->assertFalse(qtype_calculated_find_formula_errors('atan2(1.0, 1.0)'));
+        $this->assertFalse(qtype_calculated_find_formula_errors('max(1.0, 1.0)'));
+        $this->assertFalse(qtype_calculated_find_formula_errors('max(1.0, 1.0, 2.0)'));
+        $this->assertFalse(qtype_calculated_find_formula_errors('max(1.0, 1.0, 2, 3)'));
+    }
+
+    public function test_dangerous_functions_blocked() {
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('eval(1)'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('system(1)'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('base64_decode(1)'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('unserialize(1)'));
+
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('cos(tan(1) + abs(cos(eval)) * pi())'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('eval (CONSTANTREADASSTRING)'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors("eval \t ()"));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('"eval"()'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('?><?php()'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('?><?php+1'));
+    }
+
+    public function test_functions_with_wrong_num_args_caught() {
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('abs(-1, 1)'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('abs()'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('pi(1)'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('log()'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('log(64, 2, 3)'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('atan2(1.0)'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('atan2(1.0, 1.0, 2.0)'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors('max(1.0)'));
+    }
+
+    public function test_validation_of_formulas_in_text_ok() {
+        $this->assertFalse(qtype_calculated_find_formula_errors_in_text(
+                '<p>Look no equations.</p>'));
+        $this->assertFalse(qtype_calculated_find_formula_errors_in_text(
+                '<p>Simple variable: {x}.</p>'));
+        $this->assertFalse(qtype_calculated_find_formula_errors_in_text(
+                '<p>This is an equation: {=1+1}, as is this: {={x}+{y}}.</p>' .
+                '<p>Here is a more complex one: {=sin(2*pi()*{theta})}.</p>'));
+    }
+
+    public function test_validation_of_formulas_in_text_bad_function() {
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors_in_text(
+                '<p>This is an equation: {=eval(1)}.</p>'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors_in_text(
+                '<p>Good: {=1+1}, bad: {=eval(1)}, good: {={x}+{y}}.</p>'));
+        $this->assert_nonempty_string(qtype_calculated_find_formula_errors_in_text(
+                '<p>Bad: {=eval(1)}, bad: {=system(1)}.</p>'));
+    }
+}
diff --git a/question/type/calculatedmulti/edit_calculatedmulti_form.php b/question/type/calculatedmulti/edit_calculatedmulti_form.php
index eee479f1138..56fce17908b 100644
--- a/question/type/calculatedmulti/edit_calculatedmulti_form.php
+++ b/question/type/calculatedmulti/edit_calculatedmulti_form.php
@@ -228,30 +228,31 @@ class qtype_calculatedmulti_edit_form extends question_edit_form {
         return $question;
     }
 
+    /**
+     * Validate the equations in the some question content.
+     * @param array $errors where errors are being accumulated.
+     * @param string $field the field being validated.
+     * @param string $text the content of that field.
+     * @return array the updated $errors array.
+     */
+    protected function validate_text($errors, $field, $text) {
+        $problems = qtype_calculated_find_formula_errors_in_text($text);
+        if ($problems) {
+            $errors[$field] = $problems;
+        }
+        return $errors;
+    }
+
     public function validation($data, $files) {
         $errors = parent::validation($data, $files);
 
         // Verifying for errors in {=...} in question text.
-        $qtext = '';
-        $qtextremaining = $data['questiontext']['text'];
-        $possibledatasets = $this->qtypeobj->find_dataset_names($data['questiontext']['text']);
-        foreach ($possibledatasets as $name => $value) {
-            $qtextremaining = str_replace('{'.$name.'}', '1', $qtextremaining);
-        }
+        $errors = $this->validate_text($errors, 'questiontext', $data['questiontext']['text']);
+        $errors = $this->validate_text($errors, 'generalfeedback', $data['generalfeedback']['text']);
+        $errors = $this->validate_text($errors, 'correctfeedback', $data['correctfeedback']['text']);
+        $errors = $this->validate_text($errors, 'partiallycorrectfeedback', $data['partiallycorrectfeedback']['text']);
+        $errors = $this->validate_text($errors, 'incorrectfeedback', $data['incorrectfeedback']['text']);
 
-        while (preg_match('~\{=([^[:space:]}]*)}~', $qtextremaining, $regs1)) {
-            $qtextsplits = explode($regs1[0], $qtextremaining, 2);
-            $qtext = $qtext.$qtextsplits[0];
-            $qtextremaining = $qtextsplits[1];
-            if (!empty($regs1[1]) && $formulaerrors =
-                    qtype_calculated_find_formula_errors($regs1[1])) {
-                if (!isset($errors['questiontext'])) {
-                    $errors['questiontext'] = $formulaerrors.':'.$regs1[1];
-                } else {
-                    $errors['questiontext'] .= '<br/>'.$formulaerrors.':'.$regs1[1];
-                }
-            }
-        }
         $answers = $data['answer'];
         $answercount = 0;
         $maxgrade = false;
@@ -266,6 +267,7 @@ class qtype_calculatedmulti_edit_form extends question_edit_form {
                         get_string('atleastonewildcard', 'qtype_calculated');
             }
         }
+
         $totalfraction = 0;
         $maxfraction = -1;
         foreach ($answers as $key => $answer) {
@@ -279,27 +281,11 @@ class qtype_calculatedmulti_edit_form extends question_edit_form {
             }
             if ($trimmedanswer != '' || $answercount == 0) {
                 // Verifying for errors in {=...} in answer text.
-                $qanswer = '';
-                $qanswerremaining =  $trimmedanswer;
-                $possibledatasets = $this->qtypeobj->find_dataset_names($trimmedanswer);
-                foreach ($possibledatasets as $name => $value) {
-                    $qanswerremaining = str_replace('{'.$name.'}', '1', $qanswerremaining);
-                }
-
-                while (preg_match('~\{=([^[:space:]}]*)}~', $qanswerremaining, $regs1)) {
-                    $qanswersplits = explode($regs1[0], $qanswerremaining, 2);
-                    $qanswer = $qanswer . $qanswersplits[0];
-                    $qanswerremaining = $qanswersplits[1];
-                    if (!empty($regs1[1]) && $formulaerrors =
-                            qtype_calculated_find_formula_errors($regs1[1])) {
-                        if (!isset($errors['answeroptions['.$key.']'])) {
-                            $errors['answeroptions['.$key.']'] = $formulaerrors.':'.$regs1[1];
-                        } else {
-                            $errors['answeroptions['.$key.']'] .= '<br/>'.$formulaerrors.':'.$regs1[1];
-                        }
-                    }
-                }
+                $errors = $this->validate_text($errors, 'answeroptions[' . $key . ']', $answer);
+                $errors = $this->validate_text($errors, 'feedback[' . $key . ']',
+                        $data['feedback'][$key]['text']);
             }
+
             if ($trimmedanswer != '') {
                 if ('2' == $data['correctanswerformat'][$key] &&
                         '0' == $data['correctanswerlength'][$key]) {
diff --git a/question/type/calculatedmulti/questiontype.php b/question/type/calculatedmulti/questiontype.php
index 2235145855a..1cd1348d4bb 100644
--- a/question/type/calculatedmulti/questiontype.php
+++ b/question/type/calculatedmulti/questiontype.php
@@ -156,6 +156,13 @@ class qtype_calculatedmulti extends qtype_calculated {
         return true;
     }
 
+    protected function validate_answer($answer) {
+        $error = qtype_calculated_find_formula_errors_in_text($answer);
+        if ($error) {
+            throw new coding_exception($error);
+        }
+    }
+
     protected function make_question_instance($questiondata) {
         question_bank::load_question_definition_classes($this->name());
         if ($questiondata->options->single) {

From dc97145785b9ae192168659c65309bca61a58151 Mon Sep 17 00:00:00 2001
From: Marina Glancy <marina@moodle.com>
Date: Fri, 4 Jul 2014 10:40:13 +0800
Subject: [PATCH 08/22] MDL-45760 make sure to check permission before setting
 header

---
 notes/index.php | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/notes/index.php b/notes/index.php
index 62c01b9979d..b2cfe36882b 100644
--- a/notes/index.php
+++ b/notes/index.php
@@ -15,6 +15,10 @@ $userid       = optional_param('user', 0, PARAM_INT);
 $filtertype   = optional_param('filtertype', '', PARAM_ALPHA);
 $filterselect = optional_param('filterselect', 0, PARAM_INT);
 
+if (empty($CFG->enablenotes)) {
+    print_error('notesdisabled', 'notes');
+}
+
 $url = new moodle_url('/notes/index.php');
 if ($courseid != SITEID) {
     $url->param('course', $courseid);
@@ -61,11 +65,6 @@ if ($userid) {
 
 /// require login to access notes
 require_login($course);
-add_to_log($courseid, 'notes', 'view', 'index.php?course='.$courseid.'&amp;user='.$userid, 'view notes');
-
-if (empty($CFG->enablenotes)) {
-    print_error('notesdisabled', 'notes');
-}
 
 /// output HTML
 if ($course->id == SITEID) {
@@ -73,8 +72,11 @@ if ($course->id == SITEID) {
 } else {
     $coursecontext = context_course::instance($course->id);   // Course context
 }
+require_capability('moodle/notes:view', $coursecontext);
 $systemcontext = context_system::instance();   // SYSTEM context
 
+add_to_log($courseid, 'notes', 'view', 'index.php?course='.$courseid.'&amp;user='.$userid, 'view notes');
+
 $strnotes = get_string('notes', 'notes');
 if ($userid) {
     $PAGE->set_context(context_user::instance($user->id));

From f7b6562f20f6af4119c7775477cffbaa83229f74 Mon Sep 17 00:00:00 2001
From: Osanda Malith Jayathissa <osandajayathissa@gmail.com>
Date: Wed, 4 Jun 2014 13:55:11 +0800
Subject: [PATCH 09/22] MDL-45683 user: Escaping Skype ID used in profile

---
 user/profile.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/user/profile.php b/user/profile.php
index 64e0183eb43..fc96b395655 100644
--- a/user/profile.php
+++ b/user/profile.php
@@ -314,7 +314,7 @@ if ($user->icq && !isset($hiddenfields['icqnumber'])) {
 
 if ($user->skype && !isset($hiddenfields['skypeid'])) {
     $imurl = 'skype:'.urlencode($user->skype).'?call';
-    $iconurl = new moodle_url('http://mystatus.skype.com/smallicon/'.$user->skype);
+    $iconurl = new moodle_url('http://mystatus.skype.com/smallicon/'.urlencode($user->skype));
     if (strpos($CFG->httpswwwroot, 'https:') === 0) {
         // Bad luck, skype devs are lazy to set up SSL on their servers - see MDL-37233.
         $statusicon = '';

From 9eef6b5237520f0cb9874564e577c64e3a831987 Mon Sep 17 00:00:00 2001
From: Yuliya Bozhko <yuliya.bozhko@totaralms.com>
Date: Fri, 20 Jun 2014 09:44:33 +1200
Subject: [PATCH 10/22] MDL-46042 badges: Clean up external badge data

Signed-off-by: Yuliya Bozhko <yuliya.bozhko@totaralms.com>
---
 badges/renderer.php | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/badges/renderer.php b/badges/renderer.php
index 65ed4026348..8e400954f6a 100644
--- a/badges/renderer.php
+++ b/badges/renderer.php
@@ -41,7 +41,7 @@ class core_badges_renderer extends plugin_renderer_base {
                 $bname = $badge->name;
                 $imageurl = moodle_url::make_pluginfile_url($context->id, 'badges', 'badgeimage', $badge->id, '/', 'f1', false);
             } else {
-                $bname = $badge->assertion->badge->name;
+                $bname = s($badge->assertion->badge->name);
                 $imageurl = $badge->imageUrl;
             }
 
@@ -421,47 +421,48 @@ class core_badges_renderer extends plugin_renderer_base {
         }
 
         $datatable->data[] = array($this->output->heading(get_string('issuerdetails', 'badges'), 3), '');
-        $datatable->data[] = array(get_string('issuername', 'badges'), $issuer->name);
+        $datatable->data[] = array(get_string('issuername', 'badges'), s($issuer->name));
         $datatable->data[] = array(get_string('issuerurl', 'badges'),
-                html_writer::tag('a', $issuer->origin, array('href' => $issuer->origin)));
+                html_writer::tag('a', s($issuer->origin), array('href' => $issuer->origin)));
         if (isset($issuer->contact)) {
             $datatable->data[] = array(get_string('contact', 'badges'), obfuscate_mailto($issuer->contact));
         }
         $datatable->data[] = array($this->output->heading(get_string('badgedetails', 'badges'), 3), '');
-        $datatable->data[] = array(get_string('name'), $assertion->badge->name);
-        $datatable->data[] = array(get_string('description', 'badges'), $assertion->badge->description);
+        $datatable->data[] = array(get_string('name'), s($assertion->badge->name));
+        $datatable->data[] = array(get_string('description', 'badges'), s($assertion->badge->description));
         $datatable->data[] = array(get_string('bcriteria', 'badges'),
-                html_writer::tag('a', $assertion->badge->criteria, array('href' => $assertion->badge->criteria)));
+                html_writer::tag('a', s($assertion->badge->criteria), array('href' => $assertion->badge->criteria)));
         $datatable->data[] = array($this->output->heading(get_string('issuancedetails', 'badges'), 3), '');
         if (isset($assertion->issued_on)) {
-            $datatable->data[] = array(get_string('dateawarded', 'badges'), $assertion->issued_on);
+            $issuedate = !strtotime($assertion->issued_on) ? s($assertion->issued_on) : strtotime($assertion->issued_on);
+            $datatable->data[] = array(get_string('dateawarded', 'badges'), userdate($issuedate));
         }
-        if (isset($assertion->badge->expire)) {
+        if (isset($assertion->expires)) {
             $today_date = date('Y-m-d');
             $today = strtotime($today_date);
-            $expiration = strtotime($assertion->badge->expire);
+            $expiration = !strtotime($assertion->expires) ? s($assertion->expires) : strtotime($assertion->expires);
             if ($expiration < $today) {
-                $cell = new html_table_cell($assertion->badge->expire . get_string('warnexpired', 'badges'));
+                $cell = new html_table_cell(userdate($expiration) . get_string('warnexpired', 'badges'));
                 $cell->attributes = array('class' => 'notifyproblem warning');
                 $datatable->data[] = array(get_string('expirydate', 'badges'), $cell);
 
                 $image = html_writer::start_tag('div', array('class' => 'badge'));
-                $image .= html_writer::empty_tag('img', array('src' => $issued['badge']['image']));
+                $image .= html_writer::empty_tag('img', array('src' => $issued->imageUrl));
                 $image .= html_writer::start_tag('span', array('class' => 'expired'))
                             . $this->output->pix_icon('i/expired',
-                                get_string('expireddate', 'badges', $assertion->badge->expire),
+                                get_string('expireddate', 'badges', userdate($expiration)),
                                 'moodle',
                                 array('class' => 'expireimage'))
                             . html_writer::end_tag('span');
                 $image .= html_writer::end_tag('div');
                 $imagetable->data[0] = array($image);
             } else {
-                $datatable->data[] = array(get_string('expirydate', 'badges'), $assertion->badge->expire);
+                $datatable->data[] = array(get_string('expirydate', 'badges'), userdate($expiration));
             }
         }
         if (isset($assertion->evidence)) {
             $datatable->data[] = array(get_string('evidence', 'badges'),
-                html_writer::tag('a', $assertion->evidence, array('href' => $assertion->evidence)));
+                html_writer::tag('a', s($assertion->evidence), array('href' => $assertion->evidence)));
         }
         $table->attributes = array('class' => 'generalbox boxaligncenter issuedbadgebox');
         $table->data[] = array(html_writer::table($imagetable), html_writer::table($datatable));

From 555ee08b17dfe09e02391be137f60fe38c0a7865 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Mudr=C3=A1k?= <david@moodle.com>
Date: Mon, 7 Jul 2014 06:38:47 +0200
Subject: [PATCH 11/22] MDL-46223 Improve the rubric output

---
 grade/grading/form/rubric/js/rubriceditor.js |  4 ++--
 grade/grading/form/rubric/renderer.php       | 12 ++++++------
 grade/grading/form/rubric/rubriceditor.php   |  1 +
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/grade/grading/form/rubric/js/rubriceditor.js b/grade/grading/form/rubric/js/rubriceditor.js
index bff4d09b2f1..f0dd3aea297 100644
--- a/grade/grading/form/rubric/js/rubriceditor.js
+++ b/grade/grading/form/rubric/js/rubriceditor.js
@@ -93,8 +93,8 @@ M.gradingform_rubriceditor.editmode = function(el, editmode, focustb) {
             value = (el.hasClass('level')) ? M.str.gradingform_rubric.levelempty : M.str.gradingform_rubric.criterionempty
             taplain.addClass('empty')
         }
-        taplain.one('.textvalue').set('innerHTML', value)
-        if (tb) tbplain.one('.textvalue').set('innerHTML', tb.get('value'))
+        taplain.one('.textvalue').set('innerHTML', Y.Escape.html(value));
+        if (tb) tbplain.one('.textvalue').set('innerHTML', Y.Escape.html(tb.get('value')));
         // hide/display textarea, textbox and plaintexts
         taplain.removeClass('hiddenelement')
         ta.addClass('hiddenelement')
diff --git a/grade/grading/form/rubric/renderer.php b/grade/grading/form/rubric/renderer.php
index 9132f43e009..b2ba8ff44d7 100644
--- a/grade/grading/form/rubric/renderer.php
+++ b/grade/grading/form/rubric/renderer.php
@@ -79,13 +79,13 @@ class gradingform_rubric_renderer extends plugin_renderer_base {
             }
             $criteriontemplate .= html_writer::end_tag('td'); // .controls
             $criteriontemplate .= html_writer::empty_tag('input', array('type' => 'hidden', 'name' => '{NAME}[criteria][{CRITERION-id}][sortorder]', 'value' => $criterion['sortorder']));
-            $description = html_writer::tag('textarea', htmlspecialchars($criterion['description']), array('name' => '{NAME}[criteria][{CRITERION-id}][description]', 'cols' => '10', 'rows' => '5'));
+            $description = html_writer::tag('textarea', s($criterion['description']), array('name' => '{NAME}[criteria][{CRITERION-id}][description]', 'cols' => '10', 'rows' => '5'));
         } else {
             if ($mode == gradingform_rubric_controller::DISPLAY_EDIT_FROZEN) {
                 $criteriontemplate .= html_writer::empty_tag('input', array('type' => 'hidden', 'name' => '{NAME}[criteria][{CRITERION-id}][sortorder]', 'value' => $criterion['sortorder']));
                 $criteriontemplate .= html_writer::empty_tag('input', array('type' => 'hidden', 'name' => '{NAME}[criteria][{CRITERION-id}][description]', 'value' => $criterion['description']));
             }
-            $description = $criterion['description'];
+            $description = s($criterion['description']);
         }
         $descriptionclass = 'description';
         if (isset($criterion['error_description'])) {
@@ -111,12 +111,12 @@ class gradingform_rubric_renderer extends plugin_renderer_base {
                 $currentremark = $value['remark'];
             }
             if ($mode == gradingform_rubric_controller::DISPLAY_EVAL) {
-                $input = html_writer::tag('textarea', htmlspecialchars($currentremark), array('name' => '{NAME}[criteria][{CRITERION-id}][remark]', 'cols' => '10', 'rows' => '5'));
+                $input = html_writer::tag('textarea', s($currentremark), array('name' => '{NAME}[criteria][{CRITERION-id}][remark]', 'cols' => '10', 'rows' => '5'));
                 $criteriontemplate .= html_writer::tag('td', $input, array('class' => 'remark'));
             } else if ($mode == gradingform_rubric_controller::DISPLAY_EVAL_FROZEN) {
                 $criteriontemplate .= html_writer::empty_tag('input', array('type' => 'hidden', 'name' => '{NAME}[criteria][{CRITERION-id}][remark]', 'value' => $currentremark));
             }else if ($mode == gradingform_rubric_controller::DISPLAY_REVIEW || $mode == gradingform_rubric_controller::DISPLAY_VIEW) {
-                $criteriontemplate .= html_writer::tag('td', $currentremark, array('class' => 'remark'));
+                $criteriontemplate .= html_writer::tag('td', s($currentremark), array('class' => 'remark'));
             }
         }
         $criteriontemplate .= html_writer::end_tag('tr'); // .criterion
@@ -169,7 +169,7 @@ class gradingform_rubric_renderer extends plugin_renderer_base {
         $leveltemplate = html_writer::start_tag('td', $tdattributes);
         $leveltemplate .= html_writer::start_tag('div', array('class' => 'level-wrapper'));
         if ($mode == gradingform_rubric_controller::DISPLAY_EDIT_FULL) {
-            $definition = html_writer::tag('textarea', htmlspecialchars($level['definition']), array('name' => '{NAME}[criteria][{CRITERION-id}][levels][{LEVEL-id}][definition]', 'cols' => '10', 'rows' => '4'));
+            $definition = html_writer::tag('textarea', s($level['definition']), array('name' => '{NAME}[criteria][{CRITERION-id}][levels][{LEVEL-id}][definition]', 'cols' => '10', 'rows' => '4'));
             $score = html_writer::label(get_string('criterionempty', 'gradingform_rubric'), '{NAME}criteria{CRITERION-id}levels{LEVEL-id}', false, array('class' => 'accesshide'));
             $score .= html_writer::empty_tag('input', array('type' => 'text','id' => '{NAME}criteria{CRITERION-id}levels{LEVEL-id}', 'name' => '{NAME}[criteria][{CRITERION-id}][levels][{LEVEL-id}][score]', 'size' => '3', 'value' => $level['score']));
         } else {
@@ -177,7 +177,7 @@ class gradingform_rubric_renderer extends plugin_renderer_base {
                 $leveltemplate .= html_writer::empty_tag('input', array('type' => 'hidden', 'name' => '{NAME}[criteria][{CRITERION-id}][levels][{LEVEL-id}][definition]', 'value' => $level['definition']));
                 $leveltemplate .= html_writer::empty_tag('input', array('type' => 'hidden', 'name' => '{NAME}[criteria][{CRITERION-id}][levels][{LEVEL-id}][score]', 'value' => $level['score']));
             }
-            $definition = $level['definition'];
+            $definition = s($level['definition']);
             $score = $level['score'];
         }
         if ($mode == gradingform_rubric_controller::DISPLAY_EVAL) {
diff --git a/grade/grading/form/rubric/rubriceditor.php b/grade/grading/form/rubric/rubriceditor.php
index bb9cee3e42b..b3cc5bfb40e 100644
--- a/grade/grading/form/rubric/rubriceditor.php
+++ b/grade/grading/form/rubric/rubriceditor.php
@@ -106,6 +106,7 @@ class MoodleQuickForm_rubriceditor extends HTML_QuickForm_input {
         if (!$this->_flagFrozen) {
             $mode = gradingform_rubric_controller::DISPLAY_EDIT_FULL;
             $module = array('name'=>'gradingform_rubriceditor', 'fullpath'=>'/grade/grading/form/rubric/js/rubriceditor.js',
+                'requires' => array('base', 'dom', 'event', 'event-touch', 'escape'),
                 'strings' => array(array('confirmdeletecriterion', 'gradingform_rubric'), array('confirmdeletelevel', 'gradingform_rubric'),
                     array('criterionempty', 'gradingform_rubric'), array('levelempty', 'gradingform_rubric')
                     ));

From 98d5566c2270e21cbfaf1f4e8d61039f05d6aae2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Mudr=C3=A1k?= <david@moodle.com>
Date: Mon, 7 Jul 2014 09:52:37 +0200
Subject: [PATCH 12/22] MDL-46223 Improve the marking guide output

---
 grade/grading/form/guide/guideeditor.php   |  1 +
 grade/grading/form/guide/js/guideeditor.js |  4 +--
 grade/grading/form/guide/renderer.php      | 30 +++++++++++-----------
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/grade/grading/form/guide/guideeditor.php b/grade/grading/form/guide/guideeditor.php
index aebb9822fca..da430ec9027 100644
--- a/grade/grading/form/guide/guideeditor.php
+++ b/grade/grading/form/guide/guideeditor.php
@@ -100,6 +100,7 @@ class moodlequickform_guideeditor extends HTML_QuickForm_input {
             $mode = gradingform_guide_controller::DISPLAY_EDIT_FULL;
             $module = array('name'=>'gradingform_guideeditor',
                 'fullpath'=>'/grade/grading/form/guide/js/guideeditor.js',
+                'requires' => array('base', 'dom', 'event', 'event-touch', 'escape'),
                 'strings' => array(
                     array('confirmdeletecriterion', 'gradingform_guide'),
                     array('clicktoedit', 'gradingform_guide'),
diff --git a/grade/grading/form/guide/js/guideeditor.js b/grade/grading/form/guide/js/guideeditor.js
index 3aeb83be91f..94c1e157fab 100644
--- a/grade/grading/form/guide/js/guideeditor.js
+++ b/grade/grading/form/guide/js/guideeditor.js
@@ -111,9 +111,9 @@ M.gradingform_guideeditor.editmode = function(el, editmode) {
             value = M.str.gradingform_guide.clicktoedit
             taplain.addClass('empty')
         }
-        taplain.one('.textvalue').set('innerHTML', value)
+        taplain.one('.textvalue').set('innerHTML', Y.Escape.html(value))
         if (tb) {
-            tbplain.one('.textvalue').set('innerHTML', tb.get('value'))
+            tbplain.one('.textvalue').set('innerHTML', Y.Escape.html(tb.get('value')))
         }
         // hide/display textarea, textbox and plaintexts
         taplain.removeClass('hiddenelement')
diff --git a/grade/grading/form/guide/renderer.php b/grade/grading/form/guide/renderer.php
index 442d24bdf39..87e94de708e 100644
--- a/grade/grading/form/guide/renderer.php
+++ b/grade/grading/form/guide/renderer.php
@@ -93,20 +93,20 @@ class gradingform_guide_renderer extends plugin_renderer_base {
                 'name' => '{NAME}[criteria][{CRITERION-id}][sortorder]', 'value' => $criterion['sortorder']));
 
             $shortname = html_writer::empty_tag('input', array('type'=> 'text',
-                'name' => '{NAME}[criteria][{CRITERION-id}][shortname]',  'value' => htmlspecialchars($criterion['shortname']),
+                'name' => '{NAME}[criteria][{CRITERION-id}][shortname]',  'value' => $criterion['shortname'],
                 'id ' => '{NAME}[criteria][{CRITERION-id}][shortname]'));
             $shortname = html_writer::tag('div', $shortname, array('class'=>'criterionname'));
-            $description = html_writer::tag('textarea', htmlspecialchars($criterion['description']),
+            $description = html_writer::tag('textarea', s($criterion['description']),
                 array('name' => '{NAME}[criteria][{CRITERION-id}][description]', 'cols' => '65', 'rows' => '5'));
             $description = html_writer::tag('div', $description, array('class'=>'criteriondesc'));
 
-            $descriptionmarkers = html_writer::tag('textarea', htmlspecialchars($criterion['descriptionmarkers']),
+            $descriptionmarkers = html_writer::tag('textarea', s($criterion['descriptionmarkers']),
                 array('name' => '{NAME}[criteria][{CRITERION-id}][descriptionmarkers]', 'cols' => '65', 'rows' => '5'));
             $descriptionmarkers = html_writer::tag('div', $descriptionmarkers, array('class'=>'criteriondescmarkers'));
 
             $maxscore = html_writer::empty_tag('input', array('type'=> 'text',
                 'name' => '{NAME}[criteria][{CRITERION-id}][maxscore]', 'size' => '3',
-                'value' => htmlspecialchars($criterion['maxscore']),
+                'value' => $criterion['maxscore'],
                 'id' => '{NAME}[criteria][{CRITERION-id}][maxscore]'));
             $maxscore = html_writer::tag('div', $maxscore, array('class'=>'criterionmaxscore'));
         } else {
@@ -125,7 +125,7 @@ class gradingform_guide_renderer extends plugin_renderer_base {
                        $mode == gradingform_guide_controller::DISPLAY_VIEW) {
                 $descriptionclass = 'descriptionreadonly';
             }
-            $shortname   = html_writer::tag('div', $criterion['shortname'],
+            $shortname   = html_writer::tag('div', s($criterion['shortname']),
                 array('class'=>'criterionshortname', 'name' => '{NAME}[criteria][{CRITERION-id}][shortname]'));
             $descmarkerclass = '';
             $descstudentclass = '';
@@ -137,13 +137,13 @@ class gradingform_guide_renderer extends plugin_renderer_base {
                     $descstudentclass = ' hide';
                 }
             }
-            $description = html_writer::tag('div', $criterion['description'],
+            $description = html_writer::tag('div', s($criterion['description']),
                 array('class'=>'criteriondescription'.$descstudentclass,
                       'name' => '{NAME}[criteria][{CRITERION-id}][descriptionmarkers]'));
-            $descriptionmarkers   = html_writer::tag('div', $criterion['descriptionmarkers'],
+            $descriptionmarkers   = html_writer::tag('div', s($criterion['descriptionmarkers']),
                 array('class'=>'criteriondescriptionmarkers'.$descmarkerclass,
                       'name' => '{NAME}[criteria][{CRITERION-id}][descriptionmarkers]'));
-            $maxscore   = html_writer::tag('div', $criterion['maxscore'],
+            $maxscore   = html_writer::tag('div', s($criterion['maxscore']),
                 array('class'=>'criteriondescriptionscore', 'name' => '{NAME}[criteria][{CRITERION-id}][maxscore]'));
         }
 
@@ -193,7 +193,7 @@ class gradingform_guide_renderer extends plugin_renderer_base {
                 $scoreclass = 'error';
                 $currentscore = $validationerrors[$criterion['id']]['score']; // Show invalid score in form.
             }
-            $input = html_writer::tag('textarea', htmlspecialchars($currentremark),
+            $input = html_writer::tag('textarea', s($currentremark),
                 array('name' => '{NAME}[criteria][{CRITERION-id}][remark]', 'cols' => '65', 'rows' => '5',
                       'class' => 'markingguideremark'));
             $criteriontemplate .= html_writer::tag('td', $input, array('class' => 'remark'));
@@ -202,7 +202,7 @@ class gradingform_guide_renderer extends plugin_renderer_base {
             $score .= html_writer::empty_tag('input', array('type'=> 'text',
                 'name' => '{NAME}[criteria][{CRITERION-id}][score]', 'class' => $scoreclass,
                 'id' => '{NAME}[criteria][{CRITERION-id}][score]',
-                'size' => '3', 'value' => htmlspecialchars($currentscore)));
+                'size' => '3', 'value' => $currentscore));
             $score .= '/'.$maxscore;
 
             $criteriontemplate .= html_writer::tag('td', $score, array('class' => 'score'));
@@ -211,9 +211,9 @@ class gradingform_guide_renderer extends plugin_renderer_base {
                 'name' => '{NAME}[criteria][{CRITERION-id}][remark]', 'value' => $currentremark));
         } else if ($mode == gradingform_guide_controller::DISPLAY_REVIEW ||
             $mode == gradingform_guide_controller::DISPLAY_VIEW) {
-            $criteriontemplate .= html_writer::tag('td', $currentremark, array('class' => 'remark'));
+            $criteriontemplate .= html_writer::tag('td', s($currentremark), array('class' => 'remark'));
             if (!empty($options['showmarkspercriterionstudents'])) {
-                $criteriontemplate .= html_writer::tag('td', htmlspecialchars($currentscore). ' / '.$maxscore,
+                $criteriontemplate .= html_writer::tag('td', s($currentscore). ' / '.$maxscore,
                     array('class' => 'score'));
             }
         }
@@ -272,7 +272,7 @@ class gradingform_guide_renderer extends plugin_renderer_base {
             $criteriontemplate .= html_writer::end_tag('td'); // Controls.
             $criteriontemplate .= html_writer::empty_tag('input', array('type' => 'hidden',
                 'name' => '{NAME}[comments][{COMMENT-id}][sortorder]', 'value' => $comment['sortorder']));
-            $description = html_writer::tag('textarea', htmlspecialchars($comment['description']),
+            $description = html_writer::tag('textarea', s($comment['description']),
                 array('name' => '{NAME}[comments][{COMMENT-id}][description]', 'cols' => '65', 'rows' => '5'));
             $description = html_writer::tag('div', $description, array('class'=>'criteriondesc'));
         } else {
@@ -283,12 +283,12 @@ class gradingform_guide_renderer extends plugin_renderer_base {
                     'name' => '{NAME}[comments][{COMMENT-id}][description]', 'value' => $comment['description']));
             }
             if ($mode == gradingform_guide_controller::DISPLAY_EVAL) {
-                $description = html_writer::tag('span', htmlspecialchars($comment['description']),
+                $description = html_writer::tag('span', s($comment['description']),
                     array('name' => '{NAME}[comments][{COMMENT-id}][description]',
                           'title' => get_string('clicktocopy', 'gradingform_guide'),
                           'id' => '{NAME}[comments][{COMMENT-id}]', 'class'=>'markingguidecomment'));
             } else {
-                $description = $comment['description'];
+                $description = s($comment['description']);
             }
         }
         $descriptionclass = 'description';

From ec57b5669ce92acdef10a7e24f35872419219b03 Mon Sep 17 00:00:00 2001
From: Marina Glancy <marina@moodle.com>
Date: Tue, 8 Jul 2014 10:11:40 +0800
Subject: [PATCH 13/22] MDL-43948 forms: setting types to filemanager and
 editor elements

---
 lib/form/editor.php      | 22 +++++++++++++++++++++-
 lib/form/filemanager.php | 22 ++++++++++++++++++++--
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/lib/form/editor.php b/lib/form/editor.php
index f2003d2af0b..c82c05c7b58 100644
--- a/lib/form/editor.php
+++ b/lib/form/editor.php
@@ -94,6 +94,25 @@ class MoodleQuickForm_editor extends HTML_QuickForm_element {
         editors_head_setup();
     }
 
+    /**
+     * Called by HTML_QuickForm whenever form event is made on this element
+     *
+     * @param string $event Name of event
+     * @param mixed $arg event arguments
+     * @param object $caller calling object
+     * @return bool
+     */
+    function onQuickFormEvent($event, $arg, &$caller)
+    {
+        switch ($event) {
+            case 'createElement':
+                $caller->setType($arg[0] . '[format]', PARAM_ALPHANUM);
+                $caller->setType($arg[0] . '[itemid]', PARAM_INT);
+                break;
+        }
+        return parent::onQuickFormEvent($event, $arg, $caller);
+    }
+
     /**
      * Sets name of editor
      *
@@ -413,7 +432,8 @@ class MoodleQuickForm_editor extends HTML_QuickForm_element {
         if (!during_initial_install() && empty($CFG->adminsetuppending)) {
             // 0 means no files, -1 unlimited
             if ($maxfiles != 0 ) {
-                $str .= '<input type="hidden" name="'.$elname.'[itemid]" value="'.$draftitemid.'" />';
+                $str .= html_writer::empty_tag('input', array('type' => 'hidden', 'name' => $elname.'[itemid]',
+                        'value' => $draftitemid));
 
                 // used by non js editor only
                 $editorurl = new moodle_url("$CFG->wwwroot/repository/draftfiles_manager.php", array(
diff --git a/lib/form/filemanager.php b/lib/form/filemanager.php
index 77ee16903e5..8140fec153d 100644
--- a/lib/form/filemanager.php
+++ b/lib/form/filemanager.php
@@ -79,6 +79,24 @@ class MoodleQuickForm_filemanager extends HTML_QuickForm_element {
         parent::HTML_QuickForm_element($elementName, $elementLabel, $attributes);
     }
 
+    /**
+     * Called by HTML_QuickForm whenever form event is made on this element
+     *
+     * @param string $event Name of event
+     * @param mixed $arg event arguments
+     * @param object $caller calling object
+     * @return bool
+     */
+    function onQuickFormEvent($event, $arg, &$caller)
+    {
+        switch ($event) {
+            case 'createElement':
+                $caller->setType($arg[0], PARAM_INT);
+                break;
+        }
+        return parent::onQuickFormEvent($event, $arg, $caller);
+    }
+
     /**
      * Sets name of filemanager
      *
@@ -263,9 +281,9 @@ class MoodleQuickForm_filemanager extends HTML_QuickForm_element {
         $output = $PAGE->get_renderer('core', 'files');
         $html .= $output->render($fm);
 
-        $html .= '<input value="'.$draftitemid.'" name="'.$elname.'" type="hidden" />';
+        $html .= html_writer::empty_tag('input', array('value' => $draftitemid, 'name' => $elname, 'type' => 'hidden'));
         // label element needs 'for' attribute work
-        $html .= '<input value="" id="id_'.$elname.'" type="hidden" />';
+        $html .= html_writer::empty_tag('input', array('value' => '', 'id' => 'id_'.$elname, 'type' => 'hidden'));
 
         return $html;
     }

From 6eb787b873f5d3718dc8a74f798ee528d600d8fe Mon Sep 17 00:00:00 2001
From: Frederic Massart <fred@moodle.com>
Date: Thu, 8 May 2014 16:07:50 +0800
Subject: [PATCH 14/22] MDL-45471 javascript: Escape content of exception
 dialogs

---
 .../moodle-core-notification-debug.js         | 21 ++++++++++---------
 .../moodle-core-notification-min.js           |  2 +-
 .../moodle-core-notification.js               | 21 ++++++++++---------
 lib/yui/src/notification/js/notification.js   | 19 +++++++++--------
 .../src/notification/meta/notification.json   |  1 +
 5 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/lib/yui/build/moodle-core-notification/moodle-core-notification-debug.js b/lib/yui/build/moodle-core-notification/moodle-core-notification-debug.js
index eb6dbef2744..a5aedd17f2c 100644
--- a/lib/yui/build/moodle-core-notification/moodle-core-notification-debug.js
+++ b/lib/yui/build/moodle-core-notification/moodle-core-notification-debug.js
@@ -302,13 +302,13 @@ Y.extend(EXCEPTION, DIALOGUE, {
             delay = this.get('hideTimeoutDelay');
         this.get(BASE).addClass('moodle-dialogue-exception');
         this.setStdModContent(Y.WidgetStdMod.HEADER, '<h1 id="moodle-dialogue-'+this.get('COUNT')+'-header-text">'
-                + config.name + '</h1>', Y.WidgetStdMod.REPLACE);
+                + Y.Escape.html(config.name) + '</h1>', Y.WidgetStdMod.REPLACE);
         content = Y.Node.create('<div class="moodle-exception"></div>')
-                .append(Y.Node.create('<div class="moodle-exception-message">'+this.get('message')+'</div>'))
+                .append(Y.Node.create('<div class="moodle-exception-message">'+Y.Escape.html(this.get('message'))+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-filename"><label>File:</label> '
-                        + this.get('fileName')+'</div>'))
+                        + Y.Escape.html(this.get('fileName'))+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-linenumber"><label>Line:</label> '
-                        + this.get('lineNumber')+'</div>'))
+                        + Y.Escape.html(this.get('lineNumber'))+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-stacktrace"><label>Stack trace:</label> <pre>'
                         + this.get('stack')+'</pre></div>'));
         if (M.cfg.developerdebug) {
@@ -351,7 +351,7 @@ Y.extend(EXCEPTION, DIALOGUE, {
         },
         stack : {
             setter : function(str) {
-                var lines = str.split("\n"),
+                var lines = Y.Escape.html(str).split("\n"),
                     pattern = new RegExp('^(.+)@('+M.cfg.wwwroot+')?(.{0,75}).*:(\\d+)$'),
                     i;
                 for (i in lines) {
@@ -382,15 +382,15 @@ Y.extend(AJAXEXCEPTION, DIALOGUE, {
             delay = this.get('hideTimeoutDelay');
         this.get(BASE).addClass('moodle-dialogue-exception');
         this.setStdModContent(Y.WidgetStdMod.HEADER, '<h1 id="moodle-dialogue-'+this.get('COUNT')+'-header-text">'
-                + config.name + '</h1>', Y.WidgetStdMod.REPLACE);
+                + Y.Escape.html(config.name) + '</h1>', Y.WidgetStdMod.REPLACE);
         content = Y.Node.create('<div class="moodle-ajaxexception"></div>')
-                .append(Y.Node.create('<div class="moodle-exception-message">'+this.get('error')+'</div>'))
+                .append(Y.Node.create('<div class="moodle-exception-message">'+Y.Escape.html(this.get('error'))+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-debuginfo"><label>URL:</label> '
                         + this.get('reproductionlink')+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-debuginfo"><label>Debug info:</label> '
-                        + this.get('debuginfo')+'</div>'))
+                        + Y.Escape.html(this.get('debuginfo'))+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-stacktrace"><label>Stack trace:</label> <pre>'
-                        + this.get('stacktrace')+'</pre></div>'));
+                        + Y.Escape.html(this.get('stacktrace'))+'</pre></div>'));
         if (M.cfg.developerdebug) {
             content.all('.moodle-exception-param').removeClass('hidden');
         }
@@ -427,6 +427,7 @@ Y.extend(AJAXEXCEPTION, DIALOGUE, {
         reproductionlink : {
             setter : function(link) {
                 if (link !== null) {
+                    link = Y.Escape.html(link);
                     link = '<a href="'+link+'">'+link.replace(M.cfg.wwwroot, '')+'</a>';
                 }
                 return link;
@@ -448,4 +449,4 @@ M.core.exception = EXCEPTION;
 M.core.ajaxException = AJAXEXCEPTION;
 
 
-}, '@VERSION@', {"requires": ["base", "node", "panel", "event-key", "dd-plugin"]});
+}, '@VERSION@', {"requires": ["base", "node", "panel", "event-key", "escape", "dd-plugin"]});
diff --git a/lib/yui/build/moodle-core-notification/moodle-core-notification-min.js b/lib/yui/build/moodle-core-notification/moodle-core-notification-min.js
index 447093cdbcd..dd7992bfaf8 100644
--- a/lib/yui/build/moodle-core-notification/moodle-core-notification-min.js
+++ b/lib/yui/build/moodle-core-notification/moodle-core-notification-min.js
@@ -1,2 +1,2 @@
 YUI.add("moodle-core-notification",function(e,t){var n="Moodle dialogue",r="moodle-dialogue",i="Moodle confirmation dialogue",s="Moodle exception",o="Moodle AJAX exception",u="Moodle alert",a="notificationBase",f="yesLabel",l="noLabel",c="title",h="question",p={BASE:"moodle-dialogue-base",WRAP:"moodle-dialogue-wrap",HEADER:"moodle-dialogue-hd",BODY:"moodle-dialogue-bd",CONTENT:"moodle-dialogue-content",FOOTER:"moodle-dialogue-ft",HIDDEN:"hidden",LIGHTBOX:"moodle-dialogue-lightbox"},d,v,m,g,y;y=function(t){t.count=e.stamp(this);var n="moodle-dialogue-"+t.count;t.notificationBase=e.Node.create('<div class="'+p.BASE+'">').append(e.Node.create('<div id="'+n+'" role="dialog" aria-labelledby="'+n+'-header-text" class="'+p.WRAP+'"></div>').append(e.Node.create('<div id="'+n+'-header-text" class="'+p.HEADER+' yui3-widget-hd"></div>')).append(e.Node.create('<div class="'+p.BODY+' yui3-widget-bd"></div>')).append(e.Node.create('<div class="'+p.FOOTER+' yui3-widget-ft"></div>'))),e.one(document.body).append(t.notificationBase),t.srcNode="#"+n,t.width=t.width||"400px",t.visible=t.visible||!1,t.center=t.centered||!0,t.centered=!1,t.lightbox!==!1&&(t.modal=!0),delete t.lightbox,t.closeButton===!1?t.buttons=null:t.buttons=[{section:e.WidgetStdMod.HEADER,classNames:"closebutton",action:function(){this.hide()}}],y.superclass.constructor.apply(this,[t]),t.closeButton!==!1&&this.get("buttons").header[0].setAttribute("title",this.get("closeButtonTitle"))},e.extend(y,e.Panel,{initializer:function(){this.after("visibleChange",this.visibilityChanged,this),this.render(),this.show(),this.get("boundingBox").setStyle("zIndex",null)},visibilityChanged:function(t){var n;t.attrName==="visible"&&(this.get("maskNode").addClass(p.LIGHTBOX),this.get("center")&&!t.prevVal&&t.newVal&&this.centerDialogue(),this.get("draggable")&&(n="#"+this.get("id")+" ."+p.HEADER,this.plug(e.Plugin.Drag,{handles:[n]}),e.one(n).setStyle("cursor","move")))},hide:function(e){if(e&&e.type==="key"&&e.keyCode===27&&!this.get("focused"))return;return y.superclass.hide.call(this,arguments)},centerDialogue:function(){var t=this.get("boundingBox"),n=t.hasClass(r+"-hidden"),i,s;n&&t.setStyle("top","-1000px").removeClass(r+"-hidden"),i=Math.max(Math.round((t.get("winWidth")-t.get("offsetWidth"))/2),15),s=Math.max(Math.round((t.get("winHeight")-t.get("offsetHeight"))/2),15)+e.one(window).get("scrollTop"),n&&t.addClass(r+"-hidden"),t.setStyle("left",i).setStyle("top",s)}},{NAME:n,CSS_PREFIX:r,ATTRS:{notificationBase:{},lightbox:{validator:e.Lang.isBoolean,value:!0},closeButton:{validator:e.Lang.isBoolean,value:!0},closeButtonTitle:{validator:e.Lang.isString,value:"Close"},center:{validator:e.Lang.isBoolean,value:!0},draggable:{validator:e.Lang.isBoolean,value:!1},COUNT:{value:null}}}),v=function(e){e.closeButton=!1,v.superclass.constructor.apply(this,[e])},e.extend(v,y,{_closeEvents:null,initializer:function(){this._closeEvents=[],this.publish("complete");var t=e.Node.create('<input type="button" id="id_yuialertconfirm-'+this.get("COUNT")+'" value="'+this.get(f)+'" />'),n=e.Node.create('<div class="confirmation-dialogue"></div>').append(e.Node.create('<div class="confirmation-message">'+this.get("message")+"</div>")).append(e.Node.create('<div class="confirmation-buttons"></div>').append(t));this.get(a).addClass("moodle-dialogue-confirm"),this.setStdModContent(e.WidgetStdMod.BODY,n,e.WidgetStdMod.REPLACE),this.setStdModContent(e.WidgetStdMod.HEADER,'<h1 id="moodle-dialogue-'+this.get("COUNT")+'-header-text">'+this.get(c)+"</h1>",e.WidgetStdMod.REPLACE),this.after("destroyedChange",function(){this.get(a).remove()},this),this._closeEvents.push(e.on("key",this.submit,window,"down:13",this),t.on("click",this.submit,this));var r=this.get("boundingBox").one(".closebutton");r&&this._closeEvents.push(r.on("click",this.submit,this))},submit:function(){(new e.EventHandle(this._closeEvents)).detach(),this.fire("complete"),this.hide(),this.destroy()}},{NAME:u,CSS_PREFIX:r,ATTRS:{title:{validator:e.Lang.isString,value:"Alert"},message:{validator:e.Lang.isString,value:"Confirm"},yesLabel:{validator:e.Lang.isString,setter:function(e){return e||(e="Ok"),e},value:"Ok"}}}),m=function(e){m.superclass.constructor.apply(this,[e])},e.extend(m,y,{_closeEvents:null,initializer:function(){this._closeEvents=[],this.publish("complete"),this.publish("complete-yes"),this.publish("complete-no");var t=e.Node.create('<input type="button" id="id_yuiconfirmyes-'+this.get("COUNT")+'" value="'+this.get(f)+'" />'),n=e.Node.create('<input type="button" id="id_yuiconfirmno-'+this.get("COUNT")+'" value="'+this.get(l)+'" />'),r=e.Node.create('<div class="confirmation-dialogue"></div>').append(e.Node.create('<div class="confirmation-message">'+this.get(h)+"</div>")).append(e.Node.create('<div class="confirmation-buttons"></div>').append(t).append(n));this.get(a).addClass("moodle-dialogue-confirm"),this.setStdModContent(e.WidgetStdMod.BODY,r,e.WidgetStdMod.REPLACE),this.setStdModContent(e.WidgetStdMod.HEADER,'<h1 id="moodle-dialogue-'+this.get("COUNT")+'-header-text">'+this.get(c)+"</h1>",e.WidgetStdMod.REPLACE),this.after("destroyedChange",function(){this.get(a).remove()},this),this._closeEvents.push(e.on("key",this.submit,window,"down:27",this,!1),t.on("click",this.submit,this,!0),n.on("click",this.submit,this,!1));var i=this.get("boundingBox").one(".closebutton");i&&this._closeEvents.push(i.on("click",this.submit,this))},submit:function(t,n){(new e.EventHandle(this._closeEvents)).detach(),this.fire("complete",n),n?this.fire("complete-yes"):this.fire("complete-no"),this.hide(),this.destroy()}},{NAME:i,CSS_PREFIX:r,ATTRS:{yesLabel:{validator:e.Lang.isString,value:"Yes"},noLabel:{validator:e.Lang.isString,value:"No"},title:{validator:e.Lang.isString,value:"Confirm"},question:{validator:e.Lang.isString,value:"Are you sure?"}}}),e.augment(m,e.EventTarget),d=function(t){t.width=t.width||M.cfg.developerdebug?Math.floor(e.one(document.body).get("winWidth")/3)+"px":null,t.closeButton=!0,d.superclass.constructor
-.apply(this,[t])},e.extend(d,y,{_hideTimeout:null,_keypress:null,initializer:function(t){var n,r=this,i=this.get("hideTimeoutDelay");this.get(a).addClass("moodle-dialogue-exception"),this.setStdModContent(e.WidgetStdMod.HEADER,'<h1 id="moodle-dialogue-'+this.get("COUNT")+'-header-text">'+t.name+"</h1>",e.WidgetStdMod.REPLACE),n=e.Node.create('<div class="moodle-exception"></div>').append(e.Node.create('<div class="moodle-exception-message">'+this.get("message")+"</div>")).append(e.Node.create('<div class="moodle-exception-param hidden param-filename"><label>File:</label> '+this.get("fileName")+"</div>")).append(e.Node.create('<div class="moodle-exception-param hidden param-linenumber"><label>Line:</label> '+this.get("lineNumber")+"</div>")).append(e.Node.create('<div class="moodle-exception-param hidden param-stacktrace"><label>Stack trace:</label> <pre>'+this.get("stack")+"</pre></div>")),M.cfg.developerdebug&&n.all(".moodle-exception-param").removeClass("hidden"),this.setStdModContent(e.WidgetStdMod.BODY,n,e.WidgetStdMod.REPLACE),i&&(this._hideTimeout=setTimeout(function(){r.hide()},i)),this.after("visibleChange",this.visibilityChanged,this),this.after("destroyedChange",function(){this.get(a).remove()},this),this._keypress=e.on("key",this.hide,window,"down:13,27",this),this.centerDialogue()},visibilityChanged:function(e){if(e.attrName==="visible"&&e.prevVal&&!e.newVal){this._keypress&&this._keypress.detach();var t=this;setTimeout(function(){t.destroy()},1e3)}}},{NAME:s,CSS_PREFIX:r,ATTRS:{message:{value:""},name:{value:""},fileName:{value:""},lineNumber:{value:""},stack:{setter:function(e){var t=e.split("\n"),n=new RegExp("^(.+)@("+M.cfg.wwwroot+")?(.{0,75}).*:(\\d+)$"),r;for(r in t)t[r]=t[r].replace(n,"<div class='stacktrace-line'>ln: $4</div><div class='stacktrace-file'>$3</div><div class='stacktrace-call'>$1</div>");return t.join("")},value:""},hideTimeoutDelay:{validator:e.Lang.isNumber,value:null}}}),g=function(e){e.name=e.name||"Error",e.closeButton=!0,g.superclass.constructor.apply(this,[e])},e.extend(g,y,{_keypress:null,initializer:function(t){var n,r=this,i=this.get("hideTimeoutDelay");this.get(a).addClass("moodle-dialogue-exception"),this.setStdModContent(e.WidgetStdMod.HEADER,'<h1 id="moodle-dialogue-'+this.get("COUNT")+'-header-text">'+t.name+"</h1>",e.WidgetStdMod.REPLACE),n=e.Node.create('<div class="moodle-ajaxexception"></div>').append(e.Node.create('<div class="moodle-exception-message">'+this.get("error")+"</div>")).append(e.Node.create('<div class="moodle-exception-param hidden param-debuginfo"><label>URL:</label> '+this.get("reproductionlink")+"</div>")).append(e.Node.create('<div class="moodle-exception-param hidden param-debuginfo"><label>Debug info:</label> '+this.get("debuginfo")+"</div>")).append(e.Node.create('<div class="moodle-exception-param hidden param-stacktrace"><label>Stack trace:</label> <pre>'+this.get("stacktrace")+"</pre></div>")),M.cfg.developerdebug&&n.all(".moodle-exception-param").removeClass("hidden"),this.setStdModContent(e.WidgetStdMod.BODY,n,e.WidgetStdMod.REPLACE),i&&(this._hideTimeout=setTimeout(function(){r.hide()},i)),this.after("visibleChange",this.visibilityChanged,this),this._keypress=e.on("key",this.hide,window,"down:13, 27",this),this.centerDialogue()},visibilityChanged:function(e){if(e.attrName==="visible"&&e.prevVal&&!e.newVal){var t=this;this._keypress.detach(),setTimeout(function(){t.destroy()},1e3)}}},{NAME:o,CSS_PREFIX:r,ATTRS:{error:{validator:e.Lang.isString,value:"Unknown error"},debuginfo:{value:null},stacktrace:{value:null},reproductionlink:{setter:function(e){return e!==null&&(e='<a href="'+e+'">'+e.replace(M.cfg.wwwroot,"")+"</a>"),e},value:null},hideTimeoutDelay:{validator:e.Lang.isNumber,value:null}}}),M.core=M.core||{},M.core.dialogue=y,M.core.alert=v,M.core.confirm=m,M.core.exception=d,M.core.ajaxException=g},"@VERSION@",{requires:["base","node","panel","event-key","dd-plugin"]});
+.apply(this,[t])},e.extend(d,y,{_hideTimeout:null,_keypress:null,initializer:function(t){var n,r=this,i=this.get("hideTimeoutDelay");this.get(a).addClass("moodle-dialogue-exception"),this.setStdModContent(e.WidgetStdMod.HEADER,'<h1 id="moodle-dialogue-'+this.get("COUNT")+'-header-text">'+e.Escape.html(t.name)+"</h1>",e.WidgetStdMod.REPLACE),n=e.Node.create('<div class="moodle-exception"></div>').append(e.Node.create('<div class="moodle-exception-message">'+e.Escape.html(this.get("message"))+"</div>")).append(e.Node.create('<div class="moodle-exception-param hidden param-filename"><label>File:</label> '+e.Escape.html(this.get("fileName"))+"</div>")).append(e.Node.create('<div class="moodle-exception-param hidden param-linenumber"><label>Line:</label> '+e.Escape.html(this.get("lineNumber"))+"</div>")).append(e.Node.create('<div class="moodle-exception-param hidden param-stacktrace"><label>Stack trace:</label> <pre>'+this.get("stack")+"</pre></div>")),M.cfg.developerdebug&&n.all(".moodle-exception-param").removeClass("hidden"),this.setStdModContent(e.WidgetStdMod.BODY,n,e.WidgetStdMod.REPLACE),i&&(this._hideTimeout=setTimeout(function(){r.hide()},i)),this.after("visibleChange",this.visibilityChanged,this),this.after("destroyedChange",function(){this.get(a).remove()},this),this._keypress=e.on("key",this.hide,window,"down:13,27",this),this.centerDialogue()},visibilityChanged:function(e){if(e.attrName==="visible"&&e.prevVal&&!e.newVal){this._keypress&&this._keypress.detach();var t=this;setTimeout(function(){t.destroy()},1e3)}}},{NAME:s,CSS_PREFIX:r,ATTRS:{message:{value:""},name:{value:""},fileName:{value:""},lineNumber:{value:""},stack:{setter:function(t){var n=e.Escape.html(t).split("\n"),r=new RegExp("^(.+)@("+M.cfg.wwwroot+")?(.{0,75}).*:(\\d+)$"),i;for(i in n)n[i]=n[i].replace(r,"<div class='stacktrace-line'>ln: $4</div><div class='stacktrace-file'>$3</div><div class='stacktrace-call'>$1</div>");return n.join("")},value:""},hideTimeoutDelay:{validator:e.Lang.isNumber,value:null}}}),g=function(e){e.name=e.name||"Error",e.closeButton=!0,g.superclass.constructor.apply(this,[e])},e.extend(g,y,{_keypress:null,initializer:function(t){var n,r=this,i=this.get("hideTimeoutDelay");this.get(a).addClass("moodle-dialogue-exception"),this.setStdModContent(e.WidgetStdMod.HEADER,'<h1 id="moodle-dialogue-'+this.get("COUNT")+'-header-text">'+e.Escape.html(t.name)+"</h1>",e.WidgetStdMod.REPLACE),n=e.Node.create('<div class="moodle-ajaxexception"></div>').append(e.Node.create('<div class="moodle-exception-message">'+e.Escape.html(this.get("error"))+"</div>")).append(e.Node.create('<div class="moodle-exception-param hidden param-debuginfo"><label>URL:</label> '+this.get("reproductionlink")+"</div>")).append(e.Node.create('<div class="moodle-exception-param hidden param-debuginfo"><label>Debug info:</label> '+e.Escape.html(this.get("debuginfo"))+"</div>")).append(e.Node.create('<div class="moodle-exception-param hidden param-stacktrace"><label>Stack trace:</label> <pre>'+e.Escape.html(this.get("stacktrace"))+"</pre></div>")),M.cfg.developerdebug&&n.all(".moodle-exception-param").removeClass("hidden"),this.setStdModContent(e.WidgetStdMod.BODY,n,e.WidgetStdMod.REPLACE),i&&(this._hideTimeout=setTimeout(function(){r.hide()},i)),this.after("visibleChange",this.visibilityChanged,this),this._keypress=e.on("key",this.hide,window,"down:13, 27",this),this.centerDialogue()},visibilityChanged:function(e){if(e.attrName==="visible"&&e.prevVal&&!e.newVal){var t=this;this._keypress.detach(),setTimeout(function(){t.destroy()},1e3)}}},{NAME:o,CSS_PREFIX:r,ATTRS:{error:{validator:e.Lang.isString,value:"Unknown error"},debuginfo:{value:null},stacktrace:{value:null},reproductionlink:{setter:function(t){return t!==null&&(t=e.Escape.html(t),t='<a href="'+t+'">'+t.replace(M.cfg.wwwroot,"")+"</a>"),t},value:null},hideTimeoutDelay:{validator:e.Lang.isNumber,value:null}}}),M.core=M.core||{},M.core.dialogue=y,M.core.alert=v,M.core.confirm=m,M.core.exception=d,M.core.ajaxException=g},"@VERSION@",{requires:["base","node","panel","event-key","escape","dd-plugin"]});
diff --git a/lib/yui/build/moodle-core-notification/moodle-core-notification.js b/lib/yui/build/moodle-core-notification/moodle-core-notification.js
index eb6dbef2744..a5aedd17f2c 100644
--- a/lib/yui/build/moodle-core-notification/moodle-core-notification.js
+++ b/lib/yui/build/moodle-core-notification/moodle-core-notification.js
@@ -302,13 +302,13 @@ Y.extend(EXCEPTION, DIALOGUE, {
             delay = this.get('hideTimeoutDelay');
         this.get(BASE).addClass('moodle-dialogue-exception');
         this.setStdModContent(Y.WidgetStdMod.HEADER, '<h1 id="moodle-dialogue-'+this.get('COUNT')+'-header-text">'
-                + config.name + '</h1>', Y.WidgetStdMod.REPLACE);
+                + Y.Escape.html(config.name) + '</h1>', Y.WidgetStdMod.REPLACE);
         content = Y.Node.create('<div class="moodle-exception"></div>')
-                .append(Y.Node.create('<div class="moodle-exception-message">'+this.get('message')+'</div>'))
+                .append(Y.Node.create('<div class="moodle-exception-message">'+Y.Escape.html(this.get('message'))+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-filename"><label>File:</label> '
-                        + this.get('fileName')+'</div>'))
+                        + Y.Escape.html(this.get('fileName'))+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-linenumber"><label>Line:</label> '
-                        + this.get('lineNumber')+'</div>'))
+                        + Y.Escape.html(this.get('lineNumber'))+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-stacktrace"><label>Stack trace:</label> <pre>'
                         + this.get('stack')+'</pre></div>'));
         if (M.cfg.developerdebug) {
@@ -351,7 +351,7 @@ Y.extend(EXCEPTION, DIALOGUE, {
         },
         stack : {
             setter : function(str) {
-                var lines = str.split("\n"),
+                var lines = Y.Escape.html(str).split("\n"),
                     pattern = new RegExp('^(.+)@('+M.cfg.wwwroot+')?(.{0,75}).*:(\\d+)$'),
                     i;
                 for (i in lines) {
@@ -382,15 +382,15 @@ Y.extend(AJAXEXCEPTION, DIALOGUE, {
             delay = this.get('hideTimeoutDelay');
         this.get(BASE).addClass('moodle-dialogue-exception');
         this.setStdModContent(Y.WidgetStdMod.HEADER, '<h1 id="moodle-dialogue-'+this.get('COUNT')+'-header-text">'
-                + config.name + '</h1>', Y.WidgetStdMod.REPLACE);
+                + Y.Escape.html(config.name) + '</h1>', Y.WidgetStdMod.REPLACE);
         content = Y.Node.create('<div class="moodle-ajaxexception"></div>')
-                .append(Y.Node.create('<div class="moodle-exception-message">'+this.get('error')+'</div>'))
+                .append(Y.Node.create('<div class="moodle-exception-message">'+Y.Escape.html(this.get('error'))+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-debuginfo"><label>URL:</label> '
                         + this.get('reproductionlink')+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-debuginfo"><label>Debug info:</label> '
-                        + this.get('debuginfo')+'</div>'))
+                        + Y.Escape.html(this.get('debuginfo'))+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-stacktrace"><label>Stack trace:</label> <pre>'
-                        + this.get('stacktrace')+'</pre></div>'));
+                        + Y.Escape.html(this.get('stacktrace'))+'</pre></div>'));
         if (M.cfg.developerdebug) {
             content.all('.moodle-exception-param').removeClass('hidden');
         }
@@ -427,6 +427,7 @@ Y.extend(AJAXEXCEPTION, DIALOGUE, {
         reproductionlink : {
             setter : function(link) {
                 if (link !== null) {
+                    link = Y.Escape.html(link);
                     link = '<a href="'+link+'">'+link.replace(M.cfg.wwwroot, '')+'</a>';
                 }
                 return link;
@@ -448,4 +449,4 @@ M.core.exception = EXCEPTION;
 M.core.ajaxException = AJAXEXCEPTION;
 
 
-}, '@VERSION@', {"requires": ["base", "node", "panel", "event-key", "dd-plugin"]});
+}, '@VERSION@', {"requires": ["base", "node", "panel", "event-key", "escape", "dd-plugin"]});
diff --git a/lib/yui/src/notification/js/notification.js b/lib/yui/src/notification/js/notification.js
index 16c6708b2ae..0433e570467 100644
--- a/lib/yui/src/notification/js/notification.js
+++ b/lib/yui/src/notification/js/notification.js
@@ -300,13 +300,13 @@ Y.extend(EXCEPTION, DIALOGUE, {
             delay = this.get('hideTimeoutDelay');
         this.get(BASE).addClass('moodle-dialogue-exception');
         this.setStdModContent(Y.WidgetStdMod.HEADER, '<h1 id="moodle-dialogue-'+this.get('COUNT')+'-header-text">'
-                + config.name + '</h1>', Y.WidgetStdMod.REPLACE);
+                + Y.Escape.html(config.name) + '</h1>', Y.WidgetStdMod.REPLACE);
         content = Y.Node.create('<div class="moodle-exception"></div>')
-                .append(Y.Node.create('<div class="moodle-exception-message">'+this.get('message')+'</div>'))
+                .append(Y.Node.create('<div class="moodle-exception-message">'+Y.Escape.html(this.get('message'))+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-filename"><label>File:</label> '
-                        + this.get('fileName')+'</div>'))
+                        + Y.Escape.html(this.get('fileName'))+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-linenumber"><label>Line:</label> '
-                        + this.get('lineNumber')+'</div>'))
+                        + Y.Escape.html(this.get('lineNumber'))+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-stacktrace"><label>Stack trace:</label> <pre>'
                         + this.get('stack')+'</pre></div>'));
         if (M.cfg.developerdebug) {
@@ -349,7 +349,7 @@ Y.extend(EXCEPTION, DIALOGUE, {
         },
         stack : {
             setter : function(str) {
-                var lines = str.split("\n"),
+                var lines = Y.Escape.html(str).split("\n"),
                     pattern = new RegExp('^(.+)@('+M.cfg.wwwroot+')?(.{0,75}).*:(\\d+)$'),
                     i;
                 for (i in lines) {
@@ -380,15 +380,15 @@ Y.extend(AJAXEXCEPTION, DIALOGUE, {
             delay = this.get('hideTimeoutDelay');
         this.get(BASE).addClass('moodle-dialogue-exception');
         this.setStdModContent(Y.WidgetStdMod.HEADER, '<h1 id="moodle-dialogue-'+this.get('COUNT')+'-header-text">'
-                + config.name + '</h1>', Y.WidgetStdMod.REPLACE);
+                + Y.Escape.html(config.name) + '</h1>', Y.WidgetStdMod.REPLACE);
         content = Y.Node.create('<div class="moodle-ajaxexception"></div>')
-                .append(Y.Node.create('<div class="moodle-exception-message">'+this.get('error')+'</div>'))
+                .append(Y.Node.create('<div class="moodle-exception-message">'+Y.Escape.html(this.get('error'))+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-debuginfo"><label>URL:</label> '
                         + this.get('reproductionlink')+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-debuginfo"><label>Debug info:</label> '
-                        + this.get('debuginfo')+'</div>'))
+                        + Y.Escape.html(this.get('debuginfo'))+'</div>'))
                 .append(Y.Node.create('<div class="moodle-exception-param hidden param-stacktrace"><label>Stack trace:</label> <pre>'
-                        + this.get('stacktrace')+'</pre></div>'));
+                        + Y.Escape.html(this.get('stacktrace'))+'</pre></div>'));
         if (M.cfg.developerdebug) {
             content.all('.moodle-exception-param').removeClass('hidden');
         }
@@ -425,6 +425,7 @@ Y.extend(AJAXEXCEPTION, DIALOGUE, {
         reproductionlink : {
             setter : function(link) {
                 if (link !== null) {
+                    link = Y.Escape.html(link);
                     link = '<a href="'+link+'">'+link.replace(M.cfg.wwwroot, '')+'</a>';
                 }
                 return link;
diff --git a/lib/yui/src/notification/meta/notification.json b/lib/yui/src/notification/meta/notification.json
index 739c8279ec7..620897294aa 100644
--- a/lib/yui/src/notification/meta/notification.json
+++ b/lib/yui/src/notification/meta/notification.json
@@ -5,6 +5,7 @@
         "node",
         "panel",
         "event-key",
+        "escape",
         "dd-plugin"
     ]
   }

From 72d1a3ab0b002a9a5f32f3c2b61ffc9fa7f7b789 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Mudr=C3=A1k?= <david@moodle.com>
Date: Wed, 9 Jul 2014 10:21:16 +0200
Subject: [PATCH 15/22] MDL-46223 Improve the display of marking guide
 validation message

---
 grade/grading/form/guide/lib.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grade/grading/form/guide/lib.php b/grade/grading/form/guide/lib.php
index 942b19b37fb..d893e0cfe10 100644
--- a/grade/grading/form/guide/lib.php
+++ b/grade/grading/form/guide/lib.php
@@ -882,7 +882,7 @@ class gradingform_guide_instance extends gradingform_instance {
             if (!empty($this->validationerrors)) {
                 foreach ($this->validationerrors as $id => $err) {
                     $a = new stdClass();
-                    $a->criterianame = $criteria[$id]['shortname'];
+                    $a->criterianame = s($criteria[$id]['shortname']);
                     $a->maxscore = $criteria[$id]['maxscore'];
                     $html .= html_writer::tag('div', get_string('err_scoreinvalid', 'gradingform_guide', $a),
                         array('class' => 'gradingform_guide-error'));

From e42b6e20bdd5d6f09bc09be22fd7f20736e27085 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Mudr=C3=A1k?= <david@moodle.com>
Date: Wed, 9 Jul 2014 10:28:56 +0200
Subject: [PATCH 16/22] MDL-46223 Fix frequently used comments in the marking
 guide

When the frequently used comment contains a character like >, re-use it
directly instead of its HTML entitiy.
---
 grade/grading/form/guide/js/guide.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grade/grading/form/guide/js/guide.js b/grade/grading/form/guide/js/guide.js
index 1f1cd909d7d..09da8c4512b 100644
--- a/grade/grading/form/guide/js/guide.js
+++ b/grade/grading/form/guide/js/guide.js
@@ -10,7 +10,7 @@ M.gradingform_guide.init = function(Y, options) {
         currentfocus = e.currentTarget;
     });
     Y.all('.markingguidecomment').on('click', function(e) {
-        currentfocus.set('value', currentfocus.get('value') + '\n' + e.currentTarget.get('innerHTML'));
+        currentfocus.set('value', currentfocus.get('value') + '\n' + e.currentTarget.get('text'));
         currentfocus.focus();
     });
 

From 6a8d442b0f2f62fbc300d34d912bcb4e1fa3c91e Mon Sep 17 00:00:00 2001
From: Tim Hunt <T.J.Hunt@open.ac.uk>
Date: Wed, 9 Jul 2014 12:56:18 +0100
Subject: [PATCH 17/22] MDL-46148 qtype_calculated: removed unused method.

---
 question/type/calculated/questiontype.php | 43 -----------------------
 1 file changed, 43 deletions(-)

diff --git a/question/type/calculated/questiontype.php b/question/type/calculated/questiontype.php
index fb0ed991c78..9ae35362780 100644
--- a/question/type/calculated/questiontype.php
+++ b/question/type/calculated/questiontype.php
@@ -346,49 +346,6 @@ class qtype_calculated extends question_type {
         $question->datasetloader = new qtype_calculated_dataset_loader($questiondata->id);
     }
 
-    public function validate_form($form) {
-        switch($form->wizardpage) {
-            case 'question':
-                $calculatedmessages = array();
-                if (empty($form->name)) {
-                    $calculatedmessages[] = get_string('missingname', 'qtype_calculated');
-                }
-                if (empty($form->questiontext)) {
-                    $calculatedmessages[] = get_string('missingquestiontext', 'qtype_calculated');
-                }
-                // Verify formulas.
-                foreach ($form->answers as $key => $answer) {
-                    if ('' === trim($answer)) {
-                        $calculatedmessages[] = get_string(
-                                'missingformula', 'qtype_calculated');
-                    }
-                    if ($formulaerrors = qtype_calculated_find_formula_errors($answer)) {
-                        $calculatedmessages[] = $formulaerrors;
-                    }
-                    if (! isset($form->tolerance[$key])) {
-                        $form->tolerance[$key] = 0.0;
-                    }
-                    if (! is_numeric($form->tolerance[$key])) {
-                        $calculatedmessages[] = get_string('xmustbenumeric', 'qtype_numerical',
-                                get_string('tolerance', 'qtype_calculated'));
-                    }
-                }
-
-                if (!empty($calculatedmessages)) {
-                    $errorstring = "The following errors were found:<br />";
-                    foreach ($calculatedmessages as $msg) {
-                        $errorstring .= $msg . '<br />';
-                    }
-                    print_error($errorstring);
-                }
-
-                break;
-            default:
-                return parent::validate_form($form);
-                break;
-        }
-        return true;
-    }
     public function finished_edit_wizard($form) {
         return isset($form->savechanges);
     }

From b46ec6914583f78fe64589d995f229e9a963376b Mon Sep 17 00:00:00 2001
From: Tim Hunt <T.J.Hunt@open.ac.uk>
Date: Wed, 9 Jul 2014 13:35:09 +0100
Subject: [PATCH 18/22] MDL-46148 qtype_calculated: fix validation when
 importing.

In order to do this in a sane way, I cleaned up a lot of old mess,
inclduing:

1. Previously, qtype_calcuated used ->answeres when importing, and
->answer when saving the form. This was crazy, so I fixed it, and
stripped out the code that made the alternative variable name work.

2. Similarly, it could handle ->answer being either an array, such as
you would get form the HTML editor, or a simple string, which is what
you get form the form. I simplified that too.

3. Finally, I made import use a transaction around saving each
question, so we don't get half questions in the database when an error
occurs.
---
 question/format.php                           |  6 +++
 question/format/webct/format.php              |  7 +--
 question/format/xml/format.php                |  4 +-
 question/type/calculated/questiontype.php     | 54 +++++++++----------
 .../type/calculatedmulti/questiontype.php     | 15 ++++--
 .../type/calculatedsimple/questiontype.php    | 11 ++--
 6 files changed, 51 insertions(+), 46 deletions(-)

diff --git a/question/format.php b/question/format.php
index 9520ae17d76..e2292e0a0c2 100644
--- a/question/format.php
+++ b/question/format.php
@@ -357,6 +357,7 @@ class qformat_default {
         $count = 0;
 
         foreach ($questions as $question) {   // Process and store each question
+            $transaction = $DB->start_delegated_transaction();
 
             // reset the php timeout
             set_time_limit(0);
@@ -429,9 +430,14 @@ class qformat_default {
 
             if (!empty($result->error)) {
                 echo $OUTPUT->notification($result->error);
+                // Can't use $transaction->rollback(); since it requires an exception,
+                // and I don't want to rewrite this code to change the error handling now.
+                $DB->force_transaction_rollback();
                 return false;
             }
 
+            $transaction->allow_commit();
+
             if (!empty($result->notice)) {
                 echo $OUTPUT->notification($result->notice);
                 return true;
diff --git a/question/format/webct/format.php b/question/format/webct/format.php
index 9bc4f3a4672..fd6c411b736 100644
--- a/question/format/webct/format.php
+++ b/question/format/webct/format.php
@@ -606,12 +606,9 @@ class qformat_webct extends qformat_default {
                 // Calculated Question.
                 $question = $this->defaultquestion();
                 $question->qtype = 'calculated';
-                $question->answers = array(); // No problem as they go as :FORMULA: from webct.
+                $question->answer = array(); // No problem as they go as :FORMULA: from webct.
                 $question->units = array();
                 $question->dataset = array();
-
-                // To make us pass the end-of-question sanity checks.
-                $question->answer = array('dummy');
                 $question->fraction = array('1.0');
                 $question->feedback = array();
 
@@ -737,7 +734,7 @@ class qformat_webct extends qformat_default {
             if (preg_match('~^:FORMULA:(.*)~i', $line, $webctoptions)) {
                 // Answer for a calculated question.
                 ++$currentchoice;
-                $question->answers[$currentchoice] =
+                $question->answer[$currentchoice] =
                         qformat_webct_convert_formula($webctoptions[1]);
 
                 // Default settings.
diff --git a/question/format/xml/format.php b/question/format/xml/format.php
index 1005f5a653d..60c0cef552c 100644
--- a/question/format/xml/format.php
+++ b/question/format/xml/format.php
@@ -780,7 +780,7 @@ class qformat_xml extends qformat_default {
 
         // get answers array
         $answers = $question['#']['answer'];
-        $qo->answers = array();
+        $qo->answer = array();
         $qo->feedback = array();
         $qo->fraction = array();
         $qo->tolerance = array();
@@ -794,7 +794,7 @@ class qformat_xml extends qformat_default {
             if (empty($ans->answer['text'])) {
                 $ans->answer['text'] = '*';
             }
-            $qo->answers[] = $ans->answer;
+            $qo->answer[] = $ans->answer['text'];
             $qo->feedback[] = $ans->feedback;
             $qo->tolerance[] = $answer['#']['tolerance'][0]['#'];
             // fraction as a tag is deprecated
diff --git a/question/type/calculated/questiontype.php b/question/type/calculated/questiontype.php
index 9ae35362780..81c15664341 100644
--- a/question/type/calculated/questiontype.php
+++ b/question/type/calculated/questiontype.php
@@ -131,11 +131,13 @@ class qtype_calculated extends question_type {
 
     public function save_question_options($question) {
         global $CFG, $DB;
+
+        // Make it impossible to save bad formulas anywhere.
+        $this->validate_question_data($question);
+
         // The code is used for calculated, calculatedsimple and calculatedmulti qtypes.
         $context = $question->context;
-        if (isset($question->answer) && !isset($question->answers)) {
-            $question->answers = $question->answer;
-        }
+
         // Calculated options.
         $update = true;
         $options = $DB->get_record('question_calculated_options',
@@ -185,14 +187,7 @@ class qtype_calculated extends question_type {
             $units = $result->units;
         }
 
-        // Insert all the new answers.
-        if (isset($question->answer) && !isset($question->answers)) {
-            $question->answers = $question->answer;
-        }
-        foreach ($question->answers as $key => $answerdata) {
-            if (is_array($answerdata)) {
-                $answerdata = $answerdata['text'];
-            }
+        foreach ($question->answer as $key => $answerdata) {
             if (trim($answerdata) == '') {
                 continue;
             }
@@ -471,6 +466,25 @@ class qtype_calculated extends question_type {
         }
     }
 
+    /**
+     * Validate data before save.
+     * @param stdClass $question data from the form / import file.
+     */
+    protected function validate_question_data($question) {
+        $this->validate_text($question->questiontext); // Yes, really no ['text'].
+
+        if (isset($question->generalfeedback['text'])) {
+            $this->validate_text($question->generalfeedback['text']);
+        } else if (isset($question->generalfeedback)) {
+            $this->validate_text($question->generalfeedback); // Because question import is weird.
+        }
+
+        foreach ($question->answer as $key => $answer) {
+            $this->validate_answer($answer);
+            $this->validate_text($question->feedback[$key]['text']);
+        }
+    }
+
     /**
      * This method prepare the $datasets in a format similar to dadatesetdefinitions_form.php
      * so that they can be saved
@@ -483,13 +497,13 @@ class qtype_calculated extends question_type {
      * @param object $form
      * @param int $questionfromid default = '0'
      */
-    public function preparedatasets($form , $questionfromid = '0') {
+    public function preparedatasets($form, $questionfromid = '0') {
 
         // The dataset names present in the edit_question_form and edit_calculated_form
         // are retrieved.
         $possibledatasets = $this->find_dataset_names($form->questiontext);
         $mandatorydatasets = array();
-        foreach ($form->answers as $key => $answer) {
+        foreach ($form->answer as $key => $answer) {
             $mandatorydatasets += $this->find_dataset_names($answer);
         }
         // If there are identical datasetdefs already saved in the original question
@@ -579,12 +593,6 @@ class qtype_calculated extends question_type {
     public function save_question($question, $form) {
         global $DB;
 
-        if (isset($form->correctfeedback)) {
-            $this->validate_text($form->correctfeedback['text']);
-            $this->validate_text($form->partiallycorrectfeedback['text']);
-            $this->validate_text($form->incorrectfeedback['text']);
-        }
-
         if ($this->wizardpagesnumber() == 1 || $question->qtype == 'calculatedsimple') {
             $question = parent::save_question($question, $form);
             return $question;
@@ -605,14 +613,6 @@ class qtype_calculated extends question_type {
             case '' :
             case 'question': // Coming from the first page, creating the second.
                 if (empty($form->id)) { // or a new question $form->id is empty.
-                    // Make it impossible to save bad formulas anywhere.
-                    $this->validate_text($form->questiontext['text']);
-                    $this->validate_text($form->generalfeedback['text']);
-                    foreach ($form->answer as $key => $answer) {
-                        $this->validate_answer($answer);
-                        $this->validate_text($form->feedback[$key]['text']);
-                    }
-
                     $question = parent::save_question($question, $form);
                     // Prepare the datasets using default $questionfromid.
                     $this->preparedatasets($form);
diff --git a/question/type/calculatedmulti/questiontype.php b/question/type/calculatedmulti/questiontype.php
index 1cd1348d4bb..62e51571e04 100644
--- a/question/type/calculatedmulti/questiontype.php
+++ b/question/type/calculatedmulti/questiontype.php
@@ -42,6 +42,9 @@ class qtype_calculatedmulti extends qtype_calculated {
         global $CFG, $DB;
         $context = $question->context;
 
+        // Make it impossible to save bad formulas anywhere.
+        $this->validate_question_data($question);
+
         // Calculated options.
         $update = true;
         $options = $DB->get_record('question_calculated_options',
@@ -72,10 +75,7 @@ class qtype_calculatedmulti extends qtype_calculated {
         }
 
         // Insert all the new answers.
-        if (isset($question->answer) && !isset($question->answers)) {
-            $question->answers = $question->answer;
-        }
-        foreach ($question->answers as $key => $answerdata) {
+        foreach ($question->answer as $key => $answerdata) {
             if (is_array($answerdata)) {
                 $answerdata = $answerdata['text'];
             }
@@ -163,6 +163,13 @@ class qtype_calculatedmulti extends qtype_calculated {
         }
     }
 
+    protected function validate_question_data($question) {
+        parent::validate_question_data($question);
+        $this->validate_text($question->correctfeedback['text']);
+        $this->validate_text($question->partiallycorrectfeedback['text']);
+        $this->validate_text($question->incorrectfeedback['text']);
+    }
+
     protected function make_question_instance($questiondata) {
         question_bank::load_question_definition_classes($this->name());
         if ($questiondata->options->single) {
diff --git a/question/type/calculatedsimple/questiontype.php b/question/type/calculatedsimple/questiontype.php
index c98f307dbe8..0b19ca8dc5a 100644
--- a/question/type/calculatedsimple/questiontype.php
+++ b/question/type/calculatedsimple/questiontype.php
@@ -43,11 +43,9 @@ class qtype_calculatedsimple extends qtype_calculated {
     public function save_question_options($question) {
         global $CFG, $DB;
         $context = $question->context;
-        // Get old answers.
 
-        if (isset($question->answer) && !isset($question->answers)) {
-            $question->answers = $question->answer;
-        }
+        // Make it impossible to save bad formulas anywhere.
+        $this->validate_question_data($question);
 
         // Get old versions of the objects.
         if (!$oldanswers = $DB->get_records('question_answers',
@@ -69,10 +67,7 @@ class qtype_calculatedsimple extends qtype_calculated {
             $units = &$result->units;
         }
         // Insert all the new answers.
-        if (isset($question->answer) && !isset($question->answers)) {
-            $question->answers = $question->answer;
-        }
-        foreach ($question->answers as $key => $answerdata) {
+        foreach ($question->answer as $key => $answerdata) {
             if (is_array($answerdata)) {
                 $answerdata = $answerdata['text'];
             }

From 0509827f7e41915724e24b730e2c05993cbe5f02 Mon Sep 17 00:00:00 2001
From: Tim Hunt <T.J.Hunt@open.ac.uk>
Date: Wed, 9 Jul 2014 13:47:40 +0100
Subject: [PATCH 19/22] MDL-46148 qtype_calculatedsimple: fix notice

... when adding a dataset with a formula error.
---
 question/type/calculated/questiontype.php | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/question/type/calculated/questiontype.php b/question/type/calculated/questiontype.php
index 81c15664341..5a369dcccc1 100644
--- a/question/type/calculated/questiontype.php
+++ b/question/type/calculated/questiontype.php
@@ -1064,10 +1064,14 @@ class qtype_calculated extends question_type {
         }
 
         $answers = fullclone($answers);
-        $errors = '';
         $delimiter = ': ';
         $virtualqtype =  $qtypeobj->get_virtual_qtype();
         foreach ($answers as $key => $answer) {
+            $error = qtype_calculated_find_formula_errors($answer->answer);
+            if ($error) {
+                $comment->stranswers[$key] = $error;
+                continue;
+            }
             $formula = $this->substitute_variables($answer->answer, $data);
             $formattedanswer = qtype_calculated_calculate_answer(
                 $answer->answer, $data, $answer->tolerance,

From b2f2535c53eb2330735a0f18d516833deb9b979d Mon Sep 17 00:00:00 2001
From: Damyon Wiese <damyon@moodle.com>
Date: Thu, 10 Jul 2014 11:35:47 +0800
Subject: [PATCH 20/22] MDL-46148 questions: Added missing allow_commit for
 transaction

---
 question/format.php | 1 +
 1 file changed, 1 insertion(+)

diff --git a/question/format.php b/question/format.php
index e2292e0a0c2..1b5cbf541bf 100644
--- a/question/format.php
+++ b/question/format.php
@@ -372,6 +372,7 @@ class qformat_default {
                         $this->category = $newcategory;
                     }
                 }
+                $transaction->allow_commit();
                 continue;
             }
             $question->context = $this->importcontext;

From e3fd900dcda7b603d7e0749008abd0d01290bbc3 Mon Sep 17 00:00:00 2001
From: Andrew Nicols <andrew@nicols.co.uk>
Date: Tue, 27 May 2014 17:21:25 +0800
Subject: [PATCH 21/22] MDL-38990 mod_forum: Restrict ability to post to all
 groups in a forum

In order to post to all groups in a forum, you must have the
moodle/site:accessallgroups capability.

Thanks to Jakob Ackermann <jackermann@onlineschool.ca> for part of the fix
on this one.
---
 mod/forum/post_form.php                       | 14 +++-
 .../behat/separate_group_discussions.feature  | 84 +++++++++++++++++++
 ...ate_group_single_group_discussions.feature | 84 +++++++++++++++++++
 3 files changed, 179 insertions(+), 3 deletions(-)
 create mode 100644 mod/forum/tests/behat/separate_group_discussions.feature
 create mode 100644 mod/forum/tests/behat/separate_group_single_group_discussions.feature

diff --git a/mod/forum/post_form.php b/mod/forum/post_form.php
index 20d6bf29607..73ea0dae68c 100644
--- a/mod/forum/post_form.php
+++ b/mod/forum/post_form.php
@@ -147,13 +147,21 @@ class mod_forum_post_form extends moodleform {
             $mform->setConstants(array('timestart'=> 0, 'timeend'=>0));
         }
 
-        if (groups_get_activity_groupmode($cm, $course)) { // hack alert
+        if ($groupmode = groups_get_activity_groupmode($cm, $course)) { // hack alert
             $groupdata = groups_get_activity_allowed_groups($cm);
             $groupcount = count($groupdata);
+            $groupinfo = array();
             $modulecontext = context_module::instance($cm->id);
-            $contextcheck = has_capability('mod/forum:movediscussions', $modulecontext) && empty($post->parent) && $groupcount;
-            if ($contextcheck) {
+
+            // Check whether the user has access to all groups in this forum from the accessallgroups cap.
+            if ($groupmode == VISIBLEGROUPS || has_capability('moodle/site:accessallgroups', $modulecontext)) {
+                // Only allow posting to all groups if the user has access to all groups.
                 $groupinfo = array('0' => get_string('allparticipants'));
+                $groupcount++;
+            }
+
+            $contextcheck = has_capability('mod/forum:movediscussions', $modulecontext) && empty($post->parent) && $groupcount > 1;
+            if ($contextcheck) {
                 foreach ($groupdata as $grouptemp) {
                     $groupinfo[$grouptemp->id] = $grouptemp->name;
                 }
diff --git a/mod/forum/tests/behat/separate_group_discussions.feature b/mod/forum/tests/behat/separate_group_discussions.feature
new file mode 100644
index 00000000000..3b5978f4441
--- /dev/null
+++ b/mod/forum/tests/behat/separate_group_discussions.feature
@@ -0,0 +1,84 @@
+@mod @mod_forum
+Feature: Posting to all groups in a separate group discussion is restricted to users with access to all groups
+  In order to post to all groups in a forum with separate groups
+  As a teacher
+  I need to have the accessallgroups capability or be a member of all of the groups
+
+  Background:
+    Given the following "users" exist:
+      | username | firstname | lastname | email |
+      | teacher1 | Teacher | 1 | teacher1@asd.com |
+      | noneditor1 | Non-editing teacher | 1 | noneditor1@asd.com |
+      | noneditor2 | Non-editing teacher | 2 | noneditor2@asd.com |
+      | student1 | Student | 1 | student1@asd.com |
+    And the following "courses" exist:
+      | fullname | shortname | category |
+      | Course 1 | C1 | 0 |
+    And the following "course enrolments" exist:
+      | user | course | role |
+      | teacher1 | C1 | editingteacher |
+      | noneditor1 | C1 | teacher |
+      | noneditor2 | C1 | teacher |
+      | student1 | C1 | student |
+    And the following "groups" exist:
+      | name | course | idnumber |
+      | Group A | C1 | G1 |
+      | Group B | C1 | G2 |
+      | Group C | C1 | G3 |
+    And the following "group members" exist:
+      | user | group |
+      | teacher1 | G1 |
+      | teacher1 | G2 |
+      | noneditor1 | G1 |
+      | noneditor1 | G2 |
+      | noneditor1 | G3 |
+      | noneditor2 | G1 |
+      | noneditor2 | G2 |
+      | student1 | G1 |
+      | student1 | G2 |
+    And I log in as "teacher1"
+    And I follow "Course 1"
+    And I turn editing mode on
+    And I add a "Forum" to section "1" and I fill the form with:
+      | Forum name | Standard forum name |
+      | Forum type | Standard forum for general use |
+      | Description | Standard forum description |
+      | Group mode | Separate groups |
+    And I log out
+
+  Scenario: Teacher with accessallgroups can post in all groups
+    Given I log in as "teacher1"
+    And I follow "Course 1"
+    And I follow "Standard forum name"
+    When I click on "Add a new discussion topic" "button"
+    Then the "Group" select box should contain "All participants"
+    And the "Group" select box should contain "Group A"
+    And the "Group" select box should contain "Group B"
+
+  @javascript
+  Scenario: Teacher in all groups but without accessallgroups can only post in their groups
+    And I log in as "admin"
+    And I set the following system permissions of "Non-editing teacher" role:
+      | moodle/site:accessallgroups | Prohibit |
+    And I log out
+    Given I log in as "noneditor1"
+    And I follow "Course 1"
+    And I follow "Standard forum name"
+    When I click on "Add a new discussion topic" "button"
+    Then the "Group" select box should not contain "All participants"
+    And the "Group" select box should contain "Group A"
+    And the "Group" select box should contain "Group B"
+
+  @javascript
+  Scenario: Teacher in some groups and without accessallgroups can only post in their groups
+    And I log in as "admin"
+    And I set the following system permissions of "Non-editing teacher" role:
+      | moodle/site:accessallgroups | Prohibit |
+    And I log out
+    Given I log in as "noneditor1"
+    And I follow "Course 1"
+    And I follow "Standard forum name"
+    When I click on "Add a new discussion topic" "button"
+    Then the "Group" select box should not contain "All participants"
+    And the "Group" select box should contain "Group A"
+    And the "Group" select box should contain "Group B"
diff --git a/mod/forum/tests/behat/separate_group_single_group_discussions.feature b/mod/forum/tests/behat/separate_group_single_group_discussions.feature
new file mode 100644
index 00000000000..d8745d11a77
--- /dev/null
+++ b/mod/forum/tests/behat/separate_group_single_group_discussions.feature
@@ -0,0 +1,84 @@
+@mod @mod_forum
+Feature: Posting to groups in a separate group discussion when restricted to groupings
+  In order to post to groups in a forum with separate groups and groupings
+  As a teacher
+  I need to have groups configured to post to a group
+
+  Background:
+    Given the following "users" exist:
+      | username | firstname | lastname | email            |
+      | teacher1 | teacher1  | teacher1 | teacher1@asd.com |
+    And the following "courses" exist:
+      | fullname | shortname | category |
+      | Course 1 | C1 | 0 |
+    And the following "course enrolments" exist:
+      | user | course | role |
+      | teacher1 | C1 | teacher |
+    And the following "groups" exist:
+      | name | course | idnumber |
+      | G1G1 | C1 | G1G1 |
+      | G1G2 | C1 | G1G2 |
+      | G2G1 | C1 | G2G1 |
+    And the following "groupings" exist:
+      | name | course | idnumber |
+      | G1   | C1     | G1       |
+      | G2   | C1     | G2       |
+    And the following "group members" exist:
+      | user        | group |
+      | teacher1    | G1G1  |
+      | teacher1    | G1G2  |
+      | teacher1    | G2G1  |
+    And the following "grouping groups" exist:
+      | grouping | group |
+      | G1       | G1G1    |
+      | G1       | G1G2    |
+      | G2       | G2G1    |
+    And I log in as "admin"
+    And I follow "Course 1"
+    And I turn editing mode on
+    And I add a "Forum" to section "1" and I fill the form with:
+      | Forum name  | Multiple groups forum             |
+      | Forum type  | Standard forum for general use    |
+      | Description | Standard forum description        |
+      | Group mode  | Separate groups                   |
+      | Grouping    | G1                                |
+    And I add a "Forum" to section "1" and I fill the form with:
+      | Forum name  | Single groups forum               |
+      | Forum type  | Standard forum for general use    |
+      | Description | Standard forum description        |
+      | Group mode  | Separate groups                   |
+      | Grouping    | G2                                |
+    And I log out
+
+  Scenario: Teacher with accessallgroups can post in all groups
+    Given I log in as "teacher1"
+    And I follow "Course 1"
+    And I follow "Multiple groups forum"
+    When I click on "Add a new discussion topic" "button"
+    Then the "Group" select box should contain "All participants"
+    And the "Group" select box should contain "G1G1"
+    And the "Group" select box should contain "G1G2"
+    And I follow "Course 1"
+    And I follow "Single groups forum"
+    And I click on "Add a new discussion topic" "button"
+    And the "Group" select box should contain "All participants"
+    And the "Group" select box should contain "G2G1"
+
+  @javascript
+  Scenario: Teacher in all groups but without accessallgroups can post in either group but not to All Participants
+    And I log in as "admin"
+    And I set the following system permissions of "Non-editing teacher" role:
+      | moodle/site:accessallgroups | Prohibit |
+    And I log out
+    Given I log in as "teacher1"
+    And I follow "Course 1"
+    And I follow "Multiple groups forum"
+    When I click on "Add a new discussion topic" "button"
+    Then the "Group" select box should not contain "All participants"
+    And the "Group" select box should contain "G1G1"
+    And the "Group" select box should contain "G1G2"
+    And I follow "Course 1"
+    And I follow "Single groups forum"
+    And I click on "Add a new discussion topic" "button"
+    And I should see "G2G1"
+    And "Group" "select" should not exist

From ec6f010b225d6beb097e11ee18f69907a7757857 Mon Sep 17 00:00:00 2001
From: Ankit Agarwal <ankit@moodle.com>
Date: Thu, 10 Jul 2014 17:44:59 +0800
Subject: [PATCH 22/22] MDL-46148 qtype_calculated: low-level defence against
 bad formulas

This catches things like:
 * Malicious equations coming from backup files.
 * Malicious equations in old questions in the database.
---
 question/type/calculated/question.php | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/question/type/calculated/question.php b/question/type/calculated/question.php
index 43f92b6647c..d4a48726676 100644
--- a/question/type/calculated/question.php
+++ b/question/type/calculated/question.php
@@ -342,6 +342,10 @@ class qtype_calculated_variable_substituter {
      * @return float the computed result.
      */
     public function calculate($expression) {
+        // Make sure no malicious code is present in the expression. Refer MDL-46148 for details.
+        if ($error = qtype_calculated_find_formula_errors($expression)) {
+            throw new moodle_exception('illegalformulasyntax', 'qtype_calculated', '', $error);
+        }
         return $this->calculate_raw($this->substitute_values_for_eval($expression));
     }