classification
Title: del os.environ[key] ignores errors
Type: Stage: resolved
Components: Unicode, Windows Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, ezio.melotti, haypo, neologix, python-dev
Priority: normal Keywords: needs review, patch

Created on 2011-11-16 22:14 by haypo, last changed 2011-11-28 06:33 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
unsetenv.patch haypo, 2011-11-17 00:45 review
broken_unsetenv.diff neologix, 2011-11-23 18:43 review
Messages (13)
msg147773 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-11-16 22:14
os.unsetenv(name) encodes name to UTF-8. I think that the ANSI code page (or another code page?) should be used.
msg147781 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-11-17 00:06
But... there is no os.unsetenv on Windows!
2.7 used to have one, which called os.putenv(key, "")
3.2 has a os._unsetenv, which is a lambda key: _putenv(key, "")
msg147783 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-11-17 00:45
> But... there is no os.unsetenv on Windows!

Correct, even unsetenv() doesn't exist on Windows: putenv() can be used to unset a variable using an empty value. And it's exactly what Python does.

It is confusing because posix_unsetenv() is not build on Windows, but it contains code specific for Windows.

While testing del os.environ[key], I found another bug: del os.environ['x'*50000] does crash Python on Windows.

Attached patch (for Python 3.3) does:

 - Remove the Windows specific code from posix_unsetenv()
 - Check if unsetenv() failed on UNIX
 - Check environment variable length on Windows

The Windows bug does affect Python 2.7 too. "Check if unsetenv() failed on UNIX" change may be skipped on Python 2.7 and 3.2.
msg148145 - (view) Author: Roundup Robot (python-dev) Date: 2011-11-22 21:22
New changeset 3e892f428278 by Victor Stinner in branch '3.2':
Issue #13415: os.unsetenv() doesn't ignore errors anymore.
http://hg.python.org/cpython/rev/3e892f428278

New changeset aa55b7dc43f7 by Victor Stinner in branch 'default':
(Merge 3.2) Issue #13415: os.unsetenv() doesn't ignore errors anymore.
http://hg.python.org/cpython/rev/aa55b7dc43f7

New changeset 53cf6f9f374e by Victor Stinner in branch '2.7':
Issue #13415: os.unsetenv() doesn't ignore errors anymore.
http://hg.python.org/cpython/rev/53cf6f9f374e
msg148151 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-11-22 21:47
Oh, it looks like unsetenv() has no return value on Mac OS X Tiger:

http://www.python.org/dev/buildbot/all/builders/PPC%20Tiger%202.7/builds/100/steps/compile/logs/stdio

./Modules/posixmodule.c: In function 'posix_unsetenv':
./Modules/posixmodule.c:7052: error: void value not ignored as it ought to be
make: *** [Modules/posixmodule.o] Error 1
msg148172 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-11-23 08:22
> Oh, it looks like unsetenv() has no return value on Mac OS X Tiger

And neither does FreeBSD < 7:
http://python.org/dev/buildbot/all/builders/x86%20FreeBSD%206.4%203.x/builds/2015/steps/compile/logs/stdio

Note that ignoring unsetenv() return value is a bad idea:
http://xorl.wordpress.com/category/freebsd/page/2/

We could maybe add a configure-time check.
msg148176 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-11-23 09:42
> Note that ignoring unsetenv() return value is a bad idea:
> http://xorl.wordpress.com/category/freebsd/page/2/

Oh yeah, I remember this critical (local) vulnerability!

> We could maybe add a configure-time check.

Yes, it sounds like the best solution.
msg148242 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-11-24 11:12
Using broken_unsetenv.diff + autoconf, Python compiles correctly on Mac OS X Tiger.
msg148245 - (view) Author: Roundup Robot (python-dev) Date: 2011-11-24 12:52
New changeset 1b83fd683e28 by Victor Stinner in branch 'default':
Close #13415: Test in configure if unsetenv() has a return value or not.
http://hg.python.org/cpython/rev/1b83fd683e28
msg148387 - (view) Author: Roundup Robot (python-dev) Date: 2011-11-26 10:33
New changeset 23ed66484ff2 by Charles-François Natali in branch 'default':
Issue #13415: Skip test_os.test_unset_error on FreeBSD < 7 and OS X < 10.6
http://hg.python.org/cpython/rev/23ed66484ff2
msg148426 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-11-26 23:00
Oh, thanks Charles François for your two patches.
msg148450 - (view) Author: Roundup Robot (python-dev) Date: 2011-11-27 11:51
New changeset 26275a1c229c by Charles-François Natali in branch '3.2':
Issue #13415: Test in configure if unsetenv() has a return value or not.
http://hg.python.org/cpython/rev/26275a1c229c

New changeset bceb6aea8554 by Charles-François Natali in branch '3.2':
Issue #13415: Skip test_os.test_unset_error on FreeBSD and OS X.
http://hg.python.org/cpython/rev/bceb6aea8554
msg148451 - (view) Author: Roundup Robot (python-dev) Date: 2011-11-27 12:05
New changeset 11bbeaf03894 by Charles-François Natali in branch '2.7':
Issue #13415: Test in configure if unsetenv() has a return value or not.
http://hg.python.org/cpython/rev/11bbeaf03894

New changeset 99f5a0475ead by Charles-François Natali in branch '2.7':
Issue #13415: Skip test_os.test_unset_error on FreeBSD and OS X.
http://hg.python.org/cpython/rev/99f5a0475ead
History
Date User Action Args
2011-11-28 06:33:13hayposetmessages: - msg148467
2011-11-28 06:23:53python-devsetmessages: + msg148467
2011-11-27 12:05:50python-devsetmessages: + msg148451
2011-11-27 11:51:35python-devsetmessages: + msg148450
2011-11-26 23:21:09hayposetmessages: - msg148428
2011-11-26 23:20:16python-devsetmessages: + msg148428
2011-11-26 23:00:48hayposetmessages: + msg148426
2011-11-26 10:33:20python-devsetmessages: + msg148387
2011-11-24 12:52:50python-devsetstatus: open -> closed
resolution: fixed
messages: + msg148245

stage: patch review -> resolved
2011-11-24 11:12:54hayposetmessages: + msg148242
2011-11-24 00:17:18hayposettitle: del os.environ -> del os.environ[key] ignores errors
2011-11-23 18:43:32neologixsetkeywords: + needs review
files: + broken_unsetenv.diff
stage: patch review
2011-11-23 09:42:19hayposetmessages: + msg148176
title: del os.environ[key] ignores errors -> del os.environ
2011-11-23 08:22:43neologixsetnosy: + neologix
messages: + msg148172
2011-11-22 21:47:36hayposetmessages: + msg148151
2011-11-22 21:22:07python-devsetnosy: + python-dev
messages: + msg148145
2011-11-17 00:45:42hayposetfiles: + unsetenv.patch
versions: + Python 2.7
title: os.unsetenv() on Windows should not use UTF-8 -> del os.environ[key] ignores errors
messages: + msg147783

keywords: + patch
2011-11-17 00:06:55amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg147781
2011-11-16 22:14:47haypocreate