Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-8582

Ampersand in Course Name/Shortname Breaks Strict

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.8
    • Fix Version/s: 1.8
    • Component/s: Accessibility
    • Labels:
      None
    • Affected Branches:
      MOODLE_18_STABLE
    • Fixed Branches:
      MOODLE_18_STABLE

      Description

      If you create a course with the title 'Design & Technology' and shortname 'D&T', then it'll break Strict in many places all over moodle (list of courses, course page etc)

        Gliffy Diagrams

        1. 28-02-07_8582.patch
          15 kB
          Nicolas Connault
        2. amp.patch
          6 kB
          Petr Skoda
        3. course_category.patch
          0.6 kB
          Dan Poltawski
        4. danp_course_lib.patch
          1 kB
          Dan Poltawski
        5. danp_revised_course_lib.patch
          4 kB
          Dan Poltawski
        6. navigation.patch
          3 kB
          Petr Skoda
        7. nick_last_ampersand.patch
          14 kB
          Nicolas Connault
        8. nick_path.diff
          30 kB
          Nicolas Connault
        9. nick_path.diff
          90 kB
          Nicolas Connault
        10. testweblib.php
          2 kB
          Nicolas Connault

          Issue Links

            Activity

            Hide
            poltawski Dan Poltawski added a comment -

            the same goes for sitename

            Show
            poltawski Dan Poltawski added a comment - the same goes for sitename
            Hide
            dougiamas Martin Dougiamas added a comment -

            Have a try of this, Nicolas.

            Show
            dougiamas Martin Dougiamas added a comment - Have a try of this, Nicolas.
            Hide
            nicolasconnault Nicolas Connault added a comment -

            Resolved:
            1. Used the weblib::s() function to clean up site, category and course name in xhtml output
            2. Extended weblib::s() to do more thorough cleanup (especially ampersand)
            3. Wrote a set of unit tests for weblib::s() (lib/simpletest/testweblib.php). If there is a centralised way to run these tests, please show me, since there is no documentation on unit tests on the Moodle wiki. This file runs itself in the browser at the moment.
            4. Edited the following files, adding calls to weblib::s() wherever applicable:
            a. course/lib.php
            b. course/index.php
            c. blocks/course_list/block_course_list.php
            d. lib/weblib.php::print_navigation()
            e. All the header.html templates in the themes
            f. user/view.php
            g. blocks/admin/block_admin.php

            Unfortunately I don't currently have CVS write privileges to any of these files

            Show
            nicolasconnault Nicolas Connault added a comment - Resolved: 1. Used the weblib::s() function to clean up site, category and course name in xhtml output 2. Extended weblib::s() to do more thorough cleanup (especially ampersand) 3. Wrote a set of unit tests for weblib::s() (lib/simpletest/testweblib.php). If there is a centralised way to run these tests, please show me, since there is no documentation on unit tests on the Moodle wiki. This file runs itself in the browser at the moment. 4. Edited the following files, adding calls to weblib::s() wherever applicable: a. course/lib.php b. course/index.php c. blocks/course_list/block_course_list.php d. lib/weblib.php::print_navigation() e. All the header.html templates in the themes f. user/view.php g. blocks/admin/block_admin.php Unfortunately I don't currently have CVS write privileges to any of these files
            Hide
            nicolasconnault Nicolas Connault added a comment -

            This is an RCS diff patch file generated by CVS diff -c -n.
            I'm attaching this because I can't yet commit my changes due to "insufficient karma".
            Maybe I need to meditate for a while, or do some good deeds...

            The only file I've created and is scheduled to be added through a commit is /lib/simpletest/testweblib.php. This is just a unit test so it isn't part of any dependencies.

            Show
            nicolasconnault Nicolas Connault added a comment - This is an RCS diff patch file generated by CVS diff -c -n. I'm attaching this because I can't yet commit my changes due to "insufficient karma". Maybe I need to meditate for a while, or do some good deeds... The only file I've created and is scheduled to be added through a commit is /lib/simpletest/testweblib.php. This is just a unit test so it isn't part of any dependencies.
            Hide
            nicolasconnault Nicolas Connault added a comment -

            One more comment:
            I initially thought about storing the html entities in the Database, to avoid having to patch a large number of files throughout Moodle which output xhtml to the browser. However, this workaround only adds to the problem: all the parts of Moodle that use that Data without outputting it to XHTML would end up with unwanted html entities which would then have to be cleanup/converted. I always prefer storing data in the database in as pure a form as possible. The following code example shows a possible solution using simple OOP:

            $category->name('html');
            instead of
            $category->name

            The name, shortname and fullname variables are often used (for site, course, category etc...) with the stdClass. if the stdClass was a proper class (with method definitions), it could become responsible for its own output methods.

            class moodleClass {
            var $name;

            function name($output)

            { return $this->$output($this->name); }

            function html($string)

            { // apply formatting return $string; }

            }

            With PHP5's reflection, you could even generate these methods during runtime, which means you could still keep using stdClass with arbitrary variables. Once you have initialised these variables, you would add the methods through reflections.

            Show
            nicolasconnault Nicolas Connault added a comment - One more comment: I initially thought about storing the html entities in the Database, to avoid having to patch a large number of files throughout Moodle which output xhtml to the browser. However, this workaround only adds to the problem: all the parts of Moodle that use that Data without outputting it to XHTML would end up with unwanted html entities which would then have to be cleanup/converted. I always prefer storing data in the database in as pure a form as possible. The following code example shows a possible solution using simple OOP: $category->name('html'); instead of $category->name The name, shortname and fullname variables are often used (for site, course, category etc...) with the stdClass. if the stdClass was a proper class (with method definitions), it could become responsible for its own output methods. class moodleClass { var $name; function name($output) { return $this->$output($this->name); } function html($string) { // apply formatting return $string; } } With PHP5's reflection, you could even generate these methods during runtime, which means you could still keep using stdClass with arbitrary variables. Once you have initialised these variables, you would add the methods through reflections.
            Hide
            poltawski Dan Poltawski added a comment -

            Martin, I wonder if you or somebody could review Nicolas' changes.

            thanks

            Show
            poltawski Dan Poltawski added a comment - Martin, I wonder if you or somebody could review Nicolas' changes. thanks
            Hide
            dougiamas Martin Dougiamas added a comment -

            Wow, Nick, that was fast! Uploading the diff was what I wanted in this case because you don't have write privs yet.

            I'm just looking at it now.

            Show
            dougiamas Martin Dougiamas added a comment - Wow, Nick, that was fast! Uploading the diff was what I wanted in this case because you don't have write privs yet. I'm just looking at it now.
            Hide
            dougiamas Martin Dougiamas added a comment -

            The first thing I notice is that the diff has a lot of lines in it that are not actually changed ... is your editor saving in "dos" text format (CRLF) by any chance? (we use "unix" format, just LF).

            Show
            dougiamas Martin Dougiamas added a comment - The first thing I notice is that the diff has a lot of lines in it that are not actually changed ... is your editor saving in "dos" text format (CRLF) by any chance? (we use "unix" format, just LF).
            Hide
            nicolasconnault Nicolas Connault added a comment -

            Sorry about that, I will generate a new diff as soon as I find out how to change this in Eclipse. Somehow I always assumed it was saving in Unix format. I guess assumptions are dangerous

            Show
            nicolasconnault Nicolas Connault added a comment - Sorry about that, I will generate a new diff as soon as I find out how to change this in Eclipse. Somehow I always assumed it was saving in Unix format. I guess assumptions are dangerous
            Hide
            nicolasconnault Nicolas Connault added a comment -

            Ok I found out what happens. I am outputting in Unix format just fine. The issue is that my editor trims empty whitespace. See the following diff for an example:

            ? nick_path.diff
            Index: view.php
            ===================================================================
            RCS file: /cvsroot/moodle/moodle/user/view.php,v
            retrieving revision 1.143.2.1
            diff -n -c -r1.143.2.1 view.php

                • view.php 21 Feb 2007 21:57:10 -0000 1.143.2.1
                • view.php 22 Feb 2007 12:47:13 -0000
                  ***************
                • 1,7 ****
                  <?PHP // $Id: view.php,v 1.143.2.1 2007/02/21 21:57:10 skodak Exp $

            // Display profile for a particular user
            !
            require_once("../config.php");
            $id = optional_param('id', 0, PARAM_INT); // user id
            $course = optional_param('course', SITEID, PARAM_INT); // course id (defaults to Site)
            — 1,7 ----
            <?PHP // $Id: view.php,v 1.143.2.1 2007/02/21 21:57:10 skodak Exp $

            // Display profile for a particular user
            !
            require_once("../config.php");
            $id = optional_param('id', 0, PARAM_INT); // user id
            $course = optional_param('course', SITEID, PARAM_INT); // course id (defaults to Site)
            ***************

            The lines with the exclamation marks refer to places where the original code has blank space characters which got trimmed by my editor.

            I can switch that off in Eclipse if you like, since it's likely that each time I make a commit it will end up making these kinds of edits. Or maybe there's a way in CVS to ignore differences in whitespace. I'll look it up.

            Show
            nicolasconnault Nicolas Connault added a comment - Ok I found out what happens. I am outputting in Unix format just fine. The issue is that my editor trims empty whitespace. See the following diff for an example: ? nick_path.diff Index: view.php =================================================================== RCS file: /cvsroot/moodle/moodle/user/view.php,v retrieving revision 1.143.2.1 diff -n -c -r1.143.2.1 view.php view.php 21 Feb 2007 21:57:10 -0000 1.143.2.1 view.php 22 Feb 2007 12:47:13 -0000 *************** 1,7 **** <?PHP // $Id: view.php,v 1.143.2.1 2007/02/21 21:57:10 skodak Exp $ // Display profile for a particular user ! require_once("../config.php"); $id = optional_param('id', 0, PARAM_INT); // user id $course = optional_param('course', SITEID, PARAM_INT); // course id (defaults to Site) — 1,7 ---- <?PHP // $Id: view.php,v 1.143.2.1 2007/02/21 21:57:10 skodak Exp $ // Display profile for a particular user ! require_once("../config.php"); $id = optional_param('id', 0, PARAM_INT); // user id $course = optional_param('course', SITEID, PARAM_INT); // course id (defaults to Site) *************** The lines with the exclamation marks refer to places where the original code has blank space characters which got trimmed by my editor. I can switch that off in Eclipse if you like, since it's likely that each time I make a commit it will end up making these kinds of edits. Or maybe there's a way in CVS to ignore differences in whitespace. I'll look it up.
            Hide
            nicolasconnault Nicolas Connault added a comment -

            Ok here's the patch file again, this time output using

            cvs diff -n -c -b -B > nick_path.diff

            thus ignoring trailing white space and blank line deletes/adds

            The resulting file looks good to me.

            Show
            nicolasconnault Nicolas Connault added a comment - Ok here's the patch file again, this time output using cvs diff -n -c -b -B > nick_path.diff thus ignoring trailing white space and blank line deletes/adds The resulting file looks good to me.
            Hide
            dougiamas Martin Dougiamas added a comment -

            OK, thanks, Nick.

            There's some issues in this case where the change has been made to all the theme headers, where it would make more sense to fix it just once in print_header. Also, s() is being called more than once on the same string in some places, eg from print_header_simple().

            Yu's fixing those and getting it into CVS, though. See you Monday!

            Show
            dougiamas Martin Dougiamas added a comment - OK, thanks, Nick. There's some issues in this case where the change has been made to all the theme headers, where it would make more sense to fix it just once in print_header. Also, s() is being called more than once on the same string in some places, eg from print_header_simple(). Yu's fixing those and getting it into CVS, though. See you Monday!
            Hide
            skodak Petr Skoda added a comment -

            Hi, why did you deprecate addslashes_js() ? If there is a better alternative please document in the code why it is depracated and what to use instead, thanks.

            Show
            skodak Petr Skoda added a comment - Hi, why did you deprecate addslashes_js() ? If there is a better alternative please document in the code why it is depracated and what to use instead, thanks.
            Hide
            nicolasconnault Nicolas Connault added a comment -

            Hi Petr, sorry I misread the comment above it, I thought it referred to the function itself. The @deprecated tag can go.

            Show
            nicolasconnault Nicolas Connault added a comment - Hi Petr, sorry I misread the comment above it, I thought it referred to the function itself. The @deprecated tag can go.
            Hide
            skodak Petr Skoda added a comment -

            Could you please also explain the changes in s() function?
            1/ why the change in regex? is it doing something else, if yes why?
            2/ why the new function parameter? we should always fix inline docs when adding params to core functions

            Show
            skodak Petr Skoda added a comment - Could you please also explain the changes in s() function? 1/ why the change in regex? is it doing something else, if yes why? 2/ why the new function parameter? we should always fix inline docs when adding params to core functions
            Hide
            skodak Petr Skoda added a comment -

            hi Nicolas, do you use skype? we could discuss the s() function, my nick is petr.skoda

            Show
            skodak Petr Skoda added a comment - hi Nicolas, do you use skype? we could discuss the s() function, my nick is petr.skoda
            Hide
            nicolasconnault Nicolas Connault added a comment -

            I used to use Skype (nicolasconnault) but took it out because of two main reasons: rumours of spyware and simply no one to talk to However I use Trillian and have an account for the following IM services:

            ICQ: 826611
            AIM: NikoZeta
            MSN: nikozeta@hotmail.com
            Yahoo: nicolasconnault

            But if you want to chat (vocally) on Skype, why not just use the telephone? Let me know if you would still prefer Skype and I will reinstall it. But since I am starting work at the office on Monday, communication probably won't be an issue.

            Show
            nicolasconnault Nicolas Connault added a comment - I used to use Skype (nicolasconnault) but took it out because of two main reasons: rumours of spyware and simply no one to talk to However I use Trillian and have an account for the following IM services: ICQ: 826611 AIM: NikoZeta MSN: nikozeta@hotmail.com Yahoo: nicolasconnault But if you want to chat (vocally) on Skype, why not just use the telephone? Let me know if you would still prefer Skype and I will reinstall it. But since I am starting work at the office on Monday, communication probably won't be an issue.
            Hide
            nicolasconnault Nicolas Connault added a comment -

            To save in lib/simpletest

            Show
            nicolasconnault Nicolas Connault added a comment - To save in lib/simpletest
            Hide
            poltawski Dan Poltawski added a comment -

            Oh, ampersands in course categories also break strict. Here are two lines missed from course/lib.php

            Show
            poltawski Dan Poltawski added a comment - Oh, ampersands in course categories also break strict. Here are two lines missed from course/lib.php
            Hide
            poltawski Dan Poltawski added a comment -

            Also, ':' isn't being caught
            (change your test course name to 'D&T: How to break things' and see)

            Show
            poltawski Dan Poltawski added a comment - Also, ':' isn't being caught (change your test course name to 'D&T: How to break things' and see)
            Hide
            poltawski Dan Poltawski added a comment -

            Hmm sorry, ':' is perfectly valid, just firefox doesn't like it when presented as xml

            Show
            poltawski Dan Poltawski added a comment - Hmm sorry, ':' is perfectly valid, just firefox doesn't like it when presented as xml
            Hide
            poltawski Dan Poltawski added a comment -

            Lots of places in course/lib.php which need s()

            Show
            poltawski Dan Poltawski added a comment - Lots of places in course/lib.php which need s()
            Hide
            poltawski Dan Poltawski added a comment -

            Attached are some places in course/category.php missing

            Show
            poltawski Dan Poltawski added a comment - Attached are some places in course/category.php missing
            Hide
            skodak Petr Skoda added a comment -

            We have discussed it a bit with Nick via ICQ - the result is new function format_title() and reverting to original s(). The reasons were:
            1/ proper handling of 𛙕 entities
            2/ security - both s() and format_title() now accept untrusted content and return safe html or plaintext

            The $plaintext parameter should be used for activity titles (where we always 'unofficially' supported html), the rest such as course title, course shortname, forum posts titles, etc. should be IMO expecting plain text only.

            TODO:

            • fix all titles
            • add detection of lonely < and > and convert them to html entities (low priority, the lonely & was much bigger problem)
            Show
            skodak Petr Skoda added a comment - We have discussed it a bit with Nick via ICQ - the result is new function format_title() and reverting to original s(). The reasons were: 1/ proper handling of 𛙕 entities 2/ security - both s() and format_title() now accept untrusted content and return safe html or plaintext The $plaintext parameter should be used for activity titles (where we always 'unofficially' supported html), the rest such as course title, course shortname, forum posts titles, etc. should be IMO expecting plain text only. TODO: fix all titles add detection of lonely < and > and convert them to html entities (low priority, the lonely & was much bigger problem)
            Hide
            nicolasconnault Nicolas Connault added a comment -

            Now using the new format_title function. However, one more issue is the outputting of the course name and shortname in the breadcrumbs: the <a> tags are part of the $navigation variable that gets cleaned up by either s() or format_title(). At the moment I have disabled format_title()'s removing of <a> tags.

            Show
            nicolasconnault Nicolas Connault added a comment - Now using the new format_title function. However, one more issue is the outputting of the course name and shortname in the breadcrumbs: the <a> tags are part of the $navigation variable that gets cleaned up by either s() or format_title(). At the moment I have disabled format_title()'s removing of <a> tags.
            Hide
            skodak Petr Skoda added a comment -

            The link removal is needed to cleanup the links created by some filters, maybe we could add new filter settings to enable only some filters in filter_string() and filter_title().

            Show
            skodak Petr Skoda added a comment - The link removal is needed to cleanup the links created by some filters, maybe we could add new filter settings to enable only some filters in filter_string() and filter_title().
            Hide
            skodak Petr Skoda added a comment -

            Looking at the code it might be better to change the way breadcrums are passed to the print_header() function - simple array 'title'=>url would be easier to construct in scripts and much easier to process in print_header

            Show
            skodak Petr Skoda added a comment - Looking at the code it might be better to change the way breadcrums are passed to the print_header() function - simple array 'title'=>url would be easier to construct in scripts and much easier to process in print_header
            Hide
            skodak Petr Skoda added a comment -

            The more I think about this the harder it seems:

            Please also consider the fact that for security reasons we 'clean' the submitted titles (==remove dangerous js) before the storage into database and we always considered the titles to be html, NOT plaintext. The patches above break existing data in database.

            There is also the problem with multilang titles, it was allowed nearly everywhere except the site name, shortname and course name AFAIK.

            IMO we have to decide on the format of text used in titles:
            1/ plaintext - then we can use current print_title() in cvs; this option breaks existing content; no way to type "less than" or "greater than" characters in titles
            2/ html - current standard with PARAM_MULTILANG cleaning; & must be entered as html entity & amp ; - we would not need format_title(), instead format_string() and sometimes s(format_string()) could do the job

            Please note that since conversion to new forms html is not allowed in activity titles and course anymore - ok for me, but I am sure MD will not like it

            I am attaching a patch with print_navigation that support also array navigation parameter and uses print_title()

            my +1 for 2/ - only fixing the multilang issue and educating people about html entities

            Show
            skodak Petr Skoda added a comment - The more I think about this the harder it seems: Please also consider the fact that for security reasons we 'clean' the submitted titles (==remove dangerous js) before the storage into database and we always considered the titles to be html, NOT plaintext. The patches above break existing data in database. There is also the problem with multilang titles, it was allowed nearly everywhere except the site name, shortname and course name AFAIK. IMO we have to decide on the format of text used in titles: 1/ plaintext - then we can use current print_title() in cvs; this option breaks existing content; no way to type "less than" or "greater than" characters in titles 2/ html - current standard with PARAM_MULTILANG cleaning; & must be entered as html entity & amp ; - we would not need format_title(), instead format_string() and sometimes s(format_string()) could do the job Please note that since conversion to new forms html is not allowed in activity titles and course anymore - ok for me, but I am sure MD will not like it I am attaching a patch with print_navigation that support also array navigation parameter and uses print_title() my +1 for 2/ - only fixing the multilang issue and educating people about html entities
            Hide
            skodak Petr Skoda added a comment -

            We should IMO discuss this on Monday with MD in HQ chat.

            The solution should not be difficult when we decide which way to go...

            Show
            skodak Petr Skoda added a comment - We should IMO discuss this on Monday with MD in HQ chat. The solution should not be difficult when we decide which way to go...
            Hide
            skodak Petr Skoda added a comment -

            ... if we want to fix only the & char, we can also add the "detection and replacement of lonely ampersands" into form submission, it could also detect the "lonely < and >" and tweak them too before the cleaning

            Show
            skodak Petr Skoda added a comment - ... if we want to fix only the & char, we can also add the "detection and replacement of lonely ampersands" into form submission, it could also detect the "lonely < and >" and tweak them too before the cleaning
            Hide
            skodak Petr Skoda added a comment -

            We might also use htmltidy to clean other submitted HTML to get xhtml strict everywhere - it would be too costly to do that before each display, but before the storage it should not be a big problem.

            Show
            skodak Petr Skoda added a comment - We might also use htmltidy to clean other submitted HTML to get xhtml strict everywhere - it would be too costly to do that before each display, but before the storage it should not be a big problem.
            Hide
            nicolasconnault Nicolas Connault added a comment -

            Petr, I'm on ICQ if you want to chat now.

            Show
            nicolasconnault Nicolas Connault added a comment - Petr, I'm on ICQ if you want to chat now.
            Hide
            nicolasconnault Nicolas Connault added a comment -

            So the main issue is at which point to clean up the user input: before DB storage or after DB retrieval. I think it's a good overall idea to do it before storage, but different inputs need different cleanups. As you pointed out, we want links for some data and plain text for others. Some require some input to convert into "sep" divs (for breadcrumbs). That cleanup should occur in a central location, not scattered among a bunch of unrelated functions. What is considered "harmful" should be clearly defined and conceptualised before being implemented in code.

            Also, we are trying to achieve both clean XHTML strict AND security. These two aspects aren't necessarily dependent or coexistent with each other: we can have code that is clean but unsafe, and have safe but dirty code. This is why I think that cleaning up user input before storage in DB should have security as a goal, while tidying up for clean XHTML can be performed during retrieval, depending on the use made of that data. Sometimes that data is not used within XHTML output at all...

            Show
            nicolasconnault Nicolas Connault added a comment - So the main issue is at which point to clean up the user input: before DB storage or after DB retrieval. I think it's a good overall idea to do it before storage, but different inputs need different cleanups. As you pointed out, we want links for some data and plain text for others. Some require some input to convert into "sep" divs (for breadcrumbs). That cleanup should occur in a central location, not scattered among a bunch of unrelated functions. What is considered "harmful" should be clearly defined and conceptualised before being implemented in code. Also, we are trying to achieve both clean XHTML strict AND security. These two aspects aren't necessarily dependent or coexistent with each other: we can have code that is clean but unsafe, and have safe but dirty code. This is why I think that cleaning up user input before storage in DB should have security as a goal, while tidying up for clean XHTML can be performed during retrieval, depending on the use made of that data. Sometimes that data is not used within XHTML output at all...
            Hide
            skodak Petr Skoda added a comment -

            The problem with cleanup is that we do not always storing data in final html format in db, the cleanup must be done after the last html processing. In general we store titles cleaned up in db while general text in raw untrusted form (and of course there is one ugly exception called trusttext).

            What I am proposing here is to use html tidy filtering in forms before the data storage - this would solve this problem and could make user submitted html much nicer.

            Anyway I have a working patch that needs some cleanup and exception handling for our custom tags. I am running to pub now to meet a team of female ice hockey players :-D

            I am very glad you started with this hard bug that is related to our security mechanisms, tomorrow I will search for all my unfinished documents/conference papers, etc. related to security and mail it to you There should be also some recordings available online.

            skodak

            Show
            skodak Petr Skoda added a comment - The problem with cleanup is that we do not always storing data in final html format in db, the cleanup must be done after the last html processing. In general we store titles cleaned up in db while general text in raw untrusted form (and of course there is one ugly exception called trusttext). What I am proposing here is to use html tidy filtering in forms before the data storage - this would solve this problem and could make user submitted html much nicer. Anyway I have a working patch that needs some cleanup and exception handling for our custom tags. I am running to pub now to meet a team of female ice hockey players :-D I am very glad you started with this hard bug that is related to our security mechanisms, tomorrow I will search for all my unfinished documents/conference papers, etc. related to security and mail it to you There should be also some recordings available online. skodak
            Hide
            poltawski Dan Poltawski added a comment -

            Ooh, I wouldn't mind seeing those!

            Show
            poltawski Dan Poltawski added a comment - Ooh, I wouldn't mind seeing those!
            Hide
            skodak Petr Skoda added a comment -

            That should not be a big problem, they want me to help them setup a web site - I will let you know when it is online

            Show
            skodak Petr Skoda added a comment - That should not be a big problem, they want me to help them setup a web site - I will let you know when it is online
            Hide
            poltawski Dan Poltawski added a comment -

            The security documents!

            Show
            poltawski Dan Poltawski added a comment - The security documents!
            Hide
            dougiamas Martin Dougiamas added a comment -

            Wow, this was supposed to be a simple bug for Nicolas to start with. :-D

            Yes, let's meet up Monday at HQ and talk about it.

            Show
            dougiamas Martin Dougiamas added a comment - Wow, this was supposed to be a simple bug for Nicolas to start with. :-D Yes, let's meet up Monday at HQ and talk about it.
            Hide
            nicolasconnault Nicolas Connault added a comment -

            Issue fixed, under review by QA before committing.

            Show
            nicolasconnault Nicolas Connault added a comment - Issue fixed, under review by QA before committing.
            Hide
            poltawski Dan Poltawski added a comment -

            Which is the proposed fixed then?

            Show
            poltawski Dan Poltawski added a comment - Which is the proposed fixed then?
            Hide
            nicolasconnault Nicolas Connault added a comment -

            Patch under review

            Show
            nicolasconnault Nicolas Connault added a comment - Patch under review
            Hide
            nicolasconnault Nicolas Connault added a comment -

            Not sure if the hotpot module is converting ampersands correctly. $heading and $title are set up differently than in other modules:
            instead of outputting $course->title, a temporary $title variable is created and assigned that value. That made it difficult for
            me to decide whether it was already formatted elsewhere or not.

            Also I didn't format_string() any of the variables included in $navigation strings, because I assume these strings get processed by
            print_navigation(), which includes format_string().

            It would be cool if a given string could contain information about which filters have been applied to it. So a String would be an
            object with a simple array used as a stack, and a few methods. When you apply a new filter to a string, it would automatically
            (and optionally) self-check and only apply the filter if it hasn't been applied beforehand

            Here are the changed files (98 + 1 new file):

            admin/handlevirus.php
            admin/oacleanup.php
            admin/process_email.php
            admin/register.php
            admin/mnet/enr_course_enrol.php
            admin/mnet/enr_courses.php
            admin/roles/assign.php
            admin/roles/override.php
            admin/roles/tabs.php
            auth/cas/forbidden.php
            auth/cas/login.php
            auth/mnet/auth.php
            backup/backup.php
            backup/restore_check.html
            backup/restorelib.php
            backup/try.php
            blocks/admin/block_admin.php
            blocks/course_list/block_course_list.php
            course/category.php
            course/delete.php
            course/edit.php
            course/grade.php
            course/grades.php
            course/groups.php
            course/importstudents.php
            course/index.php
            course/info.php
            course/lib.php
            course/pending.php
            course/recent.php
            course/scales.php
            course/search.php
            course/user.php
            course/import/activities/index.php
            course/import/groups/index.php
            course/report/log/index.php
            course/report/log/lib.php
            course/report/outline/index.php
            course/report/participation/index.php
            course/report/stats/index.php
            course/report/stats/report.php
            enrol/authorize/enrol.php
            enrol/authorize/locallib.php
            enrol/database/enrol.php
            enrol/imsenterprise/importnow.php
            enrol/mnet/allowed_courses.php
            enrol/paypal/ipn.php
            files/index.php
            grade/exceptions.php
            grade/lib.php
            group/assign.php
            group/group.php
            group/grouping.php
            group/index.php
            group/groupgui/index.php
            group/groupgui/printgrouping.php
            lib/accesslib.php
            lib/moodlelib.php
            lib/questionlib.php
            lib/weblib.php
            lib/db/mysql.php
            lib/db/postgres7.php
            lib/simpletest/testweblibphp +
            message/index.php
            mod/assignment/lib.php
            mod/chat/gui_header_js/index.php
            mod/chat/gui_sockets/index.php
            mod/choice/view.php
            mod/data/edit.php
            mod/data/view.php
            mod/forum/discuss.php
            mod/forum/index.php
            mod/forum/lib.php
            mod/forum/post.php
            mod/forum/user.php
            mod/glossary/export.php
            mod/glossary/print.php
            mod/glossary/showentry.php
            mod/hotpot/index.php
            mod/hotpot/report.php
            mod/hotpot/review.php
            mod/hotpot/view.php
            mod/lams/index.php
            mod/lesson/index.php
            mod/resource/index.php
            mod/resource/type/file/resource.class.php
            mod/resource/type/repository/resource.class.php
            mod/scorm/player.php
            mod/scorm/report.php
            mod/scorm/view.php
            mod/survey/report.php
            mod/workshop/assess.php
            question/category_class.php
            user/edit.php
            user/editadvanced.php
            user/tabs.php
            user/view.php
            user/profile/definelib.php
            user/profile/index.php

            Show
            nicolasconnault Nicolas Connault added a comment - Not sure if the hotpot module is converting ampersands correctly. $heading and $title are set up differently than in other modules: instead of outputting $course->title, a temporary $title variable is created and assigned that value. That made it difficult for me to decide whether it was already formatted elsewhere or not. Also I didn't format_string() any of the variables included in $navigation strings, because I assume these strings get processed by print_navigation(), which includes format_string(). It would be cool if a given string could contain information about which filters have been applied to it. So a String would be an object with a simple array used as a stack, and a few methods. When you apply a new filter to a string, it would automatically (and optionally) self-check and only apply the filter if it hasn't been applied beforehand Here are the changed files (98 + 1 new file): admin/handlevirus.php admin/oacleanup.php admin/process_email.php admin/register.php admin/mnet/enr_course_enrol.php admin/mnet/enr_courses.php admin/roles/assign.php admin/roles/override.php admin/roles/tabs.php auth/cas/forbidden.php auth/cas/login.php auth/mnet/auth.php backup/backup.php backup/restore_check.html backup/restorelib.php backup/try.php blocks/admin/block_admin.php blocks/course_list/block_course_list.php course/category.php course/delete.php course/edit.php course/grade.php course/grades.php course/groups.php course/importstudents.php course/index.php course/info.php course/lib.php course/pending.php course/recent.php course/scales.php course/search.php course/user.php course/import/activities/index.php course/import/groups/index.php course/report/log/index.php course/report/log/lib.php course/report/outline/index.php course/report/participation/index.php course/report/stats/index.php course/report/stats/report.php enrol/authorize/enrol.php enrol/authorize/locallib.php enrol/database/enrol.php enrol/imsenterprise/importnow.php enrol/mnet/allowed_courses.php enrol/paypal/ipn.php files/index.php grade/exceptions.php grade/lib.php group/assign.php group/group.php group/grouping.php group/index.php group/groupgui/index.php group/groupgui/printgrouping.php lib/accesslib.php lib/moodlelib.php lib/questionlib.php lib/weblib.php lib/db/mysql.php lib/db/postgres7.php lib/simpletest/testweblibphp + message/index.php mod/assignment/lib.php mod/chat/gui_header_js/index.php mod/chat/gui_sockets/index.php mod/choice/view.php mod/data/edit.php mod/data/view.php mod/forum/discuss.php mod/forum/index.php mod/forum/lib.php mod/forum/post.php mod/forum/user.php mod/glossary/export.php mod/glossary/print.php mod/glossary/showentry.php mod/hotpot/index.php mod/hotpot/report.php mod/hotpot/review.php mod/hotpot/view.php mod/lams/index.php mod/lesson/index.php mod/resource/index.php mod/resource/type/file/resource.class.php mod/resource/type/repository/resource.class.php mod/scorm/player.php mod/scorm/report.php mod/scorm/view.php mod/survey/report.php mod/workshop/assess.php question/category_class.php user/edit.php user/editadvanced.php user/tabs.php user/view.php user/profile/definelib.php user/profile/index.php

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  31/Mar/07