gh-59616: Support os.chmod(follow_symlinks=True) and os.lchmod() on Windows

Author: serhiy-storchaka

Lib/os.py

@@ -171,6 +171,7 @@ def _add(str, fn):
     _add("HAVE_FSTATAT",    "stat")
     _add("HAVE_LCHFLAGS",   "chflags")
     _add("HAVE_LCHMOD",     "chmod")
+    _add("MS_WINDOWS",      "chmod")
     if _exists("lchown"): # mac os x10.3
         _add("HAVE_LCHOWN", "chown")
     _add("HAVE_LINKAT",     "link")

Lib/tempfile.py

@@ -273,7 +273,7 @@ def _dont_follow_symlinks(func, path, *args):
     # Pass follow_symlinks=False, unless not supported on this platform.
     if func in _os.supports_follow_symlinks:
         func(path, *args, follow_symlinks=False)
-    elif _os.name == 'nt' or not _os.path.islink(path):
+    elif not _os.path.islink(path):
         func(path, *args)
 
 def _resetperms(path):

Lib/test/libregrtest/main.py

@@ -492,6 +492,7 @@ def _run_tests(self, selected: TestTuple, tests: TestList | None) -> int:
                                          self.fail_rerun)
 
     def run_tests(self, selected: TestTuple, tests: TestList | None) -> int:
+        selected = ('test_posix', 'test_tempfile')
         os.makedirs(self.tmp_dir, exist_ok=True)
         work_dir = get_work_dir(self.tmp_dir)
 

Lib/test/test_posix.py

@@ -21,8 +21,10 @@
 
 try:
     import posix
+    nt = None
 except ImportError:
-    import nt as posix
+    import nt
+    posix = nt
 
 try:
     import pwd
@@ -935,6 +937,116 @@ def test_utime(self):
         posix.utime(os_helper.TESTFN, (int(now), int(now)))
         posix.utime(os_helper.TESTFN, (now, now))
 
+    def check_chmod(self, chmod_func, target, **kwargs):
+        mode = os.stat(target).st_mode
+        try:
+            chmod_func(target, mode & ~stat.S_IWRITE, **kwargs)
+            self.assertEqual(os.stat(target).st_mode, mode & ~stat.S_IWRITE)
+            if stat.S_ISREG(mode):
+                try:
+                    with open(target, 'wb+'):
+                        pass
+                except PermissionError:
+                    pass
+            chmod_func(target, mode | stat.S_IWRITE, **kwargs)
+            self.assertEqual(os.stat(target).st_mode, mode | stat.S_IWRITE)
+            if stat.S_ISREG(mode):
+                with open(target, 'wb+'):
+                    pass
+        finally:
+            posix.chmod(target, mode)
+
+    def test_chmod_file(self):
+        self.check_chmod(posix.chmod, os_helper.TESTFN)
+
+    def tempdir(self):
+        target = os_helper.TESTFN + 'd'
+        posix.mkdir(target)
+        self.addCleanup(posix.rmdir, target)
+        return target
+
+    def test_chmod_dir(self):
+        target = self.tempdir()
+        self.check_chmod(posix.chmod, target)
+
+    @unittest.skipUnless(hasattr(posix, 'lchmod'), 'test needs os.lchmod()')
+    def test_lchmod_file(self):
+        self.check_chmod(posix.lchmod, os_helper.TESTFN)
+        self.check_chmod(posix.chmod, os_helper.TESTFN, follow_symlinks=False)
+
+    @unittest.skipUnless(hasattr(posix, 'lchmod'), 'test needs os.lchmod()')
+    def test_lchmod_dir(self):
+        target = self.tempdir()
+        self.check_chmod(posix.lchmod, target)
+        self.check_chmod(posix.chmod, target, follow_symlinks=False)
+
+    def check_chmod_link(self, chmod_func, target, link, **kwargs):
+        target_mode = os.stat(target).st_mode
+        link_mode = os.lstat(link).st_mode
+        try:
+            chmod_func(link, target_mode & ~stat.S_IWRITE, **kwargs)
+            self.assertEqual(os.stat(target).st_mode, target_mode & ~stat.S_IWRITE)
+            self.assertEqual(os.lstat(link).st_mode, link_mode)
+            chmod_func(link, target_mode | stat.S_IWRITE)
+            self.assertEqual(os.stat(target).st_mode, target_mode | stat.S_IWRITE)
+            self.assertEqual(os.lstat(link).st_mode, link_mode)
+        finally:
+            posix.chmod(target, target_mode)
+
+    def check_lchmod_link(self, chmod_func, target, link, **kwargs):
+        target_mode = os.stat(target).st_mode
+        link_mode = os.lstat(link).st_mode
+        chmod_func(link, link_mode & ~stat.S_IWRITE, **kwargs)
+        self.assertEqual(os.stat(target).st_mode, target_mode)
+        self.assertEqual(os.lstat(link).st_mode, link_mode & ~stat.S_IWRITE)
+        chmod_func(link, link_mode | stat.S_IWRITE)
+        self.assertEqual(os.stat(target).st_mode, target_mode)
+        self.assertEqual(os.lstat(link).st_mode, link_mode | stat.S_IWRITE)
+
+    @os_helper.skip_unless_symlink
+    def test_chmod_file_symlink(self):
+        target = os_helper.TESTFN
+        link = os_helper.TESTFN + '-link'
+        os.symlink(target, link)
+        self.addCleanup(posix.unlink, link)
+        if os.name == 'nt':
+            self.check_lchmod_link(posix.chmod, target, link)
+        else:
+            self.check_chmod_link(posix.chmod, target, link)
+        self.check_chmod_link(posix.chmod, target, link, follow_symlinks=True)
+
+    @os_helper.skip_unless_symlink
+    def test_chmod_dir_symlink(self):
+        target = self.tempdir()
+        link = os_helper.TESTFN + '-link'
+        os.symlink(target, link, target_is_directory=True)
+        self.addCleanup(posix.unlink, link)
+        if os.name == 'nt':
+            self.check_lchmod_link(posix.chmod, target, link)
+        else:
+            self.check_chmod_link(posix.chmod, target, link)
+        self.check_chmod_link(posix.chmod, target, link, follow_symlinks=True)
+
+    @unittest.skipUnless(hasattr(posix, 'lchmod'), 'test needs os.lchmod()')
+    @os_helper.skip_unless_symlink
+    def test_lchmod_file_symlink(self):
+        target = os_helper.TESTFN
+        link = os_helper.TESTFN + '-link'
+        os.symlink(target, link)
+        self.addCleanup(posix.unlink, link)
+        self.check_lchmod_link(posix.chmod, target, link, follow_symlinks=False)
+        self.check_lchmod_link(posix.lchmod, target, link)
+
+    @unittest.skipUnless(hasattr(posix, 'lchmod'), 'test needs os.lchmod()')
+    @os_helper.skip_unless_symlink
+    def test_lchmod_dir_symlink(self):
+        target = self.tempdir()
+        link = os_helper.TESTFN + '-link'
+        os.symlink(target, link)
+        self.addCleanup(posix.unlink, link)
+        self.check_lchmod_link(posix.chmod, target, link, follow_symlinks=False)
+        self.check_lchmod_link(posix.lchmod, target, link)
+
     def _test_chflags_regular_file(self, chflags_func, target_file, **kwargs):
         st = os.stat(target_file)
         self.assertTrue(hasattr(st, 'st_flags'))

Modules/clinic/posixmodule.c.h

@@ -3309,6 +3309,27 @@ os_fchdir_impl(PyObject *module, int fd)
 }
 #endif /* HAVE_FCHDIR */
 
+#ifdef MS_WINDOWS
+# define CHMOD_DEFAULT_FOLLOW_SYMLINKS 0
+#else
+# define CHMOD_DEFAULT_FOLLOW_SYMLINKS 1
+#endif
+
+#ifdef MS_WINDOWS
+static int
+win32_lchmod(LPCWSTR path, int mode)
+{
+    DWORD attr = GetFileAttributesW(path);
+    if (attr == INVALID_FILE_ATTRIBUTES) {
+        return 0;
+    }
+    if (mode & _S_IWRITE)
+        attr &= ~FILE_ATTRIBUTE_READONLY;
+    else
+        attr |= FILE_ATTRIBUTE_READONLY;
+    return SetFileAttributesW(path, attr);
+}
+#endif
 
 /*[clinic input]
 os.chmod
@@ -3331,7 +3352,7 @@ os.chmod
         and path should be relative; path will then be relative to that
         directory.
 
-    follow_symlinks: bool = True
+    follow_symlinks: bool(c_default="CHMOD_DEFAULT_FOLLOW_SYMLINKS") = _CHMOD_DEFAULT_FOLLOW_SYMLINKS
         If False, and the last element of the path is a symbolic link,
         chmod will modify the symbolic link itself instead of the file
         the link points to.
@@ -3348,43 +3369,55 @@ dir_fd and follow_symlinks may not be implemented on your platform.
 static PyObject *
 os_chmod_impl(PyObject *module, path_t *path, int mode, int dir_fd,
               int follow_symlinks)
-/*[clinic end generated code: output=5cf6a94915cc7bff input=674a14bc998de09d]*/
+/*[clinic end generated code: output=5cf6a94915cc7bff input=b28ffee79f567860]*/
 {
     int result;
 
-#ifdef MS_WINDOWS
-    DWORD attr;
-#endif
-
 #ifdef HAVE_FCHMODAT
     int fchmodat_nofollow_unsupported = 0;
     int fchmodat_unsupported = 0;
 #endif
 
-#if !(defined(HAVE_FCHMODAT) || defined(HAVE_LCHMOD))
+#if !(defined(HAVE_FCHMODAT) || defined(HAVE_LCHMOD) || defined(MS_WINDOWS))
     if (follow_symlinks_specified("chmod", follow_symlinks))
         return NULL;
 #endif
 
-    if (PySys_Audit("os.chmod", "Oii", path->object, mode,
-                    dir_fd == DEFAULT_DIR_FD ? -1 : dir_fd) < 0) {
+    if (PySys_Audit("os.chmod", "OiiO", path->object, mode,
+                    dir_fd == DEFAULT_DIR_FD ? -1 : dir_fd,
+                    follow_symlinks ? Py_True : Py_False) < 0) {
         return NULL;
     }
 
 #ifdef MS_WINDOWS
+    result = 0;
     Py_BEGIN_ALLOW_THREADS
-    attr = GetFileAttributesW(path->wide);
-    if (attr == INVALID_FILE_ATTRIBUTES)
-        result = 0;
+    if (follow_symlinks) {
+        HANDLE hfile;
+        FILE_BASIC_INFO info;
+
+        hfile = CreateFileW(path->wide,
+                            FILE_READ_ATTRIBUTES|FILE_WRITE_ATTRIBUTES,
+                            0, NULL,
+                            OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
+        if (hfile != INVALID_HANDLE_VALUE) {
+            if (GetFileInformationByHandleEx(hfile, FileBasicInfo,
+                                             &info, sizeof(info)))
+            {
+                if (mode & _S_IWRITE)
+                    info.FileAttributes &= ~FILE_ATTRIBUTE_READONLY;
+                else
+                    info.FileAttributes |= FILE_ATTRIBUTE_READONLY;
+                result = SetFileInformationByHandle(hfile, FileBasicInfo,
+                                                    &info, sizeof(info));
+            }
+            CloseHandle(hfile);
+        }
+    }
     else {
-        if (mode & _S_IWRITE)
-            attr &= ~FILE_ATTRIBUTE_READONLY;
-        else
-            attr |= FILE_ATTRIBUTE_READONLY;
-        result = SetFileAttributesW(path->wide, attr);
+        result = win32_lchmod(path->wide, mode);
     }
     Py_END_ALLOW_THREADS
-
     if (!result) {
         return path_error(path);
     }
@@ -3514,7 +3547,7 @@ os_fchmod_impl(PyObject *module, int fd, int mode)
 #endif /* HAVE_FCHMOD */
 
 
-#ifdef HAVE_LCHMOD
+#if defined(HAVE_LCHMOD) || defined(MS_WINDOWS)
 /*[clinic input]
 os.lchmod
 
@@ -3532,19 +3565,29 @@ os_lchmod_impl(PyObject *module, path_t *path, int mode)
 /*[clinic end generated code: output=082344022b51a1d5 input=90c5663c7465d24f]*/
 {
     int res;
-    if (PySys_Audit("os.chmod", "Oii", path->object, mode, -1) < 0) {
+    if (PySys_Audit("os.chmod", "OiiO", path->object, mode, -1, Py_False) < 0) {
         return NULL;
     }
+#ifdef MS_WINDOWS
+    Py_BEGIN_ALLOW_THREADS
+    res = win32_lchmod(path->wide, mode);
+    Py_END_ALLOW_THREADS
+    if (!res) {
+        path_error(path);
+        return NULL;
+    }
+#else /* MS_WINDOWS */
     Py_BEGIN_ALLOW_THREADS
     res = lchmod(path->narrow, mode);
     Py_END_ALLOW_THREADS
     if (res < 0) {
         path_error(path);
         return NULL;
     }
+#endif /* MS_WINDOWS */
     Py_RETURN_NONE;
 }
-#endif /* HAVE_LCHMOD */
+#endif /* HAVE_LCHMOD || MS_WINDOWS */
 
 
 #ifdef HAVE_CHFLAGS
@@ -16927,6 +16970,7 @@ all_ins(PyObject *m)
     if (PyModule_AddIntConstant(m, "_LOAD_LIBRARY_SEARCH_USER_DIRS", LOAD_LIBRARY_SEARCH_USER_DIRS)) return -1;
     if (PyModule_AddIntConstant(m, "_LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR", LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR)) return -1;
 #endif
+    if (PyModule_Add(m, "_CHMOD_DEFAULT_FOLLOW_SYMLINKS", PyBool_FromLong(CHMOD_DEFAULT_FOLLOW_SYMLINKS))) return -1;
 
     return 0;
 }