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

Improve debugging when using default cleaning

    XMLWordPrintable

    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);

        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 (@mudrd8mz)
          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:
                7 Start watching this issue

                Dates

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