[virt-tools-list] [PATCH virt-viewer 3/3] Screenshot: reject unknown image type filenames

[Pavel Grunt]

On Wed, 2017-07-19 at 16:49 -0500, Jonathon Jongsma wrote:
> If the image format cannot be determined for a screenshot filename,
> simply return an error informing the user that this is not a valid image
> format.
> 
> In the past, if we couldn't determine the file type, we simply saved it
> as a PNG, and apended a ".png" file extension to the filename. This has
appended
> several problems. First, it can result in some oddly-named files (e.g. a
> screenshot named 'Screenshot.pdf.png').
> 
> Second, modifying the filename that is returned from the GtkFileChooser
> undermines the overwrite-confirmation functionality that is built into
> the gtk file chooser. When the user specifies a filename in the file
> chooser dialog, the chooser will automatically check whether a file of
> that name exists, and if it does, it will display a dialog asking
> whether the user wants to overwrite it. But if we then append a ".png"
> extension to the filename and save it, we may be overwriting an existing
> file without warning. By returning an error for unrecognized file types,
> we avoid this problem.
> 
one can argue that suffixes mean nothing nowadays ;)

> Resolves: rhbz#1455832
Acked-by: Pavel Grunt <pgrunt at redhat.com>

Nice improvements!
Thanks,
Pavel

> ---
>  src/virt-viewer-window.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> index 95a760f..af3441f 100644
> --- a/src/virt-viewer-window.c
> +++ b/src/virt-viewer-window.c
> @@ -949,17 +949,9 @@ virt_viewer_window_save_screenshot(VirtViewerWindow
> *self,
>      gboolean result;
>  
>      if (format == NULL) {
> -        g_debug("unknown file extension, falling back to png");
> -        if (!g_str_has_suffix(file, ".png")) {
> -            char *png_filename;
> -            png_filename = g_strconcat(file, ".png", NULL);
> -            result = gdk_pixbuf_save(pix, png_filename, "png", error,
> -                                     "tEXt::Generator App", PACKAGE, NULL);
> -            g_free(png_filename);
> -        } else {
> -            result = gdk_pixbuf_save(pix, file, "png", error,
> -                                     "tEXt::Generator App", PACKAGE, NULL);
> -        }
> +        g_set_error(error, G_FILE_ERROR, G_FILE_ERROR_FAILED,
> +                    _("Unable to determine image format for file '%s'"),
> file);
> +        result = FALSE;
>      } else {
>          char *type = gdk_pixbuf_format_get_name(format);
>          g_debug("saving to %s", type);