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

mount /sdcard/ #1801

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

mount /sdcard/ #1801

wants to merge 2 commits into from

Conversation

lollilol
Copy link

@lollilol lollilol commented May 6, 2024

android apps without root access and permission granted MANAGE_EXTERNAL_STORAGE can only access /sdcard/

when trying to access sftp://192.168.178.21:1739/ this error appears:
image

after appending /sdcard/ to the sftp uri, i was able to successfully connect:
image

android apps without root access can only access /sdcard/
@ferdnyc
Copy link
Member

ferdnyc commented May 13, 2024

@lollilol

android apps without root access and permission granted MANAGE_EXTERNAL_STORAGE can only access /sdcard/

While that's true, I'm not sure it's that simple.

Looking at the latest KDEConnect Android app, I see the Filesystem Expose preferences seem to have vanished again, so you can no longer configure multiple shared storage locations.

In the past, after giving filesystem access permission, you'd be able to select one or more device locations to share, and they'd be exposed with the pathnames the user set. For example, here's the kdeconect.sftp packet that comes back from my older phone still running a version of the app with filesystem-selection preferences:

{
  "id": 1715569869701,
  "type": "kdeconnect.sftp",
  "body": {
    "ip": "L.M.N.O",
    "port": 1739,
    "user": "kdeconnect",
    "password": "XXXXXXXXX",
    "path": "/",
    "multiPaths": [
      "/primary",
      "/primary/DCIM"
    ],
    "pathNames": [
      "primary",
      "DCIM"
    ]
  }
}

...Now, that never worked very well, and it looks like it's gone again. The latest version of the app has no filesystem-expose preferences, and when it's activated and GSConnect tries to mount, this is the packet that comes back instead:

{
  "id": 1715570673060,
  "type": "kdeconnect.sftp",
  "body": {
    "ip": "L.M.N.O",
    "port": 1739,
    "user": "kdeconnect",
    "password": "XXXXXXXX",
    "path": "/storage/emulated/0",
    "multiPaths": [
      "/storage/emulated/0"
    ],
    "pathNames": [
      "Internal storage"
    ]
  }
}

Point being, I don't think there's a simple mapping. Your device is probably responding with a packet that includes:

    "path": "/sdcard",
    "multiPaths": [
      "/sdcard"
    ],
    "pathNames": [
      "Internal storage"
    ]

We likely need to start picking up that "path" from the packet (which always used to be "/", AFAICT, so it was hardcoded), and using it in the connection. But it'll likely be different for every device, so hardcoding it to /sdcard is as wrong as hardcoding it to /.

(If you could confirm that the kdeconnect.sftp packet includes /sdcard as the path on your phone, by doing a Generate Support Log collection, that'd be a big help.)

@@ -222,7 +222,7 @@ const SFTPPlugin = GObject.registerClass({

// This is the actual call to mount the device
const host = this.device.channel.host;
const uri = `sftp://${host}:${packet.body.port}/`;
const uri = `sftp://${host}:${packet.body.port}/sdcard/`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm right, then this should be...

Suggested change
const uri = `sftp://${host}:${packet.body.port}/sdcard/`;
const uri = `sftp://${host}:${packet.body.port}/${packet.body.path}/`;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks correct if we're happy to access only the Android device's internal storage.

However, if I plug a USB thumb-drive into my phone, I get:

"path": "/storage/emulated/0",
"multiPaths": [
  "/storage/emulated/0",
  "/mnt/media_rw/301B-13FB"
],
"pathNames": [
  "Internal storage",
  "Lexar USB drive"
]

We should probably mount each of these.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mavit

Ugggh, that's really ugly. The worst part of it is, the packet.body.path is no longer even an ancestor of both (all) of the multiPaths listed. I was hoping at least browsing packet.body.path and seeing both of the mounts under it would be possible.

Given the data you've provided, I agree — we should stop trying to browse a single "device filesystem root" at all, and just separately mount each of the multiPaths entries, however meany there are.

In theory we could have UI to select which one to mount, but that already feels more complicated than necessary. If the user has multiple filesystems exported on the device, and chooses to mount it, they should just get all of them.

But it's even more clear now that we should never try to browse either sftp://${host}:${packet.body.port}/ or sftp://${host}:${packet.body.port}/${packet.body.path}/, since neither one represents the root of the remote filesystem. (There is, in fact, no path that does.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The really sucky thing is that, if I'm interpreting this right, there isn't going to be anything to indicate which mount point is which. I don't think we can set volume labels on the remote mount points, and I'm not sure GVFS will pick them up from the KDEConnect side either.

I guess we can modify _addSymlink to create ${by_name_dir.get_path()}/${safe_device_name} as a directory, instead of as the symbolic link itself, and then create one symlink for each mount point inside that directory. Then at least we'll have a mapping of the remote mount points to their names.

@ferdnyc
Copy link
Member

ferdnyc commented May 13, 2024

@lollilol I've updated your branch to be in sync with main, so if you're going to make any changes locally be sure to first do a git pull to pick up the remote changes.

@ferdnyc
Copy link
Member

ferdnyc commented May 13, 2024

We'll likely need to get rid of the Mount submenu, as well — it used to be, the contents of / were just a list of the configured share locations, so it was a relatively short list. But if the mount is going to go directly into the user's storage, there's no benefit in showing the entire directory listing as a submenu of GSConnect's device menu.

async _addSubmenu(mount) {
try {
const directories = await this._listDirectories(mount);
// Submenu sections
const dirSection = new Gio.Menu();
const unmountSection = this._getUnmountSection();
for (const [name, uri] of Object.entries(directories))
dirSection.append(name, `device.openPath::${uri}`);
// Files submenu
const filesSubmenu = new Gio.Menu();
filesSubmenu.append_section(null, dirSection);
filesSubmenu.append_section(null, unmountSection);

@ferdnyc
Copy link
Member

ferdnyc commented May 13, 2024

That'll mean eliminating the UnmountSection of (what was) the submenu, and promoting the Unmount action up to replace "Mount" in the device menu, instead of it being replaced with the submenu.

@ferdnyc
Copy link
Member

ferdnyc commented May 13, 2024

Yeah, this is the kdeconnect-android commit that switched over to using MANAGE_EXTERNAL_STORAGE, and made the path value no longer hardcoded to / on their end.

@sidevesh
Copy link
Contributor

Changing the uri to add the path to it, whether its /sdcard, or /storage/emulated/0 or packet.body.path does not seem to be working for me and the mount added to Nautilus when clicked still goes to root and hence the permission error shows up.

@ferdnyc
Copy link
Member

ferdnyc commented Oct 17, 2024

@sidevesh

Changing the uri to add the path to it, whether its /sdcard, or /storage/emulated/0 or packet.body.path does not seem to be working for me and the mount added to Nautilus when clicked still goes to root and hence the permission error shows up.

That may be because we're using file.mount_enclosing_volume() to do the actual mounting, and AFAICT that'll always attempt to mount an SFTP URI at the root of the server, even if the URI is a deeper path:

// This is the actual call to mount the device
const host = this.device.channel.host;
const uri = `sftp://${host}:${packet.body.port}/`;
const file = Gio.File.new_for_uri(uri);
await file.mount_enclosing_volume(GLib.PRIORITY_DEFAULT, op,
this.cancellable);

Unfortunately, with Gvfs I don't believe there's any way to actually mount any path deeper than the root, nor is there a way to set up multiple mounts to different paths on the same remote host. Even if a mount is done to a deeper path, you'll always be able to (attempt to) browse to any path on the server, and if you can't access a given path you'll get permissions errors. The mounted path is more of a "bookmark" to a certain location on the remote server.

And the problem is, that bookmark is created by Nautilus semi-"automagically", and may be special-cased to /home/username/ paths.

Because, on login I run just gio mount sftp://ferd@$FILESERVER to establish a mount to my fileserver. When that happens, Nautilus automatically adds a "bookmark" named "ferd on $FILESERVER" to the sidebar, which points to sftp://ferd@$FILESERVER/home/ferd/. Even though I never told it to mount that path.

And from the Gvfs perspective, the mount is still just to sftp://$FILESERVER/:

$ gio mount -l
Drive(0): TEAM T253X2001T
  Type: GProxyDrive (GProxyVolumeMonitorUDisks2)
Drive(1): TSSTcorp DVD+/-RW TS-H653F
  Type: GProxyDrive (GProxyVolumeMonitorUDisks2)
Volume(0): [email protected]
  Type: GProxyVolume (GProxyVolumeMonitorGoa)
Volume(1): [email protected]
  Type: GProxyVolume (GProxyVolumeMonitorGoa)
Mount(0): ferd on $FILESERVER -> sftp://ferd@$FILESERVER/
  Type: GDaemonMount

It's possible there's no way to get Nautilus to add a "bookmark" pointing to anywhere other than sftp://foo/ or sftp://foo/home/$USERNAME, for a given mount to foo.

In which case, unless we want to make users browse to their deeper paths "by hand" (and only by typing the path in, to avoid permission errors accessing parent directories), we're back to not being able to do what we need... short of writing a GSConnect Gvfs plugin, to handle gsconnect:// URIs in custom ways. (And since all Gvfs plugins are required to be in-tree, that also means getting it accepted upstream. I know I'm not touching that with a 50' pole; you couldn't PAY me enough to try.)

(EDIT: It's also worth noting that /home/ferd is the default path for my SFTP sessions to that fileserver:

$ sftp ferd@$FILESERVER
Connected to $FILESERVER
sftp> pwd
Remote working directory: /home/ferd

...So, that may be what determines that the "bookmark" mount should point to that path. Not sure if that's based on the home directory path in the remote /etc/passwd file, or if it's hardcoded into the SFTP service. Even if it's not hardcoded and could be changed, that's not something KDE Connect could really make use of since they can potentially have multiple paths shared for the same username, and you can only have one default path.)

@ferdnyc
Copy link
Member

ferdnyc commented Oct 17, 2024

I can set actual bookmarks to deeper paths on my fileserver, and Nautilus will respect those and navigate directly to them — it'll even auto-mount sftp://ferd@$FILESERVER/ first, if necessary.

So, if we could insert bookmarks into Nautilus for paths on the remote server, that might be a workaround... but I'd be shocked if there's an API that would let us manipulate the Nautilus bookmarks list on the user's behalf.

@ferdnyc
Copy link
Member

ferdnyc commented Oct 17, 2024

(This is all made immensely harder by the fact that the SFTP service on the paired device is only open when explicitly requested, and will migrate to different ports at random — EVEN IF we're lucky enough that the actual IP address doesn't change. So any bookmarks would have to be very transient, because they'd get invalidated practically the moment we created them. Which... is a really terrible definition of "bookmark".)

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.

4 participants