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

          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