[Python-checkins] cpython (merge 3.2 -> default): Fixes Issue #14992: os.makedirs(path, exist_ok=True) would raise an OSError

[gregory.p.smith]

http://hg.python.org/cpython/rev/eed26e508b7e
changeset:   77330:eed26e508b7e
parent:      77328:0808cb8c60fd
parent:      77329:fef529f3de5b
user:        Gregory P. Smith <greg at krypto.org>
date:        Sun Jun 03 14:35:09 2012 -0700
summary:
  Fixes 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.

files:
  Lib/os.py           |  16 ++++++++++++++--
  Lib/test/test_os.py |  25 +++++++++++++++++++++++++
  Misc/NEWS           |   5 +++++
  3 files changed, 44 insertions(+), 2 deletions(-)


diff --git a/Lib/os.py b/Lib/os.py
--- a/Lib/os.py
+++ b/Lib/os.py
@@ -160,8 +160,20 @@
     try:
         mkdir(name, mode)
     except OSError as e:
-        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
--- a/Lib/test/test_os.py
+++ b/Lib/test/test_os.py
@@ -838,6 +838,31 @@
         os.makedirs(path, mode=mode, exist_ok=True)
         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
         path = os.path.join(support.TESTFN, 'dir1')
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,11 @@
 Library
 -------
 
+- 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 #14989: Make the CGI enable option to http.server available via command
   line.
 

-- 
Repository URL: http://hg.python.org/cpython