Merge lp:~evfool/software-properties/lp1060192 into lp:software-properties

[Robert Roth]

- Do not use Shell=True unnecessarily in subprocess.Popen (bug #1060192)

As the python subprocess Popen documentation states: "the use of shell=True is strongly discouraged in cases where the command string is constructed from external input".
We construct the command to execute from mostly well-known parameters, unless the hostname of the mirror to test, which comes from /usr/share/python-apt/templates/ mirrors file, which can be (mostly) anything.
As bug #1060192 states, it does not really make a difference in this context whether we use shell=True or False, but maybe it is safer to use shell=False.

[Michael Vogt (mvo)]

On Wed, Feb 13, 2013 at 03:24:18PM -0000, Robert Roth wrote:
> Robert Roth has proposed merging lp:~evfool/software-properties/lp1060192 into lp:software-properties.
>
> Commit message:
> - Do not use Shell=True unnecessarily in subprocess.Popen (bug #1060192)
[..]

Great! Thanks for working on this!

> self.parent.report_action("Pinging %s..." % host)
> - commando = subprocess.Popen("ping -q -c 2 -W 1 -i 0.5 %s" % host,
> - shell=True, stdout=subprocess.PIPE,
> + commando = subprocess.Popen(["ping", "-q", "-c 2", "-W 1", "-i 0.5", host],
> + stdout=subprocess.PIPE,
> stderr=subprocess.STDOUT,
> universal_newlines=True).stdout
> while True:

You can also go one step further and do ["ping", "-c", "2", "-W",
"1"...].

But ping is fine with both. Thanks again for getting rid of the
shell=True.

Cheers,
 Michael

=== modified file 'softwareproperties/MirrorTest.py'
--- softwareproperties/MirrorTest.py	2012-07-05 13:32:44 +0000
+++ softwareproperties/MirrorTest.py	2013-02-13 15:23:25 +0000
@@ -41,8 +41,8 @@
                     host = mirror.hostname
 
                     self.parent.report_action("Pinging %s..." % host)
-                    commando = subprocess.Popen("ping -q -c 2 -W 1 -i 0.5 %s" % host,
-                                                shell=True, stdout=subprocess.PIPE,
+                    commando = subprocess.Popen(["ping", "-q", "-c 2", "-W 1", "-i 0.5", host],
+                                                stdout=subprocess.PIPE,
                                                 stderr=subprocess.STDOUT,
                                                 universal_newlines=True).stdout
                     while True: