From 51fddaebae34f6149723349d86350bdf58e8ab7f Mon Sep 17 00:00:00 2001 From: mrbean-bremen Date: Fri, 18 May 2018 15:16:32 +0200 Subject: [PATCH 1/3] Added special handling for broken symlinks with trailing separator - used for handling of lstat(), remove() and rename() - fixes incorrect results for islink() and lexists() under MacOS - see #396 --- pyfakefs/fake_filesystem.py | 18 ++++++++-- pyfakefs/tests/fake_os_test.py | 65 ++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/pyfakefs/fake_filesystem.py b/pyfakefs/fake_filesystem.py index f3e29a23..ddeecc7a 100644 --- a/pyfakefs/fake_filesystem.py +++ b/pyfakefs/fake_filesystem.py @@ -1081,9 +1081,11 @@ def raise_for_filepath_ending_with_separator(self, entry_path, if S_ISLNK(file_object.st_mode): try: link_object = self.resolve(entry_path) - except (IOError, OSError): - if self.is_macos: + except (IOError, OSError) as exc: + if self.is_macos and exc.errno != errno.ENOENT: return + if self.is_windows_fs: + self.raise_os_error(errno.EINVAL, entry_path) raise if not follow_symlinks or self.is_windows_fs or self.is_macos: file_object = link_object @@ -2016,6 +2018,8 @@ def rename(self, old_file_path, new_file_path, force_replace=False): new_file_path = self.absnormpath(new_file_path) if not self.exists(old_file_path, check_link=True): self.raise_os_error(errno.ENOENT, old_file_path, 2) + if ends_with_sep: + self._handle_broken_link_with_trailing_sep(old_file_path) old_object = self.lresolve(old_file_path) if not self.is_windows_fs: @@ -2054,6 +2058,14 @@ def rename(self, old_file_path, new_file_path, force_replace=False): new_dir_object.remove_entry(new_name) new_dir_object.add_entry(object_to_rename) + def _handle_broken_link_with_trailing_sep(self, path): + # note that the check for trailing sep has to be done earlier + if self.islink(path): + if not self.exists(path): + error = (errno.ENOENT if self.is_macos else + errno.EINVAL if self.is_windows_fs else errno.ENOTDIR) + self.raise_os_error(error, path) + def _handle_posix_dir_link_errors(self, new_file_path, old_file_path, ends_with_sep): if (self.isdir(old_file_path, follow_symlinks=False) and @@ -2755,6 +2767,8 @@ def remove(self, path): OSError: if removal failed. """ norm_path = self.absnormpath(path) + if self.ends_with_path_separator(path): + self._handle_broken_link_with_trailing_sep(norm_path) if self.exists(norm_path): obj = self.resolve(norm_path) if S_IFMT(obj.st_mode) == S_IFDIR: diff --git a/pyfakefs/tests/fake_os_test.py b/pyfakefs/tests/fake_os_test.py index 9da7c9fa..95896134 100644 --- a/pyfakefs/tests/fake_os_test.py +++ b/pyfakefs/tests/fake_os_test.py @@ -2162,6 +2162,71 @@ def test_rename_symlink_with_trailing_sep_windows(self): self.assert_raises_os_error(errno.EEXIST, self.os.rename, path + self.os.sep, self.base_path) + def create_broken_link_path_with_trailing_sep(self): + # Regression tests for #396 + self.skip_if_symlink_not_supported() + link_path = self.make_path('link') + target_path = self.make_path('target') + self.os.symlink(target_path, link_path) + link_path += self.os.sep + return link_path + + def test_lstat_broken_link_with_trailing_sep_linux(self): + self.check_linux_only() + link_path = self.create_broken_link_path_with_trailing_sep() + self.assert_raises_os_error(errno.ENOENT, self.os.lstat, link_path) + + def test_lstat_broken_link_with_trailing_sep_macos(self): + self.check_macos_only() + link_path = self.create_broken_link_path_with_trailing_sep() + self.assert_raises_os_error(errno.ENOENT, self.os.lstat, link_path) + + def test_lstat_broken_link_with_trailing_sep_windows(self): + self.check_windows_only() + link_path = self.create_broken_link_path_with_trailing_sep() + self.assert_raises_os_error(errno.EINVAL, self.os.lstat, link_path) + + def test_remove_broken_link_with_trailing_sep_linux(self): + self.check_linux_only() + link_path = self.create_broken_link_path_with_trailing_sep() + self.assert_raises_os_error(errno.ENOTDIR, self.os.remove, link_path) + + def test_remove_broken_link_with_trailing_sep_macos(self): + self.check_macos_only() + link_path = self.create_broken_link_path_with_trailing_sep() + self.assert_raises_os_error(errno.ENOENT, self.os.remove, link_path) + + def test_remove_broken_link_with_trailing_sep_windows(self): + self.check_windows_only() + link_path = self.create_broken_link_path_with_trailing_sep() + self.assert_raises_os_error(errno.EINVAL, self.os.remove, link_path) + + def test_rename_broken_link_with_trailing_sep_linux(self): + self.check_linux_only() + link_path = self.create_broken_link_path_with_trailing_sep() + self.assert_raises_os_error( + errno.ENOTDIR, self.os.rename, link_path, self.make_path('target')) + + def test_rename_broken_link_with_trailing_sep_macos(self): + self.check_macos_only() + link_path = self.create_broken_link_path_with_trailing_sep() + self.assert_raises_os_error( + errno.ENOENT, self.os.rename, link_path, self.make_path('target')) + + def test_rename_broken_link_with_trailing_sep_windows(self): + self.check_windows_only() + link_path = self.create_broken_link_path_with_trailing_sep() + self.assert_raises_os_error( + errno.EINVAL, self.os.rename, link_path, self.make_path('target')) + + def test_islink_broken_link_with_trailing_sep(self): + link_path = self.create_broken_link_path_with_trailing_sep() + self.assertFalse(self.os.path.islink(link_path)) + + def test_lexists_broken_link_with_trailing_sep(self): + link_path = self.create_broken_link_path_with_trailing_sep() + self.assertFalse(self.os.path.lexists(link_path)) + # hard link related tests def test_link_bogus(self): # trying to create a link from a non-existent file should fail From 3a17916c8ee513b1eb0b9c66574410882ab1d7bf Mon Sep 17 00:00:00 2001 From: mrbean-bremen Date: Fri, 18 May 2018 20:20:07 +0200 Subject: [PATCH 2/3] Added handling for readlink for broken link with trailing separator - see #396 --- pyfakefs/fake_filesystem.py | 12 +++++++++--- pyfakefs/tests/fake_os_test.py | 19 ++++++++++++++++--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/pyfakefs/fake_filesystem.py b/pyfakefs/fake_filesystem.py index ddeecc7a..93cf284e 100644 --- a/pyfakefs/fake_filesystem.py +++ b/pyfakefs/fake_filesystem.py @@ -2567,11 +2567,17 @@ def readlink(self, path): if self.ends_with_path_separator(path): if not self.is_windows_fs and self.exists(path): self.raise_os_error(errno.EINVAL, path) - if not self.is_macos and not self.exists(link_obj.path): - error = errno.EINVAL if self.is_windows_fs else errno.ELOOP + if not self.exists(link_obj.path): + if self.is_windows_fs: + error = errno.EINVAL + elif link_obj.path == link_obj.contents: + if self.is_macos: + return + error = errno.ELOOP + else: + error = errno.ENOENT self.raise_os_error(error, link_obj.path) - return link_obj.contents def makedir(self, dir_name, mode=PERM_DEF): diff --git a/pyfakefs/tests/fake_os_test.py b/pyfakefs/tests/fake_os_test.py index 95896134..544f6816 100644 --- a/pyfakefs/tests/fake_os_test.py +++ b/pyfakefs/tests/fake_os_test.py @@ -637,20 +637,23 @@ def test_broken_symlink_with_trailing_separator_windows(self): self.assert_raises_os_error(errno.EINVAL, self.os.symlink, link_path + self.os.sep, link_path + self.os.sep) - def test_circular_readlink_with_trailing_separator_372_linux(self): + def test_circular_readlink_with_trailing_separator_linux(self): + # Regression test for #372 self.check_linux_only() file_path = self.make_path('foo') self.os.symlink(file_path, file_path) self.assert_raises_os_error(errno.ELOOP, self.os.readlink, file_path + self.os.sep) - def test_circular_readlink_with_trailing_separator_372_macos(self): + def test_circular_readlink_with_trailing_separator_macos(self): + # Regression test for #372 self.check_macos_only() file_path = self.make_path('foo') self.os.symlink(file_path, file_path) print(self.os.readlink(file_path + self.os.sep)) - def test_circular_readlink_with_trailing_separator_372_windows(self): + def test_circular_readlink_with_trailing_separator_windows(self): + # Regression test for #372 self.check_windows_only() self.skip_if_symlink_not_supported() file_path = self.make_path('foo') @@ -2219,6 +2222,16 @@ def test_rename_broken_link_with_trailing_sep_windows(self): self.assert_raises_os_error( errno.EINVAL, self.os.rename, link_path, self.make_path('target')) + def test_readlink_broken_link_with_trailing_sep_posix(self): + self.check_posix_only() + link_path = self.create_broken_link_path_with_trailing_sep() + self.assert_raises_os_error(errno.ENOENT, self.os.readlink, link_path) + + def test_readlink_broken_link_with_trailing_sep_windows(self): + self.check_windows_only() + link_path = self.create_broken_link_path_with_trailing_sep() + self.assert_raises_os_error(errno.EINVAL, self.os.readlink, link_path) + def test_islink_broken_link_with_trailing_sep(self): link_path = self.create_broken_link_path_with_trailing_sep() self.assertFalse(self.os.path.islink(link_path)) From b57cce16163ab5cad961896d62ae3a3b29dcafa0 Mon Sep 17 00:00:00 2001 From: mrbean-bremen Date: Fri, 18 May 2018 20:49:03 +0200 Subject: [PATCH 3/3] Added special handling for broken links with trailing separator - allow mkdir and makedirs under MacOS - see #396 --- pyfakefs/fake_filesystem.py | 14 +++++++++++++- pyfakefs/tests/fake_os_test.py | 16 ++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/pyfakefs/fake_filesystem.py b/pyfakefs/fake_filesystem.py index 93cf284e..7aed347e 100644 --- a/pyfakefs/fake_filesystem.py +++ b/pyfakefs/fake_filesystem.py @@ -2594,6 +2594,7 @@ def makedir(self, dir_name, mode=PERM_DEF): read only or as per :py:meth:`add_object`. """ dir_name = make_string_path(dir_name) + ends_with_sep = self.ends_with_path_separator(dir_name) dir_name = self._path_without_trailing_separators(dir_name) if not dir_name: self.raise_os_error(errno.ENOENT, '') @@ -2616,7 +2617,11 @@ def makedir(self, dir_name, mode=PERM_DEF): error_nr = errno.EACCES else: error_nr = errno.EEXIST - self.raise_os_error(error_nr, dir_name) + if ends_with_sep and self.is_macos and not self.exists(dir_name): + # to avoid EEXIST exception, remove the link + self.remove_object(dir_name) + else: + self.raise_os_error(error_nr, dir_name) head, tail = self.splitpath(dir_name) self.add_object( @@ -2644,7 +2649,14 @@ def makedirs(self, dir_name, mode=PERM_DEF, exist_ok=False): OSError: if the directory already exists and exist_ok=False, or as per :py:meth:`create_dir`. """ + ends_with_sep = self.ends_with_path_separator(dir_name) dir_name = self.absnormpath(dir_name) + if (ends_with_sep and self.is_macos and + self.exists(dir_name, check_link=True) and + not self.exists(dir_name)): + # to avoid EEXIST exception, remove the link + self.remove_object(dir_name) + path_components = self._path_components(dir_name) # Raise a permission denied error if the first existing directory diff --git a/pyfakefs/tests/fake_os_test.py b/pyfakefs/tests/fake_os_test.py index 544f6816..6caee9a7 100644 --- a/pyfakefs/tests/fake_os_test.py +++ b/pyfakefs/tests/fake_os_test.py @@ -2189,6 +2189,22 @@ def test_lstat_broken_link_with_trailing_sep_windows(self): link_path = self.create_broken_link_path_with_trailing_sep() self.assert_raises_os_error(errno.EINVAL, self.os.lstat, link_path) + def test_mkdir_broken_link_with_trailing_sep_linux_windows(self): + self.check_linux_and_windows() + link_path = self.create_broken_link_path_with_trailing_sep() + self.assert_raises_os_error(errno.EEXIST, self.os.mkdir, link_path) + self.assert_raises_os_error(errno.EEXIST, self.os.makedirs, link_path) + + def test_mkdir_broken_link_with_trailing_sep_macos(self): + self.check_macos_only() + link_path = self.create_broken_link_path_with_trailing_sep() + self.os.mkdir(link_path) # no error + + def test_makedirs_broken_link_with_trailing_sep_macos(self): + self.check_macos_only() + link_path = self.create_broken_link_path_with_trailing_sep() + self.os.makedirs(link_path) # no error + def test_remove_broken_link_with_trailing_sep_linux(self): self.check_linux_only() link_path = self.create_broken_link_path_with_trailing_sep()