summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--Lib/os.py16
-rw-r--r--Lib/test/test_os.py40
-rw-r--r--Misc/NEWS5
3 files changed, 53 insertions, 8 deletions
diff --git a/Lib/os.py b/Lib/os.py
index 5862383..864edfd 100644
--- a/Lib/os.py
+++ b/Lib/os.py
@@ -152,8 +152,20 @@ def makedirs(name, mode=0o777, exist_ok=False):
mkdir(name, mode)
except OSError as e:
import stat as st
- if not (e.errno == errno.EEXIST and exist_ok and path.isdir(name) and
- st.S_IMODE(lstat(name).st_mode) == _get_masked_mode(mode)):
+ 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 dir_exists and actual_mode != expected_mode:
+ e.strerror += ' (mode %o != expected mode %o)' % (
+ actual_mode, expected_mode)
raise
def removedirs(name):
diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
index 8bc8ba9..605d2a4 100644
--- a/Lib/test/test_os.py
+++ b/Lib/test/test_os.py
@@ -15,6 +15,7 @@ from test import support
import contextlib
import mmap
import uuid
+import stat
from test.script_helper import assert_python_ok
# Detect whether we're on a Linux system that uses the (now outdated
@@ -574,12 +575,39 @@ class MakedirTests(unittest.TestCase):
path = os.path.join(support.TESTFN, 'dir1')
mode = 0o777
old_mask = os.umask(0o022)
- 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, mode=mode, exist_ok=True)
- os.umask(old_mask)
+ try:
+ 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, mode=mode, exist_ok=True)
+ finally:
+ os.umask(old_mask)
+
+ def test_exist_ok_s_isgid_directory(self):
+ path = os.path.join(support.TESTFN, 'dir1')
+ S_ISGID = stat.S_ISGID
+ mode = 0o777
+ old_mask = os.umask(0o022)
+ try:
+ existing_testfn_mode = stat.S_IMODE(
+ os.lstat(support.TESTFN).st_mode)
+ os.chmod(support.TESTFN, existing_testfn_mode | S_ISGID)
+ if (os.lstat(support.TESTFN).st_mode & S_ISGID != S_ISGID):
+ raise unittest.SkipTest('No support for S_ISGID dir mode.')
+ # The os should apply S_ISGID from the parent dir for us, but
+ # this test need not depend on that behavior. Be explicit.
+ os.makedirs(path, mode | S_ISGID)
+ # http://bugs.python.org/issue14992
+ # Should not fail when the bit is already set.
+ 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)
+ finally:
+ os.umask(old_mask)
def test_exist_ok_existing_regular_file(self):
base = support.TESTFN
diff --git a/Misc/NEWS b/Misc/NEWS
index 3f99773..62c1312 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,11 @@ What's New in Python 3.2.4
Core and Builtins
-----------------
+- Issue #14992: os.makedirs(path, exist_ok=True) would raise an OSError
+ when the path existed and had the S_ISGID mode bit set when it was
+ not explicitly asked for. This is no longer an exception as mkdir
+ cannot control if the OS sets that bit for it or not.
+
- Issue #14775: Fix a potential quadratic dict build-up due to the garbage
collector repeatedly trying to untrack dicts.