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

Use provided SFTP paths if enumeration fails #1793

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mavit
Copy link
Contributor

@mavit mavit commented Apr 30, 2024

In #1610 (comment), Andy Holmes wrote:

So in fact GSConnect generally ignores the path sent by the Android device and tries to list the directories itself.

This could be fixed by either reverting that patch, or possibly falling back to the advertised paths if the listing fails. Patches for either would be okay by me.

This is that patch, to fall back to the advertised paths.

@ferdnyc
Copy link
Member

ferdnyc commented May 13, 2024

@mavit

I suspect the change in #1801 might actually be more on track here — previously, GSConnect always tried to mount sftp://user:[email protected]:port/ and get a list of the contents. But since the android app switched over to MANAGE_EXTERNAL_PERMISSIONS, the packet.body.path won't always be / (previously, it always was), and more importantly it appears / won't be accessible to us from the SFTP server — it publishes the filesystem using its on-device path, now.

So, if packet.body.path is something other than /, we should use that in the initial connection and stop trying to access the root of the SFTP filesystem.

The impression I get is that, even if multiple shared locations are configured (which doesn't appear to be possible on Android 11+ anymore, the preferences for doing that are totally gone in the Android app)... but even if multiple locations are configured, they're meant to be accessed directly and separately, not by mounting and listing /. The KDEConnect Android code makes me strongly suspect that's not supported (and maybe never really was).

@mavit
Copy link
Contributor Author

mavit commented May 21, 2024

This versus #1801 is swings and roundabouts, I think. If we mount sftp://${host}:${packet.body.port}/${packet.body.path}/ then then it'll be nicely browsable from Nautilus, but we'll loose access to any external storage connected to the Android device.

I'd be tempted to merge this for now (since I don't think it has any drawbacks compared to what we currently have). When someone finds time, they can look at reworking the code to mount each item in multiPaths.

mavit added 2 commits January 28, 2025 13:17
It’s common to encounter a “permission denied” error when trying to list the available paths.
@mavit
Copy link
Contributor Author

mavit commented Jan 28, 2025

Although it doesn't solve all the problems, I still think this is beneficial and worth merging.

@ferdnyc
Copy link
Member

ferdnyc commented Jan 28, 2025

@mavit

Here's the problem with this, I've discovered:

You can't mount sftp://${host}:${packet.body.port}/${packet.body.path}/ with GVFS. It won't let you. It will only let you mount sftp://${host}:${packet.body.port}/ — even if you try to give it a path, it will mount the root of the remote host, and you'll have to browse to ${packet.body.path} manually to see the shared directory — potentially slamming up against permission-denied errors, attempting to browse from the root sftp://${host}:${packet.body.port}/ directory to the shared path.

It SUUUUUUUCKS, but it's how GVFS works. It won't let us mount subpaths, and it won't let us mount multiple separate paths on the same remote host.

So I really don't know how to address this in a clean and user-friendly manner. The only thing I can think of is some sort of hybrid mount setup where we mount the remote root, then create a local directory ($HOME/GSConnect/ maybe?) where we place symlinks to the shared paths on the mounted remote. The user can then browse that directory to navigate to the permitted folders.

@ferdnyc
Copy link
Member

ferdnyc commented Jan 28, 2025

(This would also be a lot easier if Nautilus still supported .desktop files, because we could create a .desktop file with a link to sftp://${host}:${packet.body.port}/${packet.body.path}/. Because that no longer works, and we're stuck with real symlinks, we'd have to link to /run/user/${uid}/gvfs/sftp:host=${host},user=kdeconnect/${packet.body.path}/ instead of the SFTP URL.)

@mavit
Copy link
Contributor Author

mavit commented Jan 29, 2025

With this patch, the user can go “System menu → GSConnect → Files” to see a list of shared folders that they can click on to open in Nautilus. This is better than nothing (but see also #1794).

So that I can access the mount-point directly from Nautilus/the GTK file selector, I have manually created a bookmark for /run/user/<uid>/gsconnect/by-name/<name>/storage/emulated/0.

@mavit
Copy link
Contributor Author

mavit commented Jan 29, 2025

The only thing I can think of is some sort of hybrid mount setup where we mount the remote root, then create a local directory ($HOME/GSConnect/ maybe?) where we place symlinks to the shared paths on the mounted remote. The user can then browse that directory to navigate to the permitted folders.

We could replace the existing $XDG_RUNTIME_DIR/gsconnect/by-name/<name> symlink with a tree of subdirectories containing symlinks to the advertised directories. But I think that's an idea for a subsequent pull request.

@mavit
Copy link
Contributor Author

mavit commented Jan 29, 2025

So that I can access the mount-point directly from Nautilus/the GTK file selector, I have manually created a bookmark for /run/user/<uid>/gsconnect/by-name/<name>/storage/emulated/0.

We could create such bookmarks automatically by appending to ~/.config/gtk-3.0/bookmarks. For example,

file:///run/user/5000/gsconnect/by-name/Phone/storage/emulated/0 Phone:/storage/emulated/0

But, again, this is a task for a different pull request.

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

Successfully merging this pull request may close these issues.

2 participants