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

lib/xmlize.php doesn't care about xml errors

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.8.4, 1.9, 2.0
    • Fix Version/s: 2.0
    • Component/s: Backup
    • Labels:
      None
    • Affected Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE, MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      The library function xmlize does not have any kind of error reporting at all so even quite subtle bugs in the processed xml code silently produce incorrect data. For example the attached file contains an HTML entity and this simply truncates the xml data at the point of the entity returning an ostensibly valid yet incomplete result structure.

      This is bad. It should, were possible, return an error condition.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            timhunt Tim Hunt added a comment -

            This is not really anything to do with the question bank, so removing that component.

            Show
            timhunt Tim Hunt added a comment - This is not really anything to do with the question bank, so removing that component.
            Hide
            timhunt Tim Hunt added a comment -

            We have a proposed fix for this thanks to my colleague Mahmoud: http://github.com/mkassaei/Moodle-Question-Engine-2/commit/acfbfec23cc4065e35fe5f3d6b77d0dc0cce57f6

            Would anyone like to review this for inclusion in Moodle 2.0?

            Show
            timhunt Tim Hunt added a comment - We have a proposed fix for this thanks to my colleague Mahmoud: http://github.com/mkassaei/Moodle-Question-Engine-2/commit/acfbfec23cc4065e35fe5f3d6b77d0dc0cce57f6 Would anyone like to review this for inclusion in Moodle 2.0?
            Hide
            timhunt Tim Hunt added a comment -

            Here is a slightly cleaned up version of the patch that will apply cleanly to HEAD.

            Show
            timhunt Tim Hunt added a comment - Here is a slightly cleaned up version of the patch that will apply cleanly to HEAD.
            Hide
            timhunt Tim Hunt added a comment -

            Slightly improved patch. $index was not being used, so omit it to save memory.

            Show
            timhunt Tim Hunt added a comment - Slightly improved patch. $index was not being used, so omit it to save memory.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Added patch that simply adds lang string in error.php.

            Tested, works ok. And it's full BC.

            My +1 for improving this only in HEAD. Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Added patch that simply adds lang string in error.php. Tested, works ok. And it's full BC. My +1 for improving this only in HEAD. Ciao
            Hide
            skodak Petr Skoda added a comment -

            +1 for 2.0 too
            -5 for any stable branch

            Show
            skodak Petr Skoda added a comment - +1 for 2.0 too -5 for any stable branch
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            The patch has landed to HEAD. Thanks for proposal/feedback!

            Resolving as fixed..ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - The patch has landed to HEAD. Thanks for proposal/feedback! Resolving as fixed..ciao
            Hide
            timhunt Tim Hunt added a comment -

            Thanks guys. Sorry about missing the lang string out of the patch.

            Show
            timhunt Tim Hunt added a comment - Thanks guys. Sorry about missing the lang string out of the patch.

              People

              • Votes:
                1 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  24/Nov/10