]> git.vanrenterghem.biz Git - git.ikiwiki.info.git/commitdiff
protect $@ whenever a block using $@ is non-trivial
authorSimon McVittie <smcv@debian.org>
Fri, 21 Feb 2014 17:06:36 +0000 (17:06 +0000)
committerSimon McVittie <smcv@debian.org>
Fri, 21 Feb 2014 17:06:36 +0000 (17:06 +0000)
As noted in the Try::Tiny man page, eval/$@ can be quite awkward in
corner cases, because $@ has the same properties and problems as C's
errno. While writing a regression test for definetemplate
in which it couldn't find an appropriate template, I received

    <span class="error">Error: failed to process template
    <span class="createlink">deftmpl</span> </span>

instead of the intended

    <span class="error">Error: failed to process template
    <span class="createlink">deftmpl</span> template deftmpl not
    found</span>

which turned out to be because the "catch"-analogous block called
gettext before it used $@, and gettext can call define_gettext,
which uses eval.

This commit alters all current "catch"-like blocks that use $@, except
those that just do trivial things with $@ (string interpolation, string
concatenation) and call a function (die, error, print, etc.)

IkiWiki/CGI.pm
IkiWiki/Plugin/aggregate.pm
IkiWiki/Plugin/attachment.pm
IkiWiki/Plugin/editpage.pm
IkiWiki/Plugin/edittemplate.pm
IkiWiki/Plugin/inline.pm
IkiWiki/Plugin/mdwn.pm
IkiWiki/Plugin/template.pm

index 5baa6c1798ef02e1f45680daa032da17674fafb0..c0d8f598b5f882b27bd2d5671d476aa5164a812b 100644 (file)
@@ -351,7 +351,8 @@ sub cgi_getsession ($) {
                        { FileName => "$config{wikistatedir}/sessions.db" })
        };
        if (! $session || $@) {
-               error($@." ".CGI::Session->errstr());
+               my $error = $@;
+               error($error." ".CGI::Session->errstr());
        }
        
        umask($oldmask);
index 28c445913f436e8ee7269fb1b3eea6ca35351bab..fbf88c62738d783c210d27cf6aa57c1b81c71fef 100644 (file)
@@ -553,7 +553,9 @@ sub aggregate (@) {
                        };
                }
                if ($@) {
-                       $feed->{message}=gettext("feed crashed XML::Feed!")." ($@)";
+                       # gettext can clobber $@
+                       my $error = $@;
+                       $feed->{message}=gettext("feed crashed XML::Feed!")." ($error)";
                        $feed->{error}=1;
                        debug($feed->{message});
                        next;
@@ -675,7 +677,9 @@ sub write_page ($$$$$) {
                $template=template($feed->{template}, blind_cache => 1);
        };
        if ($@) {
-               print STDERR gettext("failed to process template:")." $@";
+               # gettext can clobber $@
+               my $error = $@;
+               print STDERR gettext("failed to process template:")." $error";
                return;
        }
        $template->param(title => $params{title})
index 83dd120f649ea13898ee9cc479bb576cb94043b7..d56dd18ad8ded90e3dd6f46598e7bc632359d4b5 100644 (file)
@@ -229,8 +229,10 @@ sub attachment_store {
                check_canattach($session, $final_filename, $tempfile);
        };
        if ($@) {
-               json_response($q, $form, $dest."/".$filename, $@);
-               error $@;
+               # save error in case called functions clobber $@
+               my $error = $@;
+               json_response($q, $form, $dest."/".$filename, $error);
+               error $error;
        }
 
        # Move the attachment into holding directory.
index d15607990db2412b8a14f281864f82f98b3cac7f..3047869c4a0ad4da75f715937b8fff185dc80970 100644 (file)
@@ -400,10 +400,12 @@ sub cgi_editpage ($$) {
                eval { writefile($file, $config{srcdir}, $content) };
                $config{cgi}=1;
                if ($@) {
+                       # save $@ in case a called function clobbers it
+                       my $error = $@;
                        $form->field(name => "rcsinfo", value => rcs_prepedit($file),
                                force => 1);
                        my $mtemplate=template("editfailedsave.tmpl");
-                       $mtemplate->param(error_message => $@);
+                       $mtemplate->param(error_message => $error);
                        $form->tmpl_param("message", $mtemplate->output);
                        $form->field("editcontent", value => $content, force => 1);
                        $form->tmpl_param("page_select", 0);
index c7f1e4fa7f7117660b5d89207a1795caec9fc6af..e3ce5e3d97437392c681f20f6c3a23b0b9690790 100644 (file)
@@ -130,9 +130,11 @@ sub filltemplate ($$) {
                $template=template("/".$template_page);
        };
        if ($@) {
+               # gettext can clobber $@
+               my $error = $@;
                # Indicate that the earlier preprocessor directive set 
                # up a template that doesn't work.
-               return "[[!edittemplate ".gettext("failed to process template:")." $@]]";
+               return "[[!edittemplate ".gettext("failed to process template:")." $error]]";
        }
 
        $template->param(name => $page);
index 0380bec3d6cd9948ec92229117ea3655212af59a..123dfd36489c8536450971c95267ef30261abcc9 100644 (file)
@@ -391,7 +391,9 @@ sub preprocess_inline (@) {
                                        blind_cache => 1);
                        };
                        if ($@) {
-                               error sprintf(gettext("failed to process template %s"), $params{template}.".tmpl").": $@";
+                               # gettext can clobber $@
+                               my $error = $@;
+                               error sprintf(gettext("failed to process template %s"), $params{template}.".tmpl").": $error";
                        }
                }
                my $needcontent=$raw || (!($archive && $quick) && $template->query(name => 'content'));
index 430194bffdcc6444f34036709d9dc3767187af85..014e78eea124604e249d35d42a6b9ccf6524c254 100644 (file)
@@ -92,8 +92,9 @@ sub htmlize (@) {
                                        $markdown_sub=\&Markdown::Markdown;
                                }
                                else {
+                                       my $error = $@;
                                        do "/usr/bin/markdown" ||
-                                               error(sprintf(gettext("failed to load Markdown.pm perl module (%s) or /usr/bin/markdown (%s)"), $@, $!));
+                                               error(sprintf(gettext("failed to load Markdown.pm perl module (%s) or /usr/bin/markdown (%s)"), $error, $!));
                                        $markdown_sub=\&Markdown::Markdown;
                                }
                        }
index 3df06e652eeee285d51690b9d2178d6e0fb2b8ca..ccc9cb6664e5617c514ba6715107a2c4d1ddc448 100644 (file)
@@ -41,9 +41,11 @@ sub preprocess (@) {
                        blind_cache => 1);
        };
        if ($@) {
+               # gettext can clobber $@
+               my $error = $@;
                error sprintf(gettext("failed to process template %s"),
                        htmllink($params{page}, $params{destpage},
-                               "/templates/$params{id}"))." $@";
+                               "/templates/$params{id}"))." $error";
        }
 
        $params{basename}=IkiWiki::basename($params{page});