catalyst · bwalkerl · Jan 3, 2025 · Jan 3, 2025
diff --git a/classes/helper.php b/classes/helper.php
@@ -171,4 +171,30 @@ public static function merge_duplicate_records(array $records): \stdClass {
 
         return $baserecord;
     }
+
+    /**
+     * Estimate row count for local_csp table
+     *
+     * @return int
+     */
+    public static function get_row_estimate(): int {
+        global $CFG, $DB;
+
+        if ($DB->get_dbfamily() === 'mysql') {
+            // Use explain to keep accurate count after truncate.
+            $explainsql = 'EXPLAIN SELECT COUNT(1) FROM {local_csp}';
+            $record = $DB->get_record_sql($explainsql);
+            return $record->rows ?? 0;
+        } else if ($DB->get_dbfamily() === 'postgres') {
+            $sql = "SELECT relname AS table_name, reltuples::BIGINT AS row_count
+                      FROM pg_class
+                     WHERE relname = :tablename";
+            $params = ['tablename' => $CFG->prefix . 'local_csp'];
+            $record = $DB->get_record_sql($sql, $params);
+            return $record->row_count ?? 0;
+        } else {
+            // Estimates not currently supported for other databases, use a full count.
+            return $DB->count_records('local_csp');
+        }
+    }
 }
diff --git a/classes/table/csp_report.php b/classes/table/csp_report.php
@@ -51,8 +51,7 @@ protected function col_failcounter($record) {
         // Get blocked URI, and set as param for page if clicked on.
         $url = new \moodle_url('/local/csp/csp_report.php',
             [
-                'blockeddomain' => $record->blockeddomain,
-                'blockeddirective' => $record->violateddirective
+                'violation' => $record->violationhash,
             ]
         );
         return \html_writer::link($url, $record->failcounter);
@@ -179,16 +178,14 @@ protected function col_blockedurlpaths($record) {
                           blockedurlpath,
                           SUM(failcounter) AS failcounter
                      FROM {local_csp} csp
-                    WHERE violateddirective = :directive
-                      AND blockeddomain = :blockeddomain
+                    WHERE violationhash = :violationhash
                  GROUP BY blockeduri,
                           blockedurlpath
                  ORDER BY SUM(failcounter) DESC,
                           blockeduri";
 
         $params = [
-            'directive' => $record->violateddirective,
-            'blockeddomain' => $record->blockeddomain
+            'violationhash' => $record->violationhash,
         ];
 
         $blockedpaths = $DB->get_records_sql($subsql, $params, 0, 3);
@@ -218,15 +215,13 @@ protected function col_highestviolaters($record) {
         $subsql = "SELECT documenturi,
                           SUM(failcounter) AS failcounter
                      FROM {local_csp} csp
-                    WHERE violateddirective = :directive
-                      AND blockeddomain = :blockeddomain
+                    WHERE violationhash = :violationhash
                  GROUP BY documenturi
                  ORDER BY SUM(failcounter) DESC,
                           documenturi ASC";
 
         $params = [
-            'directive' => $record->violateddirective,
-            'blockeddomain' => $record->blockeddomain
+            'violationhash' => $record->violationhash,
         ];
 
         $violators = $DB->get_records_sql($subsql, $params, 0, 3);
@@ -259,15 +254,13 @@ protected function col_courses($record) {
                      FROM {local_csp} csp
                      JOIN {course} c
                        ON c.id = csp.courseid
-                    WHERE violateddirective = :directive
-                      AND blockeddomain = :blockeddomain
+                    WHERE violationhash = :violationhash
                  GROUP BY courseid, shortname
                  ORDER BY SUM(failcounter) DESC,
                           shortname ASC";
 
         $params = [
-            'directive' => $record->violateddirective,
-            'blockeddomain' => $record->blockeddomain
+            'violationhash' => $record->violationhash,
         ];
 
         $courses = $DB->get_records_sql($subsql, $params, 0, 3);
@@ -299,9 +292,8 @@ protected function col_action($record) {
         global $OUTPUT;
 
         // Find whether we are drilling down.
-        $viewblockeddomain = optional_param('blockeddomain', false, PARAM_TEXT);
-        $viewdirective = optional_param('blockeddirective', false, PARAM_TEXT);
-        if ($viewblockeddomain && $viewdirective) {
+        $viewviolation = optional_param('violation', false, PARAM_TEXT);
+        if ($viewviolation) {
             $action = new \confirm_action(get_string('areyousuretodeleteonerecord', 'local_csp'));
             $url = new \moodle_url($this->baseurl);
             $url->params(array(
@@ -318,8 +310,7 @@ protected function col_action($record) {
             $url = new \moodle_url($this->baseurl);
             $url->params(
                 [
-                    'removedirective' => $record->violateddirective,
-                    'removedomain' => $record->blockeddomain,
+                    'removeviolation' => $record->violationhash,
                     'sesskey' => sesskey(),
                     'redirecttopage' => $this->currpage,
                 ]

diff --git a/classes/task/add_violationhash_task.php b/classes/task/add_violationhash_task.php
@@ -0,0 +1,66 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+namespace local_csp\task;
+
+use core\task\adhoc_task;
+
+/**
+ * Adds violationhash to all CSP records.
+ *
+ * This is essentially a one off upgrade task that was moved to the background
+ * because it can take a long time for large datasets.
+ *
+ * @package    local_csp
+ * @author     Benjamin Walker <[email protected]>
+ * @copyright  2025 Catalyst IT
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class add_violationhash_task extends adhoc_task {
+    /**
+     * Get task name.
+     */
+    public function get_name(): string {
+        return get_string('add_violationhash_task', 'local_csp');
+    }
+
+    /**
+     * Execute the task.
+     */
+    public function execute(): void {
+        global $DB;
+
+        // Get a recordset of all the unique violateddirect and blockeddomain pairs.
+        $recordset = $DB->get_recordset_sql("
+            SELECT violateddirective, blockeddomain, count(*) as count
+              FROM {local_csp}
+             WHERE violationhash IS NULL
+          GROUP BY violateddirective, blockeddomain
+          ORDER BY count DESC
+        ");
+
+        // Iterate through each pair and update the records.
+        foreach ($recordset as $record) {
+            $violationhash = sha1($record->violateddirective . $record->blockeddomain);
+            $DB->set_field('local_csp', 'violationhash', $violationhash, [
+                'violationhash' => null,
+                'violateddirective' => $record->violateddirective,
+                'blockeddomain' => $record->blockeddomain,
+            ]);
+        }
+        $recordset->close();
+    }
+}
diff --git a/collector.php b/collector.php
@@ -77,6 +77,7 @@
             $dataobject->timecreated = $timestamp;
             $dataobject->timeupdated = $timestamp;
             $dataobject->sha1hash = $hash;
+            $dataobject->violationhash = sha1($dataobject->violateddirective . $dataobject->blockeddomain);
             $dataobject->failcounter = 1;
             $DB->insert_record('local_csp', $dataobject);
             echo "OK\n";

diff --git a/csp_report.php b/csp_report.php
@@ -26,22 +26,18 @@
 require_once(__DIR__ . '/../../config.php');
 require_once($CFG->libdir.'/adminlib.php');
 
-$viewblockeddomain = optional_param('blockeddomain', false, PARAM_TEXT);
-if ($viewblockeddomain) {
-    $viewdirective = required_param('blockeddirective', PARAM_TEXT);
-}
-$removedirective = optional_param('removedirective', false, PARAM_TEXT);
-$removedomain = optional_param('removedomain', false, PARAM_TEXT);
+$viewviolation = optional_param('violation', false, PARAM_TEXT);
+$removeviolation = optional_param('removeviolation', false, PARAM_TEXT);
 $removerecordwithid = optional_param('removerecordwithid', false, PARAM_TEXT);
+$limitedreport = optional_param('limited', null, PARAM_INT);
 $download = optional_param('download', '', PARAM_ALPHA);
 
 admin_externalpage_setup('local_csp_report', '', null, '', array('pagelayout' => 'report'));
 
 // Delete violation class if param set.
-if ($removedirective && $removedomain && confirm_sesskey()) {
+if ($removeviolation && confirm_sesskey()) {
     $DB->delete_records('local_csp', [
-            'violateddirective' => $removedirective,
-            'blockeddomain' => $removedomain
+            'violationhash' => $removeviolation,
         ]
     );
     $PAGE->set_url('/local/csp/csp_report.php', array(
@@ -53,19 +49,18 @@
 // Delete individual violation records if set.
 if ($removerecordwithid && confirm_sesskey()) {
     $DB->delete_records('local_csp', array('id' => $removerecordwithid));
-    $PAGE->set_url('/local/csp/csp_report.php', array(
+    $PAGE->set_url('/local/csp/csp_report.php', array_filter([
+        'violation' => $viewviolation,
         'page' => optional_param('redirecttopage', 0, PARAM_INT),
-    ));
+    ]));
     redirect($PAGE->url);
 }
 
-$PAGE->set_url('/local/csp/csp_report.php', [
-    'blockeddomain' => $viewblockeddomain,
-    'blockeddirective' => $viewdirective ?? '',
-    'removedirective' => $removedirective,
-    'removedomain' => $removedomain,
+$PAGE->set_url('/local/csp/csp_report.php', array_filter([
+    'violation' => $viewviolation,
+    'removeviolation' => $removeviolation,
     'removerecordwithid' => $removerecordwithid,
-]);
+]));
 
 $resetallcspstatistics = optional_param('resetallcspstatistics', 0, PARAM_INT);
 if ($resetallcspstatistics == 1 && confirm_sesskey()) {
@@ -119,12 +114,18 @@
     $action,
 ));
 
+// The local_csp table can grow very large if a policy is not set up correctly.
+// This report can be slow even with indexes, so this can be used to load a limited version.
+if (!isset($limitedreport)) {
+    $limitedreport = \local_csp\helper::get_row_estimate() > 5000000;
+}
+
 // If user has clicked on a violation to view all violation entries.
-if ($viewblockeddomain && $viewdirective) {
-    $fields = 'id, sha1hash, blockeduri, blockeddomain, violateddirective, failcounter, timeupdated, documenturi';
+if ($viewviolation) {
+    $fields = 'id, sha1hash, blockeduri, blockeddomain, violateddirective, failcounter, timeupdated, documenturi, violationhash';
     $from = "{local_csp}";
-    $where = "blockeddomain = ? AND violateddirective = ?";
-    $params = [$viewblockeddomain, $viewdirective];
+    $where = "violationhash = ?";
+    $params = [$viewviolation];
 
     // Redefine columns to display Violation source.
     $table->define_columns(array(
@@ -144,24 +145,68 @@
         $action,
     ));
 
+    // Disable sorting on large tables as sorting by failcounter requires the full table.
+    if ($limitedreport) {
+        $table->sortable(false);
+    }
+
+} else if ($limitedreport) {
+    // If a table is too large only load the fields that can be loaded using hashes and no aggregation.
+    $table->define_columns([
+        'failcounter',
+        'violateddirective',
+        'blockeddomain',
+        'timecreated',
+        'action',
+    ]);
+    $table->define_headers([
+        $failcounter,
+        $violateddirective,
+        $blockeddomain,
+        $timeupdated,
+        $action,
+    ]);
+
+    // Summing failcounter may be too slow on huge tables, so use violationcount as a rough indicator instead.
+    $fields = 'id, blockeddomain, violateddirective, violationhash, violationcount as failcounter, timecreated';
+    $from = "(SELECT MAX(id) AS maxid, count(1) AS violationcount
+                FROM {local_csp}
+               WHERE violationhash IS NOT NULL
+            GROUP BY violationhash) AS A
+                JOIN {local_csp} ON id = maxid";
+    $where = '1 = 1';
+    $params = [];
+
 } else {
-    $fields = 'id, blockeddomain, violateddirective, failcounter, timecreated';
+    $fields = 'id, blockeddomain, violateddirective, violationhash, A.failcounter, A.timecreated';
     // Select the first blockedURI of a type, and collapse the rest while summing failcounter.
-    $from = "(SELECT MAX(id) AS id,
-                     blockeddomain,
-                     violateddirective,
+    $from = "(SELECT MAX(id) AS maxid,
                      SUM(failcounter) AS failcounter,
                      MAX(timecreated) AS timecreated
                 FROM {local_csp}
-            GROUP BY violateddirective, blockeddomain) AS A";
+               WHERE violationhash IS NOT NULL
+            GROUP BY violationhash) AS A
+                JOIN {local_csp} on id = maxid";
     $where = '1 = 1';
-    $params = array();
+    $params = [];
+}
+
+if (!$viewviolation) {
+    // Simplify count SQL to also speed up processing.
+    $table->set_count_sql("SELECT COUNT(DISTINCT violationhash)
+                             FROM {local_csp}
+                            WHERE violationhash IS NOT NULL");
 }
 
 if (!$table->is_downloading()) {
     echo $OUTPUT->header();
     echo $OUTPUT->heading($title);
 
+    if ($limitedreport) {
+        $fullreport = new moodle_url($PAGE->url, ['limited' => 0]);
+        echo $OUTPUT->notification(get_string('limitedreport', 'local_csp', $fullreport->out()), 'warning');
+    }
+
     $action = new \confirm_action(get_string('areyousuretodeleteallrecords', 'local_csp'));
     $urlresetallcspstatistics = new moodle_url($PAGE->url, array(
         'resetallcspstatistics' => 1,

diff --git a/db/install.xml b/db/install.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" ?>
-<XMLDB PATH="local/csp/db" VERSION="20200324" COMMENT="XMLDB file for Moodle local/csp"
+<XMLDB PATH="local/csp/db" VERSION="20250102" COMMENT="XMLDB file for Moodle local/csp"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:noNamespaceSchemaLocation="../../../lib/xmldb/xmldb.xsd"
 >
@@ -17,13 +17,15 @@
         <FIELD NAME="timecreated" TYPE="int" LENGTH="20" NOTNULL="false" SEQUENCE="false"/>
         <FIELD NAME="timeupdated" TYPE="int" LENGTH="20" NOTNULL="false" SEQUENCE="false"/>
         <FIELD NAME="sha1hash" TYPE="char" LENGTH="40" NOTNULL="false" SEQUENCE="false" COMMENT="This is a sha1 hash over columns documenturi, blockeduri and violateddirective."/>
+        <FIELD NAME="violationhash" TYPE="char" LENGTH="40" NOTNULL="false" SEQUENCE="false" COMMENT="This is a sha1 hash over columns violateddirective and blockeddomain."/>
       </FIELDS>
       <KEYS>
         <KEY NAME="primary" TYPE="primary" FIELDS="id"/>
         <KEY NAME="courseid" TYPE="foreign" FIELDS="courseid" REFTABLE="course" REFFIELDS="id"/>
       </KEYS>
       <INDEXES>
         <INDEX NAME="mdl_local_csp_sha1hash_ix" UNIQUE="false" FIELDS="sha1hash" COMMENT="To help searching for existing records."/>
+        <INDEX NAME="violationhash" UNIQUE="false" FIELDS="violationhash"/>
       </INDEXES>
     </TABLE>
   </TABLES>

diff --git a/db/upgrade.php b/db/upgrade.php
@@ -113,6 +113,30 @@ function xmldb_local_csp_upgrade($oldversion) {
         upgrade_plugin_savepoint(true, 2022060300, 'local', 'csp');
     }
 
+    if ($oldversion < 2025010300) {
+
+        // Define field violationhash to be added to local_csp.
+        $table = new xmldb_table('local_csp');
+        $field = new xmldb_field('violationhash', XMLDB_TYPE_CHAR, '40', null, null, null, null, 'sha1hash');
+
+        // Conditionally launch add field violationhash.
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+        }
+
+        $index = new xmldb_index('violationhash', XMLDB_INDEX_NOTUNIQUE, ['violationhash']);
+
+        // Conditionally launch add index violationhash.
+        if (!$dbman->index_exists($table, $index)) {
+            $dbman->add_index($table, $index);
+        }
+
+        \core\task\manager::queue_adhoc_task(new \local_csp\task\add_violationhash_task());
+
+        // CSP savepoint reached.
+        upgrade_plugin_savepoint(true, 2025010300, 'local', 'csp');
+    }
+
     return true;
 }