daniellockyer / apkdiff

Diff between two APK files.
121 stars 24 forks source link

Issues on Windows (and other platforms), is `shell=True` required? #13

Open IanGallacher opened 1 year ago

IanGallacher commented 1 year ago

I believe that the current apkdiff.py is broken on windows (tested on 64 bit win10 with 64 bit python 3.11). I always get the following output:

C:\Users\dev\Downloads\test_dir_for_safety>python C:\Users\dev\Documents\GitHub\apkdiff\apkdiff.py C:\Users\dev\Downloads\file1.apk  C:\Users\dev\Downloads\file2.apk

                         apktool

Running apktool against 'C:\Users\dev\Downloads\file1.apk'
Traceback (most recent call last):
  File "C:\Users\dev\Documents\GitHub\apkdiff\apkdiff.py", line 194, in <module>
    main()
  File "C:\Users\dev\Documents\GitHub\apkdiff\apkdiff.py", line 61, in main
    apktoolit(args.apk1, temp1)
  File "C:\Users\dev\Documents\GitHub\apkdiff\apkdiff.py", line 81, in apktoolit
    call(["apktool", "d", "-r", "-f", "-o", dir, file], stdout=open(os.devnull, 'w'), stderr=STDOUT)
  File "C:\Users\dev\AppData\Local\Programs\Python\Python311\Lib\subprocess.py", line 389, in call
    with Popen(*popenargs, **kwargs) as p:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\dev\AppData\Local\Programs\Python\Python311\Lib\subprocess.py", line 1024, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "C:\Users\dev\AppData\Local\Programs\Python\Python311\Lib\subprocess.py", line 1493, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [WinError 2] The system cannot find the file specified

I believe I'm able to fix the issue if I change what is currently line 81 from

call(["apktool", "d", "-r", "-f", "-o", dir, file], stdout=open(os.devnull, 'w'), stderr=STDOUT)

to

call(["apktool", "d", "-r", "-f", "-o", dir, file], stdout=open(os.devnull, 'w'), stderr=STDOUT, shell=True)

or

call(["cmd", "/c", "apktool", "d", "-r", "-f", "-o", dir, file], stdout=open(os.devnull, 'w'), stderr=STDOUT)

Obviously, the "cmd", "/c" is windows-exclusive and isn't cross-platform. However, shell=True might be safe to add.

I looked at making a PR to add shell=True, but I was surprised to see that there are already a few PRs and issues that dabble with that have dabbled with shell=True.

It sounds like shell=True is required for Ubuntu according to #4, and in my experience it is also required on Windows.

PR #9 mentions that breaks "most other operating systems".

I'm raising this issue to see if it is safe to re-add shell=True. If you know of any issues or specific problematic operating systems with adding shell=True, please call them out.

This might not be the ideal fix, but I imagine it would be good if we got the tool working out of the box on Windows.