From ee5f1c13d1ea21c628068fdf142823177f5526c2 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Tue, 1 Apr 2014 19:13:18 -0400 Subject: remove directory mode check from makedirs (closes #21082) --- Doc/library/os.rst | 14 +++++++++----- Lib/os.py | 28 +++++----------------------- Lib/test/test_os.py | 7 +++---- Misc/NEWS | 3 +++ 4 files changed, 20 insertions(+), 32 deletions(-) diff --git a/Doc/library/os.rst b/Doc/library/os.rst index abacd24..d168963 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -1188,11 +1188,8 @@ Files and Directories The default *mode* is ``0o777`` (octal). On some systems, *mode* is ignored. Where it is used, the current umask value is first masked out. - If *exists_ok* is ``False`` (the default), an :exc:`OSError` is raised if - the target directory already exists. If *exists_ok* is ``True`` an - :exc:`OSError` is still raised if the umask-masked *mode* is different from - the existing mode, on systems where the mode is used. :exc:`OSError` will - also be raised if the directory creation fails. + If *exist_ok* is ``False`` (the default), an :exc:`OSError` is raised if the + target directory already exists. .. note:: @@ -1204,6 +1201,13 @@ Files and Directories .. versionadded:: 3.2 The *exist_ok* parameter. + .. versionchanged:: 3.2.5 + + Before Python 3.2.5, if *exist_ok* was ``True`` and the directory existed, + :func:`makedirs` would still raise an error if *mode* did not match the + mode of the existing directory. Since this behavior was impossible to + implement safely, it was removed in Python 3.2.6. See :issue:`21082`. + .. function:: pathconf(path, name) diff --git a/Lib/os.py b/Lib/os.py index 81e037a..34cbdc9 100644 --- a/Lib/os.py +++ b/Lib/os.py @@ -114,12 +114,6 @@ SEEK_SET = 0 SEEK_CUR = 1 SEEK_END = 2 - -def _get_masked_mode(mode): - mask = umask(0) - umask(mask) - return mode & ~mask - #' # Super directory utilities. @@ -128,11 +122,10 @@ def _get_masked_mode(mode): def makedirs(name, mode=0o777, exist_ok=False): """makedirs(path [, mode=0o777][, exist_ok=False]) - Super-mkdir; create a leaf directory and all intermediate ones. - Works like mkdir, except that any intermediate path segment (not - just the rightmost) will be created if it does not exist. If the - target directory with the same mode as we specified already exists, - raises an OSError if exist_ok is False, otherwise no exception is + Super-mkdir; create a leaf directory and all intermediate ones. Works like + mkdir, except that any intermediate path segment (not just the rightmost) + will be created if it does not exist. If the target directory already + exists, raise an OSError if exist_ok is False. Otherwise no exception is raised. This is recursive. """ @@ -154,18 +147,7 @@ def makedirs(name, mode=0o777, exist_ok=False): try: mkdir(name, mode) except OSError as e: - import stat as st - dir_exists = path.isdir(name) - expected_mode = _get_masked_mode(mode) - if dir_exists: - # S_ISGID is automatically copied by the OS from parent to child - # directories on mkdir. Don't consider it being set to be a mode - # mismatch as mkdir does not unset it when not specified in mode. - actual_mode = st.S_IMODE(lstat(name).st_mode) & ~st.S_ISGID - else: - actual_mode = -1 - if not (e.errno == errno.EEXIST and exist_ok and dir_exists and - actual_mode == expected_mode): + if not exist_ok or e.errno != errno.EEXIST or not path.isdir(name): raise def removedirs(name): diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 720e78b..a08edd6 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -579,7 +579,7 @@ class MakedirTests(unittest.TestCase): os.makedirs(path, mode) self.assertRaises(OSError, os.makedirs, path, mode) self.assertRaises(OSError, os.makedirs, path, mode, exist_ok=False) - self.assertRaises(OSError, os.makedirs, path, 0o776, exist_ok=True) + os.makedirs(path, 0o776, exist_ok=True) os.makedirs(path, mode=mode, exist_ok=True) finally: os.umask(old_mask) @@ -606,9 +606,8 @@ class MakedirTests(unittest.TestCase): os.makedirs(path, mode, exist_ok=True) # remove the bit. os.chmod(path, stat.S_IMODE(os.lstat(path).st_mode) & ~S_ISGID) - with self.assertRaises(OSError): - # Should fail when the bit is not already set when demanded. - os.makedirs(path, mode | S_ISGID, exist_ok=True) + # May work even when the bit is not already set when demanded. + os.makedirs(path, mode | S_ISGID, exist_ok=True) finally: os.umask(old_mask) diff --git a/Misc/NEWS b/Misc/NEWS index 693a0c8..3913f94 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,9 @@ What's New in Python 3.2.6? Library ------- +- Issue #21082: In os.makedirs, do not set the process-wide umask. Note this + changes behavior of makedirs when exist_ok=True. + - Issue #20246: Fix buffer overflow in socket.recvfrom_into. - Issue #12226: HTTPS is now used by default when connecting to PyPI. -- cgit v0.12