Moodle
  1. Moodle
  2. MDL-37506

unit tests using pattern match not correctly escaped

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor 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:
    • Rank:
      47150

      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.

        Activity

        Hide
        Tim Hunt added a comment -

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

        Show
        Tim Hunt added a comment - Good catch Jamie. Are you going to do a patch for this, or shall I?
        Hide
        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
        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
        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
        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
        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
        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
        Dan Poltawski added a comment -

        Integrated to master - thanks!

        Show
        Dan Poltawski added a comment - Integrated to master - thanks!
        Hide
        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
        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
        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
        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: