pyinfra-dev / pyinfra

pyinfra turns Python code into shell commands and runs them on your servers. Execute ad-hoc commands and write declarative operations. Target SSH servers, local machine and Docker containers. Fast and scales from one server to thousands.
https://pyinfra.com
MIT License
3.91k stars 383 forks source link

Single quote parsing in `server.shell` commands #678

Closed AbcSxyZ closed 3 years ago

AbcSxyZ commented 3 years ago

Describe the bug

Looks like there is an issue with signle quote (') parsing & command creation using server.shell.

To Reproduce

from pyinfra.operations import server

server.shell(name="Quote parsing error",
        commands="ls '-l'")

Will produce :

>>> sh -c 'ls '"'"'-l'"'"''

Expected behavior

To have less than 8 quotes around the option.

Meta

System: Linux
Platform: Linux-5.10.0-9-amd64-x86_64-with-glibc2.31
Release: 5.10.0-9-amd64
Machine: x86_64
pyinfra: v1.4.16
Python: 3.9.2 (CPython, GCC 10.2.1 20210110)

This --support option is really a good idea !

Many thanks for the tool, tried it a couple of times, pleasant to use :)

Fizzadar commented 3 years ago

Hi @AbcSxyZ this is expected - it is quoted such that the string passed is executed literally, this uses the shlex Python stdlib module:

import shlex
print(shlex.quote("ls '-l'"))
'ls '"'"'-l'"'"''

Is there a specific use-case you have in mind? It would be fairly easy to add a flag to disable such quoting for the server.shell operation.

AbcSxyZ commented 3 years ago

I see, there is actually an example in shlex documentation showing this scenario. Looks weird, I'm not able to understand the logic, but seem definitively expected.

When I reported it, I had trouble with a more complex command containing command substitution, quote & redirections, thought it may be the issue, but it wasn't. It was just strange to see all those quotes, even for readability.

It would be fairly easy to add a flag to disable such quoting for the server.shell operation.

Please, don't do that :)

Fizzadar commented 3 years ago

I thought I'd added this to my original message, apologies; basically the reason is that double quotes allow variables to be rendered, so wrapping the command "ls '-l'" may lead to other problems with variables being rendered. So the workaround, horrible as it is, is just to stop the string, double quote the single quote, and then return to the string!

ie, splitting it into parts:

'ls' + "'" + '-l' + "'" + ''

Something I learnt during pyinfra development, indeed looks very odd at first :)