0

I am trying to call an executable called foo, and pass it some command line arguments. An external script calls into the executable and uses the following command:

./main/foo --config config_file 2>&1 | /usr/bin/tee temp.log

The script uses Popen to execute this command as follows:

from subprocess import Popen
from subprocess import PIPE
def run_command(command, returnObject=False):
    cmd = command.split(' ')
    print('%s' % cmd)
    p = None
    print('command : %s' % command)
    if returnObject:
        p = Popen(cmd)
    else:
        p = Popen(cmd)
        p.communicate()
        print('returncode: %s' % p.returncode)
        return p.returncode
    return p

command = "./main/foo --config config_file 2>&1 | /usr/bin/tee temp.log
"
run_command(command)

However, this passes extra arguments ['2>&1', '|', '/usr/bin/tee', 'temp.log'] to the foo executable.

How can I get rid of these extra arguments getting passed to foo while maintaining the functionality? I have tried shell=True but read about avoiding it for security purposes (shell injection attack). Looking for a neat solution.

Thanks

UPDATE: - Updated the file following the tee command

8
  • First say what you want to to with the extra arguments. Next say what your are trying to achieve: write in python a full shell with pipes and redirections or just a proof of concept. And finally, you should weight the risk for shell injection (can be mitigated by forcing a full path to a known shell) against the risk for bad implementation. But without a minimum context I cannot guess what you really want. Commented Aug 11, 2015 at 21:32
  • It is clear from the problem description what I want. I do not want to pass ['2>&1', '|', '/usr/bin/tee', '>temp.log'] to foo. Commented Aug 11, 2015 at 21:38
  • Those arguments are shell constructs -- they have no meaning to anything but a shell. Commented Aug 11, 2015 at 21:58
  • ...also, calling tee without passing it any arguments is silly -- with no arguments, it reads from stdin and passes to stdout; instead of redirecting that stdout to a file, you might as well have just redirected the original program's stdout directly; in shell terms: ./main/foo --config config_file >temp.log 2>&1? Commented Aug 11, 2015 at 22:03
  • (to be clear, >temp.log is not an argument to tee; it tells a shell to direct FD 1 of the tee process to temp.log before starting tee at all). Commented Aug 11, 2015 at 22:06

3 Answers 3

3

The string

./main/foo --config config_file 2>&1 | /usr/bin/tee >temp.log

...is full of shell constructs. These have no meaning to anything without a shell in play. Thus, you have two options:

  • Set shell=True
  • Replace them with native Python code.

For instance, 2>&1 is the same thing as passing stderr=subprocess.STDOUT to Popen, and your tee -- since its output is redirected and it's passed no arguments -- could just be replaced with stdout=open('temp.log', 'w').


Thus:

p = subprocess.Popen(['./main/foo', '--config', 'config_file'],
  stderr=subprocess.STDOUT,
  stdout=open('temp.log', 'w'))

...or, if you really did want the tee command, but were just using it incorrectly (that is, if you wanted tee temp.log, not tee >temp.log):

p1 = subprocess.Popen(['./main/foo', '--config', 'config_file'],
  stderr=subprocess.STDOUT,
  stdout=subprocess.PIPE)
p2 = subprocess.Popen(['tee', 'temp.log'], stdin=p1.stdout)
p1.stdout.close() # drop our own handle so p2's stdin is the only handle on p1.stdout
stdout, _ = p2.communicate()

Wrapping this in a function, and checking success for both ends might look like:

def run():
    p1 = subprocess.Popen(['./main/foo', '--config', 'config_file'],
      stderr=subprocess.STDOUT,
      stdout=subprocess.PIPE)
    p2 = subprocess.Popen(['tee', 'temp.log'], stdin=p1.stdout)
    p1.stdout.close() # drop our own handle so p2's stdin is the only handle on p1.stdout
    # True if both processes were successful, False otherwise
    return (p2.wait() == 0 && p1.wait() == 0)

By the way -- if you want to use shell=True and return the exit status of foo, rather than tee, things get a bit more interesting. Consider the following:

p = subprocess.Popen(['bash', '-c', 'set -o pipefail; ' + command_str])

...the pipefail bash extension will force the shell to exit with the status of the first pipeline component to fail (and 0 if no components fail), rather than using only the exit status of the final component.

Sign up to request clarification or add additional context in comments.

33 Comments

Just setting shell=True does not seem to redirect the output to the console.
@shank22, of course it doesn't -- you're redirecting stdout from tee to a file. Why would you think it would go to the console as well?
@shank22, which is to say: If you want output to go to the console as well as the file, you'd have to do tee temp.log, not tee >temp.log, passing temp.log as an argument to tee, rather than passing tee no arguments but giving the shell a command to redirect its output.
How would it look like with shell=True?
With shell=True, you wouldn't do any splitting yourself at all -- you'd just pass the whole input string straight to subprocess.Popen() as the first argument.
|
1

Here's a couple of "neat" code examples in addition to the explanation from @Charles Duffy answer.

To run the shell command in Python:

#!/usr/bin/env python
from subprocess import check_call

check_call("./main/foo --config config_file 2>&1 | /usr/bin/tee temp.log",
           shell=True)

without the shell:

#!/usr/bin/env python
from subprocess import Popen, PIPE, STDOUT

tee = Popen(["/usr/bin/tee", "temp.log"], stdin=PIPE)
foo = Popen("./main/foo --config config_file".split(), 
            stdout=tee.stdin, stderr=STDOUT)
pipestatus = [foo.wait(), tee.wait()]

Note: don't use "command arg".split() with non-literal strings.

See How do I use subprocess.Popen to connect multiple processes by pipes?

15 Comments

This is where I grumble about the use of string.split() -- creating bugs if handling arguments with literal spaces (or with code containing shell quoting, escaping or redirection directives, all of which accepting a string can be read to imply support for); at least shlex.split() doesn't restrict the range of possible commands to exclude those with literal whitespace in arguments. But otherwise, the further examples are appreciated.
@CharlesDuffy: there is nothing wrong with using .split() for literal strings
What's wrong is that it becomes habit, and gets used in other scenarios as well -- such as the OP's original code above.
@CharlesDuffy: I don't buy it unless it is backed up by evidence. It reminds me about "gateway drug", "games cause violence", etc that sound plausible but that are bogus in reality. You can't accidently confuse a literal and non-literal strings. shlex.split() is not a panacea. You have to correct its results manually (though it it useful for creating a "draft" list of arguments).
The OP's misuse in the question here -- wherein they have a reusable function used by callers they don't control -- is not example enough?! Keep in mind that your example above was not explicitly commenting or noting that it was demonstrating a practice safe only with literals. If someone is copying practices espoused by high-rep users on StackOverflow -- something we should support, should we not -- then can they not expect to be able to use that practice widely, unless it's presented next to an explicit warning or disclaimer limiting its applicability?
|
0

You may combine answers to two StackOverflow questions:
1. piping together several subprocesses
x | y problem
2. Merging a Python script's subprocess' stdout and stderr (while keeping them distinguishable)
2>&1 problem

8 Comments

For the current use case, would setting shell=True be bad? (piping together several subprocesses)
Also, I am interested in knowing how this can be leveraged with the current command. I only have access to run_command method to make the changes.
It depends. The command you included in your question has no "variable" parts - security is not big risk (IMHO). Running every part of pipeline directly from python gives you exit codes of every program/command in the chain. So it is your choice in case simplicity v. full/detailed control. [I have assumed you provided "sample" command => command you really want may be different]
Short question: Are any of these components user-configurable? The config file name? The log name? Anything else?
The command is not user configurable when it comes to run_command. It is a string that is just split and given to Popen. So wondering how it can be done.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.