L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'MDL-73317-master' of https://github.com/matthewhilton/moodle

Browse Source
This commit is contained in:
Andrew Nicols
2023-02-15 22:56:35 +08:00
parent 77c4493a11 5ea4d886ee
commit f60c946408
6 changed files with 196 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backup/controller/restore_controller.class.php
+3
View File
@@ -384,6 +384,9 @@ class restore_controller extends base_controller {
// Release the session so other tabs in the same session are not blocked.
if ($this->get_releasesession() === backup::RELEASESESSION_YES) {
// Preemptively reset the navcache before closing, so it remains the same on shutdown.
navigation_cache::destroy_volatile_caches();
\core\session\manager::write_close();
}
lib/classes/session/manager.php
+112 -15
View File
@@ -60,6 +60,9 @@ class manager {
/** @var array Stores the the SESSION before a request is performed, used to check incorrect read-only modes */
private static $priorsession = [];
/** @var array Stores the the SESSION after write_close is called, used to check if it was mutated after the session is closed */
private static $sessionatclose = [];
/**
* @var bool Used to trigger the SESSION mutation warning without actually preventing SESSION mutation.
* This variable is used to "copy" what the $requireslock parameter does in start_session().
@@ -686,6 +689,12 @@ class manager {
global $PERF, $ME, $CFG;
if (self::$sessionactive) {
// If debugging, take a snapshot of session at close and compare on shutdown to detect any accidental mutations.
if (debugging()) {
self::$sessionatclose = (array) $_SESSION['SESSION'];
\core_shutdown_manager::register_function('\core\session\manager::check_mutated_closed_session');
}
// Grab the time when session lock is released.
$PERF->sessionlock['released'] = microtime(true);
if (!empty($PERF->sessionlock['gained'])) {
@@ -735,6 +744,50 @@ class manager {
self::$sessionactive = false;
}
/**
* Checks if the session has been mutated since it was closed.
* In write_close the session is saved to the variable $sessionatclose
* If there is a difference between $sessionatclose and the current session,
* it means a script has erroneously closed the session too early.
* Script is usually called in shutdown_manager
*/
public static function check_mutated_closed_session() {
global $ME;
// Session is still open, mutations are allowed.
if (self::$sessionactive) {
return;
}
// Detect if session was cleared.
if (!isset($_SESSION['SESSION']) && isset(self::$sessionatclose)) {
debugging("Script $ME cleared the session after it was closed.");
return;
} else if (!isset($_SESSION['SESSION'])) {
// Else session is empty, nothing to check.
return;
}
// Session is closed - compare the current session to the session when write_close was called.
$arraydiff = self::array_session_diff(
self::$sessionatclose,
(array) $_SESSION['SESSION']
);
if ($arraydiff) {
$error = "Script $ME mutated the session after it was closed:";
foreach ($arraydiff as $key => $value) {
$error .= ' $SESSION->' . $key;
// Extra debugging for cachestore session changes.
if (strpos($key, 'cachestore_') === 0 && is_array($value)) {
$error .= ': ' . implode(',', array_keys($value));
}
}
debugging($error);
}
}
/**
* Does the PHP session with given id exist?
*
@@ -1384,25 +1437,69 @@ class manager {
}
/**
* Compares two arrays outputs the difference.
* Compares two arrays and outputs the difference.
*
* Note this does not use array_diff_assoc due to
* the use of stdClasses in Moodle sessions.
* Note - checking between objects and array type is only done at the top level.
* Any changes in types below the top level will not be detected.
* However, if their values are the same, they will be treated as equal.
*
* @param array $array1
* @param array $array2
* Any changes, such as removals, edits or additions will be detected.
*
* @param array $previous
* @param array $current
* @return array
*/
private static function array_session_diff(array $array1, array $array2) : array {
$difference = [];
foreach ($array1 as $key => $value) {
if (!isset($array2[$key])) {
$difference[$key] = $value;
} else if ($array2[$key] !== $value) {
$difference[$key] = $value;
}
}
private static function array_session_diff(array $previous, array $current) : array {
// To use array_udiff_uassoc, the first array must have the most keys; this ensures every key is checked.
// To do this, we first need to sort them by the length of their keys.
$arrays = [$current, $previous];
return $difference;
// Sort them by the length of their keys.
usort($arrays, function ($a, $b) {
return count(array_keys($b)) - count(array_keys($a));
});
// The largest is the first value in the $arrays, after sorting.
// The smallest is then the last one.
// If they are the same size, it does not matter which is which.
$largest = $arrays[0];
$smallest = $arrays[1];
// Defines a function that casts the values to arrays.
// This is so the properties are compared, instead any object's identities.
$casttoarray = function ($value) {
return json_decode(json_encode($value), true);
};
// Defines a function that compares all keys by their string value.
$keycompare = function ($a, $b) {
return strcmp($a, $b);
};
// Defines a function that compares all values by first their type, and then their values.
// If the value contains any objects, they are cast to arrays before comparison.
$valcompare = function ($a, $b) use ($casttoarray) {
// First compare type.
// If they are not the same type, they are definitely not the same.
// Note we do not check types recursively.
if (gettype($a) !== gettype($b)) {
return 1;
}
// Next compare value. Cast any objects to arrays to compare their properties,
// instead of the identitiy of the object itself.
$v1 = $casttoarray($a);
$v2 = $casttoarray($b);
if ($v1 !== $v2) {
return 1;
}
return 0;
};
// Apply the comparison functions to the two given session arrays,
// making sure to use the largest array first, so that all keys are considered.
return array_udiff_uassoc($largest, $smallest, $valcompare, $keycompare);
}
}
lib/tests/session_manager_test.php
+68 -47
View File
@@ -846,60 +846,81 @@ class session_manager_test extends \advanced_testcase {
$this->assertEquals('/good.php?id=4', $SESSION->recentsessionlocks[0]['url']);
}
public function test_array_session_diff_same_array() {
$a = [];
$a['c'] = new \stdClass();
$a['c']->o = new \stdClass();
$a['c']->o->o = new \stdClass();
$a['c']->o->o->l = 'cool';
/**
* Data provider for the array_session_diff function.
*
* @return array
*/
public function array_session_diff_provider() {
// Create an instance of this object so the comparison object's identities are the same.
// Used in one of the tests below.
$compareobjectb = (object) ['array' => 'b'];
$class = new \ReflectionClass('\core\session\manager');
$method = $class->getMethod('array_session_diff');
$method->setAccessible(true);
$result = $method->invokeArgs(null, [$a, $a]);
$this->assertEmpty($result);
return [
'both same objects' => [
'a' => ['example' => (object) ['array' => 'a']],
'b' => ['example' => (object) ['array' => 'a']],
'expected' => [],
],
'both same arrays' => [
'a' => ['example' => ['array' => 'a']],
'b' => ['example' => ['array' => 'a']],
'expected' => [],
],
'both the same with nested objects' => [
'a' => ['example' => (object) ['array' => 'a', 'deeper' => (object) []]],
'b' => ['example' => (object) ['array' => 'a', 'deeper' => (object) []]],
'expected' => [],
],
'first array larger' => [
'a' => ['x' => 1, 'y' => 2],
'b' => ['x' => 1],
'expected' => ['y' => 2]
],
'second array larger' => [
'a' => ['x' => 1],
'b' => ['x' => 1, 'y' => 2],
'expected' => ['y' => 2]
],
'objects with different values but same keys' => [
'a' => ['example' => (object) ['array' => 'a']],
'b' => ['example' => $compareobjectb],
'expected' => ['example' => $compareobjectb]
],
'different arrays with top level indexes' => [
'a' => ['x', 'y'],
'b' => ['x', 'y', 'z'],
'expected' => [2 => 'z']
],
'different types but same values as first level' => [
'a' => ['example' => (object) ['array' => 'a']],
'b' => ['example' => ['array' => 'a']],
'expected' => ['example' => ['array' => 'a']]
],
'different types but same values nested' => [
'a' => ['example' => (object) ['array' => ['a' => 'test']]],
'b' => ['example' => (object) ['array' => (object) ['a' => 'test']]],
// Type checking is not done further than the first level, so we expect no difference.
'expected' => []
]
];
}
public function test_array_session_diff_first_array_larger() {
$a = [];
$a['stdClass'] = new \stdClass();
$a['stdClass']->attribute = 'This is an attribute';
$a['array'] = ['array', 'contents'];
$b = [];
$b['array'] = ['array', 'contents'];
/**
* Tests array diff method in various situations.
*
* @dataProvider array_session_diff_provider
* @covers \core\session\manager::array_session_diff
* @param array $a first value.
* @param array $b second value to compare to $a.
* @param array $expected the expected difference.
*/
public function test_array_session_diff(array $a, array $b, array $expected) {
$class = new \ReflectionClass('\core\session\manager');
$method = $class->getMethod('array_session_diff');
$method->setAccessible(true);
$result = $method->invokeArgs(null, [$a, $b]);
$expected = [];
$expected['stdClass'] = new \stdClass();
$expected['stdClass']->attribute = 'This is an attribute';
$this->assertEquals($expected, $result);
}
public function test_array_session_diff_second_array_larger() {
$a = [];
$a['array'] = ['array', 'contents'];
$b = [];
$b['stdClass'] = new \stdClass();
$b['stdClass']->attribute = 'This is an attribute';
$b['array'] = ['array', 'contents'];
$class = new \ReflectionClass('\core\session\manager');
$method = $class->getMethod('array_session_diff');
$method->setAccessible(true);
$result = $method->invokeArgs(null, [$a, $b]);
// It's empty because the first array contains all the contents of the second.
$expected = [];
$this->assertEquals($expected, $result);
$this->assertSame($expected, $result);
}
}
mod/assign/locallib.php
+7 -6
View File
@@ -208,8 +208,6 @@ class assign {
* otherwise this class will load one from the context as required.
*/
public function __construct($coursemodulecontext, $coursemodule, $course) {
global $SESSION;
$this->context = $coursemodulecontext;
$this->course = $course;
@@ -224,10 +222,6 @@ class assign {
// Extra entropy is required for uniqid() to work on cygwin.
$this->useridlistid = clean_param(uniqid('', true), PARAM_ALPHANUM);
if (!isset($SESSION->mod_assign_useridlist)) {
$SESSION->mod_assign_useridlist = [];
}
}
/**
@@ -9345,6 +9339,13 @@ class assign {
* @return string The key for the id, or new entry if no $id is passed.
*/
public function get_useridlist_key($id = null) {
global $SESSION;
// Ensure the user id list cache is initialised.
if (!isset($SESSION->mod_assign_useridlist)) {
$SESSION->mod_assign_useridlist = [];
}
if ($id === null) {
$id = $this->get_useridlist_key_id();
}
report/participation/index.php
+3 -2
View File
@@ -86,8 +86,6 @@ echo $OUTPUT->header();
// Print the selector dropdown.
$pluginname = get_string('pluginname', 'report_participation');
report_helper::print_report_selector($pluginname);
// Release session lock.
\core\session\manager::write_close();
// Logs will not have been recorded before the course timecreated time.
$minlog = $course->timecreated;
@@ -185,6 +183,9 @@ if (!empty($instanceid) && !empty($roleid)) {
));
$table->setup();
// Unlock the session only after outputting the table, since the table writes to the session.
\core\session\manager::write_close();
// We want to query both the current context and parent contexts.
list($relatedctxsql, $params) = $DB->get_in_or_equal($context->get_parent_context_ids(true), SQL_PARAMS_NAMED, 'relatedctx');
$params['roleid'] = $roleid;
search/index.php
+3 -3
View File
@@ -48,9 +48,6 @@ if (!empty($CFG->forcelogin)) {
require_login();
}
// Unlock the session during a search.
\core\session\manager::write_close();
require_capability('moodle/search:query', $context);
$searchrenderer = $PAGE->get_renderer('core_search');
@@ -169,6 +166,9 @@ $PAGE->set_url($url);
// We are ready to render.
echo $OUTPUT->header();
// Unlock the session only after outputting the header as this modifies the session cachestore.
\core\session\manager::write_close();
// Get the results.
if ($data) {
$results = $search->paged_search($data, $page);