img: stop ImageMagick trying to be clever if filenames contain a colon

$im->Read() takes a filename-like argument with several sets of special
syntax. Most of the possible metacharacters are escaped by the
default `wiki_file_chars` (and in any case not particularly disruptive),
but the colon ":" is not.

It seems the way to force ImageMagick to treat colons within the
filename as literal is to prepend a colon, so do that.

diff --git a/IkiWiki/Plugin/img.pm b/IkiWiki/Plugin/img.pm
index 17a58ca7a476b9b01e953d5ce10ce6c3200f8e7b..169f5e7137a99739501c170561b164d3c4e8825d 100644 (file)
--- a/IkiWiki/Plugin/img.pm
+++ b/IkiWiki/Plugin/img.pm
@@ -76,7 +76,7 @@ sub preprocess (@) {
        my $im = Image::Magick->new();
        my $imglink;
        my $imgdatalink;
        my $im = Image::Magick->new();
        my $imglink;
        my $imgdatalink;

-       my $r = $im->Read("$srcfile\[$pagenumber]");
+       my $r = $im->Read(":$srcfile\[$pagenumber]");

        error sprintf(gettext("failed to read %s: %s"), $file, $r) if $r;
 
        if (! defined $im->Get("width") || ! defined $im->Get("height")) {
        error sprintf(gettext("failed to read %s: %s"), $file, $r) if $r;
 
        if (! defined $im->Get("width") || ! defined $im->Get("height")) {

diff --git a/debian/changelog b/debian/changelog
index d29819f97fb41b868557d2c700ba5866171cdbbd..2b79e51ab1c5392448542ab401c22fbf81104591 100644 (file)
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,6 +1,7 @@
 ikiwiki (3.20150611) UNRELEASED; urgency=medium
 
   * inline: change default sort order from age to "age title" for determinism
 ikiwiki (3.20150611) UNRELEASED; urgency=medium
 
   * inline: change default sort order from age to "age title" for determinism

+  * img: avoid ImageMagick misinterpreting filenames containing a colon

 
  -- Simon McVittie <smcv@debian.org>  Thu, 11 Jun 2015 08:32:35 +0100
 
 
  -- Simon McVittie <smcv@debian.org>  Thu, 11 Jun 2015 08:32:35 +0100
 

diff --git a/doc/bugs/Colons___8216__:__8217___should_not_be_allowed_in_image_filenames.mdwn b/doc/bugs/Colons___8216__:__8217___should_not_be_allowed_in_image_filenames.mdwn
index 3ecde81f81697923748ad316cbf796c2d59cc7bb..0524a9a9882237f36ac0eacd8bffd3151ca43b62 100644 (file)
--- a/doc/bugs/Colons___8216__:__8217___should_not_be_allowed_in_image_filenames.mdwn
+++ b/doc/bugs/Colons___8216__:__8217___should_not_be_allowed_in_image_filenames.mdwn
@@ -9,4 +9,28 @@ However, `Image::Magick` doesn't seem to like page selection for filenames conta
     $ identify 'screenshot_2015-06-06_18-37-53.png[0]'
     screenshot_2015-06-06_18-37-53.png[0]=>screenshot_2015-06-06_18-37-53.png PNG 453x122 453x122+0+0 8-bit sRGB 11.2KB 0.000u 0:00.000
 
     $ identify 'screenshot_2015-06-06_18-37-53.png[0]'
     screenshot_2015-06-06_18-37-53.png[0]=>screenshot_2015-06-06_18-37-53.png PNG 453x122 453x122+0+0 8-bit sRGB 11.2KB 0.000u 0:00.000
 

-This might be an imagemagick bug, but it's also possible that colons are interpreted somehow.  Anyway, to render such images properly in ikiwiki I had to remove the colons.  An easy fix is to remove ‘:’ from `wiki_file_chars`, but this can break existing installations.  A better solution would be to make `IkiWiki::Plugin::img` croak on such image filenames (which anyway are currently not rendered, but `Image::Magick`'s error message is quite cryptic).
+This might be an imagemagick bug, but it's also possible that colons
+are interpreted somehow.
+
+> Yes they are: ImageMagick has syntax to force the file to be
+> interpreted as a particular format, like gif:foobar.img to
+> read foobar.img as a GIF, or to use unusual I/O patterns, like
+> fd:5. --[[smcv]]
+
+Anyway, to render such images properly in ikiwiki I had to remove
+the colons.  An easy fix is to remove ‘:’ from `wiki_file_chars`,
+but this can break existing installations.
+
+> I think we should remove `:` from the default `wiki_file_chars`
+> either as soon as we have a way to handle backwards compatibility,
+> or in ikiwiki 4; but you're right that it's a compat problem,
+> so we can't just do that as a solution. --s
+
+A better solution would be to make `IkiWiki::Plugin::img` croak on
+such image filenames (which anyway are currently not rendered, but
+`Image::Magick`'s error message is quite cryptic).
+
+> Better still would be to fix the bug by escaping the filename
+> so ImageMagick treats it as just a filename. It seems the way
+> to do that is to call `Read(":hello:world.png")` instead of
+> `Read("hello:world.png")`, which I have now [[done]]. --s

diff --git a/t/img.t b/t/img.t
index 6c295737e53a7da2705d6b04af7f394b720fe899..7e7753aab50d7934f8a5d901bd01848a16423b36 100755 (executable)
--- a/t/img.t
+++ b/t/img.t
@@ -24,6 +24,11 @@ my $SVGS_WORK = defined $magick->QueryFormat("svg");
 ok(! system("rm -rf t/tmp; mkdir -p t/tmp/in"));
 
 ok(! system("cp t/img/redsquare.png t/tmp/in/redsquare.png"));
 ok(! system("rm -rf t/tmp; mkdir -p t/tmp/in"));
 
 ok(! system("cp t/img/redsquare.png t/tmp/in/redsquare.png"));

+# colons in filenames are a corner case for img
+ok(! system("cp t/img/redsquare.png t/tmp/in/hello:world.png"));
+ok(! system("cp t/img/redsquare.png t/tmp/in/a:b:c.png"));
+ok(! system("cp t/img/redsquare.png t/tmp/in/a:b:c:d.png"));
+ok(! system("cp t/img/redsquare.png t/tmp/in/a:b:c:d:e:f:g:h:i:j.png"));

 
 if ($SVGS_WORK) {
        writefile("emptysquare.svg", "t/tmp/in",
 
 if ($SVGS_WORK) {
        writefile("emptysquare.svg", "t/tmp/in",


@@ -42,6 +47,9 @@ writefile("imgconversions.mdwn", "t/tmp/in", <<EOF
 [[!img redsquare.png]]
 [[!img redsquare.png size=10x]]
 [[!img redsquare.png size=30x50]] expecting 30x30
 [[!img redsquare.png]]
 [[!img redsquare.png size=10x]]
 [[!img redsquare.png size=30x50]] expecting 30x30

+[[!img hello:world.png size=x8]] expecting 8x8
+[[!img a:b:c.png size=x4]]
+[[!img a:b:c:d:e:f:g:h:i:j.png size=x6]]

 $maybe_svg_img
 [[!img twopages.pdf size=12x]]
 [[!img twopages.pdf size=16x pagenumber=1]]
 $maybe_svg_img
 [[!img twopages.pdf size=12x]]
 [[!img twopages.pdf size=16x pagenumber=1]]


@@ -57,7 +65,7 @@ ok(! system($command));
 sub size($) {
        my $filename = shift;
        my $im = Image::Magick->new();
 sub size($) {
        my $filename = shift;
        my $im = Image::Magick->new();

-       my $r = $im->Read($filename);
+       my $r = $im->Read(":$filename");

        return "no image" if $r;
        my $w = $im->Get("width");
        my $h = $im->Get("height");
        return "no image" if $r;
        my $w = $im->Get("width");
        my $h = $im->Get("height");


@@ -78,6 +86,10 @@ if ($SVGS_WORK) {
 
 is(size("$outpath/12x-twopages.png"), "12x12");
 is(size("$outpath/16x-p1-twopages.png"), "16x2");
 
 is(size("$outpath/12x-twopages.png"), "12x12");
 is(size("$outpath/16x-p1-twopages.png"), "16x2");

+ok($outhtml =~ /width="8" height="8".*expecting 8x8/);
+is(size("$outpath/x8-hello:world.png"), "8x8");
+is(size("$outpath/x4-a:b:c.png"), "4x4");
+is(size("$outpath/x6-a:b:c:d:e:f:g:h:i:j.png"), "6x6");

 
 # now let's remove them again
 
 
 # now let's remove them again
 


@@ -90,6 +102,9 @@ if (1) { # for easier testing
        ok(! -e "$outpath/10x-simple-svg.png");
        ok(! -e "$outpath/10x-simple-pdf.png");
        ok(! -e "$outpath/10x-p1-simple-pdf.png");
        ok(! -e "$outpath/10x-simple-svg.png");
        ok(! -e "$outpath/10x-simple-pdf.png");
        ok(! -e "$outpath/10x-p1-simple-pdf.png");

+       ok(! -e "$outpath/x8-hello:world.png");
+       ok(! -e "$outpath/x4-a:b:c.png");
+       ok(! -e "$outpath/x6-a:b:c:d:e:f:g:h:i:j.png");

 
        # cleanup
        ok(! system("rm -rf t/tmp"));
 
        # cleanup
        ok(! system("rm -rf t/tmp"));