doc/bugs/404_when_cancel_create_page.mdwn

   1 If you
   2
   3  * Add a link to a non-existant page and save. (e.g. somewhere-over-the-rainbow)
   4  * Click the question mark to create the page.
   5  * Click the cancel button.
   6
   7 You get a 404 as the page doesn't exist. This patch redirects to the from location
   8 if it is known.
   9
  10
  11         === modified file 'IkiWiki/CGI.pm'
  12         --- IkiWiki/CGI.pm
  13         +++ IkiWiki/CGI.pm
  14         @@ -427,7 +427,11 @@
  15                 }
  16
  17                 if ($form->submitted eq "Cancel") {
  18         -               redirect($q, "$config{url}/".htmlpage($page));
  19         +               if ( $newpage && defined $from ) {
  20         +                       redirect($q, "$config{url}/".htmlpage($from));
  21         +               } else {
  22         +                       redirect($q, "$config{url}/".htmlpage($page));
  23         +               }
  24                         return;
  25                 }
  26                 elsif ($form->submitted eq "Preview") {
  27
  28 > I think you mean to use `$newfile`? I've applied a modieid version
  29 > that also deal with creating a new page with no defined $from location.
  30 > [[bugs/done]] --[[Joey]]
  31
  32 >> Yes of course, that's what I get for submitting an untested patch!
  33 >> I must stop doing that.
  34
  35 [P.S. just above that is
  36
  37                 $type=$form->param('type');
  38                 if (defined $type && length $type && $hooks{htmlize}{$type}) {
  39                         $type=possibly_foolish_untaint($type);
  40                 }
  41                 ....
  42                 $file=$page.".".$type;
  43
  44 I'm a little worried by the `possibly_foolish_untaint` (good name for it by the way,
  45 makes it stick out). I don't think much can be done to exploit this (if anything),
  46 but it seems like you could have a very strict regex there rather than the untaint,
  47 is there aren't going to be many possible extensions. Something like `/(.\w+)+/`
  48 (groups of dot separated alpha-num chars if my perl-foo isn't failing me). You could
  49 at least exclude `/` and `..`. I'm happy to turn this in to a patch if you agree.]
  50
  51 > The reason it's safe to use `possibly_foolish_untaint` here is because
  52 > of the check for $hooks{htmlize}{$type}. This limits it to types
  53 > that have a registered htmlize hook (mdwn, etc), and not whatever random
  54 > garbage an attacker might try to put in. If it wasn't for that check,
  55 > using `possibly_foolish_untaint` there would be _very_ foolish indeed..
  56 > --[[Joey]]
  57
  58 >> Nice, sorry I missed it.
  59 >> I must say thankyou for creating ikiwiki.
  60 >> The more I look at it, the more I admire what you are doing with it and how you are going about it