distutils._msvccompiler._get_vc_env() fails with UnicodeDecodeError if an env var is not encodable

[Vgr255]

BPO	27048
Nosy	@pfmoore, @vstinner, @tjguk, @ezio-melotti, @merwok, @bitdancer, @vadmium, @zware, @eryksun, @zooba, @dstufft, @Vgr255
Files	subprocess_errors_simple_1.patch subprocess_distutils_errors_1.patch subprocess_errors_simple_2.patch subprocess_distutils_errors_2.patch subprocess_distutils_errors_3.patch distutils_windows_non_ascii_1.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/zooba'
closed_at = <Date 2016-06-17.16:36:07.104>
created_at = <Date 2016-05-17.20:09:12.783>
labels = ['type-bug', 'library', 'expert-unicode', 'OS-windows']
title = 'distutils._msvccompiler._get_vc_env() fails with UnicodeDecodeError if an env var is not encodable'
updated_at = <Date 2016-06-17.19:38:41.701>
user = 'https://github.com/Vgr255'

bugs.python.org fields:

activity = <Date 2016-06-17.19:38:41.701>
actor = 'eryksun'
assignee = 'steve.dower'
closed = True
closed_date = <Date 2016-06-17.16:36:07.104>
closer = 'steve.dower'
components = ['Distutils', 'Library (Lib)', 'Unicode', 'Windows']
creation = <Date 2016-05-17.20:09:12.783>
creator = 'abarry'
dependencies = []
files = ['42880', '42881', '42882', '42883', '42884', '42885']
hgrepos = []
issue_num = 27048
keywords = ['patch']
message_count = 18.0
messages = ['265774', '265775', '265776', '265777', '265778', '265781', '265783', '265784', '265786', '265788', '265790', '265791', '266062', '266071', '268732', '268733', '268734', '268737']
nosy_count = 13.0
nosy_names = ['paul.moore', 'vstinner', 'tim.golden', 'ezio.melotti', 'eric.araujo', 'r.david.murray', 'python-dev', 'martin.panter', 'zach.ware', 'eryksun', 'steve.dower', 'dstufft', 'abarry']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue27048'
versions = ['Python 3.5', 'Python 3.6']

[Vgr255]

I got a random UnicodeDecodeError while trying to install a module with distutils. Traced it back and it being my name having a non-ascii character floating around in my environment.

I'm including two patches: subprocess_errors_simple_1.patch simply adds 'errors="replace"' to the stdin, stdout and stderr, while subprocess_distutils_errors_1.patch fixes this a bit more thoroughly.

I haven't added any tests yet, I'll add them to the proper patch when a concensus on which one to use is reached.

[bitdancer]

I would not think errors=replace would be the right answer...that would lose information, wouldn't it? What is the data being used for that causes the problem? Normally in situations like this we'd use surrogateescape at the system boundary, but I don't know enough about distutils to know if that is the right answer.

[Vgr255]

The data that causes the issue is my name (Émanuel), which is present in a couple of places in my environment. I also completely forgot about surrogateescape, my bad. Here are two new patches with the surrogateescape error handler instead of the replace one.

[vstinner]

Hi,

I got a random UnicodeDecodeError while trying to install a module with distutils.

Please provide more context to your issue:

• What is your operating system?
• What is your locale? (locale encoding on UNIX, ANSI and OEM code pages on Windows)
• Where does the data come from?
• etc.

subprocess_errors_simple_1.patch

I don't think that it's ok to change the *default* error handler in the subprocess module.

Adding encoding/errors parameters to subprocess.Popen is an old topic: see for example the issue bpo-6135.

[Vgr255]

Oops, I thought I had written that - must have accidentally deleted it. I'm on Windows 7, my locale is fr-CA / cp1252, and the non-ascii data (my name =) comes from my environment.

I discussed a bit with R. David Murray off-issue, and in the end it seems that it would be better to have subprocess accept parameters, but not do anything by default, and then have distutils pass these parameters.

Here is a new patch.

[vstinner]

Copy of https://gist.github.com/Vgr255/67fc0245056454d6a68608790a9d417b

E:\LGP_Test\LGP-0.1>py setup.py install
running install
running build
running build_ext
building 'lgp' extension
Traceback (most recent call last):
  File "setup.py", line 10, in <module>
    Extension("lgp", ["src/lgpmodule.c"]),
  File "E:\Python\3.5.0\lib\distutils\core.py", line 148, in setup
    dist.run_commands()
  File "E:\Python\3.5.0\lib\distutils\dist.py", line 955, in run_commands
    self.run_command(cmd)
  File "E:\Python\3.5.0\lib\distutils\dist.py", line 974, in run_command
    cmd_obj.run()
  File "E:\Python\3.5.0\lib\distutils\command\install.py", line 539, in run
    self.run_command('build')
  File "E:\Python\3.5.0\lib\distutils\cmd.py", line 313, in run_command
    self.distribution.run_command(command)
  File "E:\Python\3.5.0\lib\distutils\dist.py", line 974, in run_command
    cmd_obj.run()
  File "E:\Python\3.5.0\lib\distutils\command\build.py", line 135, in run
    self.run_command(cmd_name)
  File "E:\Python\3.5.0\lib\distutils\cmd.py", line 313, in run_command
    self.distribution.run_command(command)
  File "E:\Python\3.5.0\lib\distutils\dist.py", line 974, in run_command
    cmd_obj.run()
  File "E:\Python\3.5.0\lib\distutils\command\build_ext.py", line 338, in run
    self.build_extensions()
  File "E:\Python\3.5.0\lib\distutils\command\build_ext.py", line 447, in build_
extensions
    self._build_extensions_serial()
  File "E:\Python\3.5.0\lib\distutils\command\build_ext.py", line 472, in _build
_extensions_serial
    self.build_extension(ext)
  File "E:\Python\3.5.0\lib\distutils\command\build_ext.py", line 532, in build_
extension
    depends=ext.depends)
  File "E:\Python\3.5.0\lib\distutils\_msvccompiler.py", line 317, in compile
    self.initialize()
  File "E:\Python\3.5.0\lib\distutils\_msvccompiler.py", line 210, in initialize

    vc_env = _get_vc_env(plat_spec)
  File "E:\Python\3.5.0\lib\distutils\_msvccompiler.py", line 92, in _get_vc_env

    universal_newlines=True,
  File "E:\Python\3.5.0\lib\subprocess.py", line 629, in check_output
    **kwargs).stdout
  File "E:\Python\3.5.0\lib\subprocess.py", line 698, in run
    stdout, stderr = process.communicate(input, timeout=timeout)
  File "E:\Python\3.5.0\lib\subprocess.py", line 1055, in communicate
    stdout = self.stdout.read()
  File "E:\Python\3.5.0\lib\encodings\cp1252.py", line 23, in decode
    return codecs.charmap_decode(input,self.errors,decoding_table)[0]
UnicodeDecodeError: 'charmap' codec can't decode byte 0x90 in position 49: chara
cter maps to <undefined>

[vstinner]

File "E:\Python\3.5.0\lib\distutils\_msvccompiler.py", line 92, in _get_vc_env
universal_newlines=True,

This function was added in Python 3.5 by the issue bpo-23970: "Adds distutils._msvccompiler for new Visual Studio versions." (VS 2015, I guess). The module distutils._msvccompiler doesn't exist in Python 3.5.

[vstinner]

The module distutils._msvccompiler doesn't exist in Python 3.5.

Oops, I mean: it *only* exists in Python 3.5 (and newer, it doesn't exist in Python 3.4).

[vstinner]

Python 3.4 has a similar function: query_vcvarsall() in Lib/distutils/msvc9compiler.py.

[vadmium]

IMO adding a Popen(errors=...) parameter would be a new feature for 3.6+ only. Also, the patch does not take the error handler into account when encoding and decoding multiple PIPEs in communicate().

I think it would be better to fix this bug in Lib/distutils/_msvccompiler.py only. The equivalent bug fix would look like:

out = subprocess.check_output(
    ...,
    stderr=subprocess.STDOUT,
    # No universal_newlines!
)
...
out = io.TextIOWrapper(io.BytesIO(out), errors="surrogateescape").read()

Or maybe a simpler version is sufficient: (I’m not familiar enough with the use case or Windows to say.)

out = out.decode("ascii", "surrogateescape")