search

codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.35k stars 1.9k forks source link

Bug: `is_cli()` returns `true` on Apache #5336

Closed kenjis closed 2 years ago

kenjis commented 2 years ago

PHP Version

7.4

CodeIgniter4 Version

4.1.5

Which operating systems have you tested for this bug?

Linux

Which server did you use?

apache

Database

No response

What happened?

is_cli() returns true after upgrade to 4.1.5 from 4.0.

Steps to Reproduce

Navigate to the site.

Expected Output

is_cli() returns false.

Anything else?

From https://forum.codeigniter.com/thread-80530-post-391559.html

The environment is:

vlakoff commented 2 years ago

I'd suggest the following code, inspired from Laravel:

function is_cli(): bool
{
    return \PHP_SAPI === 'cli' || \PHP_SAPI === 'phpdbg';
}

Refs:

vlakoff commented 2 years ago

It's also what Symfony does, see in VarDumper.php:

\in_array(\PHP_SAPI, ['cli', 'phpdbg'], true)

I investigated a bit, and it seems it would be useless to keep the if (defined('STDIN')) in addition.

kenjis commented 2 years ago

CI3:

return (PHP_SAPI === 'cli' OR defined('STDIN'));

https://github.com/bcit-ci/CodeIgniter/blob/develop/system/core/Common.php#L378-L382

lonnieezell commented 2 years ago

PHPUnit also uses the phpdbg check, so that's probably a good addition.

paulbalandan commented 2 years ago

I did a bit of pondering and I think the compound isset should be split to 2 isset calls:

-! isset($_SERVER['REMOTE_ADDR'], $_SERVER['HTTP_USER_AGENT'])
+! isset($_SERVER['REMOTE_ADDR']) && ! isset($_SERVER['HTTP_USER_AGENT'])

In the original, the two server variables are evaluated as a single condition, that's why when one fails, the whole condition is true. They should be checked individually.

If multiple parameters are supplied then isset() will return true only if all of the parameters are considered set.

! isset($_SERVER['REMOTE_ADDR'], $_SERVER['HTTP_USER_AGENT']);
// ! isset(true, false)
// ! false
// true # unintended

! isset($_SERVER['REMOTE_ADDR']) && ! isset($_SERVER['HTTP_USER_AGENT']);
// ! true && ! false
// false && true
// false # intended
vlakoff commented 2 years ago

It's absolutely possible for $_SERVER['HTTP_USER_AGENT'] to be missing because the user agent omitted it.

At the very most it could be considered a hint, but we definitely can't rely on it.

kenjis commented 2 years ago

I sent a PR #5393.