Skip to content

Commit

Permalink
MDL-70537 Availability: Avoid recursive calls to $cm->name
Browse files Browse the repository at this point in the history
These recursive calls didn't work in PHP 7.3 and below, but in PHP
7.4 they also cause a fatal error which means if you have invalid
availability data, the whole page might die.
  • Loading branch information
sammarshallou committed Mar 18, 2021
1 parent fc335f5 commit c89ea67
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 9 deletions.
12 changes: 6 additions & 6 deletions availability/classes/info.php
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,9 @@ public function get_full_information(\course_modinfo $modinfo = null) {
*/
protected function warn_about_invalid_availability(\coding_exception $e) {
$name = $this->get_thing_name();
// If it occurs while building modinfo based on somebody calling $cm->name,
// we can't get $cm->name, and this line will cause a warning.
$htmlname = @$this->format_info($name, $this->course);
$htmlname = $this->format_info($name, $this->course);
// Because we call format_info here, likely in the middle of building dynamic data for the
// activity, there could be a chance that the name might not be available.
if ($htmlname === '') {
// So instead use the numbers (cmid) from the tag.
$htmlname = preg_replace('~[^0-9]~', '', $name);
Expand Down Expand Up @@ -739,11 +739,11 @@ public static function format_info($inforenderable, $courseorid) {
$info = preg_replace_callback('~<AVAILABILITY_CMNAME_([0-9]+)/>~',
function($matches) use($modinfo, $context) {
$cm = $modinfo->get_cm($matches[1]);
if ($cm->has_view() and $cm->uservisible) {
if ($cm->has_view() and $cm->get_user_visible()) {
// Help student by providing a link to the module which is preventing availability.
return \html_writer::link($cm->url, format_string($cm->name, true, array('context' => $context)));
return \html_writer::link($cm->get_url(), format_string($cm->get_name(), true, ['context' => $context]));
} else {
return format_string($cm->name, true, array('context' => $context));
return format_string($cm->get_name(), true, ['context' => $context]);
}
}, $info);

Expand Down
29 changes: 29 additions & 0 deletions availability/tests/info_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -511,4 +511,33 @@ public function test_filter_user_list() {
sort($result);
$this->assertEquals($expected, $result);
}

/**
* Tests the info_module class when involved in a recursive call to $cm->name.
*/
public function test_info_recursive_name_call() {
global $DB;

$this->resetAfterTest();

// Create a course and page.
$generator = $this->getDataGenerator();
$course = $generator->create_course();
$page1 = $generator->create_module('page', ['course' => $course->id, 'name' => 'Page1']);

// Set invalid availability.
$DB->set_field('course_modules', 'availability', 'not valid', ['id' => $page1->cmid]);

// Get the cm_info object.
$this->setAdminUser();
$modinfo = get_fast_modinfo($course);
$cm1 = $modinfo->get_cm($page1->cmid);

// At this point we will generate dynamic data for $cm1, which will cause the debugging
// call below.
$this->assertEquals('Page1', $cm1->name);

$this->assertDebuggingCalled('Error processing availability data for ' .
'&lsquo;Page1&rsquo;: Invalid availability text');
}
}
21 changes: 18 additions & 3 deletions lib/modinfolib.php
Original file line number Diff line number Diff line change
Expand Up @@ -1312,9 +1312,15 @@ public function has_view() {
}

/**
* Gets the URL to link to for this module.
*
* This method is normally called by the property ->url, but can be called directly if
* there is a case when it might be called recursively (you can't call property values
* recursively).
*
* @return moodle_url URL to link to for this module, or null if it doesn't have a view page
*/
private function get_url() {
public function get_url() {
$this->obtain_dynamic_data();
return $this->url;
}
Expand Down Expand Up @@ -1360,9 +1366,13 @@ public function get_formatted_content($options = array()) {

/**
* Getter method for property $name, ensures that dynamic data is obtained.
*
* This method is normally called by the property ->name, but can be called directly if there
* is a case when it might be called recursively (you can't call property values recursively).
*
* @return string
*/
private function get_name() {
public function get_name() {
$this->obtain_dynamic_data();
return $this->name;
}
Expand Down Expand Up @@ -1915,9 +1925,14 @@ private function obtain_dynamic_data() {

/**
* Getter method for property $uservisible, ensures that dynamic data is retrieved.
*
* This method is normally called by the property ->uservisible, but can be called directly if
* there is a case when it might be called recursively (you can't call property values
* recursively).
*
* @return bool
*/
private function get_user_visible() {
public function get_user_visible() {
$this->obtain_dynamic_data();
return $this->uservisible;
}
Expand Down

0 comments on commit c89ea67

Please sign in to comment.