Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include . and .. in readdir() output #28

Closed
albertvaka opened this issue Aug 29, 2016 · 9 comments
Closed

Include . and .. in readdir() output #28

albertvaka opened this issue Aug 29, 2016 · 9 comments

Comments

@albertvaka
Copy link

Inside a sshfs mount point, ls -a should show the two "." and ".." entries, but it doesn't.

KDE KIO 5.25 (and hence Dolphin) uses the "." entry to get info about the current directory and determine if the 'Paste' action should be enabled or not, which causes it to be disabled inside sshfs mounts.

@Nikratio
Copy link
Contributor

Do you have a reference for that? I have a vague recollection that reporting . and .. from readdir() is optional - though I may be mistaken. That said, are you sure this is the source of the problem with KDE? I'd expect that KDE doesn't look for . in the directory listing, but just calls stat on . directly - and that should always work.

@albertvaka
Copy link
Author

It was problem in KIO 5.25 [1] which has been patched for the next relesse, because it affected several filesystem implementations.

However, I think that is was fair for them to assume that every directory will have a . entry in it, and it is indeed a bug in the underlying filesystems to not report it. Of course, it's good to have a workaround in the KDE side, but you might want to change it on your side as well. Both for consistency with local filesystems and for potential future issues.

[1] https://bugs.kde.org/show_bug.cgi?id=367878

@Nikratio
Copy link
Contributor

I don't think it's fair to assume that at all. As I said above, I think the better assumption is that there will be no . and .. entries.

That said, if you want to provide a patch to report . and .. in sshfs, I'd be happy to review the pull request :-).

@Nikratio Nikratio changed the title sshfs-mounted paths miss the "." and ".." entries Include . and .. in readdir() output Aug 31, 2016
@Nikratio
Copy link
Contributor

The root of the issue is that the SFTP protocol doesn't include . and .. in the SSH_FXP_READDIR response (cf https://tools.ietf.org/html/draft-ietf-secsh-filexfer-13). So we would have to manually sneak these entries in.

@albertvaka
Copy link
Author

I think it would be a good idea to add these two entries if they are not there: For example KDE's dolphin and other file browsers rely on "." to check the permissions of the current directory.

@Nikratio
Copy link
Contributor

There is a difference between expecting . in the readdir() output and calling stat('.') to get information about the current directory. Dolphin most likely does the second, and that is supported by sshfs. If not, please provide some evidence.

@albertvaka
Copy link
Author

https://cgit.kde.org/kio.git/tree/src/core/kcoredirlister.cpp#n1237

As you can see here in KIO (the filesystem abstraction used by Dolphin), rootFileItem is only set when an entry with the "." name is found.

https://cgit.kde.org/dolphin.git/tree/src/dolphinpart.cpp#n389

Here you can see how Dolphin queries the permissions on the rootItem (from above), for example, to check if it should display the "Paste here" option on the left-click menu of the directory (so it's only visible when it's writable).

It could of course be changed on Dolphin's side, but regardless of that, my point is that we can't assume people is just gonna rely on stat("."). It might not be the case, specially when using abstractions like that.

And actually, in the case of Dolphin (actually KIO), there is already a patch to workaround this: the "." entry is manually added if it's not there after enumerating all the items in a directory. See it here:

https://cgit.kde.org/kio.git/tree/src/core/slavebase.cpp#n465

But I still think this could affect other file browsers, and it feels like a hack to have it on KIO's side.

@matthewcurry
Copy link

I have a small number of points:

  • It is NOT a bug for a file system to not include parent and current directory entries. The standard clearly states that the file system is allowed to determine whether it should provide these entries.
  • Currently, in the case of SSHFS, I consider not providing these entries to be the correct behavior, because (I think) inode numbers for directories aren't always stable under operations from multiple clients, so they shouldn't be cacheable. Readdir does read-ahead buffering, and thus caches.
  • Almost all file managers already handle these missing entries correctly.

@kalvdans
Copy link
Contributor

I can add that Python's os.listdir() function automatically strips . and .. from the returned list. My recommendation is to close this issue without making any changes to sshfs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants