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

fixed several compatibility issues with windows and php-cgi, see #15 #20

Merged
merged 1 commit into from
Oct 11, 2021

Conversation

ZsgsDesign
Copy link
Contributor

@ZsgsDesign ZsgsDesign commented Oct 11, 2021

This PR fixed #15.

Also, it provides a potential approach to #16, #8, and #9.

The issue is mainly caused by Laravel and Symphony, in Symfony\Component\Process\PhpExecutableFinder, it uses PHP_BINARY to locale the PHP executable:

        // PHP_BINARY return the current sapi executable
        if (\PHP_BINARY && \in_array(\PHP_SAPI, ['cgi-fcgi', 'cli', 'cli-server', 'phpdbg'], true)) {
            return \PHP_BINARY.$args;
        }

Here PHP_BINARY is set during runtime and so did PHP_SAPI. That works fine on normal conditions, but on conditions like running a command from the web interface, problems emerge.

If you are using the web interface, PHP would use its CGI Server API instead of CLI Server API. On some Linux distributions this is fine (they share the same executable), but on Windows they are different, one is php.exe or php-cli.exe, another php-cgi.exe.

Thus PhpExecutableFinder returns wrong PHP executable (php-cgi.exe) to Illuminate\Console\Application, in method formatCommandString and phpBinary:

    /**
     * Determine the proper PHP executable.
     *
     * @return string
     */
    public static function phpBinary()
    {
        return ProcessUtils::escapeArgument((new PhpExecutableFinder)->find(false));
    }

    /**
     * Determine the proper Artisan executable.
     *
     * @return string
     */
    public static function artisanBinary()
    {
        return defined('ARTISAN_BINARY') ? ProcessUtils::escapeArgument(ARTISAN_BINARY) : 'artisan';
    }

    /**
     * Format the given command as a fully-qualified executable command.
     *
     * @param  string  $string
     * @return string
     */
    public static function formatCommandString($string)
    {
        return sprintf('%s %s %s', static::phpBinary(), static::artisanBinary(), $string);
    }

And later formatCommandString is returned as command string to Illuminate\Console\Scheduling\Illuminate\Console\Schedule, of which build scheduling commands:

    /**
     * Add a new callback event to the schedule.
     *
     * @param  string|callable  $callback
     * @param  array  $parameters
     * @return \Illuminate\Console\Scheduling\CallbackEvent
     */
    public function call($callback, array $parameters = [])
    {
        $this->events[] = $event = new CallbackEvent(
            $this->eventMutex, $callback, $parameters, $this->timezone
        );

        return $event;
    }

    /**
     * Add a new Artisan command event to the schedule.
     *
     * @param  string  $command
     * @param  array  $parameters
     * @return \Illuminate\Console\Scheduling\Event
     */
    public function command($command, array $parameters = [])
    {
        if (class_exists($command)) {
            $command = Container::getInstance()->make($command)->getName();
        }

        return $this->exec(
            Application::formatCommandString($command), $parameters
        );
    }

Note that Application::formatCommandString($command) was called and at this stage the wrongly formed string is injected in a protected array defined $events = [];.

At last, in Encore\Admin\Scheduling\Scheduling, method runTask fetches that array by using event():

    /**
     * Get all events in console kernel.
     *
     * @return array
     */
    protected function getKernelEvents()
    {
        app()->make('Illuminate\Contracts\Console\Kernel');

        return app()->make('Illuminate\Console\Scheduling\Schedule')->events();
    }

    /**
     * Run specific task.
     *
     * @param int $id
     *
     * @return string
     */
    public function runTask($id)
    {
        set_time_limit(0);

        /** @var \Illuminate\Console\Scheduling\Event $event */
        $event = $this->getKernelEvents()[$id - 1];

        $event->sendOutputTo($this->getOutputTo());

        $event->run(app());

        return $this->readOutput();
    }

The solution this PR proposed is simple, just add a php-cgi to php conversion, however, this approach is fraud on extreme conditions, but for most of the users, this approach would work fine.

        if (PHP_OS_FAMILY === 'Windows') {
            $event->command = Str::of($event->command)->replace('php-cgi.exe', 'php.exe');
        }

Also, Windows uses ", so this PR adds a small compatibility patch for that.

Issue #16, #8, and #9 might also have something to do with Symfony\Component\Process\PhpExecutableFinder, I might look for that in the further.

Another patch fix might come out later this week, basically, Artisan::call() works fine because it just creates a container then run the class. But scheduling forms a command then runs it, that's why Artisan::call() is not affected. That patch would permanently resolve issues like #16, #8, and #9.

@jxlwqq jxlwqq merged commit 84483c6 into laravel-admin-extensions:master Oct 11, 2021
@jxlwqq
Copy link
Member

jxlwqq commented Oct 11, 2021

thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Status: 419 unknown status :CSRF token mismatch,麻烦加一下
2 participants