MDL-60738 webservice: Clean theme and lang properly

Sometimes the "theme" and "lang" fields in the user and course tables
in the database are set to incorrect values (uninstalled or
non-existent themes and language packs).
This makes Web Services functions to fail because the WS server
validate the returned data using the validate_param function that clean
parameters.
This commit is contained in:
Juan Leyva 2017-11-08 15:35:33 +01:00
parent 0164f50e80
commit 6db2423551
8 changed files with 60 additions and 13 deletions

View File

@ -504,10 +504,10 @@ class core_course_external extends external_api {
$courseinfo['groupmode'] = $course->groupmode;
$courseinfo['groupmodeforce'] = $course->groupmodeforce;
$courseinfo['defaultgroupingid'] = $course->defaultgroupingid;
$courseinfo['lang'] = $course->lang;
$courseinfo['lang'] = clean_param($course->lang, PARAM_LANG);
$courseinfo['timecreated'] = $course->timecreated;
$courseinfo['timemodified'] = $course->timemodified;
$courseinfo['forcetheme'] = $course->theme;
$courseinfo['forcetheme'] = clean_param($course->theme, PARAM_THEME);
$courseinfo['enablecompletion'] = $course->enablecompletion;
$courseinfo['completionnotify'] = $course->completionnotify;
$courseinfo['courseformatoptions'] = array();
@ -1726,7 +1726,7 @@ class core_course_external extends external_api {
$categoryinfo['visible'] = $category->visible;
$categoryinfo['visibleold'] = $category->visibleold;
$categoryinfo['timemodified'] = $category->timemodified;
$categoryinfo['theme'] = $category->theme;
$categoryinfo['theme'] = clean_param($category->theme, PARAM_THEME);
}
$categoriesinfo[] = $categoryinfo;
@ -3068,6 +3068,14 @@ class core_course_external extends external_api {
foreach ($coursefields as $field) {
$coursesdata[$course->id][$field] = $course->{$field};
}
// Clean lang and auth fields for external functions (it may content uninstalled themes or language packs).
if (isset($coursesdata[$course->id]['theme'])) {
$coursesdata[$course->id]['theme'] = clean_param($coursesdata[$course->id]['theme'], PARAM_THEME);
}
if (isset($coursesdata[$course->id]['lang'])) {
$coursesdata[$course->id]['lang'] = clean_param($coursesdata[$course->id]['lang'], PARAM_LANG);
}
}
return array(

View File

@ -2211,6 +2211,21 @@ class core_course_externallib_testcase extends externallib_advanced_testcase {
$this->assertCount(0, $result['courses']);
}
/**
* Test get_courses_by_field_invalid_theme_and_lang
*/
public function test_get_courses_by_field_invalid_theme_and_lang() {
$this->resetAfterTest(true);
$this->setAdminUser();
$course = self::getDataGenerator()->create_course(array('theme' => 'kkt', 'lang' => 'kkl'));
$result = core_course_external::get_courses_by_field('id', $course->id);
$result = external_api::clean_returnvalue(core_course_external::get_courses_by_field_returns(), $result);
$this->assertEmpty($result['courses']['0']['theme']);
$this->assertEmpty($result['courses']['0']['lang']);
}
public function test_check_updates() {
global $DB;
$this->resetAfterTest(true);

View File

@ -343,7 +343,7 @@ class core_enrol_external extends external_api {
'summaryformat' => $course->summaryformat,
'format' => $course->format,
'showgrades' => $course->showgrades,
'lang' => $course->lang,
'lang' => clean_param($course->lang, PARAM_LANG),
'enablecompletion' => $course->enablecompletion,
'category' => $course->category,
'progress' => $progress,

View File

@ -376,8 +376,12 @@ class core_enrol_externallib_testcase extends externallib_advanced_testcase {
'enddate' => $timenow + WEEKSECS
);
$coursedata2 = array(
'lang' => 'kk', // Check invalid language pack.
);
$course1 = self::getDataGenerator()->create_course($coursedata1);
$course2 = self::getDataGenerator()->create_course();
$course2 = self::getDataGenerator()->create_course($coursedata2);
$courses = array($course1, $course2);
// Enrol $USER in the courses.
@ -414,6 +418,9 @@ class core_enrol_externallib_testcase extends externallib_advanced_testcase {
foreach ($coursedata1 as $fieldname => $value) {
$this->assertEquals($courseenrol[$fieldname], $course1->$fieldname);
}
} else {
// Check language pack. Should be empty since an incorrect one was used when creating the course.
$this->assertEmpty($courseenrol['lang']);
}
}
}

View File

@ -564,6 +564,14 @@ function user_get_user_details($user, $course = null, array $userfields = array(
}
}
// Clean lang and auth fields for external functions (it may content uninstalled themes or language packs).
if (isset($userdetails['lang'])) {
$userdetails['lang'] = clean_param($userdetails['lang'], PARAM_LANG);
}
if (isset($userdetails['theme'])) {
$userdetails['theme'] = clean_param($userdetails['theme'], PARAM_THEME);
}
return $userdetails;
}

View File

@ -222,8 +222,10 @@ class core_user_externallib_testcase extends externallib_advanced_testcase {
'descriptionformat' => FORMAT_MOODLE,
'city' => 'Perth',
'url' => 'http://moodle.org',
'country' => 'AU'
);
'country' => 'AU',
'lang' => 'kkl',
'theme' => 'kkt',
);
$user1 = self::getDataGenerator()->create_user($user1);
if (!empty($CFG->usetags)) {
require_once($CFG->dirroot . '/user/editlib.php');
@ -328,6 +330,11 @@ class core_user_externallib_testcase extends externallib_advanced_testcase {
if (!empty($CFG->usetags) and !empty($generateduser->interests)) {
$this->assertEquals(implode(', ', $generateduser->interests), $returneduser['interests']);
}
// Check empty since incorrect values were used when creating the user.
if ($returneduser['id'] == $user1->id) {
$this->assertEmpty($returneduser['lang']);
$this->assertEmpty($returneduser['theme']);
}
}
}

View File

@ -720,14 +720,16 @@ class core_userliblib_testcase extends advanced_testcase {
* calling user_get_user_details() function.
*/
public function test_user_get_user_details_missing_fields() {
global $CFG;
$this->resetAfterTest(true);
$this->setAdminUser(); // We need capabilities to view the data.
$user = self::getDataGenerator()->create_user([
'auth' => 'auth_something',
'confirmed' => '0',
'idnumber' => 'someidnumber',
'lang' => 'en_ar',
'theme' => 'mytheme',
'lang' => 'en',
'theme' => $CFG->theme,
'timezone' => '50',
'mailformat' => '0',
]);
@ -737,8 +739,8 @@ class core_userliblib_testcase extends advanced_testcase {
self::assertSame('auth_something', $got['auth']);
self::assertSame('0', $got['confirmed']);
self::assertSame('someidnumber', $got['idnumber']);
self::assertSame('en_ar', $got['lang']);
self::assertSame('mytheme', $got['theme']);
self::assertSame('en', $got['lang']);
self::assertSame($CFG->theme, $got['theme']);
self::assertSame('50', $got['timezone']);
self::assertSame('0', $got['mailformat']);
}

View File

@ -90,7 +90,7 @@ class core_webservice_external extends external_api {
'firstname' => $USER->firstname,
'lastname' => $USER->lastname,
'fullname' => fullname($USER),
'lang' => current_language(),
'lang' => clean_param(current_language(), PARAM_LANG),
'userid' => $USER->id,
'userpictureurl' => $profileimageurl->out(false),
'siteid' => SITEID
@ -223,7 +223,7 @@ class core_webservice_external extends external_api {
'firstname' => new external_value(PARAM_TEXT, 'first name'),
'lastname' => new external_value(PARAM_TEXT, 'last name'),
'fullname' => new external_value(PARAM_TEXT, 'user full name'),
'lang' => new external_value(PARAM_LANG, 'user language'),
'lang' => new external_value(PARAM_LANG, 'Current language.'),
'userid' => new external_value(PARAM_INT, 'user id'),
'siteurl' => new external_value(PARAM_RAW, 'site url'),
'userpictureurl' => new external_value(PARAM_URL, 'the user profile picture.