From 7a2117bf8c2cf5372e64ab7b368803eec7e6f5d7 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sat, 13 Jun 2015 20:00:08 +0100 Subject: [PATCH] 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. --- IkiWiki/Plugin/img.pm | 2 +- debian/changelog | 1 + ...uld_not_be_allowed_in_image_filenames.mdwn | 26 ++++++++++++++++++- t/img.t | 17 +++++++++++- 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/IkiWiki/Plugin/img.pm b/IkiWiki/Plugin/img.pm index 17a58ca7a..169f5e713 100644 --- 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 $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")) { diff --git a/debian/changelog b/debian/changelog index d29819f97..2b79e51ab 100644 --- 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 + * img: avoid ImageMagick misinterpreting filenames containing a colon -- Simon McVittie 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 3ecde81f8..0524a9a98 100644 --- 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 -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 6c295737e..7e7753aab 100755 --- 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")); +# 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", @@ -42,6 +47,9 @@ writefile("imgconversions.mdwn", "t/tmp/in", <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"); @@ -78,6 +86,10 @@ if ($SVGS_WORK) { 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 @@ -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/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")); -- 2.39.2