From 23ad6d0d1a7a6145a01494f4f3913a63d1f0250c Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 22 Feb 2018 10:39:10 -0800 Subject: [PATCH] bpo-32556: nt._getfinalpathname, nt._getvolumepathname and nt._getdiskusage now correctly convert from bytes. (GH-5761) --- Lib/test/test_ntpath.py | 46 ++++++++--- .../2018-02-19-14-27-51.bpo-32556.CsRsgr.rst | 2 + Modules/clinic/posixmodule.c.h | 39 +++++---- Modules/posixmodule.c | 81 +++++++++++-------- 4 files changed, 110 insertions(+), 58 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-02-19-14-27-51.bpo-32556.CsRsgr.rst diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py index 15215e497d..1eec26b200 100644 --- a/Lib/test/test_ntpath.py +++ b/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'] diff --git a/Misc/NEWS.d/next/Library/2018-02-19-14-27-51.bpo-32556.CsRsgr.rst b/Misc/NEWS.d/next/Library/2018-02-19-14-27-51.bpo-32556.CsRsgr.rst new file mode 100644 index 0000000000..1a475b308f --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-02-19-14-27-51.bpo-32556.CsRsgr.rst @@ -0,0 +1,2 @@ +nt._getfinalpathname, nt._getvolumepathname and nt._getdiskusage now +correctly convert from bytes. diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h index d6af15ffc2..4054389d15 100644 --- a/Modules/clinic/posixmodule.c.h +++ b/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]*/ diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 0bbf7c5796..4b59229883 100644 --- a/Modules/posixmodule.c +++ b/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); -- 2.40.0