From c910982c4900e29806e0d6129753cfb730395126 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sat, 2 Nov 2019 16:20:44 +0100 Subject: [PATCH 1/3] MDL-67114 mustache: php74 fix. Fix use of scalar as array Trying to use values of type null, bool, int, float or resource as an array (such as $null["key"]) will now generate a notice. This does not affect array accesses performed by list(). RFC: https://wiki.php.net/rfc/notice-for-non-valid-array-container --- lib/mustache/readme_moodle.txt | 9 +++++++++ lib/mustache/src/Mustache/Parser.php | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/mustache/readme_moodle.txt b/lib/mustache/readme_moodle.txt index 6bc4acffe4f..586ce769f8d 100644 --- a/lib/mustache/readme_moodle.txt +++ b/lib/mustache/readme_moodle.txt @@ -11,3 +11,12 @@ unzip v2.12.0.zip cd mustache.php-2.12.0/ mv src /path/to/moodle/lib/mustache/ mv LICENSE /path/to/moodle/lib/mustache/ + +Local changes: + +Note: All this changes need to be reviewed on every upgrade and, if they have +been already applied upstream for the release being used, can be removed +from the list. If still not available upstream, they will need to be re-applied. + +- MDL-67114: PHP 7.4 compatibility. Array operations on scalar value. + This corresponds to upstream https://github.com/bobthecow/mustache.php/pull/352 diff --git a/lib/mustache/src/Mustache/Parser.php b/lib/mustache/src/Mustache/Parser.php index c36a84afeab..0ec419220e0 100644 --- a/lib/mustache/src/Mustache/Parser.php +++ b/lib/mustache/src/Mustache/Parser.php @@ -149,7 +149,7 @@ class Mustache_Parser case Mustache_Tokenizer::T_BLOCK_VAR: if ($this->pragmaBlocks) { // BLOCKS pragma is enabled, let's do this! - if ($parent[Mustache_Tokenizer::TYPE] === Mustache_Tokenizer::T_PARENT) { + if (isset($parent) && $parent[Mustache_Tokenizer::TYPE] === Mustache_Tokenizer::T_PARENT) { $token[Mustache_Tokenizer::TYPE] = Mustache_Tokenizer::T_BLOCK_ARG; } $this->clearStandaloneLines($nodes, $tokens); @@ -275,7 +275,7 @@ class Mustache_Parser */ private function checkIfTokenIsAllowedInParent($parent, array $token) { - if ($parent[Mustache_Tokenizer::TYPE] === Mustache_Tokenizer::T_PARENT) { + if (isset($parent) && $parent[Mustache_Tokenizer::TYPE] === Mustache_Tokenizer::T_PARENT) { throw new Mustache_Exception_SyntaxException('Illegal content in < parent tag', $token); } } From c3a64372207388d4ff6b620b226b4320426730b8 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sun, 1 Dec 2019 17:44:52 +0100 Subject: [PATCH 2/3] MDL-67114 scssphp: php74 compatibility local fixes Note they are already available upstream, versions 1.0.4 and up so, in next upgrade they will be fixed. --- lib/scssphp/Compiler.php | 6 +++++- lib/scssphp/moodle_readme.txt | 9 +++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/scssphp/Compiler.php b/lib/scssphp/Compiler.php index 8f26740dbe2..75c5d675282 100644 --- a/lib/scssphp/Compiler.php +++ b/lib/scssphp/Compiler.php @@ -2664,11 +2664,15 @@ class Compiler * @param array $value * @param boolean $inExp * - * @return array|\ScssPhp\ScssPhp\Node\Number + * @return null|array|\ScssPhp\ScssPhp\Node\Number */ protected function reduce($value, $inExp = false) { + if (is_null($value)) { + return null; + } + switch ($value[0]) { case Type::T_EXPRESSION: list(, $op, $left, $right, $inParens) = $value; diff --git a/lib/scssphp/moodle_readme.txt b/lib/scssphp/moodle_readme.txt index 592a0663ca8..f0b5311d5e5 100644 --- a/lib/scssphp/moodle_readme.txt +++ b/lib/scssphp/moodle_readme.txt @@ -7,5 +7,14 @@ Import procedure: - Copy all the files from the folder 'src' this directory. - Copy the license file from the project root. +- Review the local changes defined below, if any. Reapply + them if needed. If already available upstream, please remove + them from the list. Licensed under MIT, Copyright (c) 2015 Leaf Corcoran. + +Currenly using 1.0.2 plus these local changes: + +- MDL-67114 : Added basic compatibility with php 7.4. This corresponds to + upstream commit https://github.com/scssphp/scssphp/commit/66675c1553b7e9d7c480d8aaedbf7c72374647cf + that is available in scssphp >= 1.0.4 From abc8f149ce1bd71d18384d3b5f2f4c721663c2f0 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sat, 2 Nov 2019 20:03:40 +0100 Subject: [PATCH 3/3] MDL-67114 core: php74 fix. Fix use of scalar as array in core There are various places where it's not guaranteed that the variable being used is array, and instead, can be null, bool, int... We need to check that because php74 warns about it. Where possible we have used the coalesce operator as replacement for isset() ternary operations. --- competency/tests/privacy_test.php | 4 ++-- enrol/meta/lib.php | 6 ++++-- lib/editor/tests/fixtures/editor_form.php | 2 +- lib/filestorage/file_system.php | 5 +++++ lib/form/duration.php | 2 +- lib/form/filetypes.php | 2 ++ lib/tests/session_manager_test.php | 2 +- mod/glossary/import_form.php | 2 +- mod/wiki/comments_form.php | 4 ++-- mod/wiki/parser/parser.php | 2 +- question/engine/questionusage.php | 2 +- repository/dropbox/classes/dropbox.php | 2 +- 12 files changed, 22 insertions(+), 13 deletions(-) diff --git a/competency/tests/privacy_test.php b/competency/tests/privacy_test.php index 77c5a5a45ce..7105753f50d 100644 --- a/competency/tests/privacy_test.php +++ b/competency/tests/privacy_test.php @@ -2302,7 +2302,7 @@ class core_competency_privacy_testcase extends provider_testcase { $this->assertEquals('-', $comp['rating']['rating']); $comp = $data->competencies[2]; $this->assertEquals($comp4->get('shortname'), $comp['name']); - $this->assertNull($comp['rating']['rating']); + $this->assertNull($comp['rating']); $data = writer::with_context($u1ctx)->get_data(array_merge($path, ["{$p1a->get('name')} ({$p1a->get('id')})", get_string('commentsubcontext', 'core_comment')])); $this->assert_exported_comments(['Hello.', 'It\'s me.', 'After all these years...'], $data->comments); @@ -2320,7 +2320,7 @@ class core_competency_privacy_testcase extends provider_testcase { $this->assertEquals('C', $comp['rating']['rating']); $comp = $data->competencies[2]; $this->assertEquals($comp4->get('shortname'), $comp['name']); - $this->assertNull($comp['rating']['rating']); + $this->assertNull($comp['rating']); // This plan is complete. $data = writer::with_context($u1ctx)->get_data(array_merge($path, ["{$p1c->get('name')} ({$p1c->get('id')})"])); diff --git a/enrol/meta/lib.php b/enrol/meta/lib.php index b64817b7140..632860a4bb9 100644 --- a/enrol/meta/lib.php +++ b/enrol/meta/lib.php @@ -120,10 +120,12 @@ class enrol_meta_plugin extends enrol_plugin { require_once("$CFG->dirroot/enrol/meta/locallib.php"); // Support creating multiple at once. - if (is_array($fields['customint1'])) { + if (isset($fields['customint1']) && is_array($fields['customint1'])) { $courses = array_unique($fields['customint1']); - } else { + } else if (isset($fields['customint1'])) { $courses = array($fields['customint1']); + } else { + $courses = array(null); // Strange? Yes, but that's how it's working or instance is not created ever. } foreach ($courses as $courseid) { if (!empty($fields['customint2']) && $fields['customint2'] == ENROL_META_CREATE_GROUP) { diff --git a/lib/editor/tests/fixtures/editor_form.php b/lib/editor/tests/fixtures/editor_form.php index 645728ad919..cc81222b01b 100644 --- a/lib/editor/tests/fixtures/editor_form.php +++ b/lib/editor/tests/fixtures/editor_form.php @@ -44,7 +44,7 @@ class editor_form extends moodleform { */ protected function definition() { $mform = $this->_form; - $editoroptions = $this->_customdata['editoroptions']; + $editoroptions = $this->_customdata['editoroptions'] ?? null; // Add header. $mform->addElement('header', 'myheader', 'Editor in Moodle form'); diff --git a/lib/filestorage/file_system.php b/lib/filestorage/file_system.php index ae1d3621d1c..8cf9ffcdc6d 100644 --- a/lib/filestorage/file_system.php +++ b/lib/filestorage/file_system.php @@ -416,11 +416,16 @@ abstract class file_system { protected function get_imageinfo_from_path($path) { $imageinfo = getimagesize($path); + if (!is_array($imageinfo)) { + return false; // Nothing to process, the file was not recognised as image by GD. + } + $image = array( 'width' => $imageinfo[0], 'height' => $imageinfo[1], 'mimetype' => image_type_to_mime_type($imageinfo[2]), ); + if (empty($image['width']) or empty($image['height']) or empty($image['mimetype'])) { // GD can not parse it, sorry. return false; diff --git a/lib/form/duration.php b/lib/form/duration.php index d1c8eee40aa..91ed7686450 100644 --- a/lib/form/duration.php +++ b/lib/form/duration.php @@ -200,7 +200,7 @@ class MoodleQuickForm_duration extends MoodleQuickForm_group { break; case 'createElement': - if ($arg[2]['optional']) { + if (!empty($arg[2]['optional'])) { $caller->disabledIf($arg[0], $arg[0] . '[enabled]'); } $caller->setType($arg[0] . '[number]', PARAM_FLOAT); diff --git a/lib/form/filetypes.php b/lib/form/filetypes.php index 9e4e3b06a53..9c70b5a8ff5 100644 --- a/lib/form/filetypes.php +++ b/lib/form/filetypes.php @@ -221,6 +221,8 @@ class MoodleQuickForm_filetypes extends MoodleQuickForm_group { */ public function validateSubmitValue($value) { + $value = $value ?? ['filetypes' => null]; // A null $value can arrive here. Coalesce, creating the default array. + if (!$this->allowall) { // Assert that there is an actual list provided. $normalized = $this->util->normalize_file_types($value['filetypes']); diff --git a/lib/tests/session_manager_test.php b/lib/tests/session_manager_test.php index 0c715523634..bbafd02aa10 100644 --- a/lib/tests/session_manager_test.php +++ b/lib/tests/session_manager_test.php @@ -835,7 +835,7 @@ class core_session_manager_testcase extends advanced_testcase { $SESSION->recentsessionlocks = $this->sessionlock_history(); $page = \core\session\manager::get_locked_page_at($time); - $this->assertEquals($url, $page['url']); + $this->assertEquals($url, is_array($page) ? $page['url'] : null); } /** diff --git a/mod/glossary/import_form.php b/mod/glossary/import_form.php index 2f37a149e4d..22255c1a1c7 100644 --- a/mod/glossary/import_form.php +++ b/mod/glossary/import_form.php @@ -10,7 +10,7 @@ class mod_glossary_import_form extends moodleform { function definition() { global $CFG; $mform =& $this->_form; - $cmid = $this->_customdata['id']; + $cmid = $this->_customdata['id'] ?? null; $mform->addElement('filepicker', 'file', get_string('filetoimport', 'glossary')); $mform->addHelpButton('file', 'filetoimport', 'glossary'); diff --git a/mod/wiki/comments_form.php b/mod/wiki/comments_form.php index fe53834d317..c8769f9379f 100644 --- a/mod/wiki/comments_form.php +++ b/mod/wiki/comments_form.php @@ -10,8 +10,8 @@ class mod_wiki_comments_form extends moodleform { protected function definition() { $mform = $this->_form; - $current = $this->_customdata['current']; - $commentoptions = $this->_customdata['commentoptions']; + $current = $this->_customdata['current'] ?? null; + $commentoptions = $this->_customdata['commentoptions'] ?? null; // visible elements $mform->addElement('editor', 'entrycomment_editor', get_string('comment', 'glossary'), null, $commentoptions); diff --git a/mod/wiki/parser/parser.php b/mod/wiki/parser/parser.php index 07a8df5c72d..9616e3b19f5 100644 --- a/mod/wiki/parser/parser.php +++ b/mod/wiki/parser/parser.php @@ -47,7 +47,7 @@ class wiki_parser_proxy { return $content; } else { - return $content[1]; + return is_array($content) ? $content[1] : null; } } else { diff --git a/question/engine/questionusage.php b/question/engine/questionusage.php index 606c30b12bb..061df4a3024 100644 --- a/question/engine/questionusage.php +++ b/question/engine/questionusage.php @@ -701,7 +701,7 @@ class question_usage_by_activity { // Behaviour vars should not be processed by question type, just add prefix. $behaviourvars = $this->get_question_attempt($slot)->get_behaviour()->get_expected_data(); foreach (array_keys($responsedata) as $responsedatakey) { - if ($responsedatakey[0] === '-') { + if (is_string($responsedatakey) && $responsedatakey[0] === '-') { $behaviourvarname = substr($responsedatakey, 1); if (isset($behaviourvars[$behaviourvarname])) { // Expected behaviour var found. diff --git a/repository/dropbox/classes/dropbox.php b/repository/dropbox/classes/dropbox.php index 81724b5c14c..dceee7d6b50 100644 --- a/repository/dropbox/classes/dropbox.php +++ b/repository/dropbox/classes/dropbox.php @@ -181,7 +181,7 @@ class dropbox extends \oauth2_client { * @throws moodle_exception */ protected function check_and_handle_api_errors($data) { - if ($this->info['http_code'] == 200) { + if (!is_array($this->info) or $this->info['http_code'] == 200) { // Dropbox only returns errors on non-200 response codes. return; }