Update codebase for php 7.3 and php 7.4

Bug #1839411 reported by Rebecca Blundell
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
Wishlist
Rebecca Blundell

Bug Description

I tried running behat tests in php 7.3 and among other issues got the following error due to a PHP 7.3 deprecation:

/code/mahara/test/behat/features/site_features/smart_evidence_editor.feature:8
        8192: strpos(): Non-string needles will be interpreted as strings in the future. Use an explicit chr() call to preserve the current behavior in ~/code/mahara/htdocs/lib/mahara.php line 1620

This made me think we should check the code for this and other 7.3 issues. Notes on migrating to 7.3:
https://www.php.net/manual/en/migration73.php

And on migrating to 7.4:
https://www.php.net/manual/en/migration74.php

Changed in mahara:
status: New → Confirmed
importance: Undecided → Wishlist
summary: - Update codebase for php 7.3
+ Update codebase for php 7.3 and php 7.4
Changed in mahara:
assignee: nobody → Rebecca Blundell (rjb-dev)
description: updated
Revision history for this message
Rebecca Blundell (rjb-dev) wrote :

Notes after reading documentation on what might affect Mahara. I have included a note about anything here that has changed that we use in the code, regardless of whether a change is needed. Many of these are in the category of 'check this to make sure' rather than 'we have to make changes'. Some things are hard to check for issues with, so it may be necessary to upgrade and see what breaks.

PHP 7.3

continue in switch statements behaves as break. This is not new, but the correct usage is break or continue2 depending on what you want.

some differences with how doc strings work, implicit conversion of array object "123" to 123 won't happen; static props can't be separated from the parent class; Assigning by reference happens on access;

getimagesize() now report the mime type of BMP images as image/bmp instead of image/x-ms-bmp (do we rely on this behaviour anywhere?)

simplexml uses float as well as int now, implicitally. (Any probs with this?)

 libcurl ≥ 7.15.5 is now required.

https://www.php.net/manual/en/migration73.incompatible.php

7.4

Strong typing is supported.
checkout str_split - possibly better to use mb_str_split in 7.4??(but not backwards, so no.)

PDO stuff important to know.

strip_tags() easier

check fread() for tests of ''. Now a non-read will be false (7.4)
in cli.php/ file.php ...

Check for comparisons on DateTime objects

Check usage of get_object_vars(array) for use to get values.

Check for non-bracketed nested ternary operators

adodb has calls to deprecated functions get_magic_quotes_gpc() and get_magic_quotes_runtime(). Look for update
Also calls fgetcsv which has changed function.

Passing parameters to implode() in reverse order is deprecated, use implode($glue, $parts) instead of implode($parts, $glue). Fix this.

Check behaviour of parse_data in csvfile.php (change to funct fgetcsv())

Check behaviour of imagescale() in skin.php (Change to funct)

check libxml version (needs to be at least 2.7.6)

Do we need PEAR for stuff (I think so.) Check what happens on install. We may need to enable it. (It's no longer enabled by default).

Changes to constants in ReflectionClass may affect $refClass in errors.php
Changes to constants in ReflectionFunction may affect $refFunc in errors.php

Elasticsearch and Dwoo also rely on ReflectionFunction - check for update.

Changes to constants in ReflectionMethod may affect Dwoo, Elasticsearch and htmlpurifier - check for update

SImpleXMLElement implements Countable now - check if that means any new errors/usage changes

Does removal of libzip have any flow-on effects (e.g. need to include another syslib in instructions?)

Revision history for this message
Mahara Bot (dev-mahara) wrote : A patch has been submitted for review

Patch for "master" branch: https://reviews.mahara.org/10711

Changed in mahara:
milestone: none → 20.04rc2
Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/10711
Committed: https://git.mahara.org/mahara/mahara/commit/4263848b4e4cc87a57d8719123b7a363bcbebf45
Submitter: Cecilia Vela Gurovic (<email address hidden>)
Branch: master

commit 4263848b4e4cc87a57d8719123b7a363bcbebf45
Author: Rebecca Blundell <email address hidden>
Date: Mon Jan 13 16:19:13 2020 +1300

Bug 1839411: Update Mahara to work with PHP7.3/7.4

Fixed strpos error picked up by 7.3
I haven't found any other 7.3 issues and none have been caught by behat

For 7.4:
-Fixed implode param order (glue, parts)
Note that Mink has a reversed implode not fixed upstream:
external/vendor/behat/mink/src/Selector/Xpath/Escaper.php:50
I have added a function to copy the correct code over in the interim
I also pushed a change to an HTMLPurifier file as it is not fixed upstream
-Changed a variable read by fread to allow for false return type
-Calling a non-array as an array now causes an error, meaning we need to
confirm an array value before attempting to use a variable. An obvious
case is in multi-record db calls the return value is either an array of
results or false. E.g.
$array = db_call();//returns false as no results
//do sth with result
$var = $array[0]//error, trying to access type false as array
//instead we need to check the value before accessing:
$var = !empty($array[0]) ? $array[0] : false //or some equivalent
check of the db_call's return
Because of this change, I have attempted to check all the multi-db
calls for possible false results and include a check for that if there
wasn't already one.
-In conjunction with the previous work, I noticed that trying to use
foreach on a non-iterable result causes an error and that array
functions (i.e. array_keys()) called on a non-array also cause an
error. Where I found them I added checks as well.
-A change in PHP7.2 was that items counted with count() must implement
Countable. (i.e, int/bool is not OK). I fixed the ones I found.
-I used the regex \{\$?\d?[^\s]\} to search for array access using {}
instead of [] and changed those. (Note: the regex pulls in a lot more)
CSStidy has a lot of these that aren't fixed, but those are covered by
the CSStidy upgrade (Bug 1840099)

Change-Id: I64a8feb821433ecd99463762a9999449c50ee32e

Revision history for this message
Mahara Bot (dev-mahara) wrote : A patch has been submitted for review

Patch for "20.04_STABLE" branch: https://reviews.mahara.org/10929

Revision history for this message
Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/10929
Committed: https://git.mahara.org/mahara/mahara/commit/01eed8e415421e4fd4c8b1976a1d1669287d3305
Submitter: Cecilia Vela Gurovic (<email address hidden>)
Branch: 20.04_STABLE

commit 01eed8e415421e4fd4c8b1976a1d1669287d3305
Author: Rebecca Blundell <email address hidden>
Date: Mon Jan 13 16:19:13 2020 +1300

Bug 1839411: Update Mahara to work with PHP7.3/7.4

Fixed strpos error picked up by 7.3
I haven't found any other 7.3 issues and none have been caught by behat

For 7.4:
-Fixed implode param order (glue, parts)
Note that Mink has a reversed implode not fixed upstream:
external/vendor/behat/mink/src/Selector/Xpath/Escaper.php:50
I have added a function to copy the correct code over in the interim
I also pushed a change to an HTMLPurifier file as it is not fixed upstream
-Changed a variable read by fread to allow for false return type
-Calling a non-array as an array now causes an error, meaning we need to
confirm an array value before attempting to use a variable. An obvious
case is in multi-record db calls the return value is either an array of
results or false. E.g.
$array = db_call();//returns false as no results
//do sth with result
$var = $array[0]//error, trying to access type false as array
//instead we need to check the value before accessing:
$var = !empty($array[0]) ? $array[0] : false //or some equivalent
check of the db_call's return
Because of this change, I have attempted to check all the multi-db
calls for possible false results and include a check for that if there
wasn't already one.
-In conjunction with the previous work, I noticed that trying to use
foreach on a non-iterable result causes an error and that array
functions (i.e. array_keys()) called on a non-array also cause an
error. Where I found them I added checks as well.
-A change in PHP7.2 was that items counted with count() must implement
Countable. (i.e, int/bool is not OK). I fixed the ones I found.
-I used the regex \{\$?\d?[^\s]\} to search for array access using {}
instead of [] and changed those. (Note: the regex pulls in a lot more)
CSStidy has a lot of these that aren't fixed, but those are covered by
the CSStidy upgrade (Bug 1840099)

Change-Id: I64a8feb821433ecd99463762a9999449c50ee32e
(cherry picked from commit 4263848b4e4cc87a57d8719123b7a363bcbebf45)

Changed in mahara:
status: Confirmed → Fix Committed
Robert Lyon (robertl-9)
Changed in mahara:
milestone: 20.04rc2 → 20.04.0
Revision history for this message
Mahara Bot (dev-mahara) wrote :

Patch for "19.10_STABLE" branch: https://reviews.mahara.org/10930

Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/10930
Committed: https://git.mahara.org/mahara/mahara/commit/81061243204a63c66fd558deffef433c8a1e8b01
Submitter: Robert Lyon (<email address hidden>)
Branch: 19.10_STABLE

commit 81061243204a63c66fd558deffef433c8a1e8b01
Author: Robert Lyon <email address hidden>
Date: Fri Apr 24 16:46:49 2020 +1200

Bug 1839411: Update Mahara to work with PHP7.3

Fixed strpos error picked up by 7.3
I haven't found any other 7.3 issues and none have been caught by behat

Change-Id: Icfea5fa01fa854ca0d49bd977b4ad507497b4b02
Signed-off-by: Robert Lyon <email address hidden>

Robert Lyon (robertl-9)
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.