Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrading from PHP 5 to PHP 7 #2102

Closed
grctest opened this issue Sep 9, 2017 · 6 comments · Fixed by #2971
Closed

Upgrading from PHP 5 to PHP 7 #2102

grctest opened this issue Sep 9, 2017 · 6 comments · Fixed by #2971

Comments

@grctest
Copy link
Contributor

grctest commented Sep 9, 2017

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor

brevilo commented Jan 18, 2019

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
Copy link
Member

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

@ChristianBeer ChristianBeer reopened this Jan 19, 2019
@ChristianBeer
Copy link
Member

Regarding func_get_arg() in

for ($i=1; $i<func_num_args(); $i++){
$replacements["%".$i] = func_get_arg($i);
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

boinc/html/inc/result.inc

Lines 506 to 514 in 2a58c1f

case "0xC0000005": return "STATUS_ACCESS_VIOLATION";
case "0xC000001D": return "STATUS_ILLEGAL_INSTRUCTION";
case "0xC0000094": return "STATUS_INTEGER_DIVIDE_BY_ZERO";
case "0xC00000FD": return "STATUS_STACK_OVERFLOW";
case "0xC0000135": return "STATUS_DLL_NOT_FOUND";
case "0xC0000139": return "STATUS_ENTRYPOINT_NOT_FOUND";
case "0xC0000142": return "STATUS_DLL_INIT_FAILED";
case "0xC000026B": return "STATUS_DLL_INIT_FAILED_LOGOFF";
case "0xC0000409": return "STATUS_STACK_BUFFER_OVERRUN";
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

if(function_exists('curl_init') && !ini_get('safe_mode') && !ini_get('open_basedir')) {
as is to be backwards compatible. In PHP7 the statement returns false because safe_mode doesn't exist anymore.

@brevilo
Copy link
Contributor

brevilo commented Jan 21, 2019

Nice one! Good to see these addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants