Fix handling of umask

- umask is now applied per default

Author: mrbean-bremen

CHANGES.md

@@ -30,6 +30,7 @@ The released versions correspond to PyPI releases.
 * fixed permission problem with `shutil.rmtree` if emulating Windows under POSIX
   (see [#979](../../issues/979))
 * fixed handling of errors on opening files via file descriptor (see [#967](../../issues/967))
+* fixed handling of `umask` - it is now applied by default
 
 ### Infrastructure
 * replace `undefined` by own minimal implementation to avoid importing it

pyfakefs/fake_filesystem.py

@@ -2117,7 +2117,7 @@ def create_dir(
         # set the permission after creating the directories
         # to allow directory creation inside a read-only directory
         for new_dir in new_dirs:
-            new_dir.st_mode = S_IFDIR | perm_bits
+            new_dir.st_mode = S_IFDIR | (perm_bits & ~self.umask)
 
         return current_dir
 
@@ -2128,7 +2128,7 @@ def create_file(
         contents: AnyString = "",
         st_size: Optional[int] = None,
         create_missing_dirs: bool = True,
-        apply_umask: bool = False,
+        apply_umask: bool = True,
         encoding: Optional[str] = None,
         errors: Optional[str] = None,
         side_effect: Optional[Callable] = None,
@@ -2403,7 +2403,7 @@ def create_file_internally(
         contents: AnyString = "",
         st_size: Optional[int] = None,
         create_missing_dirs: bool = True,
-        apply_umask: bool = False,
+        apply_umask: bool = True,
         encoding: Optional[str] = None,
         errors: Optional[str] = None,
         read_from_real_fs: bool = False,
@@ -2543,6 +2543,7 @@ def create_symlink(
             st_mode=S_IFLNK | helpers.PERM_DEF,
             contents=link_target_path,
             create_missing_dirs=create_missing_dirs,
+            apply_umask=self.is_macos,
         )
 
     def create_link(
@@ -2790,7 +2791,7 @@ def makedirs(
             else:
                 current_dir = cast(FakeDirectory, current_dir.entries[component])
         try:
-            self.create_dir(dir_name, mode & ~self.umask)
+            self.create_dir(dir_name, mode)
         except OSError as e:
             if e.errno == errno.EACCES:
                 # permission denied - propagate exception

pyfakefs/tests/fake_os_test.py

@@ -40,26 +40,36 @@
 
 
 class FakeOsModuleTestBase(RealFsTestCase):
+    def setUp(self):
+        super().setUp()
+        self.umask = self.os.umask(0o022)
+
+    def tearDown(self):
+        self.os.umask(self.umask)
+
     def createTestFile(self, path):
         self.create_file(path)
         self.assertTrue(self.os.path.exists(path))
         st = self.os.stat(path)
-        self.assertEqual(0o666, stat.S_IMODE(st.st_mode))
+        # under Windows, the umask has no effect
+        mode = 0o666 if self.is_windows_fs else 0o0644
+        self.assertEqual(mode, stat.S_IMODE(st.st_mode))
         self.assertTrue(st.st_mode & stat.S_IFREG)
         self.assertFalse(st.st_mode & stat.S_IFDIR)
 
     def createTestDirectory(self, path):
         self.create_dir(path)
         self.assertTrue(self.os.path.exists(path))
         st = self.os.stat(path)
-        self.assertEqual(0o777, stat.S_IMODE(st.st_mode))
+        mode = 0o777 if self.is_windows_fs else 0o755
+        self.assertEqual(mode, stat.S_IMODE(st.st_mode))
         self.assertFalse(st.st_mode & stat.S_IFREG)
         self.assertTrue(st.st_mode & stat.S_IFDIR)
 
 
 class FakeOsModuleTest(FakeOsModuleTestBase):
     def setUp(self):
-        super(FakeOsModuleTest, self).setUp()
+        super().setUp()
         self.rwx = self.os.R_OK | self.os.W_OK | self.os.X_OK
         self.rw = self.os.R_OK | self.os.W_OK
 
@@ -2082,7 +2092,8 @@ def test_chmod_no_follow_symlink(self):
         else:
             self.os.chmod(link_path, 0o6543, follow_symlinks=False)
             st = self.os.stat(link_path)
-            self.assert_mode_equal(0o666, st.st_mode)
+            mode = 0o644 if self.is_macos else 0o666
+            self.assert_mode_equal(mode, st.st_mode)
             st = self.os.stat(link_path, follow_symlinks=False)
             self.assert_mode_equal(0o6543, st.st_mode)
 
@@ -2097,7 +2108,7 @@ def test_lchmod(self):
         self.os.lchmod(link_path, 0o6543)
 
         st = self.os.stat(link_path)
-        self.assert_mode_equal(0o666, st.st_mode)
+        self.assert_mode_equal(0o644, st.st_mode)
         st = self.os.lstat(link_path)
         self.assert_mode_equal(0o6543, st.st_mode)
 
@@ -2807,9 +2818,9 @@ def test_nlink_for_directories(self):
 
     def test_umask(self):
         self.check_posix_only()
-        umask = os.umask(0o22)
-        os.umask(umask)
-        self.assertEqual(umask, self.os.umask(0o22))
+        umask = self.os.umask(0o22)
+        self.assertEqual(umask, self.os.umask(0o12))
+        self.os.umask(umask)
 
     def test_mkdir_umask_applied(self):
         """mkdir creates a directory with umask applied."""
@@ -2826,7 +2837,7 @@ def test_mkdir_umask_applied(self):
     def test_makedirs_umask_applied(self):
         """makedirs creates a directories with umask applied."""
         self.check_posix_only()
-        self.os.umask(0o22)
+        umask = self.os.umask(0o22)
         self.os.makedirs(self.make_path("p1", "dir1"))
         self.assert_mode_equal(0o755, self.os.stat(self.make_path("p1")).st_mode)
         self.assert_mode_equal(
@@ -2838,6 +2849,7 @@ def test_makedirs_umask_applied(self):
         self.assert_mode_equal(
             0o710, self.os.stat(self.make_path("p2", "dir2")).st_mode
         )
+        self.os.umask(umask)
 
     def test_mknod_umask_applied(self):
         """mkdir creates a device with umask applied."""
@@ -2998,7 +3010,7 @@ def use_real_fs(self):
 
 class FakeOsModuleTestCaseInsensitiveFS(FakeOsModuleTestBase):
     def setUp(self):
-        super(FakeOsModuleTestCaseInsensitiveFS, self).setUp()
+        super().setUp()
         self.check_case_insensitive_fs()
         self.rwx = self.os.R_OK | self.os.W_OK | self.os.X_OK
         self.rw = self.os.R_OK | self.os.W_OK
@@ -3072,7 +3084,7 @@ def test_listdir_impossible_without_read_permission(self):
         with self.assertRaises(PermissionError):
             self.os.scandir(directory)
         # we can access the file if we know the file name
-        assert self.os.stat(file_path).st_mode & 0o777 == 0o777
+        assert self.os.stat(file_path).st_mode & 0o777 == 0o755
         with self.open(file_path) as f:
             assert f.read() == "hey"
 
@@ -4109,10 +4121,6 @@ def test_utime_uses_open_fd_as_path(self):
 class FakeOsModuleLowLevelFileOpTest(FakeOsModuleTestBase):
     """Test low level functions `os.open()`, `os.read()` and `os.write()`."""
 
-    def setUp(self):
-        os.umask(0o022)
-        super(FakeOsModuleLowLevelFileOpTest, self).setUp()
-
     def test_open_read_only(self):
         file_path = self.make_path("file1")
         self.create_file(file_path, contents=b"contents")
@@ -4830,7 +4838,7 @@ def use_real_fs(self):
 
 class FakeOsModuleDirFdTest(FakeOsModuleTestBase):
     def setUp(self):
-        super(FakeOsModuleDirFdTest, self).setUp()
+        super().setUp()
         self.check_posix_only()
         if not self.use_real_fs():
             # in the real OS, we test the option as is, in the fake OS
@@ -4948,11 +4956,11 @@ def os_stat():
                 os_stat()
         self.add_supported_function(self.os.stat)
         if self.os.stat in self.os.supports_dir_fd:
-            self.assertEqual(os_stat().st_mode, 0o100666)
+            self.assertEqual(os_stat().st_mode, 0o100644)
 
     def test_lstat(self):
         st = self.os.lstat(self.fname, dir_fd=self.dir_fd)
-        self.assertEqual(st.st_mode, 0o100666)
+        self.assertEqual(st.st_mode, 0o100644)
 
     def test_mkdir(self):
         def os_mkdir():
@@ -5185,7 +5193,7 @@ class FakeScandirTest(FakeOsModuleTestBase):
     LINKED_FILE_SIZE = 10
 
     def setUp(self):
-        super(FakeScandirTest, self).setUp()
+        super().setUp()
         self.supports_symlinks = not self.is_windows or not self.use_real_fs()
 
         if use_scandir_package:
@@ -5484,7 +5492,7 @@ def use_real_fs(self):
 class FakeScandirFdTest(FakeScandirTest):
     def tearDown(self):
         self.os.close(self.dir_fd)
-        super(FakeScandirFdTest, self).tearDown()
+        super().tearDown()
 
     def scandir_path(self):
         # When scandir is called with a filedescriptor, only the name of the
@@ -5514,7 +5522,7 @@ def use_real_fs(self):
 
 class FakeExtendedAttributeTest(FakeOsModuleTestBase):
     def setUp(self):
-        super(FakeExtendedAttributeTest, self).setUp()
+        super().setUp()
         self.check_linux_only()
         self.dir_path = self.make_path("foo")
         self.file_path = self.os.path.join(self.dir_path, "bar")
@@ -5570,7 +5578,7 @@ def setUp(self):
             # and cannot be created in the real OS using file system
             # functions only
             self.check_posix_only()
-        super(FakeOsUnreadableDirTest, self).setUp()
+        super().setUp()
         self.dir_path = self.make_path("some_dir")
         self.file_path = self.os.path.join(self.dir_path, "some_file")
         self.create_file(self.file_path)
@@ -5618,7 +5626,7 @@ def test_listdir_user_readable_dir_from_other_user(self):
         self.check_posix_only()
         user_id = get_uid()
         set_uid(user_id + 1)
-        dir_path = self.make_path("dir1")
+        dir_path = "/dir1"
         self.create_dir(dir_path, perm=0o600)
         self.assertTrue(self.os.path.exists(dir_path))
         reset_ids()
@@ -5630,8 +5638,9 @@ def test_listdir_user_readable_dir_from_other_user(self):
 
     def test_listdir_group_readable_dir_from_other_user(self):
         self.skip_real_fs()  # won't change user in real fs
+        self.check_posix_only()
         set_uid(get_uid() + 1)
-        dir_path = self.make_path("dir1")
+        dir_path = "/dir1"
         self.create_dir(dir_path, perm=0o660)
         self.assertTrue(self.os.path.exists(dir_path))
         reset_ids()
@@ -5642,7 +5651,7 @@ def test_listdir_group_readable_dir_from_other_group(self):
         self.check_posix_only()
         group_id = self.os.getgid()
         set_gid(group_id + 1)
-        dir_path = self.make_path("dir1")
+        dir_path = "/dir1"
         self.create_dir(dir_path, perm=0o060)
         self.assertTrue(self.os.path.exists(dir_path))
         set_gid(group_id)
@@ -5687,9 +5696,10 @@ def test_remove_unreadable_dir(self):
         self.assertFalse(self.os.path.exists(dir_path))
 
     def test_remove_unreadable_dir_from_other_user(self):
+        self.check_posix_only()
         self.skip_real_fs()  # won't change user in real fs
         set_uid(get_uid() + 1)
-        dir_path = self.make_path("dir1")
+        dir_path = "/dir1"
         self.create_dir(dir_path, perm=0o000)
         self.assertTrue(self.os.path.exists(dir_path))
         reset_ids()

pyfakefs/tests/fake_pathlib_test.py

@@ -286,6 +286,7 @@ def use_real_fs(self):
 class FakePathlibFileObjectPropertyTest(RealPathlibTestCase):
     def setUp(self):
         super(FakePathlibFileObjectPropertyTest, self).setUp()
+        self.umask = self.os.umask(0o022)
         self.file_path = self.make_path("home", "jane", "test.py")
         self.create_file(self.file_path, contents=b"a" * 100)
         self.create_dir(self.make_path("home", "john"))
@@ -304,6 +305,9 @@ def setUp(self):
             self.make_path("home", "none", "test.py"),
         )
 
+    def tearDown(self):
+        self.os.umask(self.umask)
+
     def test_exists(self):
         self.skip_if_symlink_not_supported()
         self.assertTrue(self.path(self.file_path).exists())
@@ -373,14 +377,15 @@ def test_lstat_windows(self):
         self.skip_if_symlink_not_supported()
         self.check_lstat(0)
 
-    @unittest.skipIf(is_windows, "Linux specific behavior")
+    @unittest.skipIf(is_windows, "POSIX specific behavior")
     def test_chmod(self):
-        self.check_linux_only()
+        self.check_posix_only()
         file_stat = self.os.stat(self.file_path)
-        self.assertEqual(file_stat.st_mode, stat.S_IFREG | 0o666)
+        self.assertEqual(file_stat.st_mode, stat.S_IFREG | 0o644)
         link_stat = self.os.lstat(self.file_link_path)
         # we get stat.S_IFLNK | 0o755 under MacOs
-        self.assertEqual(link_stat.st_mode, stat.S_IFLNK | 0o777)
+        mode = 0o755 if self.is_macos else 0o777
+        self.assertEqual(link_stat.st_mode, stat.S_IFLNK | mode)
 
     def test_lchmod(self):
         self.skip_if_symlink_not_supported()
@@ -391,7 +396,7 @@ def test_lchmod(self):
                 self.path(self.file_link_path).lchmod(0o444)
         else:
             self.path(self.file_link_path).lchmod(0o444)
-            self.assertEqual(file_stat.st_mode, stat.S_IFREG | 0o666)
+            self.assertEqual(file_stat.st_mode, stat.S_IFREG | 0o644)
             # the exact mode depends on OS and Python version
             self.assertEqual(link_stat.st_mode & 0o777700, stat.S_IFLNK | 0o700)
 
@@ -408,7 +413,7 @@ def test_chmod_no_followsymlinks(self):
                 self.path(self.file_link_path).chmod(0o444, follow_symlinks=False)
         else:
             self.path(self.file_link_path).chmod(0o444, follow_symlinks=False)
-            self.assertEqual(file_stat.st_mode, stat.S_IFREG | 0o666)
+            self.assertEqual(file_stat.st_mode, stat.S_IFREG | 0o644)
             # the exact mode depends on OS and Python version
             self.assertEqual(link_stat.st_mode & 0o777700, stat.S_IFLNK | 0o700)
 
@@ -492,7 +497,7 @@ def test_iterdir_impossible_without_read_permission(self):
         # glob does not find the file
         assert len(list(directory.glob("*.txt"))) == 0
         # we can access the file if we know the file name
-        assert file_path.stat().st_mode & 0o777 == 0o777
+        assert file_path.stat().st_mode & 0o777 == 0o755
         assert file_path.read_text() == "hey"
 
     def test_resolve_nonexisting_file(self):

pyfakefs/tests/test_utils.py

@@ -312,7 +312,7 @@ def make_path(self, *args):
         args = [to_string(arg) for arg in args]
         return self.os.path.join(self.base_path, *args)
 
-    def create_dir(self, dir_path, perm=0o777):
+    def create_dir(self, dir_path, perm=0o777, apply_umask=True):
         """Create the directory at `dir_path`, including subdirectories.
         `dir_path` shall be composed using `make_path()`.
         """
@@ -332,9 +332,15 @@ def create_dir(self, dir_path, perm=0o777):
             existing_path = self.os.path.join(existing_path, component)
             self.os.mkdir(existing_path)
             self.os.chmod(existing_path, 0o777)
+        if apply_umask:
+            umask = self.os.umask(0o022)
+            perm &= ~umask
+            self.os.umask(umask)
         self.os.chmod(dir_path, perm)
 
-    def create_file(self, file_path, contents=None, encoding=None, perm=0o666):
+    def create_file(
+        self, file_path, contents=None, encoding=None, perm=0o666, apply_umask=True
+    ):
         """Create the given file at `file_path` with optional contents,
         including subdirectories. `file_path` shall be composed using
         `make_path()`.
@@ -347,6 +353,10 @@ def create_file(self, file_path, contents=None, encoding=None, perm=0o666):
         with self.open(file_path, mode) as f:
             if contents is not None:
                 f.write(contents)
+        if apply_umask:
+            umask = self.os.umask(0o022)
+            perm &= ~umask
+            self.os.umask(umask)
         self.os.chmod(file_path, perm)
 
     def create_symlink(self, link_path, target_path):