fix files with ' or " by using arg lists instead of strings (#581)

* replace popen with subprocess run

* add splitlines

* remove stdin

* fix xml

* fix error logging and 7zip

kindlecomicconverter/KCC_gui.py

@@ -29,7 +29,7 @@ from subprocess import STDOUT, PIPE
 # noinspection PyUnresolvedReferences
 from PyQt5 import QtGui, QtCore, QtWidgets, QtNetwork
 from xml.sax.saxutils import escape
-from psutil import Popen, Process
+from psutil import Process
 from copy import copy
 from distutils.version import StrictVersion
 from raven import Client
@@ -839,24 +839,22 @@ class KCCGUI(KCC_ui.Ui_mainWindow):
                 os.chmod('/usr/local/bin/kindlegen', 0o755)
             except Exception:
                 pass
-        kindleGenExitCode = Popen('kindlegen -locale en', stdout=PIPE, stderr=STDOUT, stdin=PIPE, shell=True)
+        kindleGenExitCode = subprocess.run(['kindlegen', '-locale', 'en'], stdout=PIPE, stderr=STDOUT, encoding='UTF-8')
-        kindleGenExitCode.communicate()
         if kindleGenExitCode.returncode == 0:
             self.kindleGen = True
-            versionCheck = Popen('kindlegen -locale en', stdout=PIPE, stderr=STDOUT, stdin=PIPE, shell=True)
+            versionCheck = subprocess.run(['kindlegen', '-locale', 'en'], stdout=PIPE, stderr=STDOUT, encoding='UTF-8')
-            for line in versionCheck.stdout:
+            for line in versionCheck.stdout.splitlines():
-                line = line.decode("utf-8")
                 if 'Amazon kindlegen' in line:
                     versionCheck = line.split('V')[1].split(' ')[0]
                     if StrictVersion(versionCheck) < StrictVersion('2.9'):
                         self.addMessage('Your <a href="https://www.amazon.com/b?node=23496309011">KindleGen</a>'
                                         ' is outdated! MOBI conversion might fail.', 'warning')
                     break
-            where_command = 'where kindlegen.exe'
+            where_command = ['where', 'kindlegen.exe']
             if os.name == 'posix':
-                where_command = 'which kindlegen'
+                where_command = ['which', 'kindlegen']
-            process = subprocess.run(where_command, stdout=PIPE, stderr=STDOUT, stdin=PIPE, shell=True)
+            process = subprocess.run(where_command, stdout=PIPE, stderr=STDOUT, encoding='UTF-8')
-            locations = process.stdout.decode('utf-8').split('\n')
+            locations = process.stdout.splitlines()
             self.addMessage(f"<b>KindleGen Found:</b> {locations[0]}", 'info')
         else:
             self.kindleGen = False
@@ -1039,8 +1037,7 @@ class KCCGUI(KCC_ui.Ui_mainWindow):
             self.addMessage('Since you are a new user of <b>KCC</b> please see few '
                             '<a href="https://github.com/ciromattia/kcc/wiki/Important-tips">important tips</a>.',
                             'info')
-        process = Popen('7z', stdout=PIPE, stderr=STDOUT, stdin=PIPE, shell=True)
+        process = subprocess.run(['7z'], stdout=PIPE, stderr=STDOUT)
-        process.communicate()
         if process.returncode == 0 or process.returncode == 7:
             self.sevenzip = True
         else:

kindlecomicconverter/comic2ebook.py

@@ -19,6 +19,7 @@
 #
 
 import os
+import subprocess
 import sys
 from argparse import ArgumentParser
 from time import strftime, gmtime
@@ -34,7 +35,7 @@ from uuid import uuid4
 from slugify import slugify as slugify_ext
 from PIL import Image
 from subprocess import STDOUT, PIPE
-from psutil import Popen, virtual_memory, disk_usage
+from psutil import virtual_memory, disk_usage
 from html import escape as hescape
 try:
     from PyQt5 import QtCore
@@ -1101,14 +1102,12 @@ def checkTools(source):
     source = source.upper()
     if source.endswith('.CB7') or source.endswith('.7Z') or source.endswith('.RAR') or source.endswith('.CBR') or \
             source.endswith('.ZIP') or source.endswith('.CBZ'):
-        process = Popen('7z', stdout=PIPE, stderr=STDOUT, stdin=PIPE, shell=True)
+        process = subprocess.run(['7z'], stdout=PIPE, stderr=STDOUT)
-        process.communicate()
         if process.returncode != 0 and process.returncode != 7:
             print('ERROR: 7z is missing!')
             sys.exit(1)
     if options.format == 'MOBI':
-        kindleGenExitCode = Popen('kindlegen -locale en', stdout=PIPE, stderr=STDOUT, stdin=PIPE, shell=True)
+        kindleGenExitCode = subprocess.run(['kindlegen', '-locale', 'en'], stdout=PIPE, stderr=STDOUT)
-        kindleGenExitCode.communicate()
         if kindleGenExitCode.returncode != 0:
             print('ERROR: KindleGen is missing!')
             sys.exit(1)
@@ -1265,10 +1264,9 @@ def makeMOBIWorker(item):
     kindlegenError = ''
     try:
         if os.path.getsize(item) < 629145600:
-            output = Popen('kindlegen -dont_append_source -locale en "' + item + '"',
+            output = subprocess.run(['kindlegen', '-dont_append_source', '-locale', 'en', item],
-                           stdout=PIPE, stderr=STDOUT, stdin=PIPE, shell=True)
+                           stdout=PIPE, stderr=STDOUT, encoding='UTF-8')
-            for line in output.stdout:
+            for line in output.stdout.splitlines():
-                line = line.decode('utf-8')
                 # ERROR: Generic error
                 if "Error(" in line:
                     kindlegenErrorCode = 1
@@ -1279,7 +1277,6 @@ def makeMOBIWorker(item):
                 if kindlegenErrorCode > 0:
                     break
                 if ":I1036: Mobi file built successfully" in line:
-                    output.communicate()
                     break
         else:
             # ERROR: EPUB too big

kindlecomicconverter/comicarchive.py

@@ -22,7 +22,6 @@ import os
 import platform
 import subprocess
 import distro
-from psutil import Popen
 from shutil import move
 from subprocess import STDOUT, PIPE
 from xml.dom.minidom import parseString
@@ -35,19 +34,17 @@ class ComicArchive:
         self.type = None
         if not os.path.isfile(self.filepath):
             raise OSError('File not found.')
-        process = Popen('7z l -y -p1 "' + self.filepath + '"', stderr=STDOUT, stdout=PIPE, stdin=PIPE, shell=True)
+        process = subprocess.run(['7z', 'l', '-y', '-p1', self.filepath], stderr=STDOUT, stdout=PIPE, encoding='UTF-8')
-        for line in process.stdout:
+        for line in process.stdout.splitlines():
-            if b'Type =' in line:
+            if 'Type =' in line:
-                self.type = line.rstrip().decode().split(' = ')[1].upper()
+                self.type = line.rstrip().split(' = ')[1].upper()
                 break
-        process.communicate()
         if process.returncode != 0 and distro.id() == 'fedora':
-            process = Popen('unrar l -y -p1 "' + self.filepath + '"', stderr=STDOUT, stdout=PIPE, stdin=PIPE, shell=True)
+            process = subprocess.run(['unrar', 'l', '-y', '-p1', self.filepath], stderr=STDOUT, stdout=PIPE, encoding='UTF-8')
-            for line in process.stdout:
+            for line in process.stdout.splitlines():
-                if b'Details: ' in line:
+                if 'Details: ' in line:
-                    self.type = line.rstrip().decode().split(' ')[1].upper()
+                    self.type = line.rstrip().split(' ')[1].upper()
                     break
-            process.communicate()
             if process.returncode != 0:
                 raise OSError('Archive is corrupted or encrypted.')
             elif self.type not in ['7Z', 'RAR', 'RAR5', 'ZIP']:
@@ -58,22 +55,20 @@ class ComicArchive:
     def extract(self, targetdir):
         if not os.path.isdir(targetdir):
             raise OSError('Target directory doesn\'t exist.')
-        process = Popen('7z x -y -xr!__MACOSX -xr!.DS_Store -xr!thumbs.db -xr!Thumbs.db -o"' + targetdir + '" "' +
+        process = subprocess.run(['7z', 'x', '-y', '-xr!__MACOSX', '-xr!.DS_Store', '-xr!thumbs.db', '-xr!Thumbs.db', '-o' + targetdir, self.filepath],
-                        self.filepath + '"', stdout=PIPE, stderr=STDOUT, stdin=PIPE, shell=True)
+                                 stdout=PIPE, stderr=STDOUT, encoding='UTF-8')
-        process.communicate()
         if process.returncode != 0 and distro.id() == 'fedora':
-            process = Popen('unrar x -y -x__MACOSX -x.DS_Store -xthumbs.db -xThumbs.db "' + self.filepath + '" "' +
+            process = subprocess.run(['unrar', 'x', '-y', '-x__MACOSX', '-x.DS_Store', '-xthumbs.db', '-xThumbs.db', self.filepath, targetdir] 
-                    targetdir + '"', stdout=PIPE, stderr=STDOUT, stdin=PIPE, shell=True)
+                    , stdout=PIPE, stderr=STDOUT)
-            process.communicate()
             if process.returncode != 0:
                 raise OSError('Failed to extract archive.')
         elif process.returncode != 0 and platform.system() == 'Darwin':
-            process = subprocess.run(f"unar '{self.filepath}' -f -o '{targetdir}'", 
+            process = subprocess.run(['unar', self.filepath, '-f', '-o', targetdir], 
-                stdout=PIPE, stderr=STDOUT, stdin=PIPE, shell=True)
+                stdout=PIPE, stderr=STDOUT, encoding='UTF-8')
             if process.returncode != 0:
-                raise Exception(process.stdout.decode("utf-8"))
+                raise Exception(process.stdout)
         elif process.returncode != 0:
-            raise OSError('Failed to extract archive. Check if p7zip-rar is installed.')
+            raise OSError(process.stdout.strip())
         tdir = os.listdir(targetdir)
         if 'ComicInfo.xml' in tdir:
             tdir.remove('ComicInfo.xml')
@@ -86,19 +81,17 @@ class ComicArchive:
     def addFile(self, sourcefile):
         if self.type in ['RAR', 'RAR5']:
             raise NotImplementedError
-        process = Popen('7z a -y "' + self.filepath + '" "' + sourcefile + '"',
+        process = subprocess.run(['7z', 'a', '-y', self.filepath, sourcefile],
-                        stdout=PIPE, stderr=STDOUT, stdin=PIPE, shell=True)
+                        stdout=PIPE, stderr=STDOUT)
-        process.communicate()
         if process.returncode != 0:
             raise OSError('Failed to add the file.')
 
     def extractMetadata(self):
-        process = Popen('7z x -y -so "' + self.filepath + '" ComicInfo.xml',
+        process = subprocess.run(['7z', 'x', '-y', '-so', self.filepath, 'ComicInfo.xml'],
-                        stdout=PIPE, stderr=STDOUT, stdin=PIPE, shell=True)
+                        stdout=PIPE, stderr=STDOUT, encoding='UTF-8')
-        xml = process.communicate()
         if process.returncode != 0:
             raise OSError('Failed to extract archive.')
         try:
-            return parseString(xml[0])
+            return parseString(process.stdout)
         except ExpatError:
             return None