Skip to content

os.path.realpath returns invalid path for junction pointing to letter-less volume #89760

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

Open
grv87 mannequin opened this issue Oct 24, 2021 · 7 comments
Open

os.path.realpath returns invalid path for junction pointing to letter-less volume #89760

grv87 mannequin opened this issue Oct 24, 2021 · 7 comments
Labels
3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes OS-windows

Comments

@grv87
Copy link
Mannequin

grv87 mannequin commented Oct 24, 2021

BPO 45597
Nosy @pfmoore, @tjguk, @zware, @eryksun, @zooba, @grv87

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 2021-10-24.09:38:45.590>
labels = ['3.10', '3.8', '3.9', 'OS-windows']
title = 'os.path.realpath returns invalid path for junction pointing to letter-less volume'
updated_at = <Date 2021-10-24.11:15:13.756>
user = 'https://github.com/grv87'

bugs.python.org fields:

activity = <Date 2021-10-24.11:15:13.756>
actor = 'eryksun'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Windows']
creation = <Date 2021-10-24.09:38:45.590>
creator = 'grv87'
dependencies = []
files = []
hgrepos = []
issue_num = 45597
keywords = []
message_count = 2.0
messages = ['404923', '404925']
nosy_count = 6.0
nosy_names = ['paul.moore', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'grv87']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue45597'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

@grv87
Copy link
Mannequin Author

grv87 mannequin commented Oct 24, 2021

If a path contains a junction pointing to a dir on a letter-less drive then os.path.realpath returns Volume{<uuid>}\dir, without \\?\ prefix.

This path, of course, doesn't work correctly. Actually, it looks relative.

Original issue: pypa/pip#10597

@grv87 grv87 mannequin added 3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes OS-windows labels Oct 24, 2021
@eryksun
Copy link
Contributor

eryksun commented Oct 24, 2021

This is from checking whether the \\?\ prefix can be stripped. The _getfinalpathname() call that it makes fails with the initial winerror (ERROR_PATH_NOT_FOUND), since nt._getfinalpathname() still lacks support for volume GUID paths. In this case, it assumes the path doesn't exist and removes the prefix.

This check should be skipped for all prefixed paths if they aren't drives or UNC shares. For example, if colon_sep (i.e. ":\\") is defined:

        # The path returned by _getfinalpathname will always start with \\?\ -
        # strip off that prefix unless it was already provided on the original
        # path.
        if not had_prefix and path.startswith(prefix):
            # For UNC paths, the prefix will be \\?\UNC\
            if path.startswith(unc_prefix):
                spath = new_unc_prefix + path[len(unc_prefix):]
            # For drive paths, the root is of the form \\?\X:\
            elif path.startswith(colon_sep, len(prefix) + 1):
                spath = path[len(prefix):]
            # For all others, the prefix must be retained.
            else:
                spath = None
            if spath is not None:
                # Ensure that the non-prefixed path resolves to the same path
                try:
                    if _getfinalpathname(spath) == path:
                        path = spath
                except OSError as ex:
                    # If the path does not exist and originally did not exist, then
                    # strip the prefix anyway.
                    if ex.winerror == initial_winerror:
                        path = spath
        return path

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@grv87
Copy link

grv87 commented Jun 9, 2022

@eryksun, are you proposing a change to discuss, or…?
Your code works for me. Although I found the comment misleading.

@eryksun
Copy link
Contributor

eryksun commented Jun 10, 2022

are you proposing a change to discuss, or…?

Yes, the suggested change in ntpath.realpath() will always keep the "\\?\" prefix if the resolved path is not on the "UNC" device or a DOS drive.

Although I found the comment misleading.

GetFinalPathNameByHandleW() supports getting the final path as VOLUME_NAME_DOS (i.e. using a DOS drive-letter volume name or a UNC share) or VOLUME_NAME_GUID (i.e. using a "Volume{GUID}" volume name). If a volume has not been assigned a DOS drive-letter name, then requesting VOLUME_NAME_DOS will fail. Python's implementation of nt._getfinalpathname() doesn't fall back on VOLUME_NAME_GUID in this case. It could, but it currently doesn't. But solving this issue by extending the implementation of _getfinalpathname() in "Modules/posixmodule.c" would make the solution overly difficult, and it still might miss edge cases. It's simpler to just change the implementation of ntpath.realpath() as mentioned above.

@grv87
Copy link

grv87 commented Jun 15, 2022

Yes, the suggested change

Why not open a PR?
Or suggested changes must be discussed/approved/… before PR? I'm not familiar with rules here.

Although I found the comment misleading

I meant, the term "drive path" is not wide-accepted.
I'd just say Regular paths start from \\?\<drive letter>:\.

Python's implementation of nt._getfinalpathname() doesn't fall back on VOLUME_NAME_GUID in this case

And what results would GetFinalPathNameByHandleW with VOLUME_NAME_GUID give in this case?

I haven't tested, but IIUC, these flags aren't fallbacks. They choose which variant should be returned when there are several possibilities (with drive letter or with volume GUID). But if there is no letter, VOLUME_NAME_GUID would give the same result as VOLUME_NAME_DOS.

@eryksun
Copy link
Contributor

eryksun commented Jun 15, 2022

Note that reproducing the issue requires the error after stripping the "\\?\" prefix to match initial_winerror. So sometimes realpath() will work correctly, given the two error codes are different.

Also, there's no misbehavior if the path of the junction is registered with the mount point manager as the canonical DOS path of the volume, e.g. a junction created by "mountvol.exe" or WinAPI SetVolumeMountPointW() (requires administrator access). nt._getfinalpathname() returns the canonical DOS path.

Why not open a PR?
Or suggested changes must be discussed/approved/… before PR?

Personally, I prefer to discuss ideas in the issue itself. It leaves a more accessible record. I've learned a lot from reading old issues. You're welcome to create a PR if you're eager to resolve this issue. I'll try to help with code review and testing.

I haven't tested, but IIUC, these flags aren't fallbacks. They choose
which variant should be returned when there are several possibilities
(with drive letter or with volume GUID). But if there is no letter,
VOLUME_NAME_GUID would give the same result as VOLUME_NAME_DOS.

If a volume has no canonical DOS mount point, then requesting VOLUME_NAME_DOS fails. In this case, I suggested that the implementation of nt._getfinalpathname() could automatically fall back on requesting the GUID name via VOLUME_NAME_GUID. This will succeed as long as the volume supports the mount point manager. In general, this would be a welcome enhancement.

But, as I said, for this issue it's better to modify realpath() to never try removing the "\\?\" prefix if the path doesn't start with a drive-letter drive or "UNC" drive.

I meant, the term "drive path" is not wide-accepted.
I'd just say Regular paths start from \\?\<drive letter>:\.

How about clarifying the comments as "For UNC drives, the path starts with \\?\UNC\", and "For drive-letter drives, the path starts with \\?\<drive letter>:\"?

@grv87
Copy link

grv87 commented Jun 16, 2022

If a volume has no canonical DOS mount point, then requesting VOLUME_NAME_DOS fails. In this case, I suggested that the implementation of nt._getfinalpathname() could automatically fall back on requesting the GUID name via VOLUME_NAME_GUID

I see. My mistake.
When VOLUME_NAME_DOS fails, Python does in _getfinalpathname_nonstrict what VOLUME_NAME_GUID would do, doesn't it?

How about clarifying the comments as
👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes OS-windows
Projects
None yet
Development

No branches or pull requests

2 participants