Skip to content

escape binary php path #147

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

Closed

Conversation

ElRochito
Copy link

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

Copy link
Collaborator

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is escapeshellarg() the correct function to use ? Or should this be escapeshellcmd() ?
Note: if it should be escapeshellcmd() - mind the warning about Windows paths below the examples.

@jrfnl
Copy link
Collaborator

jrfnl commented Aug 18, 2023

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

@ElRochito
Copy link
Author

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

@ElRochito ElRochito force-pushed the escape--php-binary-path branch from f27cef9 to ebc984b Compare August 18, 2023 08:25
@jrfnl
Copy link
Collaborator

jrfnl commented Aug 18, 2023

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
Copy link
Author

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

@jrfnl
Copy link
Collaborator

jrfnl commented Sep 12, 2023

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
Copy link
Collaborator

jrfnl commented Sep 12, 2023

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 ElRochito closed this Sep 13, 2023
@ElRochito
Copy link
Author

Indeed, it should fixed before

Sorry

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

Successfully merging this pull request may close these issues.

2 participants