-
Notifications
You must be signed in to change notification settings - Fork 30
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
cli detection is fallible #51
Comments
This is an interesting situation; thanks for reporting. I think it's a good point to be cautious about depending on the SAPI. However, that contrib comment on php.net feels sketchy:
I'm particularly struggling to see the use-case/reason/steps-to-reproduce. The whole point of the Common Gateway Interface is to define a gateway between an HTTP daemon and some worker process. The basic model is: pass in an HTTP request; send back an HTTP response. Consider these example commands:
If you run on CLI SAPI, the output is a JSON or CSV document. When I manually ran a script on CGI SAPI, the output was an HTTP response, including headers. You'd have to unpack it to get the desired content. Additionally, tldr: php-cgi is not a drop-replacement for php-cli. For CLI commands, use php-cli. The idea of an override is interesting. Maybe the above is just my lack of imagine. IMHO, it's pretty important (from security POV) to ensure that HTTP users are not allowed to invoke CLI commands. So an override should be a strong representation from the local admin that they really know what they're doing. For example, maybe this: $isCli = (PHP_SAPI === 'cli' || PHP_SAPI === 'phpdbg');
if (!$isCli && file_exists('/etc/cv.json')) {
$_cv_json = json_decode(file_get_contents('/etc/cv.json'));
if (isset($_cv_json['allow_sapi']) && in_array(PHP_SAPI, $_cv_json['allow_sapi']) {
$isCli = TRUE;
}
}
if (!$isCli) error(...); |
In the meanwhile I updated the sysadmin docs with a note about the/a workaround. |
Just adding a note here, that the security aspect is important, so is using the same PHP version as the site - for example using cv to run CiviCRM cron with a different PHP version can cause/trigger issues that would not happen when run via web etc. |
cv does the drupal8 way of detecting cli mode:
cv/bin/cv
Line 4 in 14fe9da
However this fails if it's php-cgi executing the code, since that will always return cgi-fcgi. I made #50, so it at least be obvious. See note in the docs comment. There are about five other instances of this check throughout the code.
Please either expand the detection or add a flag to force ignore it. People have been hacking around it for years: https://civicrm.stackexchange.com/questions/4723/how-do-i-import-contacts-through-the-command-line/4724#4724
In my case it was enough to call php with an absolute path, not let the shell resolve it. So perhaps just further enhancing the error message is enough.
The text was updated successfully, but these errors were encountered: