Added special handling for broken symlinks with trailing separators

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
@@ -2555,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):
@@ -2576,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, '')
@@ -2598,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(
@@ -2626,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
@@ -2755,6 +2785,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:

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')
@@ -2162,6 +2165,97 @@ 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_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()
+        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_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))
+
+    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