document-portal: Implement GetHostPaths

[grulja]

This method allows apps to get path as exists on the host filesystem for documents exported through the document portal.

This method takes a list of document IDs as a string array and returns a dictionary mapping document IDs to the paths in the host filesystem. It is expected an app making this request to have access to given list of documents.

Based on initial work made by @JakobDev

Fixes #475

[ebassi]

You also need to update the documentation of the Document portal interface, since it still mentions version 4.

[swick]

A bunch of minor issues and nits but LGTM overall.

[JakobDev]

Thanks for bringing my work to en end. glib is not easy to use.

I have one small suggestion however: I used y instead of s as type for the path, as I was told that a path is in most cases UTF-8 but not always. I think it's better to make sure that the returned path is always UTF-8 and return s instead of y, to prevent errors, as most people will treat the bytes as an UTF-8 encoded string.

[grulja]

Thanks for bringing my work to en end. glib is not easy to use.

I have one small suggestion however: I used y instead of s as type for the path, as I was told that a path is in most cases UTF-8 but not always. I think it's better to make sure that the returned path is always UTF-8 and return s instead of y, to prevent errors, as most people will treat the bytes as an UTF-8 encoded string.

It's exactly like that. It's a map<doc_id, path> where doc_id is s and path is ay.

[ebassi]

I think it's better to make sure that the returned path is always UTF-8 and return s instead of y, to prevent errors, as most people will treat the bytes as an UTF-8 encoded string.

No. "Most people" will take that path and feed it to some platform-specific API, like open() or whatever other file-related function they have. All of those functions take a natively encoded path, which may or may not be UTF-8. Converting natively encoded paths to UTF-8 is potentially lossy, because file systems can use encodings for alphabets like Cyrillic, or CJK languages.

The only place where you want UTF-8 is displaying the path into some UI; for that you will need to convert the path to UTF-8, using functions like g_filename_to_utf8().

Stop trying to enforce UTF-8 into Unix paths: that's not how Unix paths work.

[grulja]

Thanks for bringing my work to en end. glib is not easy to use.
I have one small suggestion however: I used y instead of s as type for the path, as I was told that a path is in most cases UTF-8 but not always. I think it's better to make sure that the returned path is always UTF-8 and return s instead of y, to prevent errors, as most people will treat the bytes as an UTF-8 encoded string.

It's exactly like that. It's a map<doc_id, path> where doc_id is s and path is ay.

I should have read carefully the rest of your comment as I misunderstood so ignore what I wrote above.

I think using ay is consistent with the rest of the API. For example:

    <method name="Info">
      <arg type='s' name='doc_id' direction='in'/>
      <arg type='ay' name='path' direction='out'/>
      <arg type='a{sas}' name='apps' direction='out'/>
    </method>

[JakobDev]

I mean that the returned path should be s instead of ay

[swick]

But it shouldn't.

[JakobDev]

The only place where you want UTF-8 is displaying the path into some UI

That's exactly the use case for this API. The path is only to the displayed in the UI. It doesn't exists in the Sandbox. You can't use any filesystem related functions with it. If doesn't make sense here to let programs figure out the encoding here, which will lead to errors.

But it shouldn't.

Why?

[swick]

Paths are not UTF-8, not when you get the path from syscalls, not when you get them from clipboard, not anywhere. Apps already deal with that fact and are able to display them. Let's not make this simple thing hard for no reason.

[JakobDev]

Apps already deal with that fact and are able to display them.

That's not true. Not everyone is a C developer. Many modern programming languages have a set encoding for strings (e.g. Java and JavaScript uses UTF-16, python uses UTF-8). There is simply no need to deal with path encoding as a programmer of those languages. Not everyone writes low level programs.

Let's not make this simple thing hard for no reason.

I'm trying to make this easier. If you design a function that returns an UTF-8 string in 99% and a string in another encoding in 1% of times, that will cause trouble. Where should a programmer even know, which encoding this is?

[grulja]

Hi, can we finally make a decision on this change if it's good code-wise?

[swick]

Code LGTM but this needs a maintainer to take a look as well. @GeorgesStavracas, @ebassi, anyone else?

[grulja]

Maybe @matthiasclasen can look into this? This needs mostly approval for the API change I guess since the code has received quite a good review.

[mcatanzaro]

A couple +1s from me:

• +1 for "paths are not UTF-8." Other strings are guaranteed to be UTF-8 because that makes things easier for developers, but paths are consistently "filesystem encoding" everywhere else in our stack. Violating that rule here would be confusing and would make things harder for developers and not easier. Nobody wants to deal with converting to filesystem encoding before opening a file, and developers who work only with English won't even notice their mistake. I don't even know how to figure out what encoding "filesystem encoding" is, so it's unclear how to convert to/from UTF-8 anyway.
• +1 for allowing applications to get host paths. This leaks information from the host filesystem, but it's a limited and pragmatic leak. If the user doesn't want the application to see the path to the host file, then don't open the file or copy it to a different location first.

What I'm not sure about is whether we should add this GetHostPaths, or just change the document portal to mkdir -p and then mount documents at the same location they exist on the host, because as soon as GetHostPaths exists, there's no longer much value to mounting documents under /run, unless I'm missing something? Is there value to putting files under /run other than to hide the host path?

[mcatanzaro]

Answering my own question: it'd be really confusing if the file is in the same place in the sandbox as it is on the host system. Imagine moving the file in the sandbox from one location to another, but it stays in the same place on the host. That's just asking for bugs. The question was not smart.

[smcv]

or just change the document portal to mkdir -p and then mount documents at the same location they exist on the host

In addition to what @mcatanzaro said, this might be possible some of the time, but in general is not going to be possible all of the time - for example Flatpak often mounts locations from the host read-only, and that is irreversible while the sandbox is running (the only process that ever had the necessary capabilities to be able to change mount options has already given them up by the time the app talks to portals).

[swick]

Did some xattr experimentation for other reasons and noticed that putting the host path there is pretty easy.

swick@d55f037

$ flatpak run --command=getfattr --file-forwarding  org.gnome.clocks -d -m '' -- @@ $HOME/Downloads/test.txt @@
getfattr: Removing leading '/' from absolute path names
# file: run/user/1000/doc/e5080d2/test.txt
security.selinux="system_u:object_r:fusefs_t:s0"
user.xdp-document-portal.host-path="/var/home/swick/Downloads/test.txt"

[grulja]

Did some xattr experimentation for other reasons and noticed that putting the host path there is pretty easy.

swick@d55f037

$ flatpak run --command=getfattr --file-forwarding  org.gnome.clocks -d -m '' -- @@ $HOME/Downloads/test.txt @@
getfattr: Removing leading '/' from absolute path names
# file: run/user/1000/doc/e5080d2/test.txt
security.selinux="system_u:object_r:fusefs_t:s0"
user.xdp-document-portal.host-path="/var/home/swick/Downloads/test.txt"

We can have this as an additional way to get the host path, but I think this (or any other one) DBus API will be easier to discover and use for most of the clients. I know there is an API for this in GIO, but for example Qt doesn't seem to have nice API for this.

[swick]

Yes, I'm still 100% in favor of this portal addition and would like to see it merged.

[GeorgesStavracas]

Thanks!

[GeorgesStavracas]

@swick I'd like to have the xattr one too, mind proposing a PR for that?

[grulja]

Thanks @GeorgesStavracas for merging this.

Also kudos to @JakobDev for initial version of this change!!