Handle `/:` for `ntpath.normpath`

[nineteendo]

Bug report

Bug description:

>>> import ntpath
>>> ntpath.normpath("/:foo")
'/:foo'

Expected: \\:foo, like the python implementation for ntpath.normpath.

CPython versions tested on:

CPython main branch

Operating systems tested on:

Windows

[eryksun]

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() on Windows.

>>> nt._path_normpath('/:foo')
'/:foo'

It's implemented by _Py_normpath_and_size() in "Python/fileutils.c", which skips a DOS drive using the following snippet of code:

    // 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. p1[0] && !IS_SEP(p1) && p1[1] == L':'.

[zooba]

It needs an additional check for a leading path separator, e.g. p1[0] && !IS_SEP(p1) && p1[1] == L':'.

Do we need the separator check? Or do we need to check that p1[0] is a letter?

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.

[nineteendo]

Do we need the separator check? Or do we need to check that p1[0] is a letter?

A separator check: #101363 (comment)

WinAPI GetFullPathNameW() will accept any character in the 16-bit BMP as a drive 'letter', except for null, slash, and backslash. For example:

[eryksun]

Do we need the separator check? Or do we need to check that p1[0] is a letter?

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')
True

For the above to work as it does, the Windows API has to handle "ñ:" as a drive instead of as a filename.

The newer PathCch* routines limit this to characters for which ntdll!iswalpha() returns true. That's 114 alphabetic characters in Latin 1, for which the ntdll implementation is hard coded. The UCRT implementation of iswalpha() is generalized to use Unicode character properties via WinAPI GetStringTypeW(). Thus to match the PathCch* routines, we have to ensure the ordinal value is less than 256, e.g. p1[0] < 256 && iswalpha(p1[0]).

I don't think that legacy isalpha() should be used. The result depends on the current locale, which can vary in the non-ASCII upper range (128, 256).

[zooba]

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.

[nineteendo]

I'd be inclined to change the fallback implementation here, honestly.

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:

cpython/Lib/ntpath.py

Lines 552 to 553 in 976bcb2

	path = path.replace(altsep, sep)
	drive, root, path = splitroot(path)

[zooba]

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.

[nineteendo]

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).
@barneygale what's your opinion on this?