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.

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

[Windows] Can't import extension modules resolved via relative paths in sys.path #87271

Closed
smunday mannequin opened this issue Feb 2, 2021 · 13 comments
Closed

[Windows] Can't import extension modules resolved via relative paths in sys.path #87271

smunday mannequin opened this issue Feb 2, 2021 · 13 comments
Assignees
Labels
3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-windows stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@smunday
Copy link
Mannequin

smunday mannequin commented Feb 2, 2021

BPO 43105
Nosy @brettcannon, @gpshead, @pfmoore, @orsenthil, @tjguk, @zware, @eryksun, @zooba, @neonene
PRs
  • bpo-43105: Importlib now resolves relative paths when creating module spec objects from file locations #25121
  • [3.9] bpo-43105: Importlib now resolves relative paths when creating module spec objects from file locations (GH-25121) #25237
  • [3.8] bpo-43105: Importlib now resolves relative paths when creating module spec objects from file locations (GH-25121) #25318
  • 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 2021-04-12.15:36:46.754>
    created_at = <Date 2021-02-02.17:03:00.187>
    labels = ['interpreter-core', '3.8', 'type-bug', 'library', 'OS-windows']
    title = "[Windows] Can't import extension modules resolved via relative paths in sys.path"
    updated_at = <Date 2021-06-14.06:28:27.775>
    user = 'https://bugs.python.org/smunday'

    bugs.python.org fields:

    activity = <Date 2021-06-14.06:28:27.775>
    actor = 'neonene'
    assignee = 'steve.dower'
    closed = True
    closed_date = <Date 2021-04-12.15:36:46.754>
    closer = 'steve.dower'
    components = ['Interpreter Core', 'Library (Lib)', 'Windows']
    creation = <Date 2021-02-02.17:03:00.187>
    creator = 'smunday'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 43105
    keywords = ['patch']
    message_count = 13.0
    messages = ['386155', '386898', '387056', '389930', '390388', '390421', '390423', '390445', '390476', '390672', '393129', '395032', '395775']
    nosy_count = 10.0
    nosy_names = ['brett.cannon', 'gregory.p.smith', 'paul.moore', 'orsenthil', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'neonene', 'smunday']
    pr_nums = ['25121', '25237', '25318']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue43105'
    versions = ['Python 3.8']

    @smunday
    Copy link
    Mannequin Author

    smunday mannequin commented Feb 2, 2021

    If I attempt to import an extension module via something like this:

    from pkg import extmodule

    and it happens that "pkg" is found in a folder that is given in sys.path as a relative path, then the import fails, with

    ImportError: DLL load failed while importing extmodule: The parameter is incorrect.

    This only happens with 3.8 and later.

    AFAICS the reason is that if the module resolution is done via a relative path, it results in dynload_win.c:_PyImport_FindSharedFuncptrWindows attempting to feed LoadLibraryExW a relative path for the .pyd file. But LoadLibraryExW treats relative paths completely differently from absolute ones, in that it searches for the given path relative to the library search path entries rather than simply loading the DLL at that path. But as of 3.8 the current directory is removed from the search path, so the .pyd is never found. Since Python knows the specific file it wants LoadLibraryExW to load, having just resolved it via the import mechanism, I guess it ought to ensure it only ever calls LoadLibraryExW with an absolute path.

    (I'm assuming use of relative paths in sys.path is officially supported, since nothing I've found in the docs says otherwise.)

    @smunday smunday mannequin added extension-modules C modules in the Modules dir 3.9 only security fixes 3.10 only security fixes 3.8 only security fixes type-bug An unexpected behavior, bug, or error labels Feb 2, 2021
    @smunday smunday mannequin changed the title Can't import extension modules resolved via relative paths in sys.path on Windows don't don't Can't import extension modules resolved via relative paths in sys.path on Windows Feb 2, 2021
    @smunday smunday mannequin changed the title Can't import extension modules resolved via relative paths in sys.path on Windows don't don't Can't import extension modules resolved via relative paths in sys.path on Windows Feb 2, 2021
    @eryksun
    Copy link
    Contributor

    eryksun commented Feb 13, 2021

    and it happens that "pkg" is found in a folder that is
    given in sys.path as a relative path

    I'd prefer that Python disallowed relative paths in sys.path [1]. But since they're allowed, I think importlib at least could try to resolve relative paths in a copy of sys.path before searching.

    as of 3.8 the current directory is removed from the search path,
    so the .pyd is never found

    It happens to work prior to 3.8 even though the load uses the flag LOAD_WITH_ALTERED_SEARCH_PATH, for which it's documented that "[i]f this value is used and lpFileName specifies a relative path, the behavior is undefined".

    The implemented behavior with LOAD_WITH_ALTERED_SEARCH_PATH is that the directory of the given DLL filename is added to the head of the DLL search path, even though it's a relative path. Then the DLL filename is searched for like any other relative filename.

    For example, loading r"foo\spam.pyd" will try to load r"foo\foo\spam.pyd" (note the double "foo"), r"C:\Windows\System32\foo\spam.pyd", r"C:\Windows\System\foo\spam.pyd", r"C:\Windows\foo\spam.pyd", and so on. If the current working directory (i.e. ".") is in the DLL search path, and r"foo\spam.pyd" isn't accidentally found relative to a directory that's searched before ".", then the loader will find r".\foo\spam.pyd". Fortunately another thread can't change the working directory while the loader is searching, since the PEB lock is held. If r"foo\spam.pyd" is found and it depends on "eggs.dll", the loader will look for it first in the DLL directory, i.e. as r"foo\eggs.dll".

    The implicit inclusion of the working directory can be disabled or replaced with another directory via SetDllDirectoryW(), in which case the working directory will only be checked if %PATH% contains a "." entry. If it's replaced with another directory, then it's even inheritable from a SetDllDirectoryW() call in an ancestor process.

    3.8+ uses the flag LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR, which requires the DLL filename to be a fully-qualified path.

    ---
    [1] That includes the "" entry in sys.path in the interactive shell. I wish it was implemented to resolve the working directory at startup instead of letting the entry vary with the current working directory.

    @eryksun eryksun added 3.7 (EOL) end of life labels Feb 13, 2021
    @zooba
    Copy link
    Member

    zooba commented Feb 15, 2021

    since they're allowed, I think importlib at least could try to resolve relative paths in a copy of sys.path before searching.

    I agree with this fix. They can be resolved for each new import, if we think that's an important behaviour to preserve (might mess with the cache... but probably necessary if it's to be backported). But at some point sys.path entries need to be made absolute, and it definitely needs to happen before we try to load any DLLs.

    @eryksun eryksun added interpreter-core (Objects, Python, Grammar, and Parser dirs) stdlib Python modules in the Lib dir and removed 3.7 (EOL) end of life extension-modules C modules in the Modules dir labels Mar 30, 2021
    @eryksun eryksun changed the title Can't import extension modules resolved via relative paths in sys.path on Windows [Windows] Can't import extension modules resolved via relative paths in sys.path Mar 30, 2021
    @zooba
    Copy link
    Member

    zooba commented Mar 31, 2021

    Added one possible change as a PR, but will need the importlib folk to weigh in on whether it's the best place.

    I also need to write a test (and possibly fix any tests that currently check that relative paths are _not_ resolved... letting CI handle that for me)

    @zooba
    Copy link
    Member

    zooba commented Apr 7, 2021

    New changeset 04732ca by Steve Dower in branch 'master':
    bpo-43105: Importlib now resolves relative paths when creating module spec objects from file locations (GH-25121)
    04732ca

    @zooba
    Copy link
    Member

    zooba commented Apr 7, 2021

    New changeset 0af99b4 by Steve Dower in branch '3.9':
    bpo-43105: Importlib now resolves relative paths when creating module spec objects from file locations (GH-25121)
    0af99b4

    @zooba
    Copy link
    Member

    zooba commented Apr 7, 2021

    Just needs the 3.8 backport - will get to that later tonight.

    @zooba
    Copy link
    Member

    zooba commented Apr 7, 2021

    The 3.8 backport is much more complicated, as we don't have access to the PathSkipRoot function there. So we can't use the native function. There's probably another way to implement the fix for 3.8, but I'm leaving that for another day. Feel free to chime in with suggestions and/or PRs.

    @zooba zooba removed the 3.9 only security fixes label Apr 7, 2021
    @zooba zooba removed the 3.10 only security fixes label Apr 7, 2021
    @eryksun
    Copy link
    Contributor

    eryksun commented Apr 7, 2021

    The 3.8 backport is much more complicated, as we don't have access to
    the PathSkipRoot function there. So we can't use the native function.

    I guess you missed the comment that I left on the PR a few days ago. The 3.8 backport can use the older PathSkipRootW() in shlwapi.dll [1]. It works similarly, except it doesn't support a drive relative root such as "C:spam" -> ("C:", "spam"), but it's easy enough to handle that case in C. Also, it's documented that it's limited to MAX_PATH, but it works fine with long paths in Windows 10, even if the process does not have long-path support enabled. Anyway, just limit the copy to the first MAX_PATH - 1 characters. Practically speaking, no root is anywhere near that long. It would require a ridiculously long device name in a "\." device path.

    Examples:

        import ctypes
        shlwapi = ctypes.WinDLL('shlwapi')
        shlwapi.PathSkipRootW.restype = ctypes.c_wchar_p
        path = (ctypes.c_wchar * 1000)()

    It returns NULL if there's no root:

        >>> path.value = r'spam'
        >>> shlwapi.PathSkipRootW(path) is None
        True

    Drive-relative paths aren't supported the same as they are in PathCchSkipRoot():

        >>> path.value = r'C:spam'
        >>> shlwapi.PathSkipRootW(path) is None
        True

    Otherwise it seems to support the same range of paths as PathCchSkipRoot():

        >>> path.value = r'\spam'
        >>> shlwapi.PathSkipRootW(path)
        'spam'
    
        >>> path.value = r'C:\spam'
        >>> shlwapi.PathSkipRootW(path)
        'spam'
    
        >>> path.value = r'\\server\share\spam'
        >>> shlwapi.PathSkipRootW(path)
        'spam'
    
        >>> path.value = r'\\?\C:\spam'
        >>> shlwapi.PathSkipRootW(path)
        'spam'
    
        >>> path.value = r'\\?\UNC\server\share\spam'
        >>> shlwapi.PathSkipRootW(path)
        'spam'
    
        >>> path.value = r'\\?\Volume{12345678-1234-1234-1234-123456789ABC}\spam'
        >>> shlwapi.PathSkipRootW(path)
        'spam'
    
        >>> path.value = r'\\.\PIPE\spam'
        >>> shlwapi.PathSkipRootW(path)
        'spam'

    [1] https://docs.microsoft.com/en-us/windows/win32/api/shlwapi/nf-shlwapi-pathskiprootw

    @zooba
    Copy link
    Member

    zooba commented Apr 9, 2021

    New changeset eed7686 by Steve Dower in branch '3.8':
    bpo-43105: Importlib now resolves relative paths when creating module spec objects from file locations (GH-25121)
    eed7686

    @zooba zooba closed this as completed Apr 12, 2021
    @zooba zooba self-assigned this Apr 12, 2021
    @gpshead
    Copy link
    Member

    gpshead commented May 6, 2021

    This caused a regression described in https://bugs.python.org/issue44061

    @orsenthil
    Copy link
    Member

    There is a report about this change might have caused behaviour change for '.' in sys.path between 3.10.0a7 and 3.10.0b1

    https://mail.python.org/archives/list/python-dev@python.org/thread/DE3MDGB2JGOJ3X4NWEGJS26BK6PJUPKW/

    @neonene
    Copy link
    Mannequin

    neonene mannequin commented Jun 14, 2021

    After this contribution, when using module at the root dir (maybe bad manners), the followings are expected behaviors?

    (1) relative drive in sys.path -> bytecode is not put in __pycache__ folder.

    >>> import sys
    >>> sys.path.append('F:')  # flash device, etc...
    >>> import foo
    >>> foo.__file__
    'F:foo.py'
    >>> foo.__cached__
    'F:foo.cpython-311.pyc'

    (2) absolute drive in sys.path -> __pycache__ is under current dir, not absolute.

    >>> import sys
    >>> sys.path.append('F:\\')
    >>> import foo
    >>> foo.__file__
    'F:\\foo.py'
    >>> foo.__cached__
    'F:__pycache__\\foo.cpython-311.pyc'

    @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
    Labels
    3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-windows stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants