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

Improve debugging when using default cleaning

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3, 2.5
    • Fix Version/s: 2.5
    • Component/s: Forms Library
    • Testing Instructions:
      Hide
      1. Ensure that the course form displays without any warnings and that course format options can be changed and saved
      2. Edit the course/edit_form.php, comment out this line:
        $mform->setType('idnumber', PARAM_RAW);
      3. VERIFY: that a DEVELOPER debug warning is thrown
      4. Change your debugging level lower.
      5. VERIFY: that a debugging warning is not thrown.
      6. Uncomment the line
      7. Try different forms and ensure they still work

      PLEASE NOTE: there may be more debugging warnings thrown around core with this. These are new bugs to address in new issues.

      Show
      Ensure that the course form displays without any warnings and that course format options can be changed and saved Edit the course/edit_form.php, comment out this line: $mform->setType('idnumber', PARAM_RAW); VERIFY: that a DEVELOPER debug warning is thrown Change your debugging level lower. VERIFY: that a debugging warning is not thrown. Uncomment the line Try different forms and ensure they still work PLEASE NOTE: there may be more debugging warnings thrown around core with this. These are new bugs to address in new issues.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull Master Branch:
      MDL-34311-master

      Description

      This is just an idea I wanted to explore...

      Right now people use mforms and it occasionally happens that devs forget to specify a PARAM type and we fall back to PARAM_RAW. This brings us relatively regular security issues. This seems preventable, if devs want PARAM_RAW the they can specify it manually and in other cases, we can throw a developer warning to catch it early.

      I attach a proof of concept patch below. It has the problem that we are throwing warnings in selects etc where not necessary, so we'd need to resolve that. Is there another reason we haven't considered this before?

      diff --git a/lib/formslib.php b/lib/formslib.php
      index 306c4e5..11b4292 100644
      --- a/lib/formslib.php
      +++ b/lib/formslib.php
      @@ -1466,6 +1466,7 @@ class MoodleQuickForm extends HTML_QuickForm_DHTMLRulesTableless {
                           $type = $this->_types[$key];
                       } else {
                           $type = PARAM_RAW;
      +                    debugging("Reverting to defaulting to DEFAULT cleaning of param $key", DEBUG_DEVELOPER);
                       }
                       if (is_array($s)) {
                           $submission[$key] = clean_param_array($s, $type, true);

        Gliffy Diagrams

          Attachments

            Issue Links

            1.
            Fix missing calls to setType in assign Sub-task Closed Dan Poltawski
             
            2.
            Error reported on question editing form Sub-task Closed Dan Poltawski
             
            3.
            PHP warnings on course restore page Sub-task Closed Dan Poltawski
             
            4.
            Review all admin pages looking for missing setType() uses Sub-task Closed Rajesh Taneja
             
            5.
            Numerous warnings during course restore. Sub-task Closed moodle.com
             
            6.
            Lesson edit page not calling setType for hidden and text inputs Sub-task Closed Rossiani Wijaya
             
            7.
            Fix missing calls to setType in repositories Sub-task Closed Dan Poltawski
             
            8.
            Take rid of missing setType() when uploading users Sub-task Closed Eloy Lafuente (stronk7)
             
            9.
            Add the correct types to all the backup/restore forms Sub-task Closed Eloy Lafuente (stronk7)
             
            10.
            Portfolio element missing setType Sub-task Closed Dan Poltawski
             
            11.
            Fix missing calls to setType in enrolment methods Sub-task Closed David Monllaó
             
            12.
            Multianswer question form not calling setType on hidden input "confirm" Sub-task Closed Dan Poltawski
             
            13.
            Glossary not calling setType for text input entbypage Sub-task Closed Dan Poltawski
             
            14.
            Fix missing setType calls when editing blocks Sub-task Closed David Monllaó
             
            15.
            Missing call to setType in "My private files" form Sub-task Closed Matteo Scaramuccia
             
            16.
            user/files.php sets $data->returnurl without defining in form Sub-task Closed Dan Poltawski
             
            17.
            Missing call to setType in signup form Sub-task Closed Dan Poltawski
             
            18.
            Apply proper cleaning to phpunit involved forms to let it pass Sub-task Closed Eloy Lafuente (stronk7)
             
            19.
            Course completion form not calling setType for criteria_grade_value and locale aware floats Sub-task Closed Dan Poltawski
             
            20.
            Error when modifying student grade Sub-task Closed Andrew Davis
             
            21.
            Fix missing calls to setType in scorm mod_form. Sub-task Closed Dan Marsden
             
            22.
            Many PARAM_RAW warnings when editing a grade forms Sub-task Closed Dan Poltawski
             
            23.
            Missing setType calls in mod_choice Sub-task Closed Dan Poltawski
             
            24.
            setType debugging should not be displayed on hardFreeze() elements Sub-task Closed Dan Poltawski
             
            25.
            Fix missing calls to setType in calendar Sub-task Closed Dan Poltawski
             
            26.
            Database pages show setType() warnings Sub-task Closed Rossiani Wijaya
             
            27.
            setType errors when adding a new category Sub-task Closed Dan Poltawski
             
            28.
            Add unit tests for the debugging change (and don't call detectMissingSetType twice) Sub-task Closed Dan Poltawski
             
            29.
            Completion settings page shows setType() warnings Sub-task Closed Dan Poltawski
             
            30.
            Fix missing setType calls in advanced grading Sub-task Closed Dan Poltawski
             
            31.
            Fix missing setType calls in acceptance testing tool Sub-task Closed David Monllaó
             
            32.
            Fix missing setType in workshop random allocator Sub-task Closed David Mudrák
             
            33.
            Fix missing calls to setType in blog Sub-task Closed Ankit Agarwal
             
            34.
            Workshop submission setType error Sub-task Closed Rossiani Wijaya
             
            35.
            Missing call to SetType in mod_folder Sub-task Closed Stephen Bourget
             
            36.
            quiz/startattempt.php show setType warning. Sub-task Closed Tim Hunt
             
            37.
            Groups import missing setType Sub-task Closed Dan Poltawski
             
            38.
            assignment upgrade tool setType warnings Sub-task Closed Damyon Wiese
             
            39.
            Missing setType() in portfolio format select form Sub-task Closed Dan Poltawski
             
            40.
            Community finder search display setType() form warnings Sub-task Closed Jérôme Mouneyrac
             
            41.
            Fix missing calls to setType in networking Sub-task Closed Rossiani Wijaya
             
            42.
            Fix missing calls to setType() for externalURL in assignment Sub-task Closed Rossiani Wijaya
             
            43.
            setType() warnings in Box.net and Flickr portfolio export forms Sub-task Closed Rossiani Wijaya
             
            44.
            Missing setType call in Feedback forms Sub-task Closed Frédéric Massart
             
            45.
            Missing setType in course legacy files Sub-task Closed Marina Glancy
             
            46.
            Missing setType call for advanced grading. Sub-task Closed Damyon Wiese
             
            47.
            setType() message when editing a tag Sub-task Closed Dan Poltawski
             
            48.
            Fix missing calls to setType in course criteria settings Sub-task Closed Yuliya Bozhko
             
            49.
            Fix missing setType calls when exporting gradebook Sub-task Closed Rajesh Taneja
             

              Activity

                People

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

                  Dates

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