diff --git a/privacy/classes/local/request/contextlist.php b/privacy/classes/local/request/contextlist.php index ea70cea8e04..4e391abcb75 100644 --- a/privacy/classes/local/request/contextlist.php +++ b/privacy/classes/local/request/contextlist.php @@ -48,28 +48,25 @@ class contextlist extends contextlist_base { $fields = \context_helper::get_preload_record_columns_sql('ctx'); if ($fieldname = $this->guess_id_field_from_sql($sql)) { - if (is_int($fieldname)) { + if (is_numeric($fieldname)) { $wrapper = " - SELECT - {$fields} + SELECT {$fields} FROM {context} ctx WHERE ctx.id = :fieldvalue"; $params = ['fieldvalue' => $fieldname]; } else { // Able to guess a field name. $wrapper = " - SELECT - {$fields} + SELECT {$fields} FROM {context} ctx JOIN ({$sql}) target ON ctx.id = target.{$fieldname}"; } } else { // No field name available. Fall back on a potentially slower version. $wrapper = " - SELECT - {$fields} + SELECT {$fields} FROM {context} ctx - WHERE ctx.id IN ({$sql})"; + WHERE ctx.id IN ({$sql})"; } $contexts = $DB->get_recordset_sql($wrapper, $params); @@ -138,23 +135,24 @@ class contextlist extends contextlist_base { * Guess the name of the contextid field from the supplied SQL. * * @param string $sql The SQL to guess from - * @return string The field name. + * @return string The field name or a numeric value representing the context id */ protected function guess_id_field_from_sql(string $sql) : string { // Get the list of relevant words from the SQL Query. // We explode the SQL by the space character, then trim any extra whitespace (e.g. newlines), before we filter // empty value, and finally we re-index the array. - $words = array_map('trim', explode(' ', $sql)); + $sql = rtrim($sql, ';'); + $words = array_map('trim', preg_split('/\s+/', $sql)); $words = array_filter($words, function($word) { return $word !== ''; }); $words = array_values($words); - if ($firstfrom = array_search('FROM', $words)) { + if ($firstfrom = array_search('FROM', array_map('strtoupper', $words))) { // Found a FROM keyword. // Select the previous word. $fieldname = $words[$firstfrom - 1]; - if (is_int($fieldname)) { + if (is_numeric($fieldname)) { return $fieldname; } @@ -164,6 +162,14 @@ class contextlist extends contextlist_base { } return $fieldname; + + } else if ((count($words) == 1) && (is_numeric($words[0]))) { + // Not a real SQL, just a single numerical value - such as one returned by {@link self::add_system_context()}. + return $words[0]; + + } else if ((count($words) == 2) && (strtoupper($words[0]) === 'SELECT') && (is_numeric($words[1]))) { + // SQL returning a constant numerical value. + return $words[1]; } return ''; diff --git a/privacy/tests/contextlist_test.php b/privacy/tests/contextlist_test.php index 736e523ef5d..a247b3d3ab9 100644 --- a/privacy/tests/contextlist_test.php +++ b/privacy/tests/contextlist_test.php @@ -105,4 +105,85 @@ class contextlist_test extends advanced_testcase { $this->assertContains(\context_user::instance($user1->id)->id, $contexts); $this->assertContains(\context_user::instance($user2->id)->id, $contexts); } + + /** + * Test {@link \core_privacy\local\request\contextlist::test_guess_id_field_from_sql()} implementation. + * + * @dataProvider data_guess_id_field_from_sql + * @param string $sql Input SQL we try to extract the context id field name from. + * @param string $expected Expected detected value. + */ + public function test_guess_id_field_from_sql($sql, $expected) { + + $rc = new \ReflectionClass(contextlist::class); + $rcm = $rc->getMethod('guess_id_field_from_sql'); + $rcm->setAccessible(true); + $actual = $rcm->invoke(new contextlist(), $sql); + + $this->assertEquals($expected, $actual, 'Unable to guess context id field in: '.$sql); + } + + /** + * Provides data sets for {@link self::test_guess_id_field_from_sql()}. + * + * @return array + */ + public function data_guess_id_field_from_sql() { + return [ + 'easy' => [ + 'SELECT contextid FROM {foo}', + 'contextid', + ], + 'with_distinct' => [ + 'SELECT DISTINCT contextid FROM {foo}', + 'contextid', + ], + 'with_dot' => [ + 'SELECT cx.id FROM {foo} JOIN {context} cx ON blahblahblah', + 'id', + ], + 'letter_case_does_not_matter' => [ + 'Select ctxid From {foo} Where bar = ?', + 'ctxid', + ], + 'alias' => [ + 'SELECT foo.contextid AS ctx FROM {bar} JOIN {foo} ON bar.id = foo.barid', + 'ctx', + ], + 'tabs' => [ + "SELECT\tctxid\t\tFROM foo f", + 'ctxid', + ], + 'whitespace' => [ + "SELECT + ctxid\t + \tFROM foo f", + 'ctxid', + ], + 'just_number' => [ + '1', + '1', + ], + 'select_number' => [ + 'SELECT 2', + '2', + ], + 'select_number_with_semicolon' => [ + 'SELECT 3;', + '3', + ], + 'select_number_from_table' => [ + 'SELECT 4 FROM users', + '4', + ], + 'invalid_1' => [ + 'SELECT 1+1', + '', + ], + 'invalid_2' => [ + 'muhehe', + '', + ], + ]; + } }