search

BOINC / boinc

Open-source software for volunteer computing and grid computing.
https://boinc.berkeley.edu
GNU Lesser General Public License v3.0
2.04k stars 449 forks source link

Upgrading from PHP 5 to PHP 7 #2102

Closed grctest closed 5 years ago

grctest commented 7 years ago

Differences between PHP 5 & 7 Removed depreciated PHP functionality in v7

If we want to implement a secure password hashing mechanism such as Bcrypt or Argon2i (instead of md5), I believe that we need to upgrade to the latest version (or at least v7) of PHP for native support.

How much of BOINC would break and how much work would be required to uplift BOINC to PHP v7+?

It would also require the drupal implementation to be uplifted, since it depends on the same PHP version currently.

ChristianBeer commented 7 years ago

IMHO the current code in master can already be used with PHP7. At least 2 projects that I know of already use PHP7 and didn't report problems.

marius311 commented 6 years ago

boinc-server-docker has PHP 7 as of about a week ago and has been running at Cosmology@Home with no modifications to any PHP file and no errors spotted. I'd say its safe to say we support PHP 7 and close this, and can open other issues if we spot specific problems.

brevilo commented 5 years ago

Not sure how representative those two projects are for the BOINC repo as a whole. I ran a compatibility check and got this (false-positives related to mysql already removed):

FILE: html/inc/translation.inc
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 196 | ERROR | Since PHP 7.0, functions inspecting arguments, like func_get_arg(), no longer report the original value as passed to a parameter, but will instead provide the current value. The parameter "$text" was changed on line 184.
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

FILE: html/inc/bolt_ex.inc
---------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------
 326 | ERROR | Extension 'ereg' is deprecated since PHP 5.3 and removed since PHP 7.0; Use pcre instead
 326 | ERROR | Function ereg() is deprecated since PHP 5.3 and removed since PHP 7.0; Use preg_match() instead
---------------------------------------------------------------------------------------------------------------

FILE: html/inc/result.inc
-------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 9 ERRORS AFFECTING 9 LINES
-------------------------------------------------------------------------------------------------------------------------------------------------------
 506 | ERROR | The behaviour of hexadecimal numeric strings was inconsistent prior to PHP 7 and support has been removed in PHP 7. Found: "0xC0000005"
 507 | ERROR | The behaviour of hexadecimal numeric strings was inconsistent prior to PHP 7 and support has been removed in PHP 7. Found: "0xC000001D"
 508 | ERROR | The behaviour of hexadecimal numeric strings was inconsistent prior to PHP 7 and support has been removed in PHP 7. Found: "0xC0000094"
 509 | ERROR | The behaviour of hexadecimal numeric strings was inconsistent prior to PHP 7 and support has been removed in PHP 7. Found: "0xC00000FD"
 510 | ERROR | The behaviour of hexadecimal numeric strings was inconsistent prior to PHP 7 and support has been removed in PHP 7. Found: "0xC0000135"
 511 | ERROR | The behaviour of hexadecimal numeric strings was inconsistent prior to PHP 7 and support has been removed in PHP 7. Found: "0xC0000139"
 512 | ERROR | The behaviour of hexadecimal numeric strings was inconsistent prior to PHP 7 and support has been removed in PHP 7. Found: "0xC0000142"
 513 | ERROR | The behaviour of hexadecimal numeric strings was inconsistent prior to PHP 7 and support has been removed in PHP 7. Found: "0xC000026B"
 514 | ERROR | The behaviour of hexadecimal numeric strings was inconsistent prior to PHP 7 and support has been removed in PHP 7. Found: "0xC0000409"
-------------------------------------------------------------------------------------------------------------------------------------------------------

FILE: html/inc/geoip.inc
---------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------
 490 | ERROR | Extension 'ereg' is deprecated since PHP 5.3 and removed since PHP 7.0; Use pcre instead
 490 | ERROR | Function ereg() is deprecated since PHP 5.3 and removed since PHP 7.0; Use preg_match() instead
---------------------------------------------------------------------------------------------------------------

FILE: html/user/bolt_sched.php
-------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------
 329 | ERROR | Using 'break' outside of a loop or switch structure is invalid and will throw a fatal error since PHP 7.0
-------------------------------------------------------------------------------------------------------------------------

FILE: html/user/openid.php
-------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------
 290 | WARNING | INI directive 'safe_mode' is deprecated since PHP 5.3 and removed since PHP 5.4
-------------------------------------------------------------------------------------------------

FILE: html/ops/mass_email_script.php
------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
------------------------------------------------------------------------------------------------------------
 270 | ERROR | Using a call-time pass-by-reference is deprecated since PHP 5.3 and prohibited since PHP 5.4
 296 | ERROR | Using a call-time pass-by-reference is deprecated since PHP 5.3 and prohibited since PHP 5.4
------------------------------------------------------------------------------------------------------------

Nothing too critical apart from html/inc/result.inc maybe but I reckon these known issues should be fixed. I propose to reopen the issue. Do you agree @ChristianBeer ?

ChristianBeer commented 5 years ago

I'm looking into this and probably will provide a PR.

ChristianBeer commented 5 years ago

Regarding func_get_arg() in https://github.com/BOINC/boinc/blob/2a58c1f8c50f2cd847e997fe780bdb4cf10c46b8/html/inc/translation.inc#L195-L196 this is a false positive as we skip over the first argument ($text) and only look at the others which don't get changed before.

The strings in https://github.com/BOINC/boinc/blob/2a58c1f8c50f2cd847e997fe780bdb4cf10c46b8/html/inc/result.inc#L506-L514 were always ment to be strings which with PHP7 this is now the case. At the time I wrote this code I wasn't aware that PHP5 interpreted them differently. But even then this wasn't a problem as the switch construct is doing an exact comparison and there is no room for a wrong interpretation.

The GeoIP part should be updated to a newer version available at https://github.com/maxmind/geoip-api-php or a new API version available at: https://github.com/maxmind/GeoIP2-php. I consider this outside of this issue as one would need to check how BOINC uses the existing API and also migrate over to a new API.

I'm leaving the !ini_get('safe_mode') statement in https://github.com/BOINC/boinc/blob/2a58c1f8c50f2cd847e997fe780bdb4cf10c46b8/html/user/openid.php#L290 as is to be backwards compatible. In PHP7 the statement returns false because safe_mode doesn't exist anymore.

brevilo commented 5 years ago

Nice one! Good to see these addressed.