Added more handling for trailing separators in readlink

- fixes #372

pyfakefs/fake_filesystem.py

@@ -1900,18 +1900,14 @@ def get_object(self, file_path):
         file_path = self.absnormpath(self._original_path(file_path))
         return self.get_object_from_normpath(file_path)
 
-    def resolve(self, file_path, follow_symlinks=True, allow_fd=False,
-                allow_trailing_separator=True):
+    def resolve(self, file_path, follow_symlinks=True, allow_fd=False):
         """Search for the specified filesystem object, resolving all links.
 
         Args:
             file_path: Specifies the target FakeFile object to retrieve.
             follow_symlinks: If `False`, the link itself is resolved,
                 otherwise the object linked to.
             allow_fd: If `True`, `file_path` may be an open file descriptor
-            allow_trailing_separator: If False and file_path is ending with
-                a separator and not pointing to a directory, raises OSError
-                under Posix.
 
         Returns:
           The FakeFile object corresponding to `file_path`.
@@ -1928,19 +1924,16 @@ def resolve(self, file_path, follow_symlinks=True, allow_fd=False,
         if follow_symlinks:
             file_path = make_string_path(file_path)
             return self.get_object_from_normpath(self.resolve_path(file_path))
-        return self.lresolve(file_path, allow_trailing_separator)
+        return self.lresolve(file_path)
 
-    def lresolve(self, path, allow_trailing_separator=True):
+    def lresolve(self, path):
         """Search for the specified object, resolving only parent links.
 
         This is analogous to the stat/lstat difference.  This resolves links
         *to* the object but not of the final object itself.
 
         Args:
             path: Specifies target FakeFile object to retrieve.
-            allow_trailing_separator: If False and file_path is ending with
-                a separator and not pointing to a directory, raises OSError
-                under Posix.
 
         Returns:
             The FakeFile object corresponding to path.
@@ -1969,11 +1962,7 @@ def lresolve(self, path, allow_trailing_separator=True):
                 if not self.is_windows_fs and isinstance(parent_obj, FakeFile):
                     self.raise_io_error(errno.ENOTDIR, path)
                 self.raise_io_error(errno.ENOENT, path)
-            obj = parent_obj.get_entry(child_name)
-            if (not self.is_windows_fs and not allow_trailing_separator and
-                    ends_with_sep and not isinstance(obj, FakeDirectory)):
-                self.raise_os_error(errno.EINVAL, path)
-            return obj
+            return parent_obj.get_entry(child_name)
         except KeyError:
             self.raise_io_error(errno.ENOENT, path)
 
@@ -2549,12 +2538,20 @@ def readlink(self, path):
         if path is None:
             raise TypeError
         try:
-            link_obj = self.lresolve(path, allow_trailing_separator=False)
+            link_obj = self.lresolve(path)
         except IOError as exc:
             self.raise_os_error(exc.errno, path)
         if S_IFMT(link_obj.st_mode) != S_IFLNK:
             self.raise_os_error(errno.EINVAL, 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
+                self.raise_os_error(error, link_obj.path)
+
+
         return link_obj.contents
 
     def makedir(self, dir_name, mode=PERM_DEF):
@@ -2657,8 +2654,7 @@ def _is_of_type(self, path, st_flag, follow_symlinks=True):
         if path is None:
             raise TypeError
         try:
-            obj = self.resolve(path, follow_symlinks,
-                               allow_trailing_separator=True)
+            obj = self.resolve(path, follow_symlinks)
             if obj:
                 self.raise_for_filepath_ending_with_separator(
                     path, obj, macos_handling=True)

pyfakefs/tests/fake_os_test.py

@@ -353,7 +353,7 @@ def test_mkdir_with_trailing_sep(self):
         self.os.mkdir(dir_path + self.os.sep + self.os.sep)
         self.assertTrue(self.os.path.exists(dir_path))
 
-    def test_read_link_ending_with_sep_posix(self):
+    def test_readlink_ending_with_sep_posix(self):
         # regression test for #359
         self.check_posix_only()
         link_path = self.make_path('foo')
@@ -379,7 +379,7 @@ def test_lstat_symlink_with_trailing_sep_macos(self):
         # used to raise
         self.assertTrue(self.os.lstat(link_path + self.os.sep).st_mode)
 
-    def test_read_link_ending_with_sep_windows(self):
+    def test_readlink_ending_with_sep_windows(self):
         self.check_windows_only()
         self.skip_if_symlink_not_supported()
         link_path = self.make_path('foo')
@@ -637,6 +637,27 @@ 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):
+        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):
+        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):
+        self.check_windows_only()
+        self.skip_if_symlink_not_supported()
+        file_path = self.make_path('foo')
+        self.os.symlink(file_path, file_path)
+        self.assert_raises_os_error(errno.EINVAL, self.os.readlink,
+                                    file_path + self.os.sep)
+
     def test_readlink_with_links_in_path(self):
         self.skip_if_symlink_not_supported()
         self.create_symlink(self.make_path('meyer', 'lemon', 'pie'),