bpo-32556: nt._getfinalpathname, nt._getvolumepathname and nt._getdiskusage...

bpo-32556: nt._getfinalpathname, nt._getvolumepathname and nt._getdiskusage now correctly convert from bytes. (GH-5761)

Lib/test/test_ntpath.py

@@ -7,6 +7,12 @@ from test.support import TestFailed
 from test import support, test_genericpath
 from tempfile import TemporaryFile
 
+try:
+    import nt
+except ImportError:
+    # Most tests can complete without the nt module,
+    # but for those that require it we import here.
+    nt = None
 
 def tester(fn, wantResult):
     fn = fn.replace("\\", "\\\\")
@@ -271,17 +277,9 @@ class TestNtpath(unittest.TestCase):
             tester('ntpath.expanduser("~/foo/bar")',
                    'C:\\idle\\eric/foo/bar')
 
+    @unittest.skipUnless(nt, "abspath requires 'nt' module")
     def test_abspath(self):
-        # ntpath.abspath() can only be used on a system with the "nt" module
-        # (reasonably), so we protect this test with "import nt".  This allows
-        # the rest of the tests for the ntpath module to be run to completion
-        # on any platform, since most of the module is intended to be usable
-        # from any platform.
-        try:
-            import nt
-            tester('ntpath.abspath("C:\\")', "C:\\")
-        except ImportError:
-            self.skipTest('nt module not available')
+        tester('ntpath.abspath("C:\\")', "C:\\")
 
     def test_relpath(self):
         tester('ntpath.relpath("a")', 'a')
@@ -424,6 +422,34 @@ class TestNtpath(unittest.TestCase):
             self.assertTrue(ntpath.ismount(b"\\\\localhost\\c$"))
             self.assertTrue(ntpath.ismount(b"\\\\localhost\\c$\\"))
 
+    @unittest.skipUnless(nt, "OS helpers require 'nt' module")
+    def test_nt_helpers(self):
+        # Trivial validation that the helpers do not break, and support both
+        # unicode and bytes (UTF-8) paths
+
+        drive, path = ntpath.splitdrive(sys.executable)
+        drive = drive.rstrip(ntpath.sep) + ntpath.sep
+        self.assertEqual(drive, nt._getvolumepathname(sys.executable))
+        self.assertEqual(drive.encode(),
+                         nt._getvolumepathname(sys.executable.encode()))
+
+        cap, free = nt._getdiskusage(sys.exec_prefix)
+        self.assertGreater(cap, 0)
+        self.assertGreater(free, 0)
+        b_cap, b_free = nt._getdiskusage(sys.exec_prefix.encode())
+        # Free space may change, so only test the capacity is equal
+        self.assertEqual(b_cap, cap)
+        self.assertGreater(b_free, 0)
+
+        for path in [sys.prefix, sys.executable]:
+            final_path = nt._getfinalpathname(path)
+            self.assertIsInstance(final_path, str)
+            self.assertGreater(len(final_path), 0)
+
+            b_final_path = nt._getfinalpathname(path.encode())
+            self.assertIsInstance(b_final_path, bytes)
+            self.assertGreater(len(b_final_path), 0)
+
 class NtCommonTest(test_genericpath.CommonTest, unittest.TestCase):
     pathmodule = ntpath
     attributes = ['relpath']

Misc/NEWS.d/next/Library/2018-02-19-14-27-51.bpo-32556.CsRsgr.rst

+nt._getfinalpathname, nt._getvolumepathname and nt._getdiskusage now
+correctly convert from bytes.

Modules/clinic/posixmodule.c.h

@@ -972,20 +972,23 @@ PyDoc_STRVAR(os__getfinalpathname__doc__,
     {"_getfinalpathname", (PyCFunction)os__getfinalpathname, METH_O, os__getfinalpathname__doc__},
 
 static PyObject *
-os__getfinalpathname_impl(PyObject *module, PyObject *path);
+os__getfinalpathname_impl(PyObject *module, path_t *path);
 
 static PyObject *
 os__getfinalpathname(PyObject *module, PyObject *arg)
 {
     PyObject *return_value = NULL;
-    PyObject *path;
+    path_t path = PATH_T_INITIALIZE("_getfinalpathname", "path", 0, 0);
 
-    if (!PyArg_Parse(arg, "U:_getfinalpathname", &path)) {
+    if (!PyArg_Parse(arg, "O&:_getfinalpathname", path_converter, &path)) {
         goto exit;
     }
-    return_value = os__getfinalpathname_impl(module, path);
+    return_value = os__getfinalpathname_impl(module, &path);
 
 exit:
+    /* Cleanup for path */
+    path_cleanup(&path);
+
     return return_value;
 }
 
@@ -1037,23 +1040,26 @@ PyDoc_STRVAR(os__getvolumepathname__doc__,
     {"_getvolumepathname", (PyCFunction)os__getvolumepathname, METH_FASTCALL|METH_KEYWORDS, os__getvolumepathname__doc__},
 
 static PyObject *
-os__getvolumepathname_impl(PyObject *module, PyObject *path);
+os__getvolumepathname_impl(PyObject *module, path_t *path);
 
 static PyObject *
 os__getvolumepathname(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames)
 {
     PyObject *return_value = NULL;
     static const char * const _keywords[] = {"path", NULL};
-    static _PyArg_Parser _parser = {"U:_getvolumepathname", _keywords, 0};
-    PyObject *path;
+    static _PyArg_Parser _parser = {"O&:_getvolumepathname", _keywords, 0};
+    path_t path = PATH_T_INITIALIZE("_getvolumepathname", "path", 0, 0);
 
     if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser,
-        &path)) {
+        path_converter, &path)) {
         goto exit;
     }
-    return_value = os__getvolumepathname_impl(module, path);
+    return_value = os__getvolumepathname_impl(module, &path);
 
 exit:
+    /* Cleanup for path */
+    path_cleanup(&path);
+
     return return_value;
 }
 
@@ -5014,23 +5020,26 @@ PyDoc_STRVAR(os__getdiskusage__doc__,
     {"_getdiskusage", (PyCFunction)os__getdiskusage, METH_FASTCALL|METH_KEYWORDS, os__getdiskusage__doc__},
 
 static PyObject *
-os__getdiskusage_impl(PyObject *module, Py_UNICODE *path);
+os__getdiskusage_impl(PyObject *module, path_t *path);
 
 static PyObject *
 os__getdiskusage(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames)
 {
     PyObject *return_value = NULL;
     static const char * const _keywords[] = {"path", NULL};
-    static _PyArg_Parser _parser = {"u:_getdiskusage", _keywords, 0};
-    Py_UNICODE *path;
+    static _PyArg_Parser _parser = {"O&:_getdiskusage", _keywords, 0};
+    path_t path = PATH_T_INITIALIZE("_getdiskusage", "path", 0, 0);
 
     if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser,
-        &path)) {
+        path_converter, &path)) {
         goto exit;
     }
-    return_value = os__getdiskusage_impl(module, path);
+    return_value = os__getdiskusage_impl(module, &path);
 
 exit:
+    /* Cleanup for path */
+    path_cleanup(&path);
+
     return return_value;
 }
 
@@ -6580,4 +6589,4 @@ exit:
 #ifndef OS_GETRANDOM_METHODDEF
     #define OS_GETRANDOM_METHODDEF
 #endif /* !defined(OS_GETRANDOM_METHODDEF) */
-/*[clinic end generated code: output=8e5d4a01257b6292 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=fc603214822bdda6 input=a9049054013a1b77]*/

Modules/posixmodule.c

@@ -3720,29 +3720,26 @@ os__getfullpathname_impl(PyObject *module, path_t *path)
 /*[clinic input]
 os._getfinalpathname
 
-    path: unicode
+    path: path_t
     /
 
 A helper function for samepath on windows.
 [clinic start generated code]*/
 
 static PyObject *
-os__getfinalpathname_impl(PyObject *module, PyObject *path)
-/*[clinic end generated code: output=9bd78d0e52782e75 input=71d5e89334891bf4]*/
+os__getfinalpathname_impl(PyObject *module, path_t *path)
+/*[clinic end generated code: output=621a3c79bc29ebfa input=2b6b6c7cbad5fb84]*/
 {
     HANDLE hFile;
     int buf_size;
     wchar_t *target_path;
     int result_length;
     PyObject *result;
-    const wchar_t *path_wchar;
-
-    path_wchar = _PyUnicode_AsUnicode(path);
-    if (path_wchar == NULL)
-        return NULL;
+    const char *err = NULL;
 
+    Py_BEGIN_ALLOW_THREADS
     hFile = CreateFileW(
-        path_wchar,
+        path->wide,
         0, /* desired access */
         0, /* share mode */
         NULL, /* security attributes */
@@ -3751,32 +3748,54 @@ os__getfinalpathname_impl(PyObject *module, PyObject *path)
         FILE_FLAG_BACKUP_SEMANTICS,
         NULL);
 
-    if(hFile == INVALID_HANDLE_VALUE)
-        return win32_error_object("CreateFileW", path);
+    if (hFile == INVALID_HANDLE_VALUE) {
+        err = "CreateFileW";
+        goto done1;
+    }
 
     /* We have a good handle to the target, use it to determine the
        target path name. */
     buf_size = GetFinalPathNameByHandleW(hFile, 0, 0, VOLUME_NAME_NT);
 
-    if(!buf_size)
-        return win32_error_object("GetFinalPathNameByHandle", path);
+    if (!buf_size) {
+        err = "GetFinalPathNameByHandle";
+        goto done1;
+    }
+done1:
+    Py_END_ALLOW_THREADS
+    if (err)
+        return win32_error_object(err, path->object);
 
     target_path = PyMem_New(wchar_t, buf_size+1);
     if(!target_path)
         return PyErr_NoMemory();
 
+    Py_BEGIN_ALLOW_THREADS
     result_length = GetFinalPathNameByHandleW(hFile, target_path,
                                               buf_size, VOLUME_NAME_DOS);
-    if(!result_length)
-        return win32_error_object("GetFinalPathNamyByHandle", path);
+    if (!result_length) {
+        err = "GetFinalPathNameByHandle";
+        goto done2;
+    }
 
-    if(!CloseHandle(hFile))
-        return win32_error_object("CloseHandle", path);
+    if (!CloseHandle(hFile)) {
+        err = "CloseHandle";
+        goto done2;
+    }
+done2:
+    Py_END_ALLOW_THREADS
+    if (err) {
+        PyMem_Free(target_path);
+        return win32_error_object(err, path->object);
+    }
 
     target_path[result_length] = 0;
     result = PyUnicode_FromWideChar(target_path, result_length);
     PyMem_Free(target_path);
+    if (path->narrow)
+        Py_SETREF(result, PyUnicode_EncodeFSDefault(result));
     return result;
+
 }
 
 /*[clinic input]
@@ -3811,28 +3830,22 @@ os__isdir_impl(PyObject *module, path_t *path)
 /*[clinic input]
 os._getvolumepathname
 
-    path: unicode
+    path: path_t
 
 A helper function for ismount on Win32.
 [clinic start generated code]*/
 
 static PyObject *
-os__getvolumepathname_impl(PyObject *module, PyObject *path)
-/*[clinic end generated code: output=cbdcbd1059ceef4c input=7eacadc40acbda6b]*/
+os__getvolumepathname_impl(PyObject *module, path_t *path)
+/*[clinic end generated code: output=804c63fd13a1330b input=722b40565fa21552]*/
 {
     PyObject *result;
-    const wchar_t *path_wchar;
     wchar_t *mountpath=NULL;
     size_t buflen;
     BOOL ret;
 
-    path_wchar = PyUnicode_AsUnicodeAndSize(path, &buflen);
-    if (path_wchar == NULL)
-        return NULL;
-    buflen += 1;
-
     /* Volume path should be shorter than entire path */
-    buflen = Py_MAX(buflen, MAX_PATH);
+    buflen = Py_MAX(path->length, MAX_PATH);
 
     if (buflen > PY_DWORD_MAX) {
         PyErr_SetString(PyExc_OverflowError, "path too long");
@@ -3844,15 +3857,17 @@ os__getvolumepathname_impl(PyObject *module, PyObject *path)
         return PyErr_NoMemory();
 
     Py_BEGIN_ALLOW_THREADS
-    ret = GetVolumePathNameW(path_wchar, mountpath,
+    ret = GetVolumePathNameW(path->wide, mountpath,
                              Py_SAFE_DOWNCAST(buflen, size_t, DWORD));
     Py_END_ALLOW_THREADS
 
     if (!ret) {
-        result = win32_error_object("_getvolumepathname", path);
+        result = win32_error_object("_getvolumepathname", path->object);
         goto exit;
     }
     result = PyUnicode_FromWideChar(mountpath, wcslen(mountpath));
+    if (path->narrow)
+        Py_SETREF(result, PyUnicode_EncodeFSDefault(result));
 
 exit:
     PyMem_Free(mountpath);
@@ -9844,20 +9859,20 @@ os_statvfs_impl(PyObject *module, path_t *path)
 /*[clinic input]
 os._getdiskusage
 
-    path: Py_UNICODE
+    path: path_t
 
 Return disk usage statistics about the given path as a (total, free) tuple.
 [clinic start generated code]*/
 
 static PyObject *
-os__getdiskusage_impl(PyObject *module, Py_UNICODE *path)
-/*[clinic end generated code: output=76d6adcd86b1db0b input=6458133aed893c78]*/
+os__getdiskusage_impl(PyObject *module, path_t *path)
+/*[clinic end generated code: output=3bd3991f5e5c5dfb input=6af8d1b7781cc042]*/
 {
     BOOL retval;
     ULARGE_INTEGER _, total, free;
 
     Py_BEGIN_ALLOW_THREADS
-    retval = GetDiskFreeSpaceExW(path, &_, &total, &free);
+    retval = GetDiskFreeSpaceExW(path->wide, &_, &total, &free);
     Py_END_ALLOW_THREADS
     if (retval == 0)
         return PyErr_SetFromWindowsErr(0);