-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Messenger][Process] add fromShellCommandline
to RunProcessMessage
#59768
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
[Messenger][Process] add fromShellCommandline
to RunProcessMessage
#59768
Conversation
243baf1
to
5746aab
Compare
What about |
the Process component has explicitly rejected the usage of |
i mean if it's a string use having both $command and $commandLine in a regular constructor here, bugs me more :) |
Also I find the dedicated method approach more explicit for developper, parameters with multiple type aren't as clear for me. |
Yeah that's a good point, maybe a whole other message ( |
personally Otheriwse i'd prefer the current approach yes over a new class. |
@ro0NL The case of static command is not the one having security issues. new RunProcessMessage(['ls', $folder]); // Secure
new RunProcessMessage(\sprintf('ls %s', $folder)); // Insecure
new RunProcessMessage(\sprintf('ls %s', escapeshellarg($folder))); // Secure on Unix system (and partially secure on Windows) |
It would be passed to |
Yes but the point here is that the user won't know that the call is inherently unsecure, having a dedicated method that is documented as such is better imo |
5746aab
to
4afeaee
Compare
src/Symfony/Component/Process/Messenger/RunProcessMessageHandler.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Process/Tests/Messenger/RunProcessMessageHandlerTest.php
Outdated
Show resolved
Hide resolved
Allows using the Process::fromShellCommandline when using a RunProcessMessage
4afeaee
to
e448596
Compare
BTW @nicolas-grekas I bisected the php-src to find out why the unit tests fails with php 8.5 and i narrowed it down to those change to reflection: php/php-src#17755 |
Ah nice! Please report to php-src. The earlier the better. |
Thank you @Staormin. |
The
RunProcess::fromShellCommandline()
method can be useful when using pipes or redirection.This feature allow the use of this method when using a RunProcessMessage.