Skip to content

Commit

Permalink
Fix handling of umask
Browse files Browse the repository at this point in the history
- umask is now applied per default
  • Loading branch information
mrbean-bremen committed Mar 16, 2024
1 parent 5283be6 commit a1621df
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 39 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions pyfakefs/fake_filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down
62 changes: 36 additions & 26 deletions pyfakefs/tests/fake_os_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)

Expand All @@ -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)

Expand Down Expand Up @@ -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."""
Expand All @@ -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(
Expand All @@ -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."""
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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)
Expand Down Expand Up @@ -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()
Expand Down
19 changes: 12 additions & 7 deletions pyfakefs/tests/fake_pathlib_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand All @@ -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())
Expand Down Expand Up @@ -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()
Expand All @@ -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)

Expand All @@ -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)

Expand Down Expand Up @@ -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):
Expand Down
14 changes: 12 additions & 2 deletions pyfakefs/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()`.
"""
Expand All @@ -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()`.
Expand All @@ -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):
Expand Down

0 comments on commit a1621df

Please sign in to comment.