From 15c667b40bde66af11b40b7d09950e8c9ec62b2d Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 13 Dec 2023 13:25:39 +0200 Subject: [PATCH] gh-59616: Support os.chmod(follow_symlinks=True) and os.lchmod() on Windows --- Lib/os.py | 1 + Lib/tempfile.py | 2 +- Lib/test/libregrtest/main.py | 1 + Lib/test/test_os.py | 50 +++++++ Lib/test/test_posix.py | 114 ++++++++++++++- Modules/clinic/posixmodule.c.h | 9 +- Modules/posixmodule.c | 252 ++++++++++++++++++++++++++++++--- 7 files changed, 401 insertions(+), 28 deletions(-) diff --git a/Lib/os.py b/Lib/os.py index a17946750ea7e74..8c4b93250918eb6 100644 --- a/Lib/os.py +++ b/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") diff --git a/Lib/tempfile.py b/Lib/tempfile.py index cbfc172a789b251..b5a15f7b72c8722 100644 --- a/Lib/tempfile.py +++ b/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): diff --git a/Lib/test/libregrtest/main.py b/Lib/test/libregrtest/main.py index 7ca1b1cb65ae40e..63fac1d6c38b708 100644 --- a/Lib/test/libregrtest/main.py +++ b/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) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index d4680ef0f0e03f7..6d1564fa5a216d3 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -3117,6 +3117,56 @@ def test_directory_link_nonlocal(self): os.symlink('some_dir', src) assert os.path.isdir(src) + def test_chmod_link(self): + TESTFN2 = support.TESTFN+"_link" + + def cleanup(): + if os.path.exists(TESTFN2): + os.chmod(TESTFN2, 0o100666) + os.unlink(TESTFN2) + + if os.path.exists(support.TESTFN): + os.chmod(support.TESTFN, 0o666) + os.unlink(support.TESTFN) + + cleanup() + + open(support.TESTFN, "w").close() + try: + os.symlink(support.TESTFN, TESTFN2) + os.chmod(TESTFN2, 0o444) + self.assertEqual(os.stat(TESTFN2).st_mode & 0o777, 0o444) + + os.chmod(TESTFN2, 0o666) + self.assertEqual(os.stat(support.TESTFN).st_mode & 0o777, 0o666) + finally: + cleanup() + + def test_chmod_link(self): + TESTFN2 = support.TESTFN+"_link" + + def cleanup(): + if os.path.exists(TESTFN2): + os.chmod(TESTFN2, 0o100666) + os.unlink(TESTFN2) + + if os.path.exists(support.TESTFN): + os.chmod(support.TESTFN, 0o666) + os.unlink(support.TESTFN) + + cleanup() + + open(support.TESTFN, "w").close() + try: + os.symlink(support.TESTFN, TESTFN2) + os.chmod(TESTFN2, 0o444) + self.assertEqual(os.stat(TESTFN2).st_mode & 0o777, 0o444) + + os.chmod(TESTFN2, 0o666) + self.assertEqual(os.stat(support.TESTFN).st_mode & 0o777, 0o666) + finally: + cleanup() + class FSEncodingTests(unittest.TestCase): def test_nop(self): diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index 1722c84727bbd89..b03037e924cf709 100644 --- a/Lib/test/test_posix.py +++ b/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')) diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h index 9d6cd337f4a2f4f..7be35476abd4f48 100644 --- a/Modules/clinic/posixmodule.c.h +++ b/Modules/clinic/posixmodule.c.h @@ -493,7 +493,8 @@ os_fchdir(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *k #endif /* defined(HAVE_FCHDIR) */ PyDoc_STRVAR(os_chmod__doc__, -"chmod($module, /, path, mode, *, dir_fd=None, follow_symlinks=True)\n" +"chmod($module, /, path, mode, *, dir_fd=None,\n" +" follow_symlinks=_CHMOD_DEFAULT_FOLLOW_SYMLINKS)\n" "--\n" "\n" "Change the access permissions of a file.\n" @@ -677,7 +678,7 @@ os_fchmod(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *k #endif /* defined(HAVE_FCHMOD) */ -#if defined(HAVE_LCHMOD) +#if (defined(HAVE_LCHMOD) || defined(MS_WINDOWS)) PyDoc_STRVAR(os_lchmod__doc__, "lchmod($module, /, path, mode)\n" @@ -747,7 +748,7 @@ os_lchmod(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *k return return_value; } -#endif /* defined(HAVE_LCHMOD) */ +#endif /* (defined(HAVE_LCHMOD) || defined(MS_WINDOWS)) */ #if defined(HAVE_CHFLAGS) @@ -12421,4 +12422,4 @@ os__supports_virtual_terminal(PyObject *module, PyObject *Py_UNUSED(ignored)) #ifndef OS__SUPPORTS_VIRTUAL_TERMINAL_METHODDEF #define OS__SUPPORTS_VIRTUAL_TERMINAL_METHODDEF #endif /* !defined(OS__SUPPORTS_VIRTUAL_TERMINAL_METHODDEF) */ -/*[clinic end generated code: output=ff0ec3371de19904 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=3009d169f2bf20e7 input=a9049054013a1b77]*/ diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index ddbb4cd43babfcf..43725f35d3a0897 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -2190,25 +2190,106 @@ static PyStructSequence_Field stat_result_fields[] = { #ifdef HAVE_STRUCT_STAT_ST_REPARSE_TAG {"st_reparse_tag", "Windows reparse tag"}, #endif +<<<<<<< {0} +======= + int st_mtime_nsec; + time_t st_ctime; + int st_ctime_nsec; + DWORD file_attribute; +>>>>>>> }; +<<<<<<< #ifdef HAVE_STRUCT_STAT_ST_BLKSIZE #define ST_BLKSIZE_IDX 16 +======= +attribute_data_to_stat(BY_HANDLE_FILE_INFORMATION *info, ULONG reparse_tag, struct win32_stat *result) +{ + memset(result, 0, sizeof(*result)); + result->file_attribute = info->dwFileAttributes; + result->st_mode = attributes_to_mode(info->dwFileAttributes); + result->st_size = (((__int64)info->nFileSizeHigh)<<32) + info->nFileSizeLow; + FILE_TIME_to_time_t_nsec(&info->ftCreationTime, &result->st_ctime, &result->st_ctime_nsec); +>>>>>>> +<<<<<<< #else #define ST_BLKSIZE_IDX 15 -#endif +======= +static int +win32_xstat_impl_w(const wchar_t *path, struct win32_stat *result, + wchar_t **finalpath, BOOL traverse); +static int +win32_xstat_impl(const char *path, struct win32_stat *result, + BOOL traverse) +>>>>>>> +<<<<<<< +#endif +======= + HANDLE hFile, hFile2; + BY_HANDLE_FILE_INFORMATION info; + ULONG reparse_tag = 0; + wchar_t *target_path, *finalpath = NULL; + const char *dot; +>>>>>>> + +<<<<<<< #ifdef HAVE_STRUCT_STAT_ST_BLOCKS #define ST_BLOCKS_IDX (ST_BLKSIZE_IDX+1) #else +======= + if (!get_target_path(hFile2, &target_path)) + return -1; + + finalpath = NULL; + code = win32_xstat_impl_w(target_path, result, &finalpath, + FALSE); + free(target_path); + free(finalpath); + + return code; + } + } else +>>>>>>> +<<<<<<< #define ST_BLOCKS_IDX ST_BLKSIZE_IDX #endif +======= + +static int +win32_xstat_impl_w(const wchar_t *path, struct win32_stat *result, + wchar_t **finalpath, BOOL traverse) +{ + int code; + HANDLE hFile, hFile2; +>>>>>>> +<<<<<<< #ifdef HAVE_STRUCT_STAT_ST_RDEV +======= + wchar_t *target_path; + const wchar_t *dot; + + *finalpath = NULL; + if(!check_GetFinalPathNameByHandle()) { + /* If the OS doesn't have GetFinalPathNameByHandle, don't + traverse reparse point. */ +>>>>>>> +<<<<<<< #define ST_RDEV_IDX (ST_BLOCKS_IDX+1) #else #define ST_RDEV_IDX ST_BLOCKS_IDX +======= + if (!get_target_path(hFile2, &target_path)) + return -1; + + code = win32_xstat_impl_w(target_path, result, finalpath, + FALSE); + free(target_path); + return code; + } +>>>>>>> #endif #ifdef HAVE_STRUCT_STAT_ST_FLAGS @@ -3309,6 +3390,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 +3433,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 +3450,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 +3628,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,9 +3646,18 @@ 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 @@ -3542,9 +3665,10 @@ os_lchmod_impl(PyObject *module, path_t *path, int mode) path_error(path); return NULL; } +#endif /* MS_WINDOWS */ Py_RETURN_NONE; } -#endif /* HAVE_LCHMOD */ +#endif /* HAVE_LCHMOD || MS_WINDOWS */ #ifdef HAVE_CHFLAGS @@ -7538,9 +7662,11 @@ check_null_or_callable(PyObject *obj, const char* obj_name) obj_name, _PyType_Name(Py_TYPE(obj))); return -1; } + *finalpath = wcsdup(path); return 0; } +<<<<<<< /*[clinic input] os.register_at_fork @@ -7550,8 +7676,89 @@ os.register_at_fork after_in_child: object=NULL A callable to be called in the child after fork(). after_in_parent: object=NULL +======= +static int +win32_xstat_w(const wchar_t *path, struct win32_stat *result, BOOL traverse) +{ + wchar_t *finalpath = NULL; + /* Protocol violation: we explicitly clear errno, instead of + setting it to a POSIX error. Callers should use GetLastError. */ + int code = win32_xstat_impl_w(path, result, &finalpath, traverse); + free(finalpath); + errno = 0; + return code; +} +>>>>>>> +<<<<<<< A callable to be called in the parent after fork(). +======= + int mode; + int dir_fd = DEFAULT_DIR_FD; + int follow_symlinks = 1; + int result, xstat_result; + wchar_t *finalpath = NULL; + PyObject *return_value = NULL; + static char *keywords[] = {"path", "mode", "dir_fd", + "follow_symlinks", NULL}; +>>>>>>> +<<<<<<< +======= +#endif + +#ifdef MS_WINDOWS + if (path.wide && follow_symlinks && check_GetFinalPathNameByHandle()) { + struct win32_stat st; + + Py_BEGIN_ALLOW_THREADS + + xstat_result = win32_xstat_impl_w(path.wide, &st, + &finalpath, TRUE); + if (!xstat_result) { + attr = st.file_attribute; + if (attr == 0xFFFFFFFF) + result = 0; + else { + if (mode & _S_IWRITE) + attr &= ~FILE_ATTRIBUTE_READONLY; + else + attr |= FILE_ATTRIBUTE_READONLY; + result = SetFileAttributesW(finalpath, attr); + } + free(finalpath); + } + Py_END_ALLOW_THREADS + + if (xstat_result) { + return_value = win32_error_object("chmod", path.object); + goto exit; + } + } + else { + Py_BEGIN_ALLOW_THREADS + + if (path.wide) + attr = GetFileAttributesW(path.wide); + else + attr = GetFileAttributesA(path.narrow); + if (attr == 0xFFFFFFFF) + result = 0; + else { + if (mode & _S_IWRITE) + attr &= ~FILE_ATTRIBUTE_READONLY; + else + attr |= FILE_ATTRIBUTE_READONLY; + if (path.wide) + result = SetFileAttributesW(path.wide, attr); + else + result = SetFileAttributesA(path.narrow, attr); + } + Py_END_ALLOW_THREADS + } + + if (!result) { + return_value = win32_error_object("chmod", path.object); +>>>>>>> Register callables to be called when forking a new process. 'before' callbacks are called in reverse order. @@ -16927,6 +17134,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; }