From 85d646b13366299d306bd017943e486650305a8c Mon Sep 17 00:00:00 2001
From: Cas Cremers <cas.cremers@gmail.com>
Date: Tue, 27 Oct 2020 23:38:48 +0100
Subject: [PATCH] Further wxPython4 fixes.

---
 gui/Gui/Attackwindow.py |  7 +++---
 gui/Gui/Makeimage.py    | 48 +++++++++++++++-----------------------
 gui/Gui/Misc.py         | 51 ++++++++++++++++++++++++++++-------------
 3 files changed, 57 insertions(+), 49 deletions(-)

diff --git a/gui/Gui/Attackwindow.py b/gui/Gui/Attackwindow.py
index 0336caa..9c76d3d 100644
--- a/gui/Gui/Attackwindow.py
+++ b/gui/Gui/Attackwindow.py
@@ -201,13 +201,12 @@ class AttackDisplay(wx.ScrolledWindow):
         res = self.askUserForFilename(style=wx.FD_SAVE, wildcard="*.%s" % (ext), defaultFile = "%s" % (suggested))
         return res
 
-    def exportImage(self, type,ext=None):
+    def exportImage(self, imgtype, ext=None):
         if ext == None:
-            ext = type
+            ext = imgtype
         res = self.saveFileName(ext)
         if res != None:
-            cmd = "dot -T%s" % (type)
-            cmdpushwrite(cmd,self.attack.scytherDot,res)
+            dotOutputWrite(self.attack.scytherDot,res,["-T" + imgtype])
 
     def OnExportPng(self, event):
         self.exportImage("png")
diff --git a/gui/Gui/Makeimage.py b/gui/Gui/Makeimage.py
index e641429..bebe17f 100644
--- a/gui/Gui/Makeimage.py
+++ b/gui/Gui/Makeimage.py
@@ -118,42 +118,32 @@ def makeImageDot(dotdata,attackthread=None):
     if Preference.usePIL():
         # If we have the PIL library, we can do postscript! great
         # stuff.
-        type = "ps"
+        imgtype = "ps"
         ext = ".ps"
     else:
         # Ye olde pnge file
-        type = "png"
+        imgtype = "png"
         ext = ".png"
 
-    # Retrieve dot command path
-    dotcommand = FindDot.findDot()
-
-    # command to write to temporary file
-    (fd2,fpname2) = Temporary.tempcleaned(ext)
-    f = os.fdopen(fd2,'w')
-
+    # Write dot source file into temporary file to simplify dealing with graphviz invocation across platforms 
+    (fd_in,fpname_in) = Temporary.tempcleaned(ext)
+    f_in = os.fdopen(fd_in,'w')
+    if attackthread:
+        writeGraph(attackthread,dotdata,f_in)
+    else:
+        f_in.write(dotdata)
+    f_in.close()
+    
     # Set up command
-    cmd = "%s -T%s" % (dotcommand,type)
+    dotcommand = FindDot.findDot()
+    cmd = [dotcommand, "-T" + imgtype, fpname_in]
 
     # execute command
-    p = Popen(cmd, shell=True, stdin=PIPE, stdout=PIPE)
+    (fd_out,fpname_out) = Temporary.tempcleaned(ext)
+    p = Popen(cmd, stdout=fd_out)
+    p.wait()
 
-
-    if attackthread:
-        writeGraph(attackthread,dotdata,p.stdin)
-    else:
-        p.stdin.write(dotdata)
-
-    p.stdin.close()
-
-    for l in p.stdout.read():
-        f.write(str(l))
-
-    p.stdout.close()
-    f.flush()
-    f.close()
-
-    return (fpname2, type)
+    return (fpname_out, imgtype)
 
 
 def makeImage(attack,attackthread=None):
@@ -161,10 +151,10 @@ def makeImage(attack,attackthread=None):
 
     """ This should clearly be a method of 'attack' """
 
-    (name,type) = makeImageDot(attack.scytherDot,attackthread)
+    (name,imgtype) = makeImageDot(attack.scytherDot,attackthread)
     # if this is done, store and report
     attack.file = name
-    attack.filetype = type
+    attack.filetype = imgtype
 
 
 def testImage():
diff --git a/gui/Gui/Misc.py b/gui/Gui/Misc.py
index 4f36705..8023fc1 100644
--- a/gui/Gui/Misc.py
+++ b/gui/Gui/Misc.py
@@ -25,7 +25,14 @@
 
 """ Import externals """
 import os.path
-from subprocess import Popen,PIPE
+from subprocess import Popen,PIPE,run
+from shlex import quote
+
+""" Import scyther components """
+from Scyther import FindDot
+
+""" Import scyther-gui components """
+from . import Temporary
 
 #---------------------------------------------------------------------------
 
@@ -67,24 +74,36 @@ def mypath(file):
     basedir = os.path.split(cmd_file)[0]
     return os.path.join(basedir,file)
 
-# commands: push data in, get fp.write out
-def cmdpushwrite(cmd,data,fname):
+# Write string to tempfile, return (filedescriptor,name)
+def stringToTempfile(data,ext="tmp"):
     """
-    Feed stdin data to cmd, write the output to a freshly created file
-    'fname'. The file is flushed and closed at the end.
+    Take data (a string) and write it to a safe temporary file.
+    Return the resulting filedescriptor and name as a pair.
     """
-    fp = open(fname,'w')
-    # execute command
-    p = Popen(cmd, shell=True, stdin=PIPE, stdout=PIPE)
-    (cin,cout) = (p.stdin, p.stdout)
+    (fd,fpname) = Temporary.tempcleaned(ext)
+    f = os.fdopen(fd,'w')
+    f.write(data)
+    f.close()
 
-    cin.write(data)
-    cin.close()
-    for l in cout.read():
-        fp.write(l)
-    cout.close()
-    fp.flush()
-    fp.close()
+    return (fd, fpname)
+
+
+# commands: push data in as file named argument to dot, get fp.write out
+def dotOutputWrite(data,fname,cmd=[]):
+    """
+    Feed stdin data to cmd array, write the output to a freshly
+    created file 'fname'. The file is flushed and closed at the end.
+
+    TODO: In reality, this particular dot data was already written to another temp file when rendering the attack graph. We should be reusing that file instead of writing a new one.
+    """
+    (fd_in,fpname_in) = stringToTempfile(data,ext="dot")
+
+    dotcommand = FindDot.findDot()
+    execcmd = [dotcommand] + cmd + ["-o" + quote(fname), quote(fpname_in)]
+    print (execcmd)
+
+    # execute command
+    run(execcmd)
 
 #---------------------------------------------------------------------------
 # vim: set ts=4 sw=4 et list lcs=tab\:>-: