Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
Fixing python 2.7 windows unicode issue with ``subprocess.Popen``.
# -*- coding: utf-8 -*-
## Copyright (C) 2021 Valentin Lab
##
## Redistribution and use in source and binary forms, with or without
## modification, are permitted provided that the following conditions
## are met:
##
## 1. Redistributions of source code must retain the above copyright
## notice, this list of conditions and the following disclaimer.
##
## 2. Redistributions in binary form must reproduce the above
## copyright notice, this list of conditions and the following
## disclaimer in the documentation and/or other materials provided
## with the distribution.
##
## 3. Neither the name of the copyright holder nor the names of its
## contributors may be used to endorse or promote products derived
## from this software without specific prior written permission.
##
## THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
## "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
## LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
## FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
## COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
## INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
## (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
## SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
## HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
## STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
## ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
## OF THE POSSIBILITY OF SUCH DAMAGE.
##
## issue: https://bugs.python.org/issue19264
import ctypes
import subprocess
import _subprocess
from ctypes import byref, windll, c_char_p, c_wchar_p, c_void_p, \
Structure, sizeof, c_wchar, WinError
from ctypes.wintypes import BYTE, WORD, LPWSTR, BOOL, DWORD, LPVOID, \
HANDLE
##
## Types
##
CREATE_UNICODE_ENVIRONMENT = 0x00000400
LPCTSTR = c_char_p
LPTSTR = c_wchar_p
LPSECURITY_ATTRIBUTES = c_void_p
LPBYTE = ctypes.POINTER(BYTE)
class STARTUPINFOW(Structure):
_fields_ = [
("cb", DWORD), ("lpReserved", LPWSTR),
("lpDesktop", LPWSTR), ("lpTitle", LPWSTR),
("dwX", DWORD), ("dwY", DWORD),
("dwXSize", DWORD), ("dwYSize", DWORD),
("dwXCountChars", DWORD), ("dwYCountChars", DWORD),
("dwFillAtrribute", DWORD), ("dwFlags", DWORD),
("wShowWindow", WORD), ("cbReserved2", WORD),
("lpReserved2", LPBYTE), ("hStdInput", HANDLE),
("hStdOutput", HANDLE), ("hStdError", HANDLE),
]
LPSTARTUPINFOW = ctypes.POINTER(STARTUPINFOW)
class PROCESS_INFORMATION(Structure):
_fields_ = [
("hProcess", HANDLE), ("hThread", HANDLE),
("dwProcessId", DWORD), ("dwThreadId", DWORD),
]
LPPROCESS_INFORMATION = ctypes.POINTER(PROCESS_INFORMATION)
class DUMMY_HANDLE(ctypes.c_void_p):
def __init__(self, *a, **kw):
super(DUMMY_HANDLE, self).__init__(*a, **kw)
self.closed = False
def Close(self):
if not self.closed:
windll.kernel32.CloseHandle(self)
self.closed = True
def __int__(self):
return self.value
CreateProcessW = windll.kernel32.CreateProcessW
CreateProcessW.argtypes = [
LPCTSTR, LPTSTR, LPSECURITY_ATTRIBUTES,
LPSECURITY_ATTRIBUTES, BOOL, DWORD, LPVOID, LPCTSTR,
LPSTARTUPINFOW, LPPROCESS_INFORMATION,
]
CreateProcessW.restype = BOOL
##
## Patched functions/classes
##
def CreateProcess(executable, args, _p_attr, _t_attr,
inherit_handles, creation_flags, env, cwd,
startup_info):
"""Create a process supporting unicode executable and args for win32
Python implementation of CreateProcess using CreateProcessW for Win32
"""
si = STARTUPINFOW(
dwFlags=startup_info.dwFlags,
wShowWindow=startup_info.wShowWindow,
cb=sizeof(STARTUPINFOW),
## XXXvlab: not sure of the casting here to ints.
hStdInput=int(startup_info.hStdInput),
hStdOutput=int(startup_info.hStdOutput),
hStdError=int(startup_info.hStdError),
)
wenv = None
if env is not None:
## LPCWSTR seems to be c_wchar_p, so let's say CWSTR is c_wchar
env = (unicode("").join([
unicode("%s=%s\0") % (k, v)
for k, v in env.items()])) + unicode("\0")
wenv = (c_wchar * len(env))()
wenv.value = env
pi = PROCESS_INFORMATION()
creation_flags |= CREATE_UNICODE_ENVIRONMENT
if CreateProcessW(executable, args, None, None,
inherit_handles, creation_flags,
wenv, cwd, byref(si), byref(pi)):
return (DUMMY_HANDLE(pi.hProcess), DUMMY_HANDLE(pi.hThread),
pi.dwProcessId, pi.dwThreadId)
raise WinError()
class Popen(subprocess.Popen):
"""This superseeds Popen and corrects a bug in cPython 2.7 implem"""
def _execute_child(self, args, executable, preexec_fn, close_fds,
cwd, env, universal_newlines,
startupinfo, creationflags, shell, to_close,
p2cread, p2cwrite,
c2pread, c2pwrite,
errread, errwrite):
"""Code from part of _execute_child from Python 2.7 (9fbb65e)
There are only 2 little changes concerning the construction of
the the final string in shell mode: we preempt the creation of
the command string when shell is True, because original function
will try to encode unicode args which we want to avoid to be able to
sending it as-is to ``CreateProcess``.
"""
if not isinstance(args, subprocess.types.StringTypes):
args = subprocess.list2cmdline(args)
if startupinfo is None:
startupinfo = subprocess.STARTUPINFO()
if shell:
startupinfo.dwFlags |= _subprocess.STARTF_USESHOWWINDOW
startupinfo.wShowWindow = _subprocess.SW_HIDE
comspec = os.environ.get("COMSPEC", unicode("cmd.exe"))
args = unicode('{} /c "{}"').format(comspec, args)
if (_subprocess.GetVersion() >= 0x80000000 or
os.path.basename(comspec).lower() == "command.com"):
w9xpopen = self._find_w9xpopen()
args = unicode('"%s" %s') % (w9xpopen, args)
creationflags |= _subprocess.CREATE_NEW_CONSOLE
super(Popen, self)._execute_child(args, executable,
preexec_fn, close_fds, cwd, env, universal_newlines,
startupinfo, creationflags, False, to_close, p2cread,
p2cwrite, c2pread, c2pwrite, errread, errwrite)
_subprocess.CreateProcess = CreateProcess
@awei78

This comment has been minimized.

Copy link

@awei78 awei78 commented Oct 18, 2017

GREAT! It solved my problem. thanks!

@JokerQyou

This comment has been minimized.

Copy link

@JokerQyou JokerQyou commented Dec 22, 2017

Actually there is an issue in this patch. Since you patched _subprocess.CreateProcess, multiprocessing module will start using your version of CreateProcess now, and will fail at line 85, where startup_info could be None.

@Wimpie-ccc

This comment has been minimized.

Copy link

@Wimpie-ccc Wimpie-ccc commented Jan 26, 2018

Thanks for this! I am forced to use python 2.7, so this saved not only my day, but my whole project!

PS: needed to add an "import os" for the "os.environ." & "os.path." calls...

@vaab

This comment has been minimized.

Copy link
Owner Author

@vaab vaab commented Feb 28, 2018

@JokerQyou sorry for the late answer. Can you make a little test script to illustrate the problem, that would help tremendously for thinking about a fix. As of now, I have no idea of the context and why startup_info can be None. I'm sure this will make more sense with your example. Thanks for you report !

@robhagemans

This comment has been minimized.

Copy link

@robhagemans robhagemans commented Mar 14, 2018

Very useful, thanks a lot!

I couldn't find a licence for your code on the page, did you have one in mind?

@KeithProctor

This comment has been minimized.

Copy link

@KeithProctor KeithProctor commented Oct 16, 2018

I'm not sure of the usage of this. I tried the following: 1) create file called win_subprocess.py 3) copy the contents above into it 3) included standard python header with utf-8 usage. 4) Use an if statement to include this file ONLY on Windows (below). 5) Include standard subprocess for any other platform 6) I also moved my library calls over to Popen. 7) make no other changes to my code?

I haven't had to do this type of modification in Python before. This type of thing is almost normal with #defines in C or C++. Can somebody confirm my steps? I'll try what I said below but thought this a good comment for others that follow.

@KeithProctor

This comment has been minimized.

Copy link

@KeithProctor KeithProctor commented Oct 16, 2018

For step 4 I do this:

sysPlat = sys.platform'
    if ( sysPlat == 'win32' or sysPlat == 'win64' ):
        import win_subprocess'
        from win_subprocess import CalledProcessError
    else:
        import subprocess
        from subprocess import CalledProcessError 

I'm about to move over to the Windows side and see if it works. :)

Could not get the patch to work. I guess my steps where not enough.

@robhagemans

This comment has been minimized.

Copy link

@robhagemans robhagemans commented Nov 3, 2018

@KeithProctor, you just need to import win_subprocess at the start of your code for Windows platforms, after that use subprocess as normal. Something like this, based on what I think the above aims to do:

if sys.platform == 'win32':
    import win_subprocess
import subprocess
from subprocess import CalledProcessError 

Note that there is no 'win64' platform string - 32-bit and 64-bit Windows are both 'win32'. There's also something weird going on with quotes and leading whitespace in your code that would make it break.

@virtualnobi

This comment has been minimized.

Copy link

@virtualnobi virtualnobi commented Nov 14, 2018

Very cool - fixes the issue with minimal effort. Thanks a lot!

@virtualnobi

This comment has been minimized.

Copy link

@virtualnobi virtualnobi commented Nov 14, 2018

Upps, a little too ecstatic... Now subprocess.call() uses the new function as well, but it breaks because all stdin, stdout, stderr are None. It dies in line 91, trying to convert None to an int(). If I just do call(['ls', '-l']), all three streams are None, and deliberately stay None (line 816, in _get_handles()). Is call() now unusable in general?

@exarkun

This comment has been minimized.

Copy link

@exarkun exarkun commented Feb 9, 2021

@vaab I wonder if you'd mind sticking a GPL-compatible license on this so I can use it in tahoe-lafs/tahoe-lafs to replace some gross unicode hacks?

@vaab

This comment has been minimized.

Copy link
Owner Author

@vaab vaab commented Feb 9, 2021

@exarkun Here you are. Is that good for you ?

@exarkun

This comment has been minimized.

Copy link

@exarkun exarkun commented Feb 9, 2021

@vaab That is perfect. Thank you kindly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment