]> granicus.if.org Git - python/commitdiff
Fixes Issue #14992: os.makedirs(path, exist_ok=True) would raise an OSError
authorGregory P. Smith <greg@krypto.org>
Sun, 3 Jun 2012 21:35:09 +0000 (14:35 -0700)
committerGregory P. Smith <greg@krypto.org>
Sun, 3 Jun 2012 21:35:09 +0000 (14:35 -0700)
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.

1  2 
Lib/os.py
Lib/test/test_os.py
Misc/NEWS

diff --cc Lib/os.py
index a1a35cd8e366c060083a3a42d393359adc2693cb,864edfd48e00eefa7f0690ace98f9ae3a7f88f53..11e4d60d15ec9844f6f5a0e425d8851498fbe2f0
+++ b/Lib/os.py
@@@ -160,8 -151,21 +160,20 @@@ def makedirs(name, mode=0o777, exist_ok
      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)):
 -        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 dir_exists and actual_mode != expected_mode:
+                 e.strerror += ' (mode %o != expected mode %o)' % (
+                         actual_mode, expected_mode)
              raise
  
  def removedirs(name):
index 9b29b37cf3d7e78bf92d7a42beeaa63de77b76ff,605d2a4632f42517064fac28d19179f3131d72d6..3ee5a1e08a3b1a8e444f6de6c02e6cd140b37b78
@@@ -831,13 -575,40 +831,38 @@@ class MakedirTests(unittest.TestCase)
          path = os.path.join(support.TESTFN, 'dir1')
          mode = 0o777
          old_mask = os.umask(0o022)
 -        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)
 +        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)
  
+     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 --cc Misc/NEWS
index e732604ae8f0f13c787eff32620539f4da5e0204,62c1312cddabcd217025f77bd5baca51920dd514..f0bac3215841282a9ed59fda358b3de46e60e29f
+++ b/Misc/NEWS
@@@ -2,92 -2,75 +2,97 @@@
  Python News
  +++++++++++
  
 -What's New in Python 3.2.4
 -==========================
 +What's New in Python 3.3.0 Beta 1?
 +==================================
  
 -*Release date: XX-XX-XXXX*
 +*Release date: XX-XXX-2012*
  
 -Core and Builtins
 ------------------
 +Library
 +-------
  
 -- Issue #14775: Fix a potential quadratic dict build-up due to the garbage
 -  collector repeatedly trying to untrack dicts.
+ - 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.
  
 -- Issue #14494: Fix __future__.py and its documentation to note that
 -  absolute imports are the default behavior in 3.0 instead of 2.7.
 -  Patch by Sven Marnach.
 +- Issue #14987: Add a missing import statement to inspect.
  
 -- Issue #14761: Fix potential leak on an error case in the import machinery.
 +- Issue #1079: email.header.decode_header now correctly parses all the examples
 +  in RFC2047.  There is a necessary visible behavior change: the leading and/or
 +  trailing whitespace on ASCII parts is now preserved.
  
 -- Issue #14699: Fix calling the classmethod descriptor directly.
 +- Issue #14969: Better handling of exception chaining in contextlib.ExitStack
  
 -- Issue #14433: Prevent msvcrt crash in interactive prompt when stdin
 -  is closed.
 +- Issue #14962: Update text coloring in IDLE shell window after changing
 +  options.  Patch by Roger Serwy.
  
 -- Issue #11603 (again): Setting __repr__ to __str__ now raises a RuntimeError
 -  when repr() or str() is called on such an object.
 +- Issue #14963: Convert contextlib.ExitStack.__exit__ to use an iterative
 +  algorithm (Patch by Alon Horev)
  
 -- Issue #14658: Fix binding a special method to a builtin implementation of a
 -  special method with a different name.
 +Tests
 +-----
  
 -- Issue #14630: Fix a memory access bug for instances of a subclass of int
 -  with value 0.
 +- Issue #14963 (partial): Add test cases for exception handling behaviour
 +  in contextlib.ExitStack (Initial patch by Alon Horev)
  
 -- Issue #14612: Fix jumping around with blocks by setting f_lineno.
  
 -- Issue #14607: Fix keyword-only arguments which started with ``__``.
 +What's New in Python 3.3.0 Alpha 4?
 +===================================
  
 -- Issue #13889: Check and (if necessary) set FPU control word before calling
 -  any of the dtoa.c string <-> float conversion functions, on MSVC builds of
 -  Python.  This fixes issues when embedding Python in a Delphi app.
 +*Release date: 31-May-2012*
  
 -- Issue #14474: Save and restore exception state in thread.start_new_thread()
 -  while writing error message if the thread leaves a unhandled exception.
 +Core and Builtins
 +-----------------
  
 -- Issue #13019: Fix potential reference leaks in bytearray.extend().  Patch
 -  by Suman Saha.
 +- Issue #14835: Make plistlib output empty arrays & dicts like OS X.
 +  Patch by Sidney San Martín.
  
 -- Issue #14378: Fix compiling ast.ImportFrom nodes with a "__future__" string as
 -  the module name that was not interned.
 +- Issue #14744: Use the new _PyUnicodeWriter internal API to speed up
 +  str%args and str.format(args).
  
 -- Issue #14331: Use significantly less stack space when importing modules by
 -  allocating path buffers on the heap instead of the stack.
 +- Issue #14930: Make memoryview objects weakrefable.
  
 -- Issue #14334: Prevent in a segfault in type.__getattribute__ when it was not
 -  passed strings.
 +- Issue #14775: Fix a potential quadratic dict build-up due to the garbage
 +  collector repeatedly trying to untrack dicts.
  
 -- Issue #1469629: Allow cycles through an object's __dict__ slot to be
 -  collected. (For example if ``x.__dict__ is x``).
 +- Issue #14857: fix regression in references to PEP 3135 implicit __class__
 +  closure variable (Reopens issue #12370)
  
 -- Issue #14172: Fix reference leak when marshalling a buffer-like object
 -  (other than a bytes object).
 +- Issue #14712 (PEP 405): Virtual environments. Implemented by Vinay Sajip.
  
 -- Issue #13521: dict.setdefault() now does only one lookup for the given key,
 -  making it "atomic" for many purposes.  Patch by Filip Gruszczyński.
 +- Issue #14660 (PEP 420): Namespace packages. Implemented by Eric Smith.
  
 -- Issue #14471: Fix a possible buffer overrun in the winreg module.
 +- Issue #14494: Fix __future__.py and its documentation to note that
 +  absolute imports are the default behavior in 3.0 instead of 2.7.
 +  Patch by Sven Marnach.
 +
 +- Issue #9260: A finer-grained import lock.  Most of the import sequence
 +  now uses per-module locks rather than the global import lock, eliminating
 +  well-known issues with threads and imports.
 +
 +- Issue #14624: UTF-16 decoding is now 3x to 4x faster on various inputs.
 +  Patch by Serhiy Storchaka.
 +
 +- asdl_seq and asdl_int_seq are now Py_ssize_t sized.
 +
 +- Issue #14133 (PEP 415): Implement suppression of __context__ display with an
 +  attribute on BaseException. This replaces the original mechanism of PEP 409.
 +
 +- Issue #14417: Mutating a dict during lookup now restarts the lookup instead
 +  of raising a RuntimeError (undoes issue #14205).
 +
 +- Issue #14738: Speed-up UTF-8 decoding on non-ASCII data.  Patch by Serhiy
 +  Storchaka.
 +
 +- Issue #14700: Fix two broken and undefined-behaviour-inducing overflow checks
 +  in old-style string formatting.
 +
 +- Issue #14705: The PyArg_Parse() family of functions now support the 'p' format
 +  unit, which accepts a "boolean predicate" argument.  It converts any Python
 +  value into an integer--0 if it is "false", and 1 otherwise.
  
  Library
  -------