platform.linux_distribution() should decode files from UTF-8, not from the locale encoding

[abadger]

BPO	17429
Nosy	@loewis, @vstinner, @abadger, @bitdancer
Files	00175-platform-unicode.patch: Patch to use surrogateescape when reading from OS vendor provided file 00175-platform-unicode.patch: Patch to default to utf-8 and fallback to surrogateescape when reading from OS vendor provided file 00175-platform-unicode.patch: 3rd version of the patch. Includes a unittest and some re-arranging of the code to allow the unittest to work. 00175-platform-unicode.patch: 4th version of patch w/ NEWS; rebased against hg default 00175-platform-unicode.patch: 5th version of patch fixing issues raised by r.david.murray

^{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 = None
closed_at = <Date 2013-12-08.23:26:22.834>
created_at = <Date 2013-03-15.15:08:30.031>
labels = ['type-bug']
title = 'platform.linux_distribution() should decode files from UTF-8, not from the locale encoding'
updated_at = <Date 2013-12-08.23:28:41.697>
user = 'https://github.com/abadger'

bugs.python.org fields:

activity = <Date 2013-12-08.23:28:41.697>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2013-12-08.23:26:22.834>
closer = 'vstinner'
components = []
creation = <Date 2013-03-15.15:08:30.031>
creator = 'a.badger'
dependencies = []
files = ['29416', '29420', '29508', '29509', '29573']
hgrepos = []
issue_num = 17429
keywords = ['patch']
message_count = 15.0
messages = ['184234', '184241', '184267', '184306', '184319', '184634', '184776', '184783', '184918', '185225', '200260', '205616', '205618', '205619', '205620']
nosy_count = 6.0
nosy_names = ['loewis', 'vstinner', 'a.badger', 'r.david.murray', 'zbysz', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'patch review'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue17429'
versions = ['Python 3.3', 'Python 3.4']

[abadger]

Tested on python-3.2 and python-3.3. platform.platform() looks for a file in /etc/ that looks like it will contain the name of the Linux distribution that python3 is running on. Once found, it reads the contents of the file to have a name for the Linux distribution. Most Linux distributions do create files inside of /etc/ with a single line which is the distribution name so this is a good heuristic. However, these files are created by the operating system vendor and so they can have a different encoding than the encoding of the locale the user uses. This means that if there are non-ascii characters inside the file, user code that invokes platform.platform() may throw a traceback.

Test:

$ LC_ALL=en_US.utf8 sudo echo ' Café' >> /etc/fedora-release
$ LC_ALL=C python3
>>> import platform
>>> platform.platform()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python3.2/platform.py", line 1538, in platform
    distname,distversion,distid = dist('')
  File "/usr/lib64/python3.2/platform.py", line 358, in dist
    full_distribution_name=0)
  File "/usr/lib64/python3.2/platform.py", line 329, in linux_distribution
    firstline = f.readline()
  File "/usr/lib64/python3.2/encodings/ascii.py", line 26, in decode
    return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 22: ordinal not in range(128)

It seems that the standard method of fixing these that we're promoting in python3 is to use surrogateescape. I'll provide a patch that does that.

[loewis]

What's the most likely encoding? UTF-8? I suggest we assume UTF-8, and use the surrogate-escape error handler to deal with the cases when it isn't.

[abadger]

I agree. In my experience, utf-8 is the most common encoding. Updated patch that defaults to utf-8 instead of the user's locale is attached.

[vstinner]

00175-platform-unicode.patch looks good to me, but it lacks an unit test.

[abadger]

I'm at pycon. I'll find someone during the sprints to teach me how the unittests are organized.

[zbysz]

At least for /etc/os-release, which is slated to replace /etc/fedora-release and other distribution specific files, the encoding in mandated to be UTF-8:

http://www.freedesktop.org/software/systemd/man/os-release.html
All strings should be in UTF-8 format, and non-printable characters
should not be used.

[abadger]

Okay, new version of the patch with a unittest.

Re: os-release; I don't believe the current code can handle that file. i\It changes format from a simple string (in most Linux distros) to key value pairs. We'll probably need an update to the code to deal with that at some point in the future.

[abadger]

Added NEWS file. Rebased against hg default. Ready for review.

[bitdancer]

Review comments added.

[abadger]

Patch fixing the issues raised in r.david.murray's review:

• Merged _find_linux_release_file() back into linux_distribution() and broke out _UNIXCONFDIR module level variable to enable mocking of the unittest data

• Fix already present style issue in linux_distribution() code

• Separate test_dist_with_unicode() into test_dist_with_utf8 and test_dist_with_latin1

• Moved test data into the setUp() method and broke long lines to under 79 chars

• Switched from NamedTempfile() to TemporaryDirectory() => I think this fixes the Windows incompatibility but I can mark the tests as Skip on Windows if that isn't true (I don't have a Windows box to test on).

• Removed Misc/NEWS portion of patch

• I've switched from os.environ['LC_ALL'] = temp_locale to setlocale(locale.LC_ALL, temp_locale). Testing showed that the former did not provoke the bug while the latter does. Could someone point me at documentation on the difference between these two? I'd like to understand what the two different calls do differently so that I use them correctly in other unittests.

[vstinner]

Ping myself, I just installed Fedora 19 and I cannot run the Python test suite with the ASCII locale encoding, because of this bug.

[python-dev]

New changeset 4580976c07cb by Victor Stinner in branch '3.3':
Issue bpo-17429: platform.linux_distribution() now decodes files from the UTF-8
http://hg.python.org/cpython/rev/4580976c07cb

New changeset 407f18c8ce8a by Victor Stinner in branch 'default':
(Merge 3.3) Issue bpo-17429: platform.linux_distribution() now decodes files from
http://hg.python.org/cpython/rev/407f18c8ce8a

[python-dev]

New changeset 831b2c80a9c9 by Victor Stinner in branch 'default':
Issue bpo-17429: some PEP-8 compliance fixes for the platform modules, add whitespaces
http://hg.python.org/cpython/rev/831b2c80a9c9

[vstinner]

Thanks Toshio Kuratomi for your patch. I simplified the unit test. I'm not sure that "resetlocale" restores the locale in its previous state. I don't want to rely on two specific locales ('pt_BR.UTF8' and 'pt_BR.ISO8859-1') for a such simple test. We have enough buildbots to test other various locales.

I prefered to only change minor syntax issues (PEP-8) in Python 3.4 to limit changes in the stable release.