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
    • Rank:
      40217

      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

        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: