[rt #72475] make the error messages from read() match reality

and some other related changes

diff --git a/Changes b/Changes
index 1cd74a6f75bdd8d71be49ed21b9902bcaeba1695..a6b1c2b0b479d964a2e65313f6c45d0c6954c3fe 100644 (file)
--- a/Changes
+++ b/Changes
@@ -13,6 +13,16 @@ Imager release history.  Older releases can be found in Changes.old
  - correctly read and write 256x256 pixel ICO files
    https://rt.cpan.org/Ticket/Display.html?id=69599
 
+ - make the error message from read() or read_multi() when they can't
+   identify the file type match reality.
+   https://rt.cpan.org/Ticket/Display.html?id=72475
+
+ - read() now uses $FORMATGUESS if it can't determine the file type
+   based on the file header, to match read_multi().
+
+ - re-work and add tests for def_guess_type().  def_guess_type() no
+   longer returns any random file extension as the file type.
+
 Imager 0.86 - 31 Oct 2011
 ===========
 

diff --git a/Imager.pm b/Imager.pm
index 105c76c454a62c56c2dde8781210b2d906b2245b..1bf83d98af66a5c440db7b85153efb72d3c8b47a 100644 (file)
--- a/Imager.pm
+++ b/Imager.pm
@@ -1414,8 +1414,15 @@ sub read {
     $type = i_test_format_probe($IO, -1);
   }
 
+  if ($input{file} && !$type) {
+    # guess the type 
+    $type = $FORMATGUESS->($input{file});
+  }
+
   unless ($type) {
-    $self->_set_error('type parameter missing and not possible to guess from extension'); 
+    my $msg = "type parameter missing and it couldn't be determined from the file contents";
+    $input{file} and $msg .= " or file name";
+    $self->_set_error($msg);
     return undef;
   }
 
@@ -1958,7 +1965,9 @@ sub read_multi {
   }
 
   unless ($type) {
-    $ERRSTR = "No type parameter supplied and it couldn't be guessed";
+    my $msg = "type parameter missing and it couldn't be determined from the file contents";
+    $opts{file} and $msg .= " or file name";
+    Imager->_set_error($msg);
     return;
   }
 
@@ -3890,21 +3899,39 @@ sub _set_error {
 
 # Default guess for the type of an image from extension
 
+my @simple_types = qw(png tga gif raw ico cur xpm mng jng ilbm pcx psd eps);
+
+my %ext_types =
+  (
+   ( map { $_ => $_ } @simple_types ),
+   tiff => "tiff",
+   tif => "tiff",
+   pbm => "pnm",
+   pgm => "pnm",
+   ppm => "pnm",
+   pnm => "pnm", # technically wrong, but historically it works in Imager
+   jpeg => "jpeg",
+   jpg => "jpeg",
+   bmp => "bmp",
+   dib => "bmp",
+   rgb => "sgi",
+   bw => "sgi",
+   sgi => "sgi",
+   fit => "fits",
+   fits => "fits",
+   rle => "utah",
+  );
+
 sub def_guess_type {
   my $name=lc(shift);
-  my $ext;
-  $ext=($name =~ m/\.([^\.]+)$/)[0];
-  return 'tiff' if ($ext =~ m/^tiff?$/);
-  return 'jpeg' if ($ext =~ m/^jpe?g$/);
-  return 'pnm'  if ($ext =~ m/^p[pgb]m$/);
-  return 'png'  if ($ext eq "png");
-  return 'bmp'  if ($ext eq "bmp" || $ext eq "dib");
-  return 'tga'  if ($ext eq "tga");
-  return 'sgi'  if ($ext eq "rgb" || $ext eq "bw" || $ext eq "sgi" || $ext eq "rgba");
-  return 'gif'  if ($ext eq "gif");
-  return 'raw'  if ($ext eq "raw");
-  return lc $ext; # best guess
-  return ();
+
+  my ($ext) = $name =~ /\.([^.]+)$/
+    or return;
+
+  my $type = $ext_types{$ext}
+    or return;
+
+  return $type;
 }
 
 sub combines {

diff --git a/lib/Imager/Files.pod b/lib/Imager/Files.pod
index 0bc8f819d19bf7498309c3e3e95c6c70534c231d..916fcf65b126ecbe9e0c8f5d1a78f28861ec244c 100644 (file)
--- a/lib/Imager/Files.pod
+++ b/lib/Imager/Files.pod
@@ -363,7 +363,9 @@ X<FORMATGUESS>
 When writing to a file, if you don't supply a C<type> parameter Imager
 will attempt to guess it from the file name.  This is done by calling
 the code reference stored in C<$Imager::FORMATGUESS>.  This is only
-done when write() or write_multi() is called with a C<file> parameter.
+done when write() or write_multi() is called with a C<file> parameter,
+or if read() or read_multi() can't determine the type from the file's
+header.
 
 The default function value of C<$Imager::FORMATGUESS> is
 C<\&Imager::def_guess_type>.

diff --git a/t/t1000files.t b/t/t1000files.t
index 10bbc4746a1738786eb93a5e351bcc2273c3c65c..808341023fbee6253efff61e268f9190a66558d3 100644 (file)
--- a/t/t1000files.t
+++ b/t/t1000files.t
@@ -4,7 +4,7 @@
 # the file format
 
 use strict;
-use Test::More tests => 43;
+use Test::More tests => 67;
 use Imager;
 
 -d "testout" or mkdir "testout";
@@ -224,6 +224,63 @@ probe_ok(<<JPEG2K, "jp2", "JPEG 2000");
 00 6A 70 32 63 FF 4F FF 51 00 2F 00 00 00 00 01
 JPEG2K
 
+{ # RT 72475
+  # check error messages from read/read_multi
+  my $data = "nothing useful";
+  my @mult_data = Imager->read_multi(data => $data);
+  is(@mult_data, 0, "read_multi with non-image input data should fail");
+  is(Imager->errstr,
+     "type parameter missing and it couldn't be determined from the file contents",
+     "check the error message");
+
+  my @mult_file = Imager->read_multi(file => "t/t1000files.t");
+  is(@mult_file, 0, "read_multi with non-image filename should fail");
+  is(Imager->errstr,
+     "type parameter missing and it couldn't be determined from the file contents or file name",
+     "check the error message");
+
+  my $im = Imager->new;
+  ok(!$im->read(data => $data), "read from non-image data should fail");
+  is($im->errstr,
+     "type parameter missing and it couldn't be determined from the file contents",
+     "check the error message");
+
+  ok(!$im->read(file => "t/t1000files.t"),
+     "read from non-image file should fail");
+  is($im->errstr,
+     "type parameter missing and it couldn't be determined from the file contents or file name",
+     "check the error message");
+}
+
+{
+  # test def_guess_type
+  my @tests =
+    (
+     pnm => "pnm",
+     GIF => "gif",
+     tif => "tiff",
+     TIFF => "tiff",
+     JPG => "jpeg",
+     rle => "utah",
+     bmp => "bmp",
+     dib => "bmp",
+     rgb => "sgi",
+     BW => "sgi",
+     TGA => "tga",
+     CUR => "cur",
+     ico => "ico",
+     ILBM => "ilbm",
+     pcx => "pcx",
+     psd => "psd",
+    );
+
+  while (my ($ext, $expect) = splice(@tests, 0, 2)) {
+    my $filename = "foo.$ext";
+    is(Imager::def_guess_type($filename), $expect,
+       "type for $filename should be $expect");
+  }
+}
+
 Imager->close_log;
 
 unless ($ENV{IMAGER_KEEP_FILES}) {