Handle /: for ntpath.normpath
#117383
Comments
|
This works as expected with the fallback implementation on POSIX: >>> os.name
'posix'
>>> ntpath.normpath('/:foo')
'\\:foo'The problem is in the C implementation of >>> nt._path_normpath('/:foo')
'/:foo'It's implemented by // Skip past drive segment and update minP2
else if (p1[0] && p1[1] == L':') {
*p2++ = *p1++;
*p2++ = *p1++;
minP2 = p2;
lastC = L':';
}It needs an additional check for a leading path separator, e.g. |
Do we need the separator check? Or do we need to check that In any case, it's an invalid path, so I'm not concerned about producing invalid results. Unless it's somehow exploitable, this is very non-urgent. |
A separator check: #101363 (comment)
|
Sure, we could restrict it further. Then we have to decide on what counts as a letter. The mount manager only supports assigning DOS volume names with letters A-Z. However, parts of Windows path handling are generic about what characters are supported as drive letters. For example: >>> os.system('subst ñ: C:\\Windows')
0
>>> os.chdir('ñ:/')
>>> os.getcwd()
'ñ:\\'
>>> os.path.exists('ñ:/explorer.exe')
TrueFor the above to work as it does, the Windows API has to handle "ñ:" as a drive instead of as a filename. The newer I don't think that legacy |
|
Okay, so we don't need a more strict check, we just need to decide whether our fallback implementation and our native implementation need to behave the same for invalid paths. I'd be inclined to change the fallback implementation here, honestly. As the input is invalid, the result is also invalid, and changing the fallback is lowest risk. |
In that case, either fix the bug, or leave it as it is. Breaking the working implementation would be regression, and is not what I had in mind when I reported this. Also, adding a bug here is probably going to slow the function down: Lines 552 to 553 in 976bcb2 |
|
Unless someone shows a real working scenario that involves normalising this invalid path, then leave it as is. I don't see the motivation to change anything here. I've suggested the best motivation I can imagine, and if that's not why you want this changed, then I need you to explain why. |
|
Sorry, I can't get behind introducing a bug in a perfectly working function to make it consistent with a function with a (minor) bug (maybe even slowing it down). But I believe that if the C implementation could be fixed, that would be the best outcome. So decide whether we want to fix the C implementation or close this as "won't fix" (obviously easier). |
|
OK, I'm closing this. If you change your mind this can be re-opened. |
Uh oh!
There was an error while loading. Please reload this page.
Bug report
Bug description:
Expected:
\\:foo, like the python implementation forntpath.normpath.CPython versions tested on:
CPython main branch
Operating systems tested on:
Windows
The text was updated successfully, but these errors were encountered: