diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala index 4168d940..c8d637bb 100644 --- a/src/client/application/geary-controller.vala +++ b/src/client/application/geary-controller.vala @@ -2051,49 +2051,24 @@ public class GearyController : Geary.BaseObject { } } - private bool do_overwrite_confirmation(File to_overwrite) { - string primary = _("A file named “%s” already exists. Do you want to replace it?").printf( - to_overwrite.get_basename()); - string secondary = _("The file already exists in “%s”. Replacing it will overwrite its contents.").printf( - to_overwrite.get_parent().get_basename()); - - ConfirmationDialog dialog = new ConfirmationDialog(main_window, primary, secondary, _("_Replace"), "destructive-action"); - - return (dialog.run() == Gtk.ResponseType.OK); - } - - private Gtk.FileChooserConfirmation on_confirm_overwrite(Gtk.FileChooser chooser) { - // this is only called when choosing one file - return do_overwrite_confirmation(chooser.get_file()) ? Gtk.FileChooserConfirmation.ACCEPT_FILENAME - : Gtk.FileChooserConfirmation.SELECT_AGAIN; + private async void save_attachment_to_file(Geary.Attachment attachment) { + string file_name = yield attachment.get_safe_file_name(); + try { + yield this.prompt_save_buffer( + file_name, new Geary.Memory.FileBuffer(attachment.file, true) + ); + } catch (Error err) { + message("Unable to save buffer to \"%s\": %s", file_name, err.message); + } } - private void on_save_attachments(Gee.Collection attachments) { - Gtk.FileChooserAction action = (attachments.size == 1) - ? Gtk.FileChooserAction.SAVE - : Gtk.FileChooserAction.SELECT_FOLDER; + private async void save_attachments_to_file(Gee.Collection attachments) { #if GTK_3_20 - Gtk.FileChooserNative dialog = new Gtk.FileChooserNative(null, main_window, action, - Stock._SAVE, Stock._CANCEL); + Gtk.FileChooserNative dialog = new_save_chooser(Gtk.FileChooserAction.SELECT_FOLDER); #else - Gtk.FileChooserDialog dialog = new Gtk.FileChooserDialog(null, main_window, action, - Stock._CANCEL, Gtk.ResponseType.CANCEL, Stock._SAVE, Gtk.ResponseType.ACCEPT, null); + Gtk.FileChooserDialog dialog = new_save_chooser(Gtk.FileChooserAction.SELECT_FOLDER); #endif - if (last_save_directory != null) - dialog.set_current_folder(last_save_directory.get_path()); - if (attachments.size == 1) { - Gee.Iterator it = attachments.iterator(); - it.next(); - Geary.Attachment attachment = it.get(); - dialog.set_current_name(attachment.file.get_basename()); - dialog.set_do_overwrite_confirmation(true); - // use custom overwrite confirmation so it looks consistent whether one or many - // attachments are being saved - dialog.confirm_overwrite.connect(on_confirm_overwrite); - } - dialog.set_create_folders(true); - dialog.set_local_only(false); - + bool accepted = (dialog.run() == Gtk.ResponseType.ACCEPT); string? filename = dialog.get_filename(); @@ -2102,89 +2077,108 @@ public class GearyController : Geary.BaseObject { if (!accepted || Geary.String.is_empty(filename)) return; - File destination = File.new_for_path(filename); - - // Proceeding, save this as last destination directory - last_save_directory = (attachments.size == 1) ? destination.get_parent() : destination; - - debug("Saving attachments to %s", destination.get_path()); - - // Save each one, checking for overwrite only if multiple attachments are being written + File dest_dir = File.new_for_path(filename); + this.last_save_directory = dest_dir; + + debug("Saving attachments to %s", dest_dir.get_path()); + foreach (Geary.Attachment attachment in attachments) { File source_file = attachment.file; - File dest_file = (attachments.size == 1) ? destination : destination.get_child(attachment.file.get_basename()); - - if (attachments.size > 1 && dest_file.query_exists() && !do_overwrite_confirmation(dest_file)) + File dest_file = dest_dir.get_child(yield attachment.get_safe_file_name()); + if (dest_file.query_exists() && !do_overwrite_confirmation(dest_file)) return; - - debug("Copying %s to %s...", source_file.get_path(), dest_file.get_path()); - - source_file.copy_async.begin(dest_file, FileCopyFlags.OVERWRITE, Priority.DEFAULT, null, - null, on_save_completed); + + try { + yield write_buffer_to_file( + new Geary.Memory.FileBuffer(source_file, true), dest_file + ); + } catch (Error error) { + message( + "Failed to copy attachment %s to destination: %s", + source_file.get_path(), error.message + ); + } } } - - private void on_save_completed(Object? source, AsyncResult result) { - try { - ((File) source).copy_async.end(result); - } catch (Error error) { - message("Failed to copy attachment %s to destination: %s", ((File) source).get_path(), - error.message); - } - } - - private void on_save_buffer_to_file(string? filename, Geary.Memory.Buffer buffer) { + + private async void prompt_save_buffer(string? filename, Geary.Memory.Buffer buffer) + throws Error { #if GTK_3_20 - Gtk.FileChooserNative dialog = new Gtk.FileChooserNative(null, main_window, Gtk.FileChooserAction.SAVE, - Stock._SAVE, Stock._CANCEL); + Gtk.FileChooserNative dialog = new_save_chooser(Gtk.FileChooserAction.SAVE); #else - Gtk.FileChooserDialog dialog = new Gtk.FileChooserDialog(null, main_window, Gtk.FileChooserAction.SAVE, - Stock._CANCEL, Gtk.ResponseType.CANCEL, Stock._SAVE, Gtk.ResponseType.ACCEPT, null); + Gtk.FileChooserDialog dialog = new_save_chooser(Gtk.FileChooserAction.SAVE); #endif - if (last_save_directory != null) - dialog.set_current_folder(last_save_directory.get_path()); if (!Geary.String.is_empty(filename)) dialog.set_current_name(filename); dialog.set_do_overwrite_confirmation(true); - dialog.confirm_overwrite.connect(on_confirm_overwrite); - dialog.set_create_folders(true); - dialog.set_local_only(false); - + dialog.confirm_overwrite.connect((chooser) => { + return do_overwrite_confirmation(chooser.get_file()) + ? Gtk.FileChooserConfirmation.ACCEPT_FILENAME + : Gtk.FileChooserConfirmation.SELECT_AGAIN; + }); bool accepted = (dialog.run() == Gtk.ResponseType.ACCEPT); string? accepted_filename = dialog.get_filename(); - + dialog.destroy(); - - if (!accepted || Geary.String.is_empty(accepted_filename)) - return; - - File destination = File.new_for_path(accepted_filename); - - // Proceeding, save this as last destination directory - last_save_directory = destination.get_parent(); - - debug("Saving buffer to %s", destination.get_path()); - - // Create the file where the image will be saved and get the output stream. - try { - FileOutputStream outs = destination.replace(null, false, FileCreateFlags.REPLACE_DESTINATION, - null); - outs.splice_async.begin(buffer.get_input_stream(), - OutputStreamSpliceFlags.CLOSE_SOURCE | OutputStreamSpliceFlags.CLOSE_TARGET, - Priority.DEFAULT, null, on_save_buffer_to_file_completed); - } catch (Error err) { - message("Unable to save buffer to \"%s\": %s", filename, err.message); + + if (accepted && !Geary.String.is_empty(accepted_filename)) { + File destination = File.new_for_path(accepted_filename); + this.last_save_directory = destination.get_parent(); + yield write_buffer_to_file(buffer, destination); } } - - private void on_save_buffer_to_file_completed(Object? source, AsyncResult result) { - try { - ((FileOutputStream) source).splice_async.end(result); - } catch (Error err) { - message("Failed to save buffer to file: %s", err.message); - } + + private async void write_buffer_to_file(Geary.Memory.Buffer buffer, File dest) + throws Error { + debug("Saving buffer to: %s", dest.get_path()); + FileOutputStream outs = dest.replace( + null, false, FileCreateFlags.REPLACE_DESTINATION, null + ); + yield outs.splice_async( + buffer.get_input_stream(), + OutputStreamSpliceFlags.CLOSE_SOURCE | OutputStreamSpliceFlags.CLOSE_TARGET, + Priority.DEFAULT, null + ); } - + + private bool do_overwrite_confirmation(File to_overwrite) { + string primary = _("A file named “%s” already exists. Do you want to replace it?").printf( + to_overwrite.get_basename()); + string secondary = _("The file already exists in “%s”. Replacing it will overwrite its contents.").printf( + to_overwrite.get_parent().get_basename()); + + ConfirmationDialog dialog = new ConfirmationDialog(main_window, primary, secondary, _("_Replace"), "destructive-action"); + + return (dialog.run() == Gtk.ResponseType.OK); + } + +#if GTK_3_20 + private inline Gtk.FileChooserNative new_save_chooser(Gtk.FileChooserAction action) { + Gtk.FileChooserNative dialog = new Gtk.FileChooserNative( + null, + this.main_window, + action, + Stock._SAVE, + Stock._CANCEL + ); +#else + private inline Gtk.FileChooserDialog new_save_chooser(Gtk.FileChooserAction action) { + Gtk.FileChooserDialog dialog = new Gtk.FileChooserDialog( + null, + this.main_window, + action, + Stock._CANCEL, Gtk.ResponseType.CANCEL, + Stock._SAVE, Gtk.ResponseType.ACCEPT, + null + ); +#endif + if (this.last_save_directory != null) + dialog.set_current_folder(this.last_save_directory.get_path()); + dialog.set_create_folders(true); + dialog.set_local_only(false); + return dialog; + } + // Opens a link in an external browser. private bool open_uri(string _link) { string link = _link; @@ -2776,7 +2770,9 @@ public class GearyController : Geary.BaseObject { open_uri(link); } }); - mview.save_image.connect(on_save_buffer_to_file); + mview.save_image.connect((filename, buf) => { + on_save_image_extended(view, filename, buf); + }); mview.search_activated.connect((op, value) => { string search = op + ":" + value; show_search_bar(search); @@ -2993,5 +2989,49 @@ public class GearyController : Geary.BaseObject { } } + private void on_save_attachments(Gee.Collection attachments) { + if (attachments.size == 1) { + this.save_attachment_to_file.begin(attachments.to_array()[0]); + } else { + this.save_attachments_to_file.begin(attachments); + } + } + + private void on_save_image_extended(ConversationEmail view, + string url, + Geary.Memory.Buffer resource_buf) { + // This is going to be either an inline image, or a remote + // image, so either treat it as an attachment ot assume we'll + // have a valid filename in the URL + + bool handled = false; + if (url.has_prefix(ClientWebView.CID_URL_PREFIX)) { + string cid = url.substring(ClientWebView.CID_URL_PREFIX.length); + Geary.Attachment? attachment = null; + try { + attachment = view.email.get_attachment_by_content_id(cid); + } catch (Error err) { + debug("Could not get attachment \"%s\": %s", cid, err.message); + } + if (attachment != null) { + this.save_attachment_to_file.begin(attachment); + handled = true; + } + } + + if (!handled) { + File source = File.new_for_uri(url); + string filename = source.get_basename(); + this.prompt_save_buffer.begin( + filename, resource_buf, + (obj, res) => { + try { + this.prompt_save_buffer.end(res); + } catch (Error err) { + message("Unable to save buffer to \"%s\": %s", filename, err.message); + } + }); + } + } } diff --git a/src/client/conversation-viewer/conversation-message.vala b/src/client/conversation-viewer/conversation-message.vala index 2b062a05..e4952586 100644 --- a/src/client/conversation-viewer/conversation-message.vala +++ b/src/client/conversation-viewer/conversation-message.vala @@ -252,7 +252,7 @@ public class ConversationMessage : Gtk.Grid { public signal void remember_remote_images(); /** Fired when the user saves an inline displayed image. */ - public signal void save_image(string? filename, Geary.Memory.Buffer buffer); + public signal void save_image(string? uri, Geary.Memory.Buffer buffer); /** Fired when the user activates a specific search shortcut. */ public signal void search_activated(string operator, string value); diff --git a/src/engine/api/geary-attachment.vala b/src/engine/api/geary-attachment.vala index 82e64cf7..226bb1a2 100644 --- a/src/engine/api/geary-attachment.vala +++ b/src/engine/api/geary-attachment.vala @@ -12,6 +12,7 @@ public abstract class Geary.Attachment : BaseObject { + /** * An identifier that can be used to locate the {@link Attachment} in an {@link Email}. * @@ -87,5 +88,73 @@ public abstract class Geary.Attachment : BaseObject { this.file = file; this.filesize = filesize; } -} + /** + * Returns a string to use as a file name, even if not specified. + * + * This checks that the extension of the given content file name + * matches the given content type, even if the attachment has the + * default content type. + * + * If no file name was specified for the attachment, it will + * construct one from the attachment's id and by guessing the file + * name extension, and also guessing the MIME content type if + * needed. + */ + public async string get_safe_file_name() { + string? file_name = this.content_filename; + if (Geary.String.is_empty(file_name)) { + string[] others = { + this.content_id, + this.id ?? "attachment", + }; + + int i = 0; + while (Geary.String.is_empty(file_name)) { + file_name = others[i++]; + } + } + + file_name = file_name.strip(); + + // Check the content type suggested by the file name is + // consistent with the declared content type. This adds an + // appropriate file name extension if missing, and ensures + // that malicious file names are fixed up. + Mime.ContentType mime_type = this.content_type; + Mime.ContentType? name_type = null; + try { + name_type = Mime.ContentType.guess_type(file_name, null); + } catch (Error err) { + debug("Error guessing attachment file name content type: %s", err.message); + } + + if (name_type == null || + name_type.is_default() || + !name_type.is_same(mime_type)) { + // Substitute file name either is of unknown type + // (e.g. it does not have an extension) or is not the + // same type as the declared type, so try to fix it. + if (mime_type.is_default()) { + // Declared type is unknown, see if we can guess + // it. Don't use GFile.query_info however since + // that will attempt to use the filename, which is + // what we are trying to guess in the first place. + try { + mime_type = Mime.ContentType.guess_type( + null, + new Geary.Memory.FileBuffer(this.file, true) + ); + } catch (Error err) { + debug("Error guessing attachment data content type: %s", err.message); + } + } + string? ext = mime_type.get_file_name_extension(); + if (!file_name.has_suffix(ext)) { + file_name = file_name + (ext ?? ""); + } + } + return file_name; + } + +} diff --git a/src/engine/api/geary-email.vala b/src/engine/api/geary-email.vala index 62093331..e9cc1cd1 100644 --- a/src/engine/api/geary-email.vala +++ b/src/engine/api/geary-email.vala @@ -297,10 +297,13 @@ public class Geary.Email : BaseObject { } /** + * Returns the attachment with the given {@link Geary.Attachment.id}. + * * Requires the REQUIRED_FOR_MESSAGE fields be present; else * EngineError.INCOMPLETE_MESSAGE is thrown. */ - public Geary.Attachment? get_attachment(string attachment_id) throws EngineError { + public Geary.Attachment? get_attachment_by_id(string attachment_id) + throws EngineError { if (!fields.fulfills(REQUIRED_FOR_MESSAGE)) throw new EngineError.INCOMPLETE_MESSAGE("Parsed email requires HEADER and BODY"); @@ -312,6 +315,25 @@ public class Geary.Email : BaseObject { return null; } + /** + * Returns the attachment with the given MIME Content ID. + * + * Requires the REQUIRED_FOR_MESSAGE fields be present; else + * EngineError.INCOMPLETE_MESSAGE is thrown. + */ + public Geary.Attachment? get_attachment_by_content_id(string cid) + throws EngineError { + if (!fields.fulfills(REQUIRED_FOR_MESSAGE)) + throw new EngineError.INCOMPLETE_MESSAGE("Parsed email requires HEADER and BODY"); + + foreach (Geary.Attachment attachment in attachments) { + if (attachment.content_id == cid) { + return attachment; + } + } + return null; + } + /** * Returns a list of this email's ancestry by Message-ID. IDs are not returned in any * particular order. The ancestry is made up from this email's Message-ID, its References, diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 70a5e4c0..68a437fd 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -6,6 +6,7 @@ set(TEST_SRC main.vala testcase.vala # Based on same file in libgee, courtesy Julien Peeters + engine/api/geary-attachment-test.vala engine/mime-content-type-test.vala engine/rfc822-mailbox-address-test.vala engine/rfc822-message-test.vala diff --git a/test/engine/api/geary-attachment-test.vala b/test/engine/api/geary-attachment-test.vala new file mode 100644 index 00000000..0efad064 --- /dev/null +++ b/test/engine/api/geary-attachment-test.vala @@ -0,0 +1,215 @@ +/* + * Copyright 2016 Michael Gratton + * + * This software is licensed under the GNU Lesser General Public License + * (version 2.1 or later). See the COPYING file in this distribution. + */ + +class Geary.AttachmentTest : Gee.TestCase { + + private const string ATTACHMENT_ID = "test-id"; + private const string CONTENT_TYPE = "image/png"; + private const string CONTENT_ID = "test-content-id"; + private const string CONTENT_DESC = "Mea navis volitans anguillis plena est"; + private const string FILE_PATH = "../icons/hicolor/16x16/apps/geary.png"; + + private Mime.ContentType? content_type; + private Mime.ContentType? default_type; + private Mime.ContentDisposition? content_disposition; + private File? file; + + private class TestAttachment : Attachment { + // A test article + + internal TestAttachment(string id, + Mime.ContentType content_type, + string? content_id, + string? content_description, + Mime.ContentDisposition content_disposition, + string? content_filename, + File file, + int64 filesize) { + base(id, content_type, content_id, content_description, + content_disposition, content_filename, file, filesize); + } + + } + + public AttachmentTest() { + base("Geary.AttachmentTest"); + add_test("get_safe_file_name_with_content_name", + get_safe_file_name_with_content_name); + add_test("get_safe_file_name_with_bad_content_name", + get_safe_file_name_with_bad_content_name); + add_test("get_safe_file_name_with_bad_file_name", + get_safe_file_name_with_bad_file_name); + add_test("get_safe_file_name_with_no_content_name", + get_safe_file_name_with_no_content_name); + add_test("get_safe_file_name_with_no_content_name_or_id", + get_safe_file_name_with_no_content_name_or_id); + add_test("get_safe_file_name_with_default_content_type", + get_safe_file_name_with_default_content_type); + add_test("get_safe_file_name_with_default_content_type_bad_file_name", + get_safe_file_name_with_default_content_type_bad_file_name); + } + + public override void set_up() { + try { + this.content_type = Mime.ContentType.deserialize(CONTENT_TYPE); + this.default_type = Mime.ContentType.deserialize(Mime.ContentType.DEFAULT_CONTENT_TYPE); + this.content_disposition = new Mime.ContentDisposition("attachment", null); + // XXX this will break as soon as the test runner is not + // launched from the project root dir + this.file = File.new_for_path("../icons/hicolor/16x16/apps/geary.png"); + + } catch (Error err) { + assert_not_reached(); + } + } + + public void get_safe_file_name_with_content_name() { + const string TEST_FILENAME = "test-filename.png"; + Attachment test = new TestAttachment( + ATTACHMENT_ID, + this.content_type, + CONTENT_ID, + CONTENT_DESC, + content_disposition, + TEST_FILENAME, + this.file, + 742 + ); + + test.get_safe_file_name.begin((obj, ret) => { + async_complete(ret); + }); + + assert(test.get_safe_file_name.end(async_result()) == TEST_FILENAME); + } + + public void get_safe_file_name_with_bad_content_name() { + const string TEST_FILENAME = "test-filename.jpg"; + const string RESULT_FILENAME = "test-filename.jpg.png"; + Attachment test = new TestAttachment( + ATTACHMENT_ID, + this.content_type, + CONTENT_ID, + CONTENT_DESC, + content_disposition, + TEST_FILENAME, + this.file, + 742 + ); + + test.get_safe_file_name.begin((obj, ret) => { + async_complete(ret); + }); + + assert(test.get_safe_file_name.end(async_result()) == RESULT_FILENAME); + } + + public void get_safe_file_name_with_bad_file_name() { + const string TEST_FILENAME = "test-filename"; + const string RESULT_FILENAME = "test-filename.png"; + Attachment test = new TestAttachment( + ATTACHMENT_ID, + this.content_type, + CONTENT_ID, + CONTENT_DESC, + content_disposition, + TEST_FILENAME, + this.file, + 742 + ); + + test.get_safe_file_name.begin((obj, ret) => { + async_complete(ret); + }); + + assert(test.get_safe_file_name.end(async_result()) == RESULT_FILENAME); + } + + public void get_safe_file_name_with_no_content_name() { + const string RESULT_FILENAME = CONTENT_ID + ".png"; + Attachment test = new TestAttachment( + ATTACHMENT_ID, + this.content_type, + CONTENT_ID, + CONTENT_DESC, + content_disposition, + null, + this.file, + 742 + ); + + test.get_safe_file_name.begin((obj, ret) => { + async_complete(ret); + }); + + assert(test.get_safe_file_name.end(async_result()) == RESULT_FILENAME); + } + + public void get_safe_file_name_with_no_content_name_or_id() { + const string RESULT_FILENAME = ATTACHMENT_ID + ".png"; + Attachment test = new TestAttachment( + ATTACHMENT_ID, + this.content_type, + null, + CONTENT_DESC, + content_disposition, + null, + this.file, + 742 + ); + + test.get_safe_file_name.begin((obj, ret) => { + async_complete(ret); + }); + + assert(test.get_safe_file_name.end(async_result()) == RESULT_FILENAME); + } + + public void get_safe_file_name_with_default_content_type() { + const string TEST_FILENAME = "test-filename.png"; + Attachment test = new TestAttachment( + ATTACHMENT_ID, + this.default_type, + CONTENT_ID, + CONTENT_DESC, + content_disposition, + TEST_FILENAME, + this.file, + 742 + ); + + test.get_safe_file_name.begin((obj, ret) => { + async_complete(ret); + }); + + assert(test.get_safe_file_name.end(async_result()) == TEST_FILENAME); + } + + public void get_safe_file_name_with_default_content_type_bad_file_name() { + const string TEST_FILENAME = "test-filename.jpg"; + const string RESULT_FILENAME = "test-filename.jpg.png"; + Attachment test = new TestAttachment( + ATTACHMENT_ID, + this.default_type, + CONTENT_ID, + CONTENT_DESC, + content_disposition, + TEST_FILENAME, + // XXX this will break as soon as the test runner is not + // launched from the project root dir + File.new_for_path("../icons/hicolor/16x16/apps/geary.png"), + 742 + ); + + test.get_safe_file_name.begin((obj, ret) => { + async_complete(ret); + }); + + assert(test.get_safe_file_name.end(async_result()) == RESULT_FILENAME); + } + +} diff --git a/test/main.vala b/test/main.vala index 7472a1a6..14a972d8 100644 --- a/test/main.vala +++ b/test/main.vala @@ -37,6 +37,7 @@ int main(string[] args) { TestSuite engine = new TestSuite("engine"); + engine.add_suite(new Geary.AttachmentTest().get_suite()); engine.add_suite(new Geary.HTML.UtilTest().get_suite()); engine.add_suite(new Geary.IdleManagerTest().get_suite()); engine.add_suite(new Geary.Inet.Test().get_suite());