fix error handling for invalid utf8 for T1

diff --git a/T1/T1.pm b/T1/T1.pm
index 45a1cd55f68e82b3114a8d369707065447565422..2f7b3bd929c80b56646aea7b69c98f137a5bf299 100644 (file)
--- a/T1/T1.pm
+++ b/T1/T1.pm
@@ -111,8 +111,14 @@ sub _bounding_box {
   $flags .= 'u' if $input{underline};
   $flags .= 's' if $input{strikethrough};
   $flags .= 'o' if $input{overline};
-  return $self->{t1font}->bbox($input{size}, $input{string},
-                              $input{utf8}, $flags);
+  my @bbox =  $self->{t1font}->bbox($input{size}, $input{string},
+                                   $input{utf8}, $flags);
+  unless (@bbox) {
+    Imager->_set_error(Imager->_error_as_msg);
+    return;
+  }
+
+  return @bbox;
 }
 
 # check if the font has the characters in the given string
@@ -126,8 +132,25 @@ sub has_chars {
     $Imager::ERRSTR = "No string supplied to \$font->has_chars()";
     return;
   }
-  return $self->{t1font}->has_chars($hsh{string}, 
-                                   _first($hsh{'utf8'}, $self->{utf8}, 0));
+  if (wantarray) {
+    my @result = $self->{t1font}
+      ->has_chars($hsh{string}, _first($hsh{'utf8'}, $self->{utf8}, 0));
+    unless (@result) {
+      Imager->_set_error(Imager->_error_as_msg);
+      return;
+    }
+
+    return @result;
+  }
+  else {
+    my $result = $self->{t1font}
+      ->has_chars($hsh{string}, _first($hsh{'utf8'}, $self->{utf8}, 0));
+    unless (defined $result) {
+      Imager->_set_error(Imager->_error_as_msg);
+      return;
+    }
+    return $result;
+  }
 }
 
 sub utf8 {
@@ -154,7 +177,13 @@ sub glyph_names {
     or return Imager->_set_error("no string parameter passed to glyph_names");
   my $utf8 = _first($input{utf8} || 0);
 
-  return $self->{t1font}->glyph_names($string, $utf8);
+  my @result = $self->{t1font}->glyph_names($string, $utf8);
+  unless (@result) {
+    Imager->_set_error(Imager->_error_as_msg);
+    return;
+  }
+
+  return @result;
 }
 
 sub set_aa_level {

diff --git a/T1/T1.xs b/T1/T1.xs
index 162920dbc655ff9aa62196aa77c7a53bc13a94d7..54bdfaa8f0fb734fcc60344a57bdb689e54e9212 100644 (file)
--- a/T1/T1.xs
+++ b/T1/T1.xs
@@ -173,12 +173,14 @@ i_t1_glyph_names(font, text_sv, utf8 = 0)
         STRLEN work_len;
         size_t len;
         char name[255];
+       SSize_t count = 0;
       PPCODE:
         text = SvPV(text_sv, work_len);
 #ifdef SvUTF8
         if (SvUTF8(text_sv))
           utf8 = 1;
 #endif
+       i_clear_error();
         len = work_len;
         while (len) {
           unsigned long ch;
@@ -186,7 +188,7 @@ i_t1_glyph_names(font, text_sv, utf8 = 0)
             ch = i_utf8_advance(&text, &len);
             if (ch == ~0UL) {
               i_push_error(0, "invalid UTF8 character");
-              break;
+             XSRETURN(0);
             }
           }
           else {
@@ -195,12 +197,14 @@ i_t1_glyph_names(font, text_sv, utf8 = 0)
           }
           EXTEND(SP, 1);
           if (i_t1_glyph_name(font, ch, name, sizeof(name))) {
-            PUSHs(sv_2mortal(newSVpv(name, 0)));
+            ST(count) = sv_2mortal(newSVpv(name, 0));
           }
           else {
-            PUSHs(&PL_sv_undef);
-          } 
+            ST(count) = &PL_sv_undef;
+          }
+         ++count;
         }
+       XSRETURN(count);
 
 int
 i_t1_CLONE_SKIP(...)

diff --git a/T1/imt1.c b/T1/imt1.c
index a883503c4072c1182348cb039d2201eef1731c7c..ef9976993fbe9c09d464567ef9474d40b0150e8d 100644 (file)
--- a/T1/imt1.c
+++ b/T1/imt1.c
@@ -286,6 +286,10 @@ i_t1_cp(i_t1_font_t font, i_img *im,i_img_dim xb,i_img_dim yb,int channel,double
   if (utf8) {
     int worklen;
     char *work = t1_from_utf8(str, len, &worklen);
+    if (work == NULL) {
+      i_mutex_unlock(mutex);
+      return 0;
+    }
     glyph=T1_AASetString( fontnum, work, worklen, 0, mod_flags, points, NULL);
     myfree(work);
   }
@@ -362,13 +366,19 @@ i_t1_bbox(i_t1_font_t font, double points,const char *str,size_t len, i_img_dim
   int fontnum = font->font_id;
   int space_position;
 
+  i_clear_error();
+
   i_mutex_lock(mutex);
 
   space_position = T1_GetEncodingIndex(fontnum, "space");
   
   mm_log((1,"i_t1_bbox(font %p (%d),points %.2f,str '%.*s', len %u)\n",
          font, fontnum,points,(int)len,str,(unsigned)len));
-  T1_LoadFont(fontnum);  /* FIXME: Here a return code is ignored - haw haw haw */ 
+  if (T1_LoadFont(fontnum) == -1) {
+    t1_push_error();
+    i_mutex_unlock(mutex);
+    return 0;
+  }
 
   if (len == 0) {
     /* len == 0 has special meaning to T1lib, but it means there's
@@ -380,6 +390,10 @@ i_t1_bbox(i_t1_font_t font, double points,const char *str,size_t len, i_img_dim
     if (utf8) {
       int worklen;
       char *work = t1_from_utf8(str, len, &worklen);
+      if (!work) {
+       i_mutex_unlock(mutex);
+       return 0;
+      }
       advance = T1_GetStringWidth(fontnum, work, worklen, 0, mod_flags);
       bbox = T1_GetStringBBox(fontnum,work,worklen,0,mod_flags);
       t1_fix_bbox(&bbox, work, worklen, advance, space_position);
@@ -422,7 +436,7 @@ i_t1_bbox(i_t1_font_t font, double points,const char *str,size_t len, i_img_dim
 
 
 /*
-=item i_t1_text(im, xb, yb, cl, fontnum, points, str, len, align, aa)
+=item i_t1_text(im, xb, yb, cl, fontnum, points, str, len, align, utf8, flags, aa)
 
 Interface to text rendering in a single color onto an image
 
@@ -435,6 +449,8 @@ Interface to text rendering in a single color onto an image
    str     - char pointer to string to render
    len     - string length
    align   - (0 - top of font glyph | 1 - baseline )
+   utf8    - str is utf8
+   flags   - formatting flags
    aa      - anti-aliasing level
 
 =cut
@@ -466,6 +482,10 @@ i_t1_text(i_t1_font_t font, i_img *im, i_img_dim xb, i_img_dim yb,const i_color
   if (utf8) {
     int worklen;
     char *work = t1_from_utf8(str, len, &worklen);
+    if (!work) {
+      i_mutex_unlock(mutex);
+      return 0;
+    }
     glyph=T1_AASetString( fontnum, work, worklen, 0, mod_flags, points, NULL);
     myfree(work);
   }
@@ -688,12 +708,13 @@ i_t1_glyph_name(i_t1_font_t font, unsigned long ch, char *name_buf,
   char *name;
   int font_num = font->font_id;
 
-  i_mutex_lock(mutex);
   i_clear_error();
   if (ch > 0xFF) {
-    i_mutex_unlock(mutex);
     return 0;
   }
+
+  i_mutex_lock(mutex);
+
   if (T1_LoadFont(font_num)) {
     t1_push_error();
     i_mutex_unlock(mutex);

diff --git a/T1/t/t10type1.t b/T1/t/t10type1.t
index 7a58afba312d2f1e56ba7641b95b581db54ef428..bb0843d29e153dcc80f32b48e8807faa1521e657 100644 (file)
--- a/T1/t/t10type1.t
+++ b/T1/t/t10type1.t
@@ -8,7 +8,7 @@ use Cwd qw(getcwd abs_path);
 
 #$Imager::DEBUG=1;
 
-plan tests => 120;
+plan tests => 132;
 
 ok($Imager::formats{t1}, "must have t1");
 
@@ -92,6 +92,14 @@ SKIP:
   ok($fnum->cp($backgr, 80, 140, 1, 32, $text, 1, 1), 
       "cp hand-encoded UTF8");
 
+  { # invalid utf8
+    my $text = pack("C", 0xC0);
+    ok(!$fnum->text($backgr, 10, 140, $bgcolor, 32, $text, 1, 1),
+       "attempt to draw invalid utf8");
+    is(Imager->_error_as_msg, "invalid UTF8 character",
+       "check message");
+  }
+
   # ok, try native perl UTF8 if available
  SKIP:
   {
@@ -218,6 +226,35 @@ SKIP:
            "display smaller than advance");
   }
 
+  { # invalid UTF8 handling at the OO level
+    my $im = Imager->new(xsize => 80, ysize => 20);
+    my $bad_utf8 = pack("C", 0xC0);
+    Imager->_set_error("");
+    ok(!$im->string(font => $font, size => 1, text => $bad_utf8, utf8 => 1,
+                   y => 18, x => 2),
+       "drawing invalid utf8 should fail");
+    is($im->errstr, "invalid UTF8 character", "check error message");
+    Imager->_set_error("");
+    ok(!$im->string(font => $font, size => 1, text => $bad_utf8, utf8 => 1,
+                   y => 18, x => 2, channel => 1),
+       "drawing invalid utf8 should fail (channel)");
+    is($im->errstr, "invalid UTF8 character", "check error message");
+    Imager->_set_error("");
+    ok(!$font->bounding_box(string => $bad_utf8, size => 30, utf8 => 1),
+       "bounding_box() bad utf8 should fail");
+    is(Imager->errstr, "invalid UTF8 character", "check error message");
+    Imager->_set_error("");
+    is_deeply([ $font->glyph_names(string => $bad_utf8, utf8 => 1) ],
+             [ ],
+             "glyph_names returns empty list for bad string");
+    is(Imager->errstr, "invalid UTF8 character", "check error message");
+    Imager->_set_error("");
+    is_deeply([ $font->has_chars(string => $bad_utf8, utf8 => 1) ],
+             [ ],
+             "has_chars returns empty list for bad string");
+    is(Imager->errstr, "invalid UTF8 character", "check error message");
+  }
+
  SKIP:
   { print "# alignment tests\n";
     my $font = Imager::Font->new(file=>$deffont, type=>'t1');

diff --git a/lib/Imager/Font.pm b/lib/Imager/Font.pm
index 7dd08d3544ceed205ebc3e20ff480756de895010..5f28e97e5afab52fab2515405c444a83dc422dab 100644 (file)
--- a/lib/Imager/Font.pm
+++ b/lib/Imager/Font.pm
@@ -857,6 +857,11 @@ say how those name tables are unreliable, or how FT2 handles them.
 Both FreeType 1.x and 2.x allow support for glyph names to not be
 included.
 
+If the supplied C<string> is marked as UTF-8 or the C<utf8> parameter
+is true and the supplied string does not contain valid UTF-8, returns
+an empty string and set an error message readable from C<<
+Imager->errstr >>,
+
 =item draw
 
 This is used by Imager's string() method to implement drawing text.