plumbum icon indicating copy to clipboard operation
plumbum copied to clipboard

Arguments are incorrectly double-quoted when nesting remote commands

Open JoshRosen opened this issue 9 years ago • 13 comments

Consider the following example:

In [2]: from plumbum import SshMachine

In [3]: remote = SshMachine('localhost', ssh_opts=['-tt'])

In [4]: ls = remote['ls']

In [5]: sudo = remote['sudo']

In [6]: print sudo[ls['directory_without_spaces_in_name']]
/usr/bin/sudo /bin/ls directory_without_spaces_in_name

In [7]: print sudo[ls['directory with spaces in name']]
/usr/bin/sudo /bin/ls "'directory with spaces in name'"

In [8]: print ls['directory with spaces in name']
/bin/ls 'directory with spaces in name'

What's happening here is that the single argument to ls, which contains spaces, gets quoted twice when nested, causing the final argument received by ls to contain an extra set of single quotes (causing the command to return incorrect results).

Note that this type of nesting works as expected with local commands:

In [9]: from plumbum import local

In [10]: ls = local['ls']

In [11]: sudo = local['sudo']

In [12]: print sudo[ls['directory_without_spaces_in_name']]
/usr/bin/sudo /bin/ls directory_without_spaces_in_name

In [13]: print sudo[ls['directory with spaces in name']]
/usr/bin/sudo /bin/ls 'directory with spaces in name'

However, it doesn't work locally without nesting:

In [14]: print ls['directory with spaces in name']
/bin/ls directory with spaces in name

Am I doing anything wrong? Is there any way to control this behavior?

JoshRosen avatar Jan 07 '16 23:01 JoshRosen

Note that I see the above output on both 1.6.0 and 1.6.1.

JoshRosen avatar Jan 07 '16 23:01 JoshRosen

I managed to work around this using a trick similar to https://github.com/tomerfiliba/plumbum/issues/124#issuecomment-54664378, using sudo[ls]['arg with spaces'], but I'm pretty confused by the behavior that I saw above and would appreciate any clarification here.

JoshRosen avatar Jan 07 '16 23:01 JoshRosen

I'll try to look into it soon, but have a paper to write and thesis too, and I'll be working in Belize with little internet for a week.

henryiii avatar Jan 09 '16 03:01 henryiii

I can have a go at implementing this. This bug is affecting me as I'm trying to run a command surrounded by sudo, consider the following:

>>> from plumbum import local
>>> make = local['make']
>>> sudo = local['sudo']
>>> sudo[make['install', 'VAR="a b c"']].formulate()
['/usr/bin/sudo', '/usr/bin/make', 'install', '\'VAR="a b c"\'']    # Notice extra quotes, make isn't happy
>>> make['install', 'VAR="a b c"'].formulate()
['/usr/bin/make', 'install', 'VAR="a b c"']     # Without sudo, works as expected

In this case I really do not want the shquote function to be called, I strictly want str. I think I know how to fix this, but I don't entirely understand the purpose of QUOTE_LEVEL, I don't want to break functionality that others are relying on.

nicholashaydensmith avatar Mar 04 '16 23:03 nicholashaydensmith

Feel free to have a go at it, I'll help review it. Hopefully the tests will fail if there's a problem. I just don't have time to attempt a fix myself. :(

henryiii avatar Mar 04 '16 23:03 henryiii

Do have any insight behind the significance behind local.py:94:

    QUOTE_LEVEL = 2

Why 2?

nicholashaydensmith avatar Mar 05 '16 00:03 nicholashaydensmith

This bug is still present in 1.6.3. Any update on this would be greatly appreciated.

thedataking avatar Jun 23 '17 00:06 thedataking

Feel free to look into it and make a PR; I'll try to look into it this weekend if I have time. I am not sure if nesting is the way to compose commands like this. I missed the comment from @nicholashaydensmith, that's probably where I'll start.

henryiii avatar Jun 23 '17 02:06 henryiii

Looking at issue #124 and poking at the code, this is my understanding of the problem:

  1. Some arguments need escaping no matter their nesting level, e.g., directory names with spaces in them as @JoshRosen points out.
  2. For some arguments, we don't want escaping since we want the shell to expand them. E.g. '*.py'.
  3. We don't want to shell-escape the same argument multiple times. Nesting three or more local commands is one way to trigger this.

We need explicit control over argument quoting. One way would be to pass some arguments as a new type RawString or whatever rather than a native python string to tell plumbum we don't want any automatic escaping for that arg. That would remove a lot of the elegance of plumbum so that is probably not the way to go. Another option, which I'd personally vote for is to make the programmer responsible for automatically shell escaping strings. Existing packages such as pipes and shlex (py3 only) provide a quote function fit for this purpose (at least for non-Windows platforms).

Note, one can disable automatic shell-escaping for local commands indirectly thru this ugly hack:

import sys
from plumbum.machines import LocalCommand
LocalCommand.QUOTE_LEVEL = sys.maxint #any number >> 2 will do

make = local.get("make")
nice = local.get("nice")
print nice["-n", "19", make["VAR='A B C'", "test"]] 
# output: /usr/bin/nice -n 19 /usr/bin/make VAR='A B C' test

thedataking avatar Jun 24 '17 00:06 thedataking

I am running into this issue as well with REMOTE commands. I want to send a pattern argument into unzip and avoid escaping it.

Would it be possible to have a Raw() class that you can enclose arguments in, so escaping is ignored for these arguments. Is this an acceptable solution? @henryiii - I'm happy to submit a patch if no one else has time to get to it - just don't want to do wasted work as I'm not a regular contributor to this project.

bendemott avatar Dec 05 '19 18:12 bendemott

This appears to be a problem still in version 1.7.0 :(

simonwiles avatar May 24 '21 21:05 simonwiles

I'm happy to accept a tested patch :)

henryiii avatar May 24 '21 21:05 henryiii

I managed to monkey-patch current default branch using:

from plumbum.machines import RemoteCommand
RemoteCommand.QUOTE_LEVEL = 3

For some arguments, we don't want escaping since we want the shell to expand them. E.g. '*.py'.

Maybe I actually misunderstood what plumbum is for, but is it really the case? I was expecting arguments to be passed through unmodified. If shell expansion is allowed, then there's no way to automatically decide what should be quoted and what shouldn't.

dcz-self avatar Feb 07 '22 17:02 dcz-self