From 814c80da137c377a45acb643ccd7e237009a69c2 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Mon, 16 Aug 2021 19:35:49 +0200 Subject: [PATCH 1/2] MDL-72312 GHA: Force use of igbinary working version And also remove memcached and redis that have dependencies on it. Versions 3.2.2 - 3.2.4 have a bug returning arrays with index messed up. This affects key() and next() operations when reading information serialised with igbinary. Notably Moodle's MUC uses igbinary when available and was leading to some wrong operations and test failures. Once a fixed version is used by GHA this can be reverted. See MDL-72399. Note this only affects to php72, newer php versions are not affected. --- .github/workflows/push.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 52e9bf70a41..697ceb6d20a 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -55,9 +55,16 @@ jobs: include: - os: ubuntu-18.04 php: 7.2 + # Need to disable both redis and memcached (both are optional) in order to keep igbinary installed. + # (there are dependencies that become broken if we just switch the igbinary version). + # Need to do this for php 7.2 because of a bug with current default igbinary + # versions used by GHA being >= 3.2.2 and <= 3.2.4 (buggy ones). + # TODO: MDL-72399 - Revert this patch once igbinary 3.2.5 or up is the default @ GHA. + extensions: :redis, :memcached, igbinary-3.2.6 db: mysqli - os: ubuntu-18.04 php: 7.4 + extensions: db: pgsql steps: @@ -88,6 +95,7 @@ jobs: uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php }} + extensions: ${{ matrix.extensions }} coverage: none - name: Checking out code from ${{ env.GITHUB_REF_SLUG }} From a4e45e8159ae7714cf3134a5a1d9e36e8c0d67b8 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Tue, 17 Aug 2021 21:21:25 +0200 Subject: [PATCH 2/2] MDL-72312 environment: Add new optional custom check for igbinary This optional environmental check will look if: - the site is running php72. - the site has igbinary extension enabled. - the igbinary extension version is a buggy one >=3.2.2 <= 3.2.4 - the bug is reproducible. And will warn if all the conditions are met. --- admin/environment.xml | 10 ++++++++ lang/en/admin.php | 1 + lib/upgradelib.php | 53 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+) diff --git a/admin/environment.xml b/admin/environment.xml index bbb6e4671cf..6faa7ded027 100644 --- a/admin/environment.xml +++ b/admin/environment.xml @@ -3163,6 +3163,11 @@ + + + + + @@ -3345,6 +3350,11 @@ + + + + + diff --git a/lang/en/admin.php b/lang/en/admin.php index 40d2e09bf77..833b5ca2fc9 100644 --- a/lang/en/admin.php +++ b/lang/en/admin.php @@ -659,6 +659,7 @@ $string['checkboxyes'] = 'Yes'; $string['choosefiletoedit'] = 'Choose file to edit'; $string['h5pgetcontenttypestask'] = 'Download available H5P content types from h5p.org'; $string['iconvrequired'] = 'Installing ICONV extension is required.'; +$string['igbinary322buggyversion'] = 'The php-igbinary extension installed on the site can lead to problems when running with PHP 7.2. You are recommended to either upgrade to php-igbinary 3.2.5 or later, or alternatively to upgrade to PHP 7.3 or later.'; $string['ignore'] = 'Ignore'; $string['includemoduleuserdata'] = 'Include module user data'; $string['incompatibleblocks'] = 'Incompatible blocks'; diff --git a/lib/upgradelib.php b/lib/upgradelib.php index 5b8445b14e1..422d1e96264 100644 --- a/lib/upgradelib.php +++ b/lib/upgradelib.php @@ -2422,6 +2422,59 @@ function check_sixtyfour_bits(environment_results $result) { return null; } +/** + * Check if the igbinary extension installed is buggy one + * + * There are a few php-igbinary versions that are buggy and + * return any unserialised array with wrong index. This defeats + * key() and next() operations on them. + * + * This library is used by MUC and also by memcached and redis + * when available. + * + * Let's inform if there is some problem when: + * - php 7.2 is being used (php 7.3 and up are immune). + * - the igbinary extension is installed. + * - the version of the extension is between 3.2.2 and 3.2.4. + * - the buggy behaviour is reproduced. + * + * @param environment_results $result object to update, if relevant. + * @return environment_results|null updated results or null. + */ +function check_igbinary322_version(environment_results $result) { + + // No problem if using PHP version 7.3 and up. + $phpversion = normalize_version(phpversion()); + if (version_compare($phpversion, '7.3', '>=')) { + return null; + } + + // No problem if igbinary is not installed.. + if (!function_exists('igbinary_serialize')) { + return null; + } + + // No problem if using igbinary < 3.2.2 or > 3.2.4. + $igbinaryversion = normalize_version(phpversion('igbinary')); + if (version_compare($igbinaryversion, '3.2.2', '<') or version_compare($igbinaryversion, '3.2.4', '>')) { + return null; + } + + // Let's verify the real behaviour to see if the bug is around. + // Note that we need this extra check because they released 3.2.5 with 3.2.4 version number, so + // over the paper, there are 3.2.4 working versions (3.2.5 ones with messed reflection version). + $data = [1, 2, 3]; + $data = igbinary_unserialize(igbinary_serialize($data)); + if (key($data) === 0) { + return null; + } + + // Arrived here, we are using PHP 7.2 and a buggy verified igbinary version, let's inform and don't allow to continue. + $result->setInfo('igbinary version problem'); + $result->setStatus(false); + return $result; +} + /** * Assert the upgrade key is provided, if it is defined. *