From 7cf0235ef7272df28051cf593c4ba24143ec927a Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Mon, 20 Dec 2021 21:16:43 +0000 Subject: [PATCH] MDL-73086 user: account for mixed casing of profile field shortname. Custom profile fields allow for shortnames containing mixed character casing, which should also be supported within the user fields API when retrieving said custom fields. Given that the DML libs convert all selected columns to lowercased equivalent, we need to do the same when requesting custom fields to ensure calling code doesn't try to access `->profile_field_Foo` property when the DB returns `->profile_field_foo` instead. --- user/classes/fields.php | 12 +++++--- user/profile/lib.php | 12 ++++++-- user/tests/profilelib_test.php | 55 ++++++++++++++++++++++++++++++++++ user/upgrade.txt | 5 ++++ 4 files changed, 78 insertions(+), 6 deletions(-) diff --git a/user/classes/fields.php b/user/classes/fields.php index f5ac4e7676e..2e7d94ad5eb 100644 --- a/user/classes/fields.php +++ b/user/classes/fields.php @@ -16,6 +16,8 @@ namespace core_user; +use core_text; + /** * Class for retrieving information about user fields that are needed for displaying user identity. * @@ -345,7 +347,8 @@ class fields { * showing lists of users (in addition to the user's name which is included as standard). * * The results include basic field names (columns from the 'user' database table) and, unless - * turned off, custom profile field names in the format 'profile_field_myfield'. + * turned off, custom profile field names in the format 'profile_field_myfield', note these + * fields will always be returned lower cased to match how they are returned by the DML library. * * This function does all the required capability checks to see if the current user is allowed * to see them in the specified context. You can pass context null to get all the fields @@ -419,7 +422,7 @@ class fields { // Re-index the entries and return. $extra = array_values($extra); - return $extra; + return array_map([core_text::class, 'strtolower'], $extra); } /** @@ -536,7 +539,8 @@ class fields { $placeholder = '?'; $params[] = $shortname; } - $joins .= " JOIN {user_info_field} $fieldalias ON $fieldalias.shortname = $placeholder + $joins .= " JOIN {user_info_field} $fieldalias ON " . + $DB->sql_equal($fieldalias . '.shortname', $placeholder, false) . " LEFT JOIN {user_info_data} $dataalias ON $dataalias.fieldid = $fieldalias.id AND $dataalias.userid = {$usertable}id"; // For Oracle we need to convert the field into a usable format. @@ -587,7 +591,7 @@ class fields { // Custom fields have special handling. if (preg_match(self::PROFILE_FIELD_REGEX, $field, $matches)) { require_once($CFG->dirroot . '/user/profile/lib.php'); - $fieldinfo = profile_get_custom_field_data_by_shortname($matches[1]); + $fieldinfo = profile_get_custom_field_data_by_shortname($matches[1], false); // Use format_string so it can be translated with multilang filter if necessary. return $fieldinfo ? format_string($fieldinfo->name) : $field; } diff --git a/user/profile/lib.php b/user/profile/lib.php index d4b59d44e86..114089039c2 100644 --- a/user/profile/lib.php +++ b/user/profile/lib.php @@ -854,9 +854,11 @@ function profile_save_custom_fields($userid, $profilefields) { * current request for all fields so that it can be used quickly. * * @param string $shortname Shortname of custom profile field + * @param bool $casesensitive Whether to perform case-sensitive matching of shortname. Note current limitations of custom profile + * fields allow the same shortname to exist differing only by it's case * @return stdClass|null Object with properties id, shortname, name, visible, datatype, categoryid, etc */ -function profile_get_custom_field_data_by_shortname(string $shortname): ?stdClass { +function profile_get_custom_field_data_by_shortname(string $shortname, bool $casesensitive = true): ?stdClass { $cache = \cache::make_from_params(cache_store::MODE_REQUEST, 'core_profile', 'customfields', [], ['simplekeys' => true, 'simpledata' => true]); $data = $cache->get($shortname); @@ -866,7 +868,13 @@ function profile_get_custom_field_data_by_shortname(string $shortname): ?stdClas $data = null; foreach ($fields as $field) { $cache->set($field->shortname, $field); - if ($field->shortname === $shortname) { + + // Perform comparison according to case sensitivity parameter. + $shortnamematch = $casesensitive + ? strcmp($field->shortname, $shortname) === 0 + : strcasecmp($field->shortname, $shortname) === 0; + + if ($shortnamematch) { $data = $field; } } diff --git a/user/tests/profilelib_test.php b/user/tests/profilelib_test.php index 77da39879f1..a49dc6bae81 100644 --- a/user/tests/profilelib_test.php +++ b/user/tests/profilelib_test.php @@ -283,4 +283,59 @@ class core_user_profilelib_testcase extends advanced_testcase { $this->assertNull(profile_get_custom_field_data_by_shortname('speciality')); } + + /** + * Data provider for {@see test_profile_get_custom_field_data_by_shortname_case_sensitivity} + * + * @return array[] + */ + public function profile_get_custom_field_data_by_shortname_case_sensitivity_provider(): array { + return [ + 'Matching case, case-sensitive search' => ['hello', 'hello', true, true], + 'Matching case, case-insensitive search' => ['hello', 'hello', false, true], + 'Non-matching case, case-sensitive search' => ['hello', 'Hello', true, false], + 'Non-matching case, case-insensitive search' => ['hello', 'Hello', false, true], + 'Non-matching, case-sensitive search' => ['hello', 'hola', true, false], + 'Non-matching, case-insensitive search' => ['hello', 'hola', false, false], + ]; + } + + /** + * Test retrieving custom field by shortname, specifying case-sensitivity when matching + * + * @param string $shortname + * @param string $shortnamesearch + * @param bool $casesensitive + * @param bool $expectmatch + * + * @dataProvider profile_get_custom_field_data_by_shortname_case_sensitivity_provider + */ + public function test_profile_get_custom_field_data_by_shortname_case_sensitivity( + string $shortname, + string $shortnamesearch, + bool $casesensitive, + bool $expectmatch + ): void { + global $CFG; + + require_once("{$CFG->dirroot}/user/profile/lib.php"); + + $this->resetAfterTest(); + + $this->getDataGenerator()->create_custom_profile_field([ + 'datatype' => 'text', + 'shortname' => $shortname, + 'name' => 'My field', + ]); + + $customfield = profile_get_custom_field_data_by_shortname($shortnamesearch, $casesensitive); + if ($expectmatch) { + $this->assertInstanceOf(\stdClass::class, $customfield); + $this->assertEquals('text', $customfield->datatype); + $this->assertEquals($shortname, $customfield->shortname); + $this->assertEquals('My field', $customfield->name); + } else { + $this->assertNull($customfield); + } + } } diff --git a/user/upgrade.txt b/user/upgrade.txt index d20af1afffd..bb08d441d75 100644 --- a/user/upgrade.txt +++ b/user/upgrade.txt @@ -1,5 +1,10 @@ This files describes API changes for code that uses the user API. +=== 3.11.5 === + +* The `profile_get_custom_field_data_by_shortname` method now accepts an optional parameter to determine whether + to use case-sensitive matching of the profile field shortname or not (default true) + === 3.11 === * Added new core_user/form_user_selector JS module that can be used as the 'ajax' handler for the autocomplete form