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

Should we escape exclude patterns in RsyncTask? #77

Closed
boedah opened this issue Nov 13, 2014 · 2 comments
Closed

Should we escape exclude patterns in RsyncTask? #77

boedah opened this issue Nov 13, 2014 · 2 comments

Comments

@boedah
Copy link
Contributor

boedah commented Nov 13, 2014

If you want to exclude patterns with a wildcard, e.g. *.scss, it will not exclude these files but (as the shell expands the wildcard) also sync matching files in the current working directory (which is certainly not what you want ;).

In exclude() method of rsync task (https://github.com/Codegyre/Robo/blob/master/src/Task/Rsync.php#L241), we could add escapeshellarg:

return $this->option('exclude', escapeshellarg($pattern));

This would be a BC break for people who already added ' themselves:

$rsync->exclude("'*.scss'");

But from my point of view, it should be escaped.

@DavertMik
Copy link
Member

Breaking BC before the stable version releases is not so painful.
This is a good time to came to agreement on defaults and interfaces.
Sure escapeshellarg should be added there....

P.S. never heard of that function before :|

@boedah
Copy link
Contributor Author

boedah commented Nov 19, 2014

Ok, will create a PR the next days.

escapeshellarg should be used everywhere a user-supplied value is used as shell argument because there can be spaces, quotes, wildcards... in it.
I have not looked in the other Robo tasks if it should be applied also.

boedah added a commit to boedah/Robo that referenced this issue Dec 4, 2014
DavertMik added a commit that referenced this issue Dec 25, 2014
…ttern

shell escape rsync exclude pattern
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

No branches or pull requests

2 participants