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

Commands Are Manipulated Incorrectly #408

Open
manuth opened this issue Apr 17, 2022 · 0 comments
Open

Commands Are Manipulated Incorrectly #408

manuth opened this issue Apr 17, 2022 · 0 comments

Comments

@manuth
Copy link

manuth commented Apr 17, 2022

This is a follow-up of #401, #406 and #407 as all of them are symptoms of the very same issue.

Description

Commands which aren not intended to be visible (such as the TestCommand and commands in the commands.hidden setting) are visible in the production environment.

Furthermore, lazy loaded commands aren't scheduled by laravel-zero as pointed out in #406.

Steps to reproduce

  1. Create a laravel-zero/laravel-zero with laravel/framework v9.1.1 in it.
  2. Make sure that the hidden-setting in config/commands.php contains at least one command
  3. Build the application php ./application app:build
  4. Run the application php ./builds/application
  5. Take note that all commands - even the hidden ones and the TestCommand - are listed
  6. Also - take note, that some commands aren't scheduled

Actual Behavior

Test- and Development commands are visible even when executing the command in production environment.
Commands should be scheduled.

Expected Behavior

Commands which are supposed to be used for testing and development should not be visible.
Some commands aren't scheduled.

Workaround

Add a customized kernel class which inherits LaravelZero\Framework\Kernel with the following content:
./app/Framework/MyKernel.php:

<?php

namespace App\Framework;

use Illuminate\Console\Application;
use Illuminate\Support\Collection;
use LaravelZero\Framework\Commands\Command as LaravelZeroCommand;
use LaravelZero\Framework\Kernel;
use NunoMaduro\Collision\Adapters\Laravel\Commands\TestCommand;
use ReflectionClass;
use Symfony\Component\Console\Application as SymfonyApplication;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputOption;

{
    /**
     * Provides the functionality to handle the execution of a command line interface.
     */
    class MyKernel extends Kernel
    {
        /**
         * {@inheritdoc}
         */
        public function getArtisan(): Application
        {
            if (is_null($this->artisan))
            {
                $artisan = new Application($this->app, $this->events, $this->app->version());
                $artisan->resolveCommands($this->commands);
                $commandsProperty = (new ReflectionClass(SymfonyApplication::class))->getProperty('commands');
                $commandMapProperty = (new ReflectionClass(Application::class))->getProperty('commandMap');
                /** @var Collection<string, string> */
                $commandMap = collect($commandMapProperty->getValue($artisan));
                /** @var string[] */
                $commands = $commandsProperty->getValue($artisan);

                $toRemove = collect($commandMap)->filter(function (string $commandClass)
                {
                    return $commandClass == TestCommand::class || in_array($commandClass, config('commands.remove'));
                });

                $availableCommands = $commandMap->diff($toRemove);
                $property->setAccessible(true); // Not necessary in PHP >= 8.1
                $commandMapProperty->setValue($artisan, $availableCommands->toArray());
                $property->setAccessible(false); // Not necessary in PHP >= 8.1
                $artisan->setContainerCommandLoader();

                collect($artisan->all())->each(function (Command $command) use ($commands)
                {
                    if (in_array($command::class, config('commands.hidden', []), true))
                    {
                        $command->setHidden(true);
                    }

                    if (
                        $command instanceof LaravelZeroCommand &&
                        !in_array($command::class, $commands)
                    )
                    {
                        $this->app->call([$command, 'schedule']);
                    }
                });

                $this->artisan = $artisan;
            }

            return parent::getArtisan();
        }
    }
}

Next, replace laravel-zeros Kernel with your newly created custom kernel by editing the Kernel singleton in your ./bootstrap/app.php file:

 $app->singleton(
     Illuminate\Contracts\Console\Kernel::class,
-    LaravelZero\Framework\Kernel::class
+    App\Framework\MyKernel::class
 );

After editing the ./bootstrap/app.php file, the Kernel singleton should look like this:

$app->singleton(
    Illuminate\Contracts\Console\Kernel::class,
    App\Framework\MyKernel::class
);

Here's what it does:

  • Create a new Artisan instance
    Don't get confused, the class is called Application in this code snippet
  • Add all commands from the Kernel::$commands array
    This array has been filtered by laravel-zeros Kernel class, so there are no unwanted commands in this array for sure.
  • Remove unwanted commands which are queued for a lazy load in the commandMap
  • Enable lazy loading (using the setContainerCommandLoader() method)
  • Hide commands which are marked for hiding
    This will work now as $artisan->all() returns all commands (even the ones queued for lazy-loading) after enabling lazy loading.
  • Schedule commands which haven't been scheduled before
    Only commands which are present in the Artisan::$commands array are scheduled by laravel-zero. Thus, scheduling all commands which aren't in the array should fix this issue.
  • Store and return tampered artisan instance

My workaround is based on @owenvoke's recommendation pointed out in #401 which can be seen in his repository:
https://github.com/owenvoke/rugby-schedule/blob/bbb78284980294f4dc288ff4da48d44c07c4e3d6/app/Providers/AppServiceProvider.php#L18-L22

More Context

As seen in the laravel/framework-repository, commands aren't added to the Artisan::$commands-array directly but rather are queued in the Artisan::$commandMap for a lazy load:
https://github.com/laravel/framework/blob/6463e1c90d73faf58b57d98f27eb5c3f5264138e/src/Illuminate/Console/Application.php#L270-L274

Then again, in laravel-zero/foundation, the Artisan::starting-bootstrapers are called first (through the Artisan-constructor call, line 330) and the lazy command loader is injected only afterwards (setContainerCommandLoader() at line 332):
https://github.com/laravel-zero/foundation/blob/88f6f9e828294949b6c56f07e9046c8e98eb62c9/src/Illuminate/Foundation/Console/Kernel.php#L329-L333

With the lazy command loader not injected, filtering commands as seen in laravel-zero/framework is not possible:
https://github.com/laravel-zero/framework/blob/e9bdb5e1338c4c874be9cb6c902c5fbc5a58e02e/src/Kernel.php#L183-L197

What's more, as lazy loaded commands aren't even stored in Artisan::$commands but rather in Artisan::$commandMap, lines 192-197 in particular won't work in this point of view at all.

Furthermore, hiding commands as seen in the Kernel class of laravel-zero/framework doesn't work either, as $artisan->all() only contains all commands once the lazy command loader has been injected using setContainerCommandLoader():
https://github.com/laravel-zero/framework/blob/e9bdb5e1338c4c874be9cb6c902c5fbc5a58e02e/src/Kernel.php#L214-L226

As pointed out in issue #406, scheduling doesn't work either cause of this issue (see line 222-224 in previously linked snippet).

A Few Last Words

I just wanted to point out that this is not some sort of a rant at all. I've been looking for some neat way to create PHP CLIs as I want to clean up an existing php project and I'm amazed about this great piece of work!

Thanks for your great work! I deeply hope my research will help to improve this project even further 😄

  • Edit: Added info & workaround for command scheduling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant