Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.6, 2.2.3, 2.3
    • Fix Version/s: 2.1.7, 2.2.4
    • Component/s: Reports
    • Labels:
    • Testing Instructions:
      Hide

      Open the log viewer
      Select some different dates and ensure that the correct log entries are displayed

      There may be some other tests you want to run here. This shouldn't make any difference, just actually sanitise the input correctly.

      Show
      Open the log viewer Select some different dates and ensure that the correct log entries are displayed There may be some other tests you want to run here. This shouldn't make any difference, just actually sanitise the input correctly.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-33040-master-1

      Description

      Just noticed that report/log/index.php has PARAM_FILE as it's type.
      It also states in it's comment:

      // Date to display - number or some string

      However, in looking into this:
      print_log() is called with $date, which passes the $date straight to build_logs_array() which in turn runs:

      if ($date) {
          $enddate = $date + 86400;
          $joins[] = "l.time > :date AND l.time < :enddate";
          $params['date'] = $date;
          $params['enddate'] = $enddate;
      }

      Specifying a string (e.g. now) just causes a database error. The string is passed in as a bind param so never evaled as a function (which is good).

      Vote to convert to a PARAM_INT and adjust the comment accordingly

        Gliffy Diagrams

          Activity

          Hide
          Sam Hemelryk added a comment -

          Good spotting thanks Andrew, fix looks perfect, $date must be an int. Putting this up for integration immediately.

          Show
          Sam Hemelryk added a comment - Good spotting thanks Andrew, fix looks perfect, $date must be an int. Putting this up for integration immediately.
          Hide
          Dan Poltawski added a comment -

          I assume this can be cherry-pciekd in other branches

          Show
          Dan Poltawski added a comment - I assume this can be cherry-pciekd in other branches
          Hide
          Andrew Nicols added a comment -

          Can be cherry-picked to MOODLE_22_STABLE
          Alternative branch for MOODLE_21_STABLE

          Show
          Andrew Nicols added a comment - Can be cherry-picked to MOODLE_22_STABLE Alternative branch for MOODLE_21_STABLE
          Hide
          Dan Poltawski added a comment -

          To confirm. You have checked that the same applies in those branches?

          Show
          Dan Poltawski added a comment - To confirm. You have checked that the same applies in those branches?
          Hide
          Andrew Nicols added a comment -

          Yes - in 2.1, the report was in /course/report/log
          In 2.2 it moved to /report/log but was otherwise unchanged in this regard.

          I've checked on 2.1 and 2.2 that the patches apply correctly.

          Show
          Andrew Nicols added a comment - Yes - in 2.1, the report was in /course/report/log In 2.2 it moved to /report/log but was otherwise unchanged in this regard. I've checked on 2.1 and 2.2 that the patches apply correctly.
          Hide
          Aparup Banerjee added a comment -

          Thanks, that's been (int)egrated into master, 21 and picked into 22. up for testing on All branches.

          Show
          Aparup Banerjee added a comment - Thanks, that's been (int)egrated into master, 21 and picked into 22. up for testing on All branches.
          Hide
          Ankit Agarwal added a comment -

          works as expected.
          Thanks

          Show
          Ankit Agarwal added a comment - works as expected. Thanks
          Hide
          Dan Poltawski added a comment -

          Congratulations!

          Your work has made into the latest Moodle release!

          You are only authorised to celebrate after testing 15 Moodle 2.3 QA tests, thanks!

          Show
          Dan Poltawski added a comment - Congratulations! Your work has made into the latest Moodle release! You are only authorised to celebrate after testing 15 Moodle 2.3 QA tests, thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: