After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 530654 - Map FUSE paths to uri's
Map FUSE paths to uri's
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: client module
git master
Other Linux
: Normal normal
: ---
Assigned To: gvfs-maint
gvfs-maint
: 509617 (view as bug list)
Depends on:
Blocks: 528670
 
 
Reported: 2008-04-30 00:05 UTC by David Zeuthen (not reading bugmail)
Modified: 2009-03-02 11:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (3.99 KB, patch)
2008-04-30 00:09 UTC, David Zeuthen (not reading bugmail)
needs-work Details | Review

Description David Zeuthen (not reading bugmail) 2008-04-30 00:05:50 UTC
There's a TODO in the gvfs source code to map FUSE POSIX paths to proper URI's

  /* TODO: detect fuse paths and convert to daemon vfs GFiles */

Will attach a patch that does this.
Comment 1 David Zeuthen (not reading bugmail) 2008-04-30 00:09:21 UTC
Created attachment 110137 [details] [review]
Proposed patch

Example of what this patch does

 GFile *f;
 const char *path;
 path = "/home/davidz/.gvfs/gphoto2 mount on usb%3A001,006/MUSIC/Nephew/09. USADSB.mp3"
 f = g_file_new_for_commandline_arg (path);
 g_print ("uri=%s", g_file_get_uri (f));

will print

 gphoto2://[usb:001,006]/MUSIC/Nephew/09. USADSB.mp3

This is pretty useful in many ways as it will allow us to bypass the fuse daemon and get better performance. See also bug 528670.
Comment 2 Alexander Larsson 2008-05-03 08:25:02 UTC
I don't think this is the right way to do this. Doing a listattr involves lots of ipc to things that could potentially be blocking a while such as the fuse daemon and the backend daemon. 

Instead we should just ask the mount tracker in the main gvfs daemon for the mount based on the fuse mount name (the dir name after ~/.gvs/). The main gvfs daemon doesn't generally do any blocking i/o, so it should reply fast, and it will cause only one level of ipc. Furthermore this can easily be cached in the already existing mount info cache in the client side code.
Comment 3 Alexander Larsson 2008-05-03 08:31:29 UTC
In the code this is:

GDaemonVfs->mount_cache - active (partial) cache of mounts (GMountInfos)
GMountInfo->fuse_mountpoint - fuse mountpoint for a mount

gdaemonvfs.c:lookup_mount_info_in_cache() - example cache lookup

_g_daemon_vfs_get_mount_info_sync() - example mount info getter based on mount spec, we'd need something similar based on fuse path.
Comment 4 A. Walton 2008-05-07 18:55:44 UTC
*** Bug 509617 has been marked as a duplicate of this bug. ***
Comment 5 Alexander Larsson 2008-09-26 10:45:00 UTC
2008-09-26  Alexander Larsson  <alexl@redhat.com>

        * client/gdaemonvfs.[ch]:
        * common/gvfsdaemonprotocol.h:
        * daemon/mount.c:
	Reverse map fuse paths to gvfs uris in
	g_file_new_for_path().

I commited this to trunk. Should we commit this to stable too?
Comment 6 David Zeuthen (not reading bugmail) 2008-09-26 16:49:04 UTC
(In reply to comment #5)
> I commited this to trunk. 

Thanks.

> Should we commit this to stable too?

The main need, I think, for this feature is bug 528670 - I think I'd like a less complicated approach than initially suggested there.

My proposal would be to _always_ expand %u and %U to file:/// URI's (if one exists) from GAppInfo and then, by virtue of this bug being fixed, it'd be mapped back correctly to gio URI's if, and only if, the consumer is actually using gio.

So that means that if I'm using Nautilus to browse ssh://quad.local/media/BigDisk and I double-click an .avi file there then both mplayer (not using gio) and Totem (using gio) will work.

Thoughts?

(To actually answer your question if it should be committed to stable: I think it only should be committed to stable if we also commit the fix for bug 528670 too. There might be other uses of this fix that I'm not aware of though.)
Comment 7 Gavin Hamill 2008-09-26 17:22:09 UTC
This seems to very adequately address the requirements. mplayer is an interesting case, but special because it already has compiled-in support for smb:// URIs (assuming no authentication is needed).

As a more generic case, I can surely add working with large archive sets on remote shares. Passing an smb:// URI to '/usr/bin/unrar' doesn't yield great results, and copying gigabytes of data locally is a bad solution :)

Every app can understand traditional POSIX paths, so your approach of mapping a URI from that would appear to be the correct one.

+1

My own interest is to try and get this into Ubuntu 8.10, so I'd be very keen for this to go into stable!
Comment 8 David Zeuthen (not reading bugmail) 2008-09-26 17:48:35 UTC
(In reply to comment #7)
> This seems to very adequately address the requirements. mplayer is an
> interesting case, but special because it already has compiled-in support for
> smb:// URIs (assuming no authentication is needed).

Yeah, but even if mplayer has (some level of) support for smb:// I really don't think you want to use it - authentication, as you bring up yourself, is a great example; it's much better to use an already existing session from gvfs because you'd be using the credentials the user already authenticated with.

Ditto for other apps and claims for support of some protocol; just because the same protocol name is used (e.g. smb, ssh, ftp) the URI's can be very different; as pointed out in bug 528670 comment 0 and, for more detail, in bug 528670 comment 12, it doesn't really work passing URI's around. Heck, even gvfs's interpretation of ftp:// URI's is different from most others (for good reasons).

So.. until everyone a) agrees on the URI format; and b) uses the same transport libraries / sessions / cookies / etc. we pretty much need to do this FUSE trick. Notably both a) and b) requires a bunch of specifications to be written, people agreeing on the specs plus code written to support these. 

When we get to a situation like that.. then we can start talking about annotating desktop files to specify that an app supports the one or more URI types and then pass the URI directly instead of something that all POSIX apps can understand. 

Until then, I think it's fine to just use FUSE, it works great.

FWIW, we've been doing something similar to this since Fedora 9 (shipping the original patch in bug 528670) and so far no complaints AFAICT (I'm adding mclasen to the Cc as he's probably on top of what bugs people have been filing). The only hiccups that I can think of was that the FUSE daemon was initially a bit wonky but that got fixed up rather quickly thanks to Hans Petter, Matthias, myself and others.
Comment 9 Alexander Larsson 2008-09-29 11:01:36 UTC
I agree that putting this alone in stable is kinda useless, we really want the patch from bug 528670 in glib too. 

Matthias: Whats the chance of getting that into the stable glib release?
Comment 10 David Zeuthen (not reading bugmail) 2008-10-01 17:52:18 UTC
Can we close this bug? The discussion of whether we want this in stable glib/gvfs is kinda orthogonal to getting the feature in trunk.
Comment 11 Vlad C. 2008-10-02 16:26:22 UTC
Is the current implementation smart enough to tell if the path is actually a *link* to a FUSE file, not the original FUSE file itself?

Let's expand on the example in comment #1 and assume that "/home/davidz/my_favorite_song.mp3" is a symbolic link pointing to "/home/davidz/.gvfs/gphoto2 mount on usb%3A001,006/MUSIC/Nephew/09.
USADSB.mp3". Will g_file_get_uri() return "gphoto2://[usb:001,006]/MUSIC/Nephew/09. USADSB.mp3" if given a Gfile constructed from "/home/davidz/my_favorite_song.mp3"?
Comment 12 Grahame Bowland 2008-10-04 04:49:54 UTC
I see potential security issues with this change. Now any file path with the string ".gvfs" in it is allowed to provide an extended attribute which determines a URI to open.

I could write a malicious FUSE filesystem, tell a user to open a file (possibly via a .desktop file that pretty much hides the path to the file they are opening) and then redirect the user anywhere I liked. If I knew the app re-opened files as part of an atomic save operation, I could use timing information to cause the user to overwrite an arbitrary path they have write privileges for by switching the URI mapping.

Even without a custom FUSE filesystem, I could create '/tmp/.gvfs/readme.txt', tell someone to open that, and then potentially use their saved authentication credentials (realms/cookies/whatever) to cause the user to access any URLs I liked. For sites that have action URLs with get-style arguments, I could do a whole bunch with the user's credentials, having effectively stolen their cookies.

We've effectively now got a system with hidden, magical symlinks anyone can set so long as the path contains ".gvfs" (not even a directory name, just that string anywhere in the entire path). Seems a bit abuse prone to me!
Comment 13 David Zeuthen (not reading bugmail) 2008-10-04 15:51:14 UTC
(In reply to comment #12)
> I see potential security issues with this change. Now any file path with the
> string ".gvfs" in it is allowed to provide an extended attribute which
> determines a URI to open.

That's not the way the patch that was committed [1] works; it only attempts to reverse map fuse paths in the given file:/// URI is inside the gvfs fuse root (e.g. $HOME/.gvfs). And even if the file is inside that root, it asks the gvfs FUSE daemon for the URI. See g_daemon_vfs_get_file_for_path() and convert_fuse_path() for details.

Hope this demonstrates there are no security issues with this patch.

     David

[1] : See http://cvs.fedoraproject.org/viewvc/devel/gvfs/gvfs-1.1.1-reverse-map-fuse-paths.patch?revision=1.1&view=markup
Comment 14 Christian Persch 2008-10-17 11:26:18 UTC
I have a few comments on the patch:

The patch seems to confuse strlen(attr) and sizeof(attr) sometimes?

+  if (list == NULL && size == 0)
+    return sizeof attr + 1;

Shouldn't that be just sizeof attr, since the terminal \0 is already included in the sizeof?

+  if (size < strlen (attr) + 1)
Can use sizeof attr here.

+  res = sizeof attr + 1;

Also I wonder about the use of g_utf8_strncpy, esp. in vfs_getxattr. Its 3rd argument is the number of *characters* to copy, not bytes. If everything is always ascii-only this is fine (the attr name is, but is the URI?) — but then g_strlcpy could simply be used here — otherwise it may overflow the buffer.

+  const char attr[] = "user.gio.uri";
This could be |static const char...| .