codeigniter4 / CodeIgniter4

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

Bug: Constant STDOUT already defined #7047

Closed jozefrebjak closed 1 year ago

jozefrebjak commented 1 year ago

PHP Version

8.1

CodeIgniter4 Version

4.2.11

CodeIgniter4 Installation Method

Composer (as dependency to an existing project)

Which operating systems have you tested for this bug?

macOS

Which server did you use?

cli

Database

MariaDB 10.9.3

What happened?

When I trying to call command from Controller with params I'm getting

ErrorException
Constant STDOUT already defined

Here is the screenshot:

CleanShot 2023-01-03 at 13 05 49@2x

Steps to Reproduce

Controller method includes line:

command('command:command')

Command includes line:

$isId = (bool) CLI::getOption('i');

where it's failing.

Expected Output

Execute command.

Anything else?

I tried to look at method in vendor/codeigniter4/framework/system/CLI/CLI.php and what I found is when I delete define('STDOUT', 'php://output'); // @codeCoverageIgnore then it's working as expected.

Do we need to define it in else statement ? Or I'm calling command wrong.

/**
     * Static "constructor".
     */
    public static function init()
    {
        if (is_cli()) {
            // Readline is an extension for PHP that makes interactivity with PHP
            // much more bash-like.
            // http://www.php.net/manual/en/readline.installation.php
            static::$readline_support = extension_loaded('readline');

            // clear segments & options to keep testing clean
            static::$segments = [];
            static::$options  = [];

            // Check our stream resource for color support
            static::$isColored = static::hasColorSupport(STDOUT);

            static::parseCommandLine();

            static::$initialized = true;
        } else {
            // If the command is being called from a controller
            // we need to define STDOUT ourselves
-          define('STDOUT', 'php://output'); // @codeCoverageIgnore
        }
    }
kenjis commented 1 year ago

The CLI SAPI defines STDOUT. https://www.php.net/manual/en/features.commandline.io-streams.php

So if you use PHP via web, the constant is not defined. Why is STDOUT defined in your environment?

jozefrebjak commented 1 year ago

@kenjis I added in vendor/codeigniter4/framework/system/CLI/CLI.php

    /**
     * Static "constructor".
     */
    public static function init()
    {
+        if (defined('STDOUT')) {
+            dd(STDOUT);
+       }
+        else {
+            dd('STDOUT not defined');
+       }

        if (is_cli()) {
            // Readline is an extension for PHP that makes interactivity with PHP
            // much more bash-like.
            // http://www.php.net/manual/en/readline.installation.php
            static::$readline_support = extension_loaded('readline');

            // clear segments & options to keep testing clean
            static::$segments = [];
            static::$options  = [];

            // Check our stream resource for color support
            static::$isColored = static::hasColorSupport(STDOUT);

            static::parseCommandLine();

            static::$initialized = true;
        } else {
            // If the command is being called from a controller
            // we need to define STDOUT ourselves
            define('STDOUT', 'php://output'); // @codeCoverageIgnore
        }
    }

too see how is STDOUT defined and it returns

CleanShot 2023-01-05 at 08 56 20@2x

and it seems it comes from psalm required package amphp/byte-stream.

So, I removed "codeigniter4/devkit": "^1.1.0", from composer.json and now it returns:

STDOUT not defined

So here is the problem.

We need to improve init() method to define define('STDOUT', 'php://output'); // @codeCoverageIgnore even if there is STDOUT defined from other package to the correct one.

What do you think ?

After removing all of the test code it's working as expected. Now I even don't need to check if some CLI method comes from CLI like

            if (is_cli()) {
                CLI::write(CLI::color('Something to update: ', 'white') . $something->something);
                CLI::newLine();
            }

and run commands from Controller.

Note:

I believe that this problem occurs only in dev environment, where we have dev tools installed. If we run code in production where are dependencies installed with --no-dev composer flag, then it should work and exception is not raised.

MGatner commented 1 year ago

We should make that elseif (! defined('STDOUT')), especially if third-party dependencies may be grabbing the output stream already.