bpo-46362: Ensure ntpath.abspath() uses the Windows API correctly (py…

…thonGH-30571) This makes ntpath.abspath()/getpath_abspath() follow normpath(), since some WinAPIs such as PathCchSkipRoot() require backslashed paths.
rjmcnamara · Jan 13, 2022 · d4e64cd · d4e64cd
1 parent b8ddf7e
commit d4e64cd
Show file tree

Hide file tree

Showing 8 changed files with 114 additions and 42 deletions.
diff --git a/Include/internal/pycore_fileutils.h b/Include/internal/pycore_fileutils.h
@@ -235,6 +235,9 @@ extern int _Py_EncodeNonUnicodeWchar_InPlace(
 
 extern int _Py_isabs(const wchar_t *path);
 extern int _Py_abspath(const wchar_t *path, wchar_t **abspath_p);
+#ifdef MS_WINDOWS
+extern int _PyOS_getfullpathname(const wchar_t *path, wchar_t **abspath_p);
+#endif
 extern wchar_t * _Py_join_relfile(const wchar_t *dirname,
                                   const wchar_t *relfile);
 extern int _Py_add_relfile(wchar_t *dirname,

diff --git a/Lib/ntpath.py b/Lib/ntpath.py
@@ -551,7 +551,7 @@ def _abspath_fallback(path):
     def abspath(path):
         """Return the absolute version of a path."""
         try:
-            return normpath(_getfullpathname(path))
+            return _getfullpathname(normpath(path))
         except (OSError, ValueError):
             return _abspath_fallback(path)
 

diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py
@@ -1404,6 +1404,33 @@ def test_init_pyvenv_cfg(self):
                                    api=API_COMPAT, env=env,
                                    ignore_stderr=True, cwd=tmpdir)
 
+    @unittest.skipUnless(MS_WINDOWS, 'specific to Windows')
+    def test_getpath_abspath_win32(self):
+        # Check _Py_abspath() is passed a backslashed path not to fall back to
+        # GetFullPathNameW() on startup, which (re-)normalizes the path overly.
+        # Currently, _Py_normpath() doesn't trim trailing dots and spaces.
+        CASES = [
+            ("C:/a. . .",  "C:\\a. . ."),
+            ("C:\\a. . .", "C:\\a. . ."),
+            ("\\\\?\\C:////a////b. . .", "\\\\?\\C:\\a\\b. . ."),
+            ("//a/b/c. . .", "\\\\a\\b\\c. . ."),
+            ("\\\\a\\b\\c. . .", "\\\\a\\b\\c. . ."),
+            ("a. . .", f"{os.getcwd()}\\a"),  # relpath gets fully normalized
+        ]
+        out, err = self.run_embedded_interpreter(
+            "test_init_initialize_config",
+            env=dict(PYTHONPATH=os.path.pathsep.join(c[0] for c in CASES))
+        )
+        self.assertEqual(err, "")
+        try:
+            out = json.loads(out)
+        except json.JSONDecodeError:
+            self.fail(f"fail to decode stdout: {out!r}")
+
+        results = out['config']["module_search_paths"]
+        for (_, expected), result in zip(CASES, results):
+            self.assertEqual(result, expected)
+
     def test_global_pathconfig(self):
         # Test C API functions getting the path configuration:
         #

diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py
@@ -613,6 +613,40 @@ def test_expanduser(self):
     @unittest.skipUnless(nt, "abspath requires 'nt' module")
     def test_abspath(self):
         tester('ntpath.abspath("C:\\")', "C:\\")
+        tester('ntpath.abspath("\\\\?\\C:////spam////eggs. . .")', "\\\\?\\C:\\spam\\eggs")
+        tester('ntpath.abspath("\\\\.\\C:////spam////eggs. . .")', "\\\\.\\C:\\spam\\eggs")
+        tester('ntpath.abspath("//spam//eggs. . .")',     "\\\\spam\\eggs")
+        tester('ntpath.abspath("\\\\spam\\\\eggs. . .")', "\\\\spam\\eggs")
+        tester('ntpath.abspath("C:/spam. . .")',  "C:\\spam")
+        tester('ntpath.abspath("C:\\spam. . .")', "C:\\spam")
+        tester('ntpath.abspath("C:/nul")',  "\\\\.\\nul")
+        tester('ntpath.abspath("C:\\nul")', "\\\\.\\nul")
+        tester('ntpath.abspath("//..")',           "\\\\")
+        tester('ntpath.abspath("//../")',          "\\\\..\\")
+        tester('ntpath.abspath("//../..")',        "\\\\..\\")
+        tester('ntpath.abspath("//../../")',       "\\\\..\\..\\")
+        tester('ntpath.abspath("//../../../")',    "\\\\..\\..\\")
+        tester('ntpath.abspath("//../../../..")',  "\\\\..\\..\\")
+        tester('ntpath.abspath("//../../../../")', "\\\\..\\..\\")
+        tester('ntpath.abspath("//server")',           "\\\\server")
+        tester('ntpath.abspath("//server/")',          "\\\\server\\")
+        tester('ntpath.abspath("//server/..")',        "\\\\server\\")
+        tester('ntpath.abspath("//server/../")',       "\\\\server\\..\\")
+        tester('ntpath.abspath("//server/../..")',     "\\\\server\\..\\")
+        tester('ntpath.abspath("//server/../../")',    "\\\\server\\..\\")
+        tester('ntpath.abspath("//server/../../..")',  "\\\\server\\..\\")
+        tester('ntpath.abspath("//server/../../../")', "\\\\server\\..\\")
+        tester('ntpath.abspath("//server/share")',        "\\\\server\\share")
+        tester('ntpath.abspath("//server/share/")',       "\\\\server\\share\\")
+        tester('ntpath.abspath("//server/share/..")',     "\\\\server\\share\\")
+        tester('ntpath.abspath("//server/share/../")',    "\\\\server\\share\\")
+        tester('ntpath.abspath("//server/share/../..")',  "\\\\server\\share\\")
+        tester('ntpath.abspath("//server/share/../../")', "\\\\server\\share\\")
+        tester('ntpath.abspath("C:\\nul. . .")', "\\\\.\\nul")
+        tester('ntpath.abspath("//... . .")',  "\\\\")
+        tester('ntpath.abspath("//.. . . .")', "\\\\")
+        tester('ntpath.abspath("//../... . .")',  "\\\\..\\")
+        tester('ntpath.abspath("//../.. . . .")', "\\\\..\\")
         with os_helper.temp_cwd(os_helper.TESTFN) as cwd_dir: # bpo-31047
             tester('ntpath.abspath("")', cwd_dir)
             tester('ntpath.abspath(" ")', cwd_dir + "\\ ")

diff --git a/Misc/NEWS.d/next/Windows/2022-01-13-22-31-09.bpo-46362.f2cuEb.rst b/Misc/NEWS.d/next/Windows/2022-01-13-22-31-09.bpo-46362.f2cuEb.rst
@@ -0,0 +1,2 @@
+os.path.abspath("C:\CON") is now fixed to return "\\.\CON", not the same path.
+The regression was true of all legacy DOS devices such as COM1, LPT1, or NUL.
diff --git a/Modules/getpath.c b/Modules/getpath.c
@@ -59,16 +59,16 @@ getpath_abspath(PyObject *Py_UNUSED(self), PyObject *args)
 {
     PyObject *r = NULL;
     PyObject *pathobj;
-    const wchar_t *path;
+    wchar_t *path;
     if (!PyArg_ParseTuple(args, "U", &pathobj)) {
         return NULL;
     }
     Py_ssize_t len;
     path = PyUnicode_AsWideCharString(pathobj, &len);
     if (path) {
         wchar_t *abs;
-        if (_Py_abspath(path, &abs) == 0 && abs) {
-            r = PyUnicode_FromWideChar(_Py_normpath(abs, -1), -1);
+        if (_Py_abspath((const wchar_t *)_Py_normpath(path, -1), &abs) == 0 && abs) {
+            r = PyUnicode_FromWideChar(abs, -1);
             PyMem_RawFree((void *)abs);
         } else {
             PyErr_SetString(PyExc_OSError, "failed to make path absolute");

diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
@@ -4240,6 +4240,48 @@ os_listdir_impl(PyObject *module, path_t *path)
 }
 
 #ifdef MS_WINDOWS
+int
+_PyOS_getfullpathname(const wchar_t *path, wchar_t **abspath_p)
+{
+    wchar_t woutbuf[MAX_PATH], *woutbufp = woutbuf;
+    DWORD result;
+
+    result = GetFullPathNameW(path,
+                              Py_ARRAY_LENGTH(woutbuf), woutbuf,
+                              NULL);
+    if (!result) {
+        return -1;
+    }
+
+    if (result >= Py_ARRAY_LENGTH(woutbuf)) {
+        if ((size_t)result <= (size_t)PY_SSIZE_T_MAX / sizeof(wchar_t)) {
+            woutbufp = PyMem_RawMalloc((size_t)result * sizeof(wchar_t));
+        }
+        else {
+            woutbufp = NULL;
+        }
+        if (!woutbufp) {
+            *abspath_p = NULL;
+            return 0;
+        }
+
+        result = GetFullPathNameW(path, result, woutbufp, NULL);
+        if (!result) {
+            PyMem_RawFree(woutbufp);
+            return -1;
+        }
+    }
+
+    if (woutbufp != woutbuf) {
+        *abspath_p = woutbufp;
+        return 0;
+    }
+
+    *abspath_p = _PyMem_RawWcsdup(woutbufp);
+    return 0;
+}
+
+
 /* A helper function for abspath on win32 */
 /*[clinic input]
 os._getfullpathname
@@ -4255,8 +4297,7 @@ os__getfullpathname_impl(PyObject *module, path_t *path)
 {
     wchar_t *abspath;
 
-    /* _Py_abspath() is implemented with GetFullPathNameW() on Windows */
-    if (_Py_abspath(path->wide, &abspath) < 0) {
+    if (_PyOS_getfullpathname(path->wide, &abspath) < 0) {
         return win32_error_object("GetFullPathNameW", path->object);
     }
     if (abspath == NULL) {

diff --git a/Python/fileutils.c b/Python/fileutils.c
@@ -2049,42 +2049,7 @@ _Py_abspath(const wchar_t *path, wchar_t **abspath_p)
     }
 
 #ifdef MS_WINDOWS
-    wchar_t woutbuf[MAX_PATH], *woutbufp = woutbuf;
-    DWORD result;
-
-    result = GetFullPathNameW(path,
-                              Py_ARRAY_LENGTH(woutbuf), woutbuf,
-                              NULL);
-    if (!result) {
-        return -1;
-    }
-
-    if (result >= Py_ARRAY_LENGTH(woutbuf)) {
-        if ((size_t)result <= (size_t)PY_SSIZE_T_MAX / sizeof(wchar_t)) {
-            woutbufp = PyMem_RawMalloc((size_t)result * sizeof(wchar_t));
-        }
-        else {
-            woutbufp = NULL;
-        }
-        if (!woutbufp) {
-            *abspath_p = NULL;
-            return 0;
-        }
-
-        result = GetFullPathNameW(path, result, woutbufp, NULL);
-        if (!result) {
-            PyMem_RawFree(woutbufp);
-            return -1;
-        }
-    }
-
-    if (woutbufp != woutbuf) {
-        *abspath_p = woutbufp;
-        return 0;
-    }
-
-    *abspath_p = _PyMem_RawWcsdup(woutbufp);
-    return 0;
+    return _PyOS_getfullpathname(path, abspath_p);
 #else
     wchar_t cwd[MAXPATHLEN + 1];
     cwd[Py_ARRAY_LENGTH(cwd) - 1] = 0;
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		os.path.abspath("C:\CON") is now fixed to return "\\.\CON", not the same path.
		The regression was true of all legacy DOS devices such as COM1, LPT1, or NUL.