Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

del os.environ[key] ignores errors #57624

Closed
vstinner opened this issue Nov 16, 2011 · 13 comments
Closed

del os.environ[key] ignores errors #57624

vstinner opened this issue Nov 16, 2011 · 13 comments
Labels
OS-windows topic-unicode

Comments

@vstinner
Copy link
Member

BPO 13415
Nosy @amauryfa, @vstinner, @ezio-melotti
Files
  • unsetenv.patch
  • broken_unsetenv.diff
    		•

    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 2011-11-24.12:52:50.675>
created_at = <Date 2011-11-16.22:14:47.392>
labels = ['expert-unicode', 'OS-windows']
title = 'del os.environ[key] ignores errors'
updated_at = <Date 2011-11-28.06:33:13.884>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2011-11-28.06:33:13.884>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2011-11-24.12:52:50.675>
closer = 'python-dev'
components = ['Unicode', 'Windows']
creation = <Date 2011-11-16.22:14:47.392>
creator = 'vstinner'
dependencies = []
files = ['23714', '23764']
hgrepos = []
issue_num = 13415
keywords = ['patch', 'needs review']
message_count = 13.0
messages = ['147773', '147781', '147783', '148145', '148151', '148172', '148176', '148242', '148245', '148387', '148426', '148450', '148451']
nosy_count = 5.0
nosy_names = ['amaury.forgeotdarc', 'vstinner', 'ezio.melotti', 'neologix', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue13415'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']
    @vstinner
    Copy link
    Member Author

    os.unsetenv(name) encodes name to UTF-8. I think that the ANSI code page (or another code page?) should be used.

    @amauryfa
    Copy link
    Member

    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, "")

    @vstinner
    Copy link
    Member Author

    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.

    @vstinner vstinner changed the title os.unsetenv() on Windows should not use UTF-8 del os.environ[key] ignores errors Nov 17, 2011
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 22, 2011

    New changeset 3e892f428278 by Victor Stinner in branch '3.2':
    Issue bpo-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 bpo-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 bpo-13415: os.unsetenv() doesn't ignore errors anymore.
    http://hg.python.org/cpython/rev/53cf6f9f374e

    @vstinner
    Copy link
    Member Author

    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

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Nov 23, 2011

    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.

    @vstinner
    Copy link
    Member Author

    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.

    @vstinner vstinner changed the title del os.environ[key] ignores errors del os.environ Nov 23, 2011
    @vstinner vstinner changed the title del os.environ del os.environ[key] ignores errors Nov 24, 2011
    @vstinner
    Copy link
    Member Author

    Using broken_unsetenv.diff + autoconf, Python compiles correctly on Mac OS X Tiger.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 24, 2011

    New changeset 1b83fd683e28 by Victor Stinner in branch 'default':
    Close bpo-13415: Test in configure if unsetenv() has a return value or not.
    http://hg.python.org/cpython/rev/1b83fd683e28

    @python-dev python-dev mannequin closed this as completed Nov 24, 2011
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 26, 2011

    New changeset 23ed66484ff2 by Charles-François Natali in branch 'default':
    Issue bpo-13415: Skip test_os.test_unset_error on FreeBSD < 7 and OS X < 10.6
    http://hg.python.org/cpython/rev/23ed66484ff2

    @vstinner
    Copy link
    Member Author

    Oh, thanks Charles François for your two patches.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 27, 2011

    New changeset 26275a1c229c by Charles-François Natali in branch '3.2':
    Issue bpo-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 bpo-13415: Skip test_os.test_unset_error on FreeBSD and OS X.
    http://hg.python.org/cpython/rev/bceb6aea8554

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 27, 2011

    New changeset 11bbeaf03894 by Charles-François Natali in branch '2.7':
    Issue bpo-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 bpo-13415: Skip test_os.test_unset_error on FreeBSD and OS X.
    http://hg.python.org/cpython/rev/99f5a0475ead

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    OS-windows topic-unicode
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @vstinner @amauryfa