-
Notifications
You must be signed in to change notification settings - Fork 98
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
Handle Thunderbird as a special case for mail with attachment. #447
Conversation
Ah so g_string_free_and_steal is only for Glib >2.76, while Ubuntu 22.04 is on 2.72. I'll rewrite that bit then. |
Special casing apps this way is not reasonable IMO. If Thunderbird stopped supporting mailto as command line argument, then you need to create a compatibility wrapper that handles this, and make that the entry point for the mailto: hander call. This wrapper thing would either be shipped by thunderbird, or by the package (flatpak or snap), thus need no special casing in all possible portals. |
Thunderbird no longer accepts the mailto URI. This breaks Nautilus right-click > Email, for example. Creates a special case for Thunberbird then, using the -compose option.
The point of not handling 'attachment=' in mailto: is to avoid making it too easy to trick users into attaching a file without noticing. Imagine a website with a 'contact us for free trial' link which points to a 'mailto:...attachement=~/.ssh/.id_rsa'.... https://www.nds.ruhr-uni-bochum.de/media/nds/veroeffentlichungen/2020/08/15/mailto-paper.pdf has some reference to the issue Adding a wrapper that would make attachment in mailto work again would just add back the vulnerability. The problem existed in evolution (CVE-2011-3201) and was fixed there by blocking attachments from /etc and hidden directories but it's somewhat fragile (you can have private content outside of those paths) and not a solution thunderbird (and some other mailers) were comfortable using. Thunderbird is not likely to change and a popular email client for linux users... |
I thought this was only about missing mailto: not about attachment. Automagic attachments must not be supported indeed. |
Sorry the PR description is probably missing some context, the problem is discussed in more details in https://gitlab.gnome.org/GNOME/nautilus/-/issues/2431 but basically the issue is that the way the portal add attachements today is by constructing a 'mailto:....attachement=' url and calling the mail client on it, which is incompatible with thunderbird, which makes the nautilus sendto feature just not work with thunderbird. One alternative would be to hide the 'sendto' entry from nautilus when the default mailer is thunderbird (or another client not handling the mailto scheme for attachements) |
I apologize, the description was not indeed correctly contextualized. I edited the main comment with the link from seb128 for better visibility. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've a bit the same concerns regarding the hardcoding app-special cases into the portal.
Wondering if we can handle this somewhere else from the launcher instead. Or even provide a new way to launch an app when using some local file that as to be passed via the document portal?
Btw, syntax style has to be fixed, see https://developer.gnome.org/documentation/guidelines/programming/coding-style.html
/* The commandline can be complicated, e.g. | ||
* env BAMF_DESKTOP_FILE_HINT=/var/lib/snapd/.../thunderbird_thunderbird.desktop /snap/bin/thunderbird %u | ||
* so just match for thunderbird. | ||
*/ | ||
old_cmdline = g_app_info_get_commandline(info); | ||
if(strstr(old_cmdline, "thunderbird")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't use g_app_info_get_executable
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that in the example command line commented above, that would be env
.
sep = "&"; | ||
|
||
for (i = 0; cc[i]; i++) | ||
is_tb_snap = strstr(old_cmdline, "/snap/bin/thunderbird"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to check this from the application type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, what is that? Does it have something to do with "content type" as in https://docs.gtk.org/gio/iface.AppInfo.html?
Drive-by comment: the coding style is all over the place, and does not match the existing one. Please, use a style consistent with the rest of the file you're modifying. |
Indeed, we should not do that.
Maybe |
How would it be different adding a wrapper doing what this PR is doing, but inside the Thunderbird snap/flatpak? |
Pardon that, I took note of 3v1n0's comment on that but given the low probability of the changes being merged I didn't act on it yet.
That should work, but I'm afraid that would have to be done for all forms of packaging, correct? |
Hum, what would the wrapper do exactly? If we add a script that 'translate' a mailto: url including an attachment to one not including one and appending the option to the cmdline instead then we would restore the problem that a click on a url from a website would allow to attach a filename no? Or we would need from the wrapper to be able to tell what is the caller and only allow that if it's nautilus... |
Isn't translating mailto into something Thunderbird understands exactly what is attempted to be introduced here? The callee won't know who the caller is, no, so it's not possible to "allow list" nautilus in Thunderbird, so I guess if that was the intention, it wont work. Allow listing doesn't seem like a sane solution either way. So it seems like nautilus send-to feature relies on a feature that has potentially significant security implications, and perhaps best to not do that anymore at all. |
yes, but the point is that we want to do it only for safe callers. Thinking about it more though the current patch wouldn't achieve that either since it's not checking the caller, and such is not really different from the wrapper (it works in our case only because firefox doesn't use the portal atm). It seems either nautilus should drop the option or go back to have custom code rather than relying on the portal... |
If a sandboxed nautilus can use custom Thunderbird specific code to make it compose an E-mail with a predetermined attachment, what'd stop any other app from doing the same? Or is your assumption that nautilus is outside the sandbox? |
I was speaking about the case outside the sandbox (I don't know if any distribution provides nautilus sandboxed today, that has other challenges for a filemanager that users would probably expect to tbe able to access any part of their filesystem). But yes, once sandboxed it has no way to invoke the email client |
Sounds like the best long term solution is to drop support for "Send file" via E-mail from file managers then. |
At least from the user perspective that seems like a nice and reasonable feature to have. The typical thing even Windows XP already had, it looks like a small but solid step back to drop that. Especially if we consider e.g. Evolution, that warns about the attachment instead of preventing it. |
Thunderbird no longer accepts attachments in the mailto URI, it just ignores them. This breaks Nautilus right-click > Email, for example. More context: https://gitlab.gnome.org/GNOME/nautilus/-/issues/2431.
Creates a special case for Thunberbird then, using the -compose option.
Note: This is essentially a smaller diff than it looks, but the indentation level change makes it seem larger. For reference,
the patch without indentation level changes.