Details

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

          Attachments

            Activity

            Hide
            samhemelryk Sam Hemelryk added a comment -

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

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

            I assume this can be cherry-pciekd in other branches

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

            Can be cherry-picked to MOODLE_22_STABLE
            Alternative branch for MOODLE_21_STABLE

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

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

            Show
            poltawski Dan Poltawski added a comment - To confirm. You have checked that the same applies in those branches?
            Hide
            dobedobedoh 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
            dobedobedoh 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
            nebgor 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
            nebgor 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_frenz Ankit Agarwal added a comment -

            works as expected.
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - works as expected. Thanks
            Hide
            poltawski 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
            poltawski 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:
                  Fix Release Date:
                  9/Jul/12