amphp / process

An async process dispatcher for Amp.
MIT License
233 stars 30 forks source link

Report a better error message when proc_open is disallowed via disabl… #27

Closed staabm closed 5 years ago

staabm commented 6 years ago

…e_functions php-ini directive.

Refs #26

staabm commented 6 years ago

Reworded the error message. Maybe its more clear now

kelunik commented 6 years ago
➜ php -d disable_functions=proc_open -r 'proc_open();' 
PHP Warning:  proc_open() has been disabled for security reasons in Command line code on line 1

~ via 🐘 v7.2.5 
➜ php -d disabled_functions=proc_open -r 'var_dump(function_exists("proc_open"));'
bool(true)

Did the behavior change between PHP versions or why do we need that check at all? The warning is already reported to the user as can be noticed in #26.

staabm commented 6 years ago

As you can see in the stacktrace in https://github.com/amphp/process/issues/26#issue-318401472 you only see that the process wasnt able to start, but you dont see why.

kelunik commented 6 years ago

Which version was that?

bwoebi commented 6 years ago

Why should error_get_last() not give you the proper error message? That's confusing me...

php -d disable_functions=proc_open -r '@proc_open(); var_dump(error_get_last());'
array(4) {
  ["type"]=>
  int(2)
  ["message"]=>
  string(50) "proc_open() has been disabled for security reasons"
  ["file"]=>
  string(17) "Command line code"
  ["line"]=>
  int(1)
}
staabm commented 6 years ago

Maybe its related to some other extensions beeing loaded (blackfire, xdebug or similar).

I will be back to the VM which reproduced this problem at least in 2 weeks

kelunik commented 5 years ago

@staabm: Any change to get it reproduced again? Otherwise I'll close this one.