1 files changed, 56 insertions, 7 deletions

diff --git a/extensions/BmpConvert/Extension.pm b/extensions/BmpConvert/Extension.pm
index ee08e04f0..efffa91b2 100644
--- a/extensions/BmpConvert/Extension.pm
+++ b/extensions/BmpConvert/Extension.pm
@@ -25,33 +25,82 @@ use strict;
 use base qw(Bugzilla::Extension);
 
 use Image::Magick;
+use File::Temp qw(:seekable);
+use File::stat qw(stat);
 
 our $VERSION = '1.0';
 
 sub attachment_process_data {
     my ($self, $args) = @_;
+
     return unless $args->{attributes}->{mimetype} eq 'image/bmp';
+    eval {
+        _try_convert_bmp_to_png($args);
+    };
+    warn $@ if $@;
+}
+
+
+# Here be dragons:
+# Image::Magick uses dualvars extensively to signal errors.
+# The documentation is either confusing or wrong in this regard.
+# This is not a great practice. dualvar(0, "foo") is a true value,
+# but dualvar(0, "foo") + 0 is not.
+# Also dualvar(1, "") is a false value, but dualvar(1, "") > 0 is true.
+#
+# "When a scalar has both string and numeric components (dualvars), Perl
+# prefers to check the string component for boolean truth."
+# From https://github.com/chromatic/modern_perl_book/blob/master/sections/coercion.pod
+sub _try_convert_bmp_to_png {
+    my ($args) = @_;
 
     my $data = ${$args->{data}};
     my $img = Image::Magick->new(magick => 'bmp');
+    my $size;
 
-    # $data is a filehandle.
     if (ref $data) {
-        $img->Read(file => \*$data);
+        my $read_error = $img->Read(file => \*$data);
+
+        # rewind so it can be read in again by other code
+        seek($data, 0, SEEK_SET);
+
+        die "Error reading in BMP: $read_error"
+          if $read_error;
+
         $img->set(magick => 'png');
-        $img->Write(file => \*$data);
+
+        my $tmp = File::Temp->new(UNLINK => 1, SUFFIX => '.png');
+        my $write_error = $img->Write(file => $tmp);
+
+        die "Error converting BMP to PNG: $write_error"
+          if $write_error;
+
+        $tmp->flush;
+        $size = stat($tmp->filename)->size;
+        die "Error converting BMP to PNG results in empty file"
+          if $size == 0;
+
+        $tmp->seek(0, SEEK_SET);
+        $data = $tmp;
     }
-    # $data is a blob.
     else {
-        $img->BlobToImage($data);
+        my $parse_error = $img->BlobToImage($data);
+        die "Error parsing in BMP: $parse_error"
+          if $parse_error;
+
         $img->set(magick => 'png');
         $data = $img->ImageToBlob();
+
+        die "Error converting BMP to PNG (empty PNG)"
+          unless length($data) > 0;
+
+        $size = length($data);
     }
-    undef $img;
 
     ${$args->{data}} = $data;
     $args->{attributes}->{mimetype} = 'image/png';
-    $args->{attributes}->{filename} =~ s/^(.+)\.bmp$/$1.png/i;
+    $args->{attributes}->{filename} =~ s/\.bmp$/.png/i;
+    $args->{attributes}->{attach_size} = $size;
 }
 
  __PACKAGE__->NAME;