From 69316d827ae8de19ab5321b2c171b900f917ac9c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= <lsedlar@redhat.com>
Date: Thu, 31 Mar 2016 09:27:22 +0200
Subject: [PATCH] [util] Remove umask manipulation from makedirs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This causes a race condition: umask is process-wide setting, so any file
created by another thread while the umask is set to 0 will have wrong
permissions. This also removes the possible problem of not resetting the
umask if exception is raised.

Fixes: #239
Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>
---
 pungi/util.py      |  2 --
 tests/test_util.py | 15 +++++++++++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/pungi/util.py b/pungi/util.py
index 857174ea..f4f5e335 100644
--- a/pungi/util.py
+++ b/pungi/util.py
@@ -138,13 +138,11 @@ def _doCheckSum(path, hash, logger):
 
 
 def makedirs(path, mode=0o775):
-    mask = os.umask(0)
     try:
         os.makedirs(path, mode=mode)
     except OSError as ex:
         if ex.errno != errno.EEXIST:
             raise
-    os.umask(mask)
 
 
 def rmtree(path, ignore_errors=False, onerror=None):
diff --git a/tests/test_util.py b/tests/test_util.py
index d730ba4c..97892b71 100755
--- a/tests/test_util.py
+++ b/tests/test_util.py
@@ -13,7 +13,7 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 from pungi import compose
 from pungi import util
 
-from tests.helpers import touch
+from tests.helpers import touch, PungiTestCase
 
 
 class TestGitRefResolver(unittest.TestCase):
@@ -193,7 +193,7 @@ class TestFindOldCompose(unittest.TestCase):
         self.assertEqual(old, self.tmp_dir + '/Fedora-Rawhide-Base-1-20160229.0')
 
 
-class TestHelpers(unittest.TestCase):
+class TestHelpers(PungiTestCase):
     def test_process_args(self):
         self.assertEqual(util.process_args('--opt={}', None), [])
         self.assertEqual(util.process_args('--opt={}', []), [])
@@ -201,6 +201,17 @@ class TestHelpers(unittest.TestCase):
                          ['--opt=foo', '--opt=bar'])
         self.assertEqual(util.process_args('--opt={}', 'foo'), ['--opt=foo'])
 
+    def test_makedirs(self):
+        util.makedirs(self.topdir + '/foo/bar/baz')
+        self.assertTrue(os.path.isdir(self.topdir + '/foo/bar/baz'))
+
+    def test_makedirs_on_existing(self):
+        os.makedirs(self.topdir + '/foo/bar/baz')
+        try:
+            util.makedirs(self.topdir + '/foo/bar/baz')
+        except OSError:
+            self.fail('makedirs raised exception on existing directory')
+
 
 if __name__ == "__main__":
     unittest.main()