Moodle
  1. Moodle
  2. MDL-14816

Escape special regex characters before applying perl compatible regex substitution of field names for database activity's advanced search

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9, 1.9.1
    • Fix Version/s: 1.9.2
    • Labels:
      None
    • Database:
      Any
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      30913

      Description

      When the name of a field contains a special perl-compatible regex character such as "/" or "+", the advanced search page will not create an input box for such fields. In the attached screenshot this is shown for the field with the name "A/X". The same field type (text) with the name "AX" works fine, but "A/X" or "A + X" does not work.

      1. MDL-14816_20080518.patch
        0.9 kB
        Robert Allerstorfer
      1. No_search_input_box.png
        13 kB

        Activity

        Hide
        Robert Allerstorfer added a comment -

        The attached patch fixes this bug.

        The solution was to escape the chars / and + with \ before applying the perl regex substitution.

        Show
        Robert Allerstorfer added a comment - The attached patch fixes this bug. The solution was to escape the chars / and + with \ before applying the perl regex substitution.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Hi Robert,

        shouldn't be preg_quote() instead of addcslashes? That's the natural function to escape characters with "meaning" in regular expressions AFAIK.

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Hi Robert, shouldn't be preg_quote() instead of addcslashes? That's the natural function to escape characters with "meaning" in regular expressions AFAIK. Ciao
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Also,

        feel free to assign yourself database bugs (I think that if you add me or Petr or Martin for confirmation/vote is a good strategy). Ususally one +1 is required to go to HEAD and +2 for stable versions.

        Thanks and ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Also, feel free to assign yourself database bugs (I think that if you add me or Petr or Martin for confirmation/vote is a good strategy). Ususally one +1 is required to go to HEAD and +2 for stable versions. Thanks and ciao
        Hide
        Robert Allerstorfer added a comment -

        Hi Eloy,

        yes, preg_quote() is the better solution. Then I think we should use
        $fieldname = preg_quote($fieldname, '/');

        And thanks for explaing the usual committing policy

        Show
        Robert Allerstorfer added a comment - Hi Eloy, yes, preg_quote() is the better solution. Then I think we should use $fieldname = preg_quote($fieldname, '/'); And thanks for explaing the usual committing policy
        Hide
        Robert Allerstorfer added a comment -

        OK, here is the new patch (MDL-14816_20080518.patch) using preg_quote($fieldname, '/'). It works fine - I have tested it successfully.

        Show
        Robert Allerstorfer added a comment - OK, here is the new patch ( MDL-14816 _20080518.patch) using preg_quote($fieldname, '/'). It works fine - I have tested it successfully.
        Hide
        Robert Allerstorfer added a comment -

        Eloy,

        as you suggested, I have re-assigned this issue to me. The solution is clear, so I'm planning to commit my patch soon unless there is feedback against it.

        Thanks for your attention.

        Show
        Robert Allerstorfer added a comment - Eloy, as you suggested, I have re-assigned this issue to me. The solution is clear, so I'm planning to commit my patch soon unless there is feedback against it. Thanks for your attention.
        Hide
        Dongsheng Cai added a comment -

        Verified, thanks, Robert.

        Show
        Dongsheng Cai added a comment - Verified, thanks, Robert.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: