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

pathlib: "Incorrect function" during resolve() #76023

Open
earonesty mannequin opened this issue Oct 22, 2017 · 12 comments
Open

pathlib: "Incorrect function" during resolve() #76023

earonesty mannequin opened this issue Oct 22, 2017 · 12 comments
Labels
3.9 only security fixes OS-windows stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@earonesty
Copy link
Mannequin

earonesty mannequin commented Oct 22, 2017

BPO 31842
Nosy @pfmoore, @tjguk, @zware, @eryksun, @zooba, @earonesty, @earonesty, @zombie110year, @christopher.pickering

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 = None
created_at = <Date 2017-10-22.21:31:13.027>
labels = ['type-bug', '3.9', 'OS-windows']
title = 'pathlib: "Incorrect function" during resolve()'
updated_at = <Date 2021-11-13.12:33:39.598>
user = 'https://github.com/earonesty'

bugs.python.org fields:

activity = <Date 2021-11-13.12:33:39.598>
actor = 'eryksun'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Windows']
creation = <Date 2017-10-22.21:31:13.027>
creator = 'earonesty2'
dependencies = []
files = []
hgrepos = []
issue_num = 31842
keywords = []
message_count = 8.0
messages = ['304767', '356199', '357702', '357725', '381676', '390089', '406212', '406274']
nosy_count = 11.0
nosy_names = ['paul.moore', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'earonesty', 'earonesty2', 'zombie110year', 'maciozo', 'christopherpickering', 'riffitos']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue31842'
versions = ['Python 3.9']

@earonesty
Copy link
Mannequin Author

earonesty mannequin commented Oct 22, 2017

When strict is "false", pathlib should not fail if the network share is inaccessible. It should, as documented, catch the exception and continue with as much of the path as it has.

>>> pathlib.Path("v:").resolve()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\erik\AppData\Local\Programs\Python\Python36\lib\pathlib.py", line 1119, in resolve
    s = self._flavour.resolve(self, strict=strict)
  File "C:\Users\erik\AppData\Local\Programs\Python\Python36\lib\pathlib.py", line 193, in resolve
    s = self._ext_to_normal(_getfinalpathname(s))
OSError: [WinError 1] Incorrect function: 'v:'

@earonesty earonesty mannequin added 3.7 (EOL) end of life OS-windows labels Oct 22, 2017
@maciozo
Copy link
Mannequin

maciozo mannequin commented Nov 7, 2019

Same error occurs when attempting to resolve a path on an ImDisk RAM disk:

>>> pathlib.Path("T:\\").resolve()
Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "C:\Users\maciozo\AppData\Local\Continuum\miniconda3\envs\brainboxes\lib\pathlib.py", line 1151, in resolve
    s = self._flavour.resolve(self, strict=strict)
  File "C:\Users\maciozo\AppData\Local\Continuum\miniconda3\envs\brainboxes\lib\pathlib.py", line 202, in resolve
    s = self._ext_to_normal(_getfinalpathname(s))
OSError: [WinError 1] Incorrect function: 'T:\\'

@Zombie110year
Copy link
Mannequin

Zombie110year mannequin commented Dec 2, 2019

Same Error.

It happend by resolving a path which is not under C: driver,
in Windows System.

>>> Path().resolve()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\zom\scoop\apps\miniconda3\current\envs\web\lib\pathlib.py", line 1159, in resolve
    s = self._flavour.resolve(self, strict=strict)
  File "C:\Users\zom\scoop\apps\miniconda3\current\envs\web\lib\pathlib.py", line 202, in resolve
    s = self._ext_to_normal(_getfinalpathname(s))
OSError: [WinError 1] Incorrect function: '.'
>>> Path().absolute()
WindowsPath('R:/fileshare/fileshare-s')
>>> Path().absolute().resolve()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\zom\scoop\apps\miniconda3\current\envs\web\lib\pathlib.py", line 1159, in resolve
    s = self._flavour.resolve(self, strict=strict)
  File "C:\Users\zom\scoop\apps\miniconda3\current\envs\web\lib\pathlib.py", line 202, in resolve
    s = self._ext_to_normal(_getfinalpathname(s))
OSError: [WinError 1] Incorrect function: 'R:\\fileshare\\fileshare-s'

@Zombie110year Zombie110year mannequin added 3.8 only security fixes and removed 3.7 (EOL) end of life labels Dec 2, 2019
@eryksun
Copy link
Contributor

eryksun commented Dec 2, 2019

When strict is "false", pathlib should not fail if the network
share is inaccessible.

A redirected filesystem (i.e. a share, whether local or remote) that's physically inaccessible should fail with a FileNotFoundError -- due to underlying errors such as ERROR_BAD_NETPATH (53) or ERROR_BAD_NET_NAME (67). This case is already handled by resolve() in non-strict mode. But error handling for other cases does need to be expanded.

A PermissionError should be ignored, from OS errors such as ERROR_ACCESS_DENIED (5), ERROR_SHARING_VIOLATION (32), and ERROR_NOT_READY (21).

It should ignore errors due to bad reparse points (e.g. filesystem symlinks and mount points), such as ERROR_INVALID_REPARSE_DATA (4392) and ERROR_REPARSE_TAG_INVALID (4393).

The resolve() method is supposed to raise a RuntimeError for a symlink loop (i.e. reparse loop). In Windows, the system detects this case as ERROR_CANT_RESOLVE_FILENAME (1921). It's not necessarily due to a reparse loop, but in practice it usually is. (It's actually due to the upper limit for the number of reparse attempts, which as of Windows 10 is no more than 63 attempts.) Maybe this error should be re-raised as a RuntimeError for the sake of compatibility with the POSIX implementation.

The above-mentioned cases are all due to WINAPI CreateFileW failing.
Additionally, the GetFinalPathNameByHandleW call in nt._getfinalpathname relies on several low-level system calls, any one of which can fail with ERROR_INVALID_FUNCTION (1) or ERROR_INVALID_PARAMETER (87) -- and possibly ERROR_NOT_SUPPORTED (50). These errors should be ignored in non-strict mode.

At a lower level, the reliability of nt._getfinalpathname could be improved for non-strict operation. It could fall back on other forms of the final name that may be supported. It could try FILE_NAME_OPENED instead of FILE_NAME_NORMALIZED to handle a filesystem that does not support normalizing 8.3 short names as long names. Try VOLUME_NAME_GUID instead of VOLUME_NAME_DOS to handle a volume that only has a GUID name (i.e. a volume device that's registered with the mount-point manager but lacks a DOS drive name or even a mountpoint on a volume that has a DOS drive name). Try VOLUME_NAME_NT to handle legacy volume/filesystem devices ('legacy' because they do not support the mount-point manager interface that was introduced almost 20 years ago in NT 5). For the sake of simplicity and performance, the latter case could be limited to well-known DOS device names such as r"\\?\PIPE" -> r"\Device\NamedPipe". The NT device path for the comparison should be queried instead of hard coded, e.g. via QueryDosDeviceW(L"PIPE", ...).

@christopherpickering
Copy link
Mannequin

christopherpickering mannequin commented Nov 23, 2020

Hi, is there a status update on this issue? Thanks!

@riffitos
Copy link
Mannequin

riffitos mannequin commented Apr 2, 2021

Still happening on Python 3.9.2 on Win10 when using a Ram Drive.

Any chance of getting this fixed?

@earonesty
Copy link
Mannequin Author

earonesty mannequin commented Nov 12, 2021

bug is worse than that:

perfectly valid redirected paths (winfsp ram drives for example) now break in python 3.9.6 (maybe fixed in later version?)

>>> import pathlib
>>> p=pathlib.Path('C:\\Users\\erik\\Atakama')
>>> p.resolve()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "c:\users\erik\.pyenv\pyenv-win\versions\3.9.6\Lib\pathlib.py", line 1205, in resolve
    s = self._flavour.resolve(self, strict=strict)
  File "c:\users\erik\.pyenv\pyenv-win\versions\3.9.6\Lib\pathlib.py", line 206, in resolve
    s = self._ext_to_normal(_getfinalpathname(s))
OSError: [WinError 1005] The volume does not contain a recognized file system.

... yet....

dir 'C:\\Users\\erik\\Atakama'
Desktop.ini Personal\ Files Vault

mounted via winfsp

@earonesty earonesty mannequin added 3.9 only security fixes and removed 3.8 only security fixes labels Nov 12, 2021
@eryksun
Copy link
Contributor

eryksun commented Nov 13, 2021

perfectly valid redirected paths (winfsp ram drives for example)

I mounted a WinFsp MEMFS filesystem on a directory, which set a mountpoint that targets the root path of a volume device in the native "\Device" object directory. It didn't create a volume GUID name, which means the mountpoint manager isn't supported in this configuration.

The error code ERROR_UNRECOGNIZED_VOLUME (1005) is meaningless in this case. The mountpoint manager queries a volume with IOCTLs such as IOCTL_MOUNTDEV_QUERY_DEVICE_NAME, which the WinFsp virtual volume (in the above configuration) doesn't support. Weirdly, it returns the error code STATUS_UNRECOGNIZED_VOLUME instead of STATUS_INVALID_DEVICE_REQUEST. It does this as a lazy workaround for various IOCTLs it receives from filesystem drivers while the volume is in the process of being mounted [1][2]. The side effect is that it returns STATUS_UNRECOGNIZED_VOLUME for unhandled IOCTLs even when it's not getting mounted. This behavior should have been restricted to when the volume parameter block (VPB) is unmounted. Otherwise it should return the expected error code STATUS_INVALID_DEVICE_REQUEST (i.e. ERROR_INVALID_FUNCTION) instead of confusing users with a meaningless error.

WinFsp does support the mountpoint manager, in a restricted fashion. The mount target has to be a drive device name in the form "\.\X:". This gets registered with the mountpoint manager as the canonical DOS name of the volume. Since it's a global name, administrator access is required. It also creates a GUID volume name. Run mountvol.exe without arguments to find the volume name that's associated with the drive letter. Then run it again as mountvol <path> <volume_name>, where is an empty directory on which to mount the volume. Note that Python's os.path.realpath() will resolve the volume to the canonical drive name, even if the path traverses a directory mountpoint for the volume.

A new issue should be created to ignore ERROR_UNRECOGNIZED_VOLUME in 3.10+, for which Path.resolve() was updated to call os.path.realpath(). For 3.9, fixing Path.resolve() is still possible. There are 3 remaining bug releases planned: 3.9.9: (2022-01-03), 3.9.10 (2022-02-28), and 3.9.11 (2022-05-02).

---
[1] https://github.com/billziss-gh/winfsp/blob/v1.9/src/sys/devctl.c#L49
[2] winfsp/winfsp#177

@barneygale
Copy link
Contributor

Could you re-test this in 3.10 or above? We changed Path.resolve() to use os.path.realpath(), which may not suffer from this bug.

@traits
Copy link

traits commented Oct 29, 2022

We are using symbolic names (example: <XYZ>/my.fil), so these files never exist in literal form. I would still expect from the documentation (3.10):

If the path doesn’t exist and strict is True, FileNotFoundError is raised. If strict is False, the path is resolved as far as possible and any remainder is appended without checking whether it exists. If an infinite loop is encountered along the resolution path, RuntimeError is raised.

that only two types of exceptions are possible: FileNotFoundError and RuntimeError, for strict==False only the second one. But we still get an OSError in this case:

OSError: [WinError 123] The filename, directory name, or volume label syntax is incorrect: '<XYZ>\my.fil'

OSError is a base class of FileNotFoundError, but not raised as the latter, but as a real OSError. It shouldn't do it in either case, because as mentioned for strict==False only RuntimeError should be allowed. And only for specific circumstances, otherwise the documentation suggests a noop/partial string resolve regarding the input.

@zooba
Copy link
Member

zooba commented Oct 31, 2022

I don't think any Python documentation exhaustively lists exceptions. There are some that are always possible dependent on the arguments passed (ValueError, TypeError, etc.) and others dependent on state outside of the caller's control (OSError, KeyboardInterrupt, etc.). So to a certain extent, your expectation is wrong, and that's the fault of the documentation for not making that more obvious.

We'd welcome a pull request to add that particular error code into the "assume the path is correct and don't check" codepath pointed out in an earlier response.

@barneygale
Copy link
Contributor

I'm removing the topic-pathlib tag because in Python 3.13+, pathlib.Path.resolve() calls straight through to os.path.realpath(). If this bug still exists, it's in realpath() and not pathlib.

cpython/Lib/pathlib.py

Lines 1437 to 1443 in 1619f43

def resolve(self, strict=False):
"""
Make the path absolute, resolving all symlinks on the way and also
normalizing it.
"""
return self.with_segments(os.path.realpath(self, strict=strict))

@iritkatriel iritkatriel added the stdlib Python modules in the Lib dir label Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.9 only security fixes 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

6 participants