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

Windows Python cannot handle an early PATH entry containing ".." and python.exe #76638

Closed
mingwandroid mannequin opened this issue Dec 30, 2017 · 8 comments
Closed

Windows Python cannot handle an early PATH entry containing ".." and python.exe #76638

mingwandroid mannequin opened this issue Dec 30, 2017 · 8 comments
Assignees
@zooba
Labels
3.7 (EOL) end of life 3.8 only security fixes OS-windows type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@mingwandroid
Copy link
Mannequin

mingwandroid mannequin commented Dec 30, 2017

BPO 32457
Nosy @pfmoore, @tjguk, @zware, @eryksun, @zooba, @mingwandroid, @miss-islington
PRs
  • bpo-32457: Improves handling of denormalized executable path when launching Python #5756
  • [3.7] bpo-32457: Improves handling of denormalized executable path when launching Python (GH-5756) #5817
  • [3.6] bpo-32457: Improves handling of denormalized executable path when launching Python (GH-5756) #5818
    		•

    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 2018-05-28.22:14:20.327>
created_at = <Date 2017-12-30.18:58:32.765>
labels = ['3.7', '3.8', 'OS-windows', 'type-crash']
title = 'Windows Python cannot handle an early PATH entry containing ".." and python.exe'
updated_at = <Date 2018-05-28.22:14:20.326>
user = 'https://github.com/mingwandroid'

    bugs.python.org fields:

    activity = <Date 2018-05-28.22:14:20.326>
actor = 'steve.dower'
assignee = 'steve.dower'
closed = True
closed_date = <Date 2018-05-28.22:14:20.327>
closer = 'steve.dower'
components = ['Windows']
creation = <Date 2017-12-30.18:58:32.765>
creator = 'Ray Donnelly'
dependencies = []
files = []
hgrepos = []
issue_num = 32457
keywords = ['patch']
message_count = 8.0
messages = ['309242', '309243', '309249', '309491', '309495', '312579', '312587', '312595']
nosy_count = 7.0
nosy_names = ['paul.moore', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'Ray Donnelly', 'miss-islington']
pr_nums = ['5756', '5817', '5818']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue32457'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']
    @mingwandroid
    Copy link
    Mannequin Author

    mingwandroid mannequin commented Dec 30, 2017

    Over on the Anaconda Distribution we received a (private) bug report about a crash when trying to use scons. I thought initially it was due to one of our patches but I tested it out with official CPython and also with WinPython and ran into the same crash.

    To reproduce this, from cmd.exe on CPython (here I installed CPython as part of Visual Studio 2017, then updated it to the latest 3.6.4 release):

    set "PATH=C:\Program Files (x86)\Microsoft Visual Studio\Shared\Python36_64\Scripts\..;%PATH%"

python.exe

..

python
Fatal Python error: Py_Initialize: unable to load the file system codec
ModuleNotFoundError: No module named 'encodings'

Current thread 0x00000328 (most recent call first):

    The trigger for this bug is the following batch code in scons.bat:

    https://bitbucket.org/scons/scons/src/c0172db149b1a151eeb76910d55c81746bfede05/src/script/scons.bat?at=default&fileviewer=file-view-default#scons.bat-19

    My current thinking is that the best fix here is to modify get_progpath()/get_program_full_path() so that it uses PathCchCanonicalizeEx() at

    cpython/PC/getpathp.c

    Lines 558 to 559 in 9bee329

    done:
    config->program_full_path = _PyMem_RawWcsdup(program_full_path);

    @mingwandroid mingwandroid mannequin added OS-windows type-crash A hard crash of the interpreter, possibly with a core dump labels Dec 30, 2017
    @mingwandroid
    Copy link
    Mannequin Author

    mingwandroid mannequin commented Dec 30, 2017

    .. though I will also ask the scons people to change this to use pushd and %CD% instead. Even if you were to make Python capable of handling such bad input, who knows what other programs will fail, and build systems should be extra careful not to mess the environment up like this.

    @eryksun
    Copy link
    Contributor

    eryksun commented Dec 30, 2017

    Here's a way to trigger this error that's unrelated to the PATH environment variable:

        >>> subprocess.call('python', executable=r'C:\Program Files\Python36\.\python.exe')
    Fatal Python error: Py_Initialize: unable to load the file system codec
    ModuleNotFoundError: No module named 'encodings'
    [...]

    Apparently Windows doesn't normalize the process image path if it uses only backslash as the path separator. It normalizes it if at least one backslash is replaced with a slash.

    @zooba
    Copy link
    Member

    zooba commented Jan 5, 2018

    I agree that explicitly normalizing in PC/getpathp.c is the correct approach.

    As far as I'm aware, GetModuleFileNameW(NULL) can never fail other than a buffer that is too small, so that whole function can probably be simplified to abort if it happens and just remove the PATH search.

    Also, we should use a similar process for PathCchCanonicalizeEx as we already use for PathCchCombineExW in this file to avoid failing on earlier OS versions.

    @zooba zooba added 3.7 (EOL) end of life 3.8 only security fixes labels Jan 5, 2018
    @eryksun
    Copy link
    Contributor

    eryksun commented Jan 5, 2018

    For extra measure, you may want to normalize prefix prior to calculating its length n in gotlandmark(). Then it would be reliable to truncate it via prefix[n] = '\0' after joining with landmark. Or at least add a comment there or in the calling function, search_for_prefix(), that gotlandmark assumes the current value of prefix is a canonical, normalized path.

    @zooba zooba self-assigned this Feb 19, 2018
    @zooba
    Copy link
    Member

    zooba commented Feb 22, 2018

    New changeset 48e8c82 by Steve Dower in branch 'master':
    bpo-32457: Improves handling of denormalized executable path when launching Python (GH-5756)
    48e8c82

    @miss-islington
    Copy link
    Contributor

    New changeset e5a9b35 by Miss Islington (bot) in branch '3.7':
    bpo-32457: Improves handling of denormalized executable path when launching Python (GH-5756)
    e5a9b35

    @zooba
    Copy link
    Member

    zooba commented Feb 22, 2018

    New changeset 1d3c518 by Steve Dower in branch '3.6':
    bpo-32457: Improves handling of denormalized executable path when launching Python (GH-5756) (bpo-5818)
    1d3c518

    @zooba zooba closed this as completed May 28, 2018
    @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

    @zooba zooba

    Labels
    3.7 (EOL) end of life 3.8 only security fixes OS-windows type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @eryksun @zooba @miss-islington