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

gh-96290: support partial/invalid UNC drives in normpath() and splitdrive() #100351

Merged
merged 11 commits into from
Jan 12, 2023

Conversation

barneygale
Copy link
Contributor

@barneygale barneygale commented Dec 19, 2022

Fix handling of partial and invalid UNC drives in ntpath.splitdrive(), and in ntpath.normpath() on non-Windows systems.

Paths such as \\server and \\ are now considered by splitdrive() to contain only a drive, and consequently are not modified by normpath() on non-Windows systems. The behaviour of normpath() on Windows systems is unaffected, as native OS APIs are used.

This brings the Python implementation of ntpath.normpath() more in line with the C implementation added in 99fcf15.

Implementation by @eryksun. I added tests and NEWS.

…splitdrive()

This brings the Python implementation of `ntpath.normpath()` in line with
the C implementation added in 99fcf15

Co-authored-by: Eryk Sun <eryksun@gmail.com>
@eryksun
Copy link
Contributor

eryksun commented Dec 22, 2022

This brings the Python implementation of ntpath.normpath() in line with the C implementation

Almost. The C implementation is incorrect for "UNC" device paths. For example:

>>> nt._path_normpath('//?/UNC/server/share/..')
'\\\\?\\UNC\\server'

The problem is that it handles "//?/UNC/" as the root instead of "//?/UNC/server/share/". The pure Python implementation gets it right.

@barneygale
Copy link
Contributor Author

I've changed "in line" to "more in line" :-)

Lib/ntpath.py Outdated Show resolved Hide resolved
barneygale and others added 2 commits December 28, 2022 22:56
Co-authored-by: Eryk Sun <eryksun@gmail.com>
@barneygale barneygale requested review from eryksun and removed request for brettcannon December 29, 2022 15:34
Lib/test/test_ntpath.py Outdated Show resolved Hide resolved
Co-authored-by: Eryk Sun <eryksun@gmail.com>
Lib/test/test_ntpath.py Outdated Show resolved Hide resolved
Co-authored-by: Eryk Sun <eryksun@gmail.com>
Lib/test/test_ntpath.py Outdated Show resolved Hide resolved
Co-authored-by: Eryk Sun <eryksun@gmail.com>
Lib/test/test_ntpath.py Outdated Show resolved Hide resolved
@barneygale barneygale added 3.11 only security fixes 3.12 bugs and security fixes needs backport to 3.11 only security fixes and removed 3.11 only security fixes 3.12 bugs and security fixes labels Jan 11, 2023
@zooba
Copy link
Member

zooba commented Jan 11, 2023

If Eryk and Barney both say this is ready, I'm happy to merge.

@barneygale
Copy link
Contributor Author

It's ready to land I think!

@eryksun
Copy link
Contributor

eryksun commented Jan 12, 2023

A long-standing issue with ntpath.isabs() is fixed, and it no longer depends on splitdrive(). We need a couple of tests in test_isabs(). I'd like to test a normal UNC path and a device path, both without a trailing backslash. They're really the same case as far as the new implementation is concerned, but I feel better with both being tested. In the old implementation, isabs() incorrectly returns false for these two cases.

        tester('ntpath.isabs("\\\\conky\\mountpoint")', 1)
        tester('ntpath.isabs("\\\\.\\C:")', 1)

@eryksun
Copy link
Contributor

eryksun commented Jan 12, 2023

@zooba, it's ready to be merged.

@zooba zooba merged commit 005e694 into python:main Jan 12, 2023
@miss-islington
Copy link
Contributor

Thanks @barneygale for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @barneygale and @zooba, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 005e69403d638f9ff8f71e59960c600016e101a4 3.11

@bedevere-bot
Copy link

GH-100999 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jan 12, 2023
zooba pushed a commit to zooba/cpython that referenced this pull request Jan 12, 2023
…() and splitdrive() (pythonGH-100351)

This brings the Python implementation of `ntpath.normpath()` in line with the C implementation added in 99fcf15

Co-authored-by: Eryk Sun <eryksun@gmail.com>
zooba added a commit that referenced this pull request Jan 12, 2023
… splitdrive() (GH-100351)

This brings the Python implementation of `ntpath.normpath()` in line with the C implementation added in 99fcf15

Co-authored-by: Barney Gale <barney.gale@gmail.com>
Co-authored-by: Eryk Sun <eryksun@gmail.com>
@barneygale
Copy link
Contributor Author

Thank you both!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants