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

unit tests using pattern match not correctly escaped

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.8
    • Fix Version/s: 2.5
    • Component/s: Questions, Quiz
    • Labels:
    • Testing Instructions:
      Hide

      1. Run all the unit tests.

      Please also do a quick test for regressions, through the Moodle UI.

      2. Create a shortanswer question that uses * wildcards to match string patterns. For example, you could ask the question "Type a string that includes |." with answer '|'.

      3. Preview that question a few times, and try a few possible responses.

      Show
      1. Run all the unit tests. Please also do a quick test for regressions, through the Moodle UI. 2. Create a shortanswer question that uses * wildcards to match string patterns. For example, you could ask the question "Type a string that includes |." with answer ' | '. 3. Preview that question a few times, and try a few possible responses.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      We need to change all places where preg_quote has been used to include a second parameter to preg_quote, the delimiter '/'. Otherwise the delimiter is not escaped.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            timhunt Tim Hunt added a comment -

            Good catch Jamie. Are you going to do a patch for this, or shall I?

            Show
            timhunt Tim Hunt added a comment - Good catch Jamie. Are you going to do a patch for this, or shall I?
            Hide
            jamiesensei Jamie Pratt added a comment -

            I would prefer if you did Tim. Not sure how many branches you would like to apply this fix to.

            Show
            jamiesensei Jamie Pratt added a comment - I would prefer if you did Tim. Not sure how many branches you would like to apply this fix to.
            Hide
            timhunt Tim Hunt added a comment -

            Wow! there were a lot of those. I think I have fixed them all. (I did a search for preg_quote.) Fortunately most of the changes were in the unit tests, so it is easy to verify that I have not broken anything.

            Since the potential bugs are mainly theoretical (it is very unlikely that anyone will use the deliminter character in the strings that are being preg_quoted), I have decided to submit this for integration in master only.

            Note, I have also fixed this (master-branch only) in the OU-developed add-on question types that I happened to have installed in my dev environment.

            Show
            timhunt Tim Hunt added a comment - Wow! there were a lot of those. I think I have fixed them all. (I did a search for preg_quote.) Fortunately most of the changes were in the unit tests, so it is easy to verify that I have not broken anything. Since the potential bugs are mainly theoretical (it is very unlikely that anyone will use the deliminter character in the strings that are being preg_quoted), I have decided to submit this for integration in master only. Note, I have also fixed this (master-branch only) in the OU-developed add-on question types that I happened to have installed in my dev environment.
            Hide
            poltawski Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            poltawski Dan Poltawski added a comment -

            Integrated to master - thanks!

            Show
            poltawski Dan Poltawski added a comment - Integrated to master - thanks!
            Hide
            salvetore Michael de Raadt added a comment -

            Test result: Success!

            Tested in Master only.

            The specific tests passed and short-answer questions containing wildcards seemed to work as expected.

            Show
            salvetore Michael de Raadt added a comment - Test result: Success! Tested in Master only. The specific tests passed and short-answer questions containing wildcards seemed to work as expected.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks!

            Closing as fixed, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks! Closing as fixed, ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  14/May/13