php-parallel-lint / PHP-Parallel-Lint

This tool check syntax of PHP files faster than serial check with fancier output.
Other
281 stars 21 forks source link

escape binary php path #147

Closed ElRochito closed 1 year ago

ElRochito commented 1 year ago

This PR allows to fix php executable path when path like this : /Users/xxxx/Application Support/Herd/bin/php82

Without escaping, binary was not executable because it stop to /Users/xxx/Application

jrfnl commented 1 year ago

Oh - and I realize this might not be straight-forward, but did you try to add a test for this ?

ElRochito commented 1 year ago

For me escapeshellarg is the right function which is used . I've added a condition for window users

jrfnl commented 1 year ago

For me escapeshellarg is the right function which is used . I've added a condition for window users

I'm not sure you understood my question - why do you think escapeshellarg() is the correct function ? and why should something different be used for Windows vs other OSes ?

ElRochito commented 1 year ago

Oh sorry! escapeshellarg because I need to add quotes around path to prevent error of whitespace in it

jrfnl commented 1 year ago

Oh sorry! escapeshellarg because I need to add quotes around path to prevent error of whitespace in it

Clearly my questions didn't have the intended effect,..

escapeshellarg() is intended to escape arguments passed to a shell command, while escapeshellcmd() is intended to escape the command. As the PHP executable is the command which will be executed, you are using the wrong function for the escaping. Please fix this up to use the correct function.

jrfnl commented 1 year ago

Oh and another thing I'm wondering about is whether this is the right place to do the escaping. Shouldn't the escaping be done at the point when the input is actually used (not just stored and passed around).

ElRochito commented 1 year ago

Indeed, it should fixed before

Sorry