From f346853710264d8186fb931eafefabb829f3a874 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Sun, 18 Jan 2015 16:25:23 -0800
Subject: [PATCH] embedart: Do not use shell for subprocess

This avoids bugs when the filename contains spaces, etc.

Pinging @Kraymer: I gave this a brief test, but can you check whether I messed
anything up more subtly?

Came up when looking at #1241.
---
 beetsplug/embedart.py | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py
index 5191a4a4b..b140320bb 100644
--- a/beetsplug/embedart.py
+++ b/beetsplug/embedart.py
@@ -176,20 +176,28 @@ class EmbedCoverArtPlugin(BeetsPlugin):
             art = self.extract(f.name, item)
 
             if art:
-                # Converting images to grayscale tends to minimize the weight
-                # of colors in the diff score
-                cmd = 'convert {0} {1} -colorspace gray MIFF:- | ' \
-                      'compare -metric PHASH - null:' \
-                      .format(syspath(imagepath), syspath(art))
+                is_windows = platform.system() == "Windows"
 
-                is_windows = platform.system() != "Windows"
-                proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
-                                        stderr=subprocess.PIPE,
-                                        close_fds=is_windows,
-                                        shell=True)
-                stdout, stderr = proc.communicate()
-                if proc.returncode:
-                    if proc.returncode != 1:
+                # Converting images to grayscale tends to minimize the weight
+                # of colors in the diff score.
+                convert_proc = subprocess.Popen(
+                    ['convert', syspath(imagepath), syspath(art),
+                     '-colorspace', 'gray', 'MIFF:-'],
+                    stdout=subprocess.PIPE,
+                    close_fds=not is_windows,
+                )
+                compare_proc = subprocess.Popen(
+                    ['compare', '-metric', 'PHASH', '-', 'null:'],
+                    stdin=convert_proc.stdout,
+                    stdout=subprocess.PIPE,
+                    stderr=subprocess.PIPE,
+                    close_fds=not is_windows,
+                )
+                convert_proc.stdout.close()
+
+                stdout, stderr = compare_proc.communicate()
+                if compare_proc.returncode:
+                    if compare_proc.returncode != 1:
                         self._log.debug(u'IM phashes compare failed for {0}, '
                                         u'{1}', displayable_path(imagepath),
                                         displayable_path(art))