Details
-
Bug
-
Status: Closed
-
Minor
-
Resolution: Won't Do
-
3.1.2
-
None
Description
The top of `cache/admin.php` does the following:
// The first time the user visits this page we are going to reparse the definitions.
|
// Just ensures that everything is up to date.
|
// We flag is session so that this only happens once as people are likely to hit
|
// this page several times if making changes.
|
if (empty($SESSION->cacheadminreparsedefinitions)) {
|
cache_helper::update_definitions();
|
$SESSION->cacheadminreparsedefinitions = true;
|
}
|
Which results in `muc/config.php` being written - even if unchanged.
This means that if for example, one systems administrator is viewing this page (in a supervisory / pair-programming/ops capacity) they are unexpectedly in a race condition with another administrator who is actually performing the changes, where their viewing of the page may overwrite the changes made by the admin who is actually trying to change the settings.
Related - the file that does get written out is prepared as follows:
// Prepare the file content.
|
$content = "<?php defined('MOODLE_INTERNAL') || die();\n \$configuration = ".var_export($configuration, true).";";
|
I'm unable to find any guarantees that `var_export` will produce deterministic output - and it our case we've observed the order of the `definitionmappings` change after subsequent views to this page - making it very difficult to make sense of running `diff` commands against a before and after.
Some suggestions (in my view of most to least important)
1. Do not write to important configuration files that affect the stability of the system without an affirmative action by an administrator.
2. Allow for this file to be edited solely outside of the application (like other frozen $CFG settings).
3. Consider a deterministic format for this file.