Item13895: fixed corrupt data in rest resizer

- working around bug in some versions of imagemagick coredumping on exit
foswiki · Apr 1, 2016 · 978c9bb · 978c9bb
1 parent fe20383
commit 978c9bb
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 46 deletions.
diff --git a/data/System/ImagePlugin.txt b/data/System/ImagePlugin.txt
@@ -226,6 +226,8 @@ $Foswiki::cfg{ImagePlugin}{Mode} = 'Image::Magick';
 ---++ Change History
 
 %TABLE{columnwidths="7em" tablewidth="100%"}%
+|  01 Apr 2016 | fixed resize rest handler returning invalid image data; \
+                 working around buggy implementations of imagemagick core-dumping on exit |
 |  26 Sep 2015 | fixed rewrite of html img tags to bail out when width/height units aren't px values; \
                  don't auto-render svg to png anymore |
 |  17 Jul 2015 | optionally extract and convert inline svg to real images; \
@@ -306,7 +308,7 @@ $Foswiki::cfg{ImagePlugin}{Mode} = 'Image::Magick';
 %META:FILEATTACHMENT{name="logo.gif" attachment="logo.gif" attr="h" date="1437152721" path="http://www.google.com/intl/en_ALL/images/logo.gif" size="8558" user="ProjectContributor" version="1"}%
 %META:FORM{name="PackageForm"}%
 %META:FIELD{name="Author" title="Author" value="Craig Meyer, Michael Daum"}%
-%META:FIELD{name="Copyright" title="Copyright" value="&copy; 2006, Craig Meyer, %BR% &copy; 2006-2015 Michael Daum http://michaeldaumconsulting.com"}%
+%META:FIELD{name="Copyright" title="Copyright" value="&copy; 2006, Craig Meyer, %BR% &copy; 2006-2016 Michael Daum http://michaeldaumconsulting.com"}%
 %META:FIELD{name="Home" title="Home" value="Foswiki:Extensions/%TOPIC%"}%
 %META:FIELD{name="License" title="License" value="GPL ([[http://www.gnu.org/copyleft/gpl.html][GNU General Public License]])"}%
 %META:FIELD{name="Release" title="Release" value="%25$RELEASE%25"}%

diff --git a/lib/Foswiki/Plugins/ImagePlugin.pm b/lib/Foswiki/Plugins/ImagePlugin.pm
@@ -1,7 +1,7 @@
 # Plugin for Foswiki - The Free and Open Source Wiki, http://foswiki.org/
 #
 # Copyright (C) 2006 Craig Meyer, meyercr@gmail.com
-# Copyright (C) 2006-2015 Michael Daum http://michaeldaumconsulting.com
+# Copyright (C) 2006-2016 Michael Daum http://michaeldaumconsulting.com
 #
 # Based on ImgPlugin
 # Copyright (C) 2006 Meredith Lesly, msnomer@spamcop.net
@@ -30,8 +30,8 @@ use warnings;
 use Foswiki::Func ();
 use Foswiki::Plugins ();
 
-our $VERSION = '7.01';
-our $RELEASE = '26 Sep 2015';
+our $VERSION = '7.10';
+our $RELEASE = '01 Apr 2016';
 our $NO_PREFS_IN_TOPIC = 1;
 our $SHORTDESCRIPTION = 'Image and thumbnail services to display and alignment images using an easy syntax';
 our $core;
@@ -222,8 +222,6 @@ sub renderLocalImage {
 sub renderExternalImage {
   my ($web, $topic, $prefix, $url) = @_;
 
-  #print STDERR "called renderExternalImage($web, $topic, $url)\n";
-
   my $href = '';
   my $title = $url;
 

diff --git a/lib/Foswiki/Plugins/ImagePlugin/Core.pm b/lib/Foswiki/Plugins/ImagePlugin/Core.pm
@@ -1,7 +1,7 @@
 # Plugin for Foswiki - The Free and Open Source Wiki, http://foswiki.org/
 #
 # Copyright (C) 2006 Craig Meyer, meyercr@gmail.com
-# Copyright (C) 2006-2015 Michael Daum http://michaeldaumconsulting.com
+# Copyright (C) 2006-2016 Michael Daum http://michaeldaumconsulting.com
 #
 # Early version Based on ImgPlugin
 # Copyright (C) 2006 Meredith Lesly, msnomer@spamcop.net
@@ -82,30 +82,39 @@ sub new {
 
   $this->{errorMsg} = '';    # from image mage
 
-  # Graphics::Magick is less buggy than Image::Magick
-  my $impl =
-       $Foswiki::cfg{ImagePlugin}{Impl}
-    || $Foswiki::cfg{ImageGalleryPlugin}{Impl}
-    || 'Image::Magick';
-
-
-  writeDebug("creating new image mage using $impl");
-  eval "require $impl";
-  die $@ if $@;
-  $this->{mage} = new $impl;
   #writeDebug("done");
 
   return $this;
 }
 
+###############################################################################
+sub mage {
+  my $this = shift;
+
+  unless ($this->{mage}) {
+
+    my $impl =
+         $Foswiki::cfg{ImagePlugin}{Impl}
+      || $Foswiki::cfg{ImageGalleryPlugin}{Impl}
+      || 'Image::Magick';
+
+    writeDebug("creating new image mage using $impl");
+    eval "require $impl";
+    die $@ if $@;
+    $this->{mage} = $impl->new();
+  }
+
+  return $this->{mage};
+}
+
 ###############################################################################
 sub finishPlugin {
   my $this = shift;
 
-  $this->{mage} = undef;
-  $this->{ua} = undef;
-  $this->{types} = undef;
-  $this->{imageplugin} = undef;
+  undef $this->{mage};
+  undef $this->{ua};
+  undef $this->{types};
+  undef $this->{imageplugin};
 }
 
 ###############################################################################
@@ -180,7 +189,7 @@ sub handleREST {
     );
 
     my $image = readImage($imgWeb, $imgTopic, $imgInfo);
-    $response->print($image);
+    $response->body($image);
   }
 
   return;
@@ -457,6 +466,9 @@ sub handleIMAGE {
   $result =~ s/\$n/\n/go;
   $result =~ s/\$dollar/\$/go;
 
+  # clean up empty 
+  $result =~ s/(style|width|height|class|alt|id)=''//go;
+
   # recursive call for delayed TML expansion
   $result = Foswiki::Func::expandCommonVariables($result, $theTopic, $theWeb);
   return $result;
@@ -487,7 +499,7 @@ sub pingImage {
 
   unless (defined $imgInfo->{width}) {
     writeDebug("pinging $imgInfo->{imgPath}\n");
-    ($imgInfo->{width}, $imgInfo->{height}, $imgInfo->{filesize}, $imgInfo->{format}) = $this->{mage}->Ping($imgInfo->{imgPath});
+    ($imgInfo->{width}, $imgInfo->{height}, $imgInfo->{filesize}, $imgInfo->{format}) = $this->mage->Ping($imgInfo->{imgPath});
     $imgInfo->{width} ||= 0;
     $imgInfo->{height} ||= 0;
   }
@@ -501,7 +513,7 @@ sub pingOrigImage {
 
   unless (defined $imgInfo->{origWidth}) {
     writeDebug("pinging $imgInfo->{origImgPath}\n");
-    ($imgInfo->{origWidth}, $imgInfo->{origHeight}, $imgInfo->{origFilesize}, $imgInfo->{origFormat}) = $this->{mage}->Ping($imgInfo->{origImgPath});
+    ($imgInfo->{origWidth}, $imgInfo->{origHeight}, $imgInfo->{origFilesize}, $imgInfo->{origFormat}) = $this->mage->Ping($imgInfo->{origImgPath});
     $imgInfo->{origWidth} ||= 0;
     $imgInfo->{origHeight} ||= 0;
   }
@@ -592,7 +604,7 @@ sub processImage {
 
       # read
       writeDebug("reading $source");
-      my $error = $this->{mage}->Read($source);
+      my $error = $this->mage->Read($source);
       if ($error =~ /(\d+)/) {
         $this->{errorMsg} = $error;
         return undef if $1 >= 400;
@@ -601,7 +613,7 @@ sub processImage {
       # merge layers
       if ($imgFile =~ /\.(xcf|psd)$/i) {
         writeDebug("merge");
-        $this->{mage} = $this->{mage}->Layers(method => 'merge');
+        $this->{mage} = $this->mage->Layers(method => 'merge');
       }
 
       # scale
@@ -615,7 +627,7 @@ sub processImage {
         }
 
         writeDebug("resize($geometry)");
-        $error = $this->{mage}->Resize(geometry => $geometry);
+        $error = $this->mage->Resize(geometry => $geometry);
         if ($error =~ /(\d+)/) {
           $this->{errorMsg} = $error;
           return undef if $1 >= 400;
@@ -625,7 +637,7 @@ sub processImage {
         if ($crop =~ /^(on|northwest|north|northeast|west|center|east|southwest|south|southeast)$/i) {
           $crop = "center" if $crop eq 'on';
           writeDebug("Set(Gravity=>$crop)");
-          $error = $this->{mage}->Set(Gravity => "$crop");
+          $error = $this->mage->Set(Gravity => "$crop");
           if ($error =~ /(\d+)/) {
             $this->{errorMsg} = $error;
             writeDebug("Error: $error");
@@ -645,7 +657,7 @@ sub processImage {
 
           # new method
           writeDebug("extent(geometry=>$geometry)");
-          $error = $this->{mage}->Extent($geometry);
+          $error = $this->mage->Extent($geometry);
           if ($error =~ /(\d+)/) {
             $this->{errorMsg} = $error;
             writeDebug("Error: $error");
@@ -656,7 +668,7 @@ sub processImage {
 
       # auto orient
       writeDebug("auto orient");
-      $error = $this->{mage}->AutoOrient();
+      $error = $this->mage->AutoOrient();
       if ($error =~ /(\d+)/) {
         $this->{errorMsg} = $error;
         writeDebug("Error: $error");
@@ -665,7 +677,7 @@ sub processImage {
 
       # strip of profiles and comments
       writeDebug("strip");
-      $error = $this->{mage}->Strip();
+      $error = $this->mage->Strip();
       if ($error =~ /(\d+)/) {
         $this->{errorMsg} = $error;
         writeDebug("Error: $error");
@@ -675,7 +687,7 @@ sub processImage {
       # rotate
       if ($rotate) {
         writeDebug("rotate");
-        $error = $this->{mage}->Rotate(degrees => $rotate);
+        $error = $this->mage->Rotate(degrees => $rotate);
         if ($error =~ /(\d+)/) {
           $this->{errorMsg} = $error;
           writeDebug("Error: $error");
@@ -685,14 +697,14 @@ sub processImage {
 
       # write
       writeDebug("writing to $imgInfo{imgPath}");
-      $error = $this->{mage}->Write($imgInfo{imgPath});
+      $error = $this->mage->Write($imgInfo{imgPath});
       if ($error =~ /(\d+)/) {
         $this->{errorMsg} .= " $error";
         writeDebug("Error: $error");
         return undef if $1 >= 400;
       }
 
-      ($imgInfo{width}, $imgInfo{height}, $imgInfo{filesize}, $imgInfo{format}) = $this->{mage}->Get('width', 'height', 'filesize', 'format');
+      ($imgInfo{width}, $imgInfo{height}, $imgInfo{filesize}, $imgInfo{format}) = $this->mage->Get('width', 'height', 'filesize', 'format');
       $imgInfo{width} ||= 0;
       $imgInfo{height} ||= 0;
     }
@@ -702,8 +714,8 @@ sub processImage {
   }
   writeDebug("done");
 
-  # forget images
-  my $mage = $this->{mage};
+  # unload images
+  my $mage = $this->mage;
   @$mage = ();
 
   return \%imgInfo;
@@ -715,6 +727,8 @@ sub beforeSaveHandler {
 
   return unless $this->{autoAttachInlineImages};
 
+  writeDebug("called beforeSaveHandler");
+
   my $wikiName = Foswiki::Func::getWikiName();
   return unless Foswiki::Func::checkAccessPermission("CHANGE", $wikiName, undef, $topic, $web);
 
@@ -736,7 +750,9 @@ sub beforeSaveHandler {
     my $fh = File::Temp->new();
     my $filename = $fh->filename;
 
+    binmode($fh);
     print $fh $data;
+
     my $size = do { use bytes; length $data };
 
     my $attachment = Digest::MD5::md5_hex($data) . '.' . $suffix;
@@ -792,6 +808,8 @@ sub afterRenameHandler {
     && $oldWeb eq $newWeb
     && $oldTopic eq $newTopic;
 
+  writeDebug("called afterRenameHandler");
+
   # attachment has been renamed, delete old thumbnails
   my $web = $oldWeb;
   my $topic = $oldTopic;
@@ -816,6 +834,8 @@ sub completePageHandler {
   my $this = shift;
   #my $text = $_[0];
 
+  writeDebug("called completePageHandler");
+
   $_[0] =~ s/(<svg.*?<\/svg>)/$this->processInlineSvg($1)/geims;
 }
 
@@ -881,7 +901,6 @@ sub processInlineSvg {
   $thumbFileUrl = urlEncode($thumbFileUrl);
 
   my $result = $this->getTemplate("plain");
-  #%TMPL:DEF{"image:plain"}%<img class='imagePlain imagePlain_$align$class' src='$src' alt='$alt' title='$title' width='$width' height='$height' $mousein $mouseout style='$style' />%TMPL:END%
 
   $result =~ s/\$src/$thumbFileUrl/g;
   $result =~ s/\$width/(pingImage($this, $imgInfo))[0]/ge;
@@ -903,6 +922,9 @@ sub processInlineSvg {
   $result =~ s/\$n/\n/go;
   $result =~ s/\$dollar/\$/go;
 
+  # clean up empty 
+  $result =~ s/(style|width|height|class|alt|id)=''//go;
+
   unlink ($svgPath);
 
   return $result;
@@ -1210,9 +1232,16 @@ sub suffixToMimeType {
 sub sanitizeAttachmentName {
   my $fileName = shift;
 
+  my $origFileName = $fileName;
+
+  my $filter = 
+    $Foswiki::cfg{AttachmentNameFilter}  ||
+    $Foswiki::cfg{NameFilter} ||
+    '[^[:alnum:]\. _-]';
+
   $fileName =~ s{[\\/]+$}{};    # Get rid of trailing slash/backslash (unlikely)
   $fileName =~ s!^.*[\\/]!!;    # Get rid of leading directory components
-  $fileName =~ s/[\\\\\*?~^\$@%`"'&;|<>\[\]#\x00-\x1f]//g;    # Get rid of a subset of Namefilter
+  $fileName =~ s/$filter+//g;
 
   return Foswiki::Sandbox::untaintUnchecked($fileName);
 }

diff --git a/lib/Foswiki/Plugins/ImagePlugin/IMAGETOOLTIP.pm b/lib/Foswiki/Plugins/ImagePlugin/IMAGETOOLTIP.pm
@@ -1,6 +1,6 @@
 # Plugin for Foswiki - The Free and Open Source Wiki, http://foswiki.org/
 # 
-# Copyright (C) 2010-2015 Michael Daum, http://michaeldaumconsulting.com
+# Copyright (C) 2010-2016 Michael Daum, http://michaeldaumconsulting.com
 # 
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License

diff --git a/templates/imageplugin.tmpl b/templates/imageplugin.tmpl
@@ -1,21 +1,21 @@
 %{ format definitions for image types }%
 
-%TMPL:DEF{"image:link"}%<a %IF{"'$id'!=''" then="id='$id' "}%class='imageLink' href='$href' title='$title' style='$style'>$text</a>%TMPL:END%
+%TMPL:DEF{"image:link"}%<a href='$href' id='$id' class='imageLink' title='$title' style='$style'>$text</a>%TMPL:END%
 
 %TMPL:DEF{"image:plain"}%<img class='imagePlain imagePlain_$align$class' src='$src' alt='$alt' title='$title' width='$width' height='$height' $mousein $mouseout style='$style' />%TMPL:END%
 
-%TMPL:DEF{"image:simple"}%<a %IF{"'$id'!=''" then="id='$id' "}%class='imageHref imageSimple imageSimple_$align$class' title='$title' href='$href' ><img src='$src' alt='$alt' width='$width' height='$height' $mousein $mouseout style='$style' /></a>%TMPL:END%
+%TMPL:DEF{"image:simple"}%<a href='$href' id='$id' class='imageHref imageSimple imageSimple_$align$class' title='$title'><img src='$src' alt='$alt' width='$width' height='$height' $mousein $mouseout style='$style' /></a>%TMPL:END%
 
-%TMPL:DEF{"image:frame"}%<div %IF{"'$id'!=''" then="id='$id' "}%class='imageFrame imageFrame_$align$class' style='width:$framewidthpx'><a class='imageHref' title='$title' href='$href'><img src='$src' alt='$alt' width='$width' height='$height' $mousein $mouseout style='$style'/></a>$caption</div>%TMPL:END%
+%TMPL:DEF{"image:frame"}%<div id='$id' class='imageFrame imageFrame_$align$class' style='width:$framewidthpx'><a href='$href' class='imageHref' title='$title'><img src='$src' alt='$alt' width='$width' height='$height' $mousein $mouseout style='$style'/></a>$caption</div>%TMPL:END%
 
-%TMPL:DEF{"image:float"}%<div %IF{"'$id'!=''" then="id='$id' "}%class='imageFloat imageFloat_$align$class'><a class='imageHref' title='$title' href='$href'><img src='$src' alt='$alt' width='$width' height='$height' $mousein $mouseout  style='$style'/></a>$caption</div>%TMPL:END%
+%TMPL:DEF{"image:float"}%<div id='$id' class='imageFloat imageFloat_$align$class'><a href='$href' class='imageHref' title='$title'><img src='$src' alt='$alt' width='$width' height='$height' $mousein $mouseout  style='$style'/></a>$caption</div>%TMPL:END%
 
-%TMPL:DEF{"image:thumb"}%<div %IF{"'$id'!=''" then="id='$id' "}%class='imageThumb imageThumb_$align$class' style='width:$framewidthpx'><a class='imageHref' title='$title' href='$href'><img src='$src' alt='$alt' width='$width' height='$height' $mousein $mouseout style='$style'/></a>$caption</div>%TMPL:END%
+%TMPL:DEF{"image:thumb"}%<div id='$id' class='imageThumb imageThumb_$align$class' style='width:$framewidthpx'><a href='$href' class='imageHref' title='$title'><img src='$src' alt='$alt' width='$width' height='$height' $mousein $mouseout style='$style'/></a>$caption</div>%TMPL:END%
 
 %{ helper snippets }%
 
 %TMPL:DEF{"image:clear"}%<br class='imageClear' clear='all' />%TMPL:END%
 
 %TMPL:DEF{"image:caption"}%<div class='imageCaption'> $caption </div>%TMPL:END%
 
-%TMPL:DEF{"image:magnify"}%<div class='imageMagnify'><a title='Enlarge' href='$href'><img src='$magnifyIcon' width='$magnifyWidth' height='$magnifyHeight' alt='Enlarge' /></a></div>%TMPL:END%
+%TMPL:DEF{"image:magnify"}%<div class='imageMagnify'><a href='$href' title='Enlarge'><img src='$magnifyIcon' width='$magnifyWidth' height='$magnifyHeight' alt='Enlarge' /></a></div>%TMPL:END%