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

shquote breaks certain sudo commands #61

Closed
bkabrda opened this issue Apr 15, 2013 · 10 comments
Closed

shquote breaks certain sudo commands #61

bkabrda opened this issue Apr 15, 2013 · 10 comments
Labels

Comments

@bkabrda
Copy link
Contributor

bkabrda commented Apr 15, 2013

Hi,
when using some sudo commands I found out that they were failing where they shouldn't have. E.g.:

from plumbum.cmd import sudo, yum

sudo(yum['install']['rubygem(rails)'])

This fails, because rubygem(rails) gets overquoted by formulate (which calls shquote). This ends up formulated like this:

['/usr/bin/sudo', '/usr/bin/yum', 'install', "'rubygem(rails)'"]

So yum in fact gets "'rubygem(rails)'" on commandline (because Popen puts the outer quotes there.

I think that the problem can be solved by adding '(' and ')' to _safechars, which seems ok to me, but I'm not sure of other consequences of this. Any other options how to solve this?

Thanks.

@tomerfiliba
Copy link
Owner

can you see if sudo(yum['install', 'rubygem(rails)']) works? (i removed one level of brackets)

@bkabrda
Copy link
Contributor Author

bkabrda commented Apr 15, 2013

Also please note, that this only happens while using sudo, which triggers the "level >= self.QUOTE_LEVEL" condition in ConcreteCommand.formulate.

@bkabrda
Copy link
Contributor Author

bkabrda commented Apr 15, 2013

@tomerfiliba doesn't work, still the same issue.

@bkabrda
Copy link
Contributor Author

bkabrda commented Apr 16, 2013

Thinking of this further, is the formulate/shquote logic needed at all? subprocess.Popen (which is used to execute these) handles quoting itself, doesn't it? So what is the reason to use these?

@tomerfiliba
Copy link
Owner

No, there's no quoting done by Popen. It only happens if shell=True.
Anyway, quoting is needed when you nest commands one into another, as each
level unescapes one level of quotes.
On Apr 16, 2013 12:32 PM, "Bohuslav Kabrda" notifications@github.com
wrote:

Thinking of this further, is the formulate/shquote logic needed at all?
subprocess.Popen (which is used to execute these) handles quoting itself,
doesn't it? So what is the reason to use these?


Reply to this email directly or view it on GitHubhttps://github.com//issues/61#issuecomment-16435073
.

@bkabrda
Copy link
Contributor Author

bkabrda commented May 3, 2013

Well, I meant quoting in the sense that when you subprocess.Popen(['yum', 'install', 'rubygem(rails)']), it actually works, whereas using "yum install rubygem(rails)" fails on commandline because of parentheses that are not quoted. So I'm not sure that this is actually quoting, but it does something with the parameters.
Anyway, I'd like to create a pull request to fix this, so there's one thing I'd like to know - what will break, if '()' are added to _safechars? I'd like to se an example so I have something to work with.
Thanks.

@tomerfiliba
Copy link
Owner

when you do

subprocess.Popen(['yum', 'install', 'rubygem(rails)'])

it just places the arguments in argv of the new process, never going through the shell. in windows it does go through the shell, but that's another story.

anyhow, "()" are not safe chars, because the shell uses them for its own purposes. for example, you can write

(prog1 && prog2) || prog3

I think it would be easiest and simplest to add a RawString type, so you could do

yum("install", RawStr("rubygem(rails)")

which won't get quoted.

this idea just came to me - did you try:

sudo[yum]("install", "rubygem(rails)")

?

@bkabrda
Copy link
Contributor Author

bkabrda commented May 3, 2013

Interesting, the

sudo[yum]("install", "rubygem(rails)")

seems to work fine.
Anyway, it seems to me that using RawStr is somehow more systematic - I get the precise control over what will and what won't get quoted.

@tomerfiliba
Copy link
Owner

come to think of it, sudo[yum]("params") is the way to go -- you want
"yum" to be sudo'ed, not the arguments.
but it would require some thinking on my side... i'll let you know


Tomer Filiba
tomerfiliba.com http://www.facebook.com/tomerfiliba
http://il.linkedin.com/in/tomerfiliba

On Fri, May 3, 2013 at 11:20 AM, Bohuslav Kabrda
notifications@github.comwrote:

Interesting, the

sudoyum

seems to work fine.
Anyway, it seems to me that using RawStr is somehow more systematic - I
get the precise control over what will and what won't get quoted.


Reply to this email directly or view it on GitHubhttps://github.com//issues/61#issuecomment-17383187
.

@tomerfiliba
Copy link
Owner

closing as "won't fix". there's a workaround and it's quite possible that #27 solves it.

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

No branches or pull requests

2 participants