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

os.path.splitext() can split UNC drive on Windows #115892

Open
barneygale opened this issue Feb 24, 2024 · 5 comments
Open

os.path.splitext() can split UNC drive on Windows #115892

barneygale opened this issue Feb 24, 2024 · 5 comments
Labels
3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes OS-windows type-bug An unexpected behavior, bug, or error

Comments

@barneygale
Copy link
Contributor

barneygale commented Feb 24, 2024

Bug report

>>> import ntpath
>>> ntpath.splitext('//server/share.jpg')
('//server/share', '.jpg')  # expected: ('//server/share.jpg', '')

This path has an empty basename, and so its suffix should also be empty. Affects all current Python versions.

Linked PRs

@barneygale barneygale added type-bug An unexpected behavior, bug, or error OS-windows labels Feb 24, 2024
@serhiy-storchaka serhiy-storchaka added 3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes labels Feb 24, 2024
@zooba
Copy link
Member

zooba commented Feb 26, 2024

I don't think this is necessarily an "obvious" change to make. We don't currently validate anything about the path, so it works on relative and absolute paths alike. This would introduce validation at the start, making new assumptions about the type of path we've been given.

Meanwhile, there's nothing actually bad about splitting the share name like this? No file operation is going to work on it anyway, so you're about to get an error even if you don't at this step, and if you know you've got a UNC share or a directory path, why are you asking for its suffix?

I feel like this is too risky to backport, and I'm not convinced of the value going forward given that we currently don't require a full or relative path (and probably shouldn't).

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Feb 27, 2024
os.path.splitext() on Windows no longer splits an "extension" from
the server or share name in the UNC path.
@nineteendo
Copy link
Contributor

It's it worth fixing this with splitroot? Or do we close this?

@zooba
Copy link
Member

zooba commented Apr 29, 2024

I think we should close. Fixing this would suggest we should also detect directories with . and avoid splitting those, and that to me is clearly crossing the line. I'd rather keep this function simple and let it blindly split at the last dot after the last separator.

@nineteendo
Copy link
Contributor

Fixing this would suggest we should also detect directories with . and avoid splitting those

I don't think that's what @barneygale had in mind. He doesn't like the inconsistency with ntpath.split():

>>> import ntpath
>>> ntpath.split('//server/share.jpg')
('//server/share.jpg', '') # empty basename
>>> ntpath.splitext('//server/share.jpg') 
('//server/share', '.jpg') # but extension?

@zooba
Copy link
Member

zooba commented Apr 29, 2024

I'm sure I can come up with a justification, but really, both operations are somewhat nonsense (or more politely, a boundary condition), so the result can be arbitrary. Arbitrarily, I'd like the splitext implementation that is simpler, faster, and more reliable.

(I guess you could reasonably argue that split(path_with_no_basename) is a valid base case for a recursive algorithm, but I think you'd struggle to say the same for splitext(path_with_no_filename). So there's a justification. But it's really just the simpler implementation that I want.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes OS-windows type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants