Merge branch 'master' of ssh://git.ikiwiki.info/srv/git/ikiwiki.info
[ikiwiki] / 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