From e674bb1445378ce43ae60720e42d6c418270001a Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Sun, 18 Jul 2010 17:30:46 -0400 Subject: [PATCH] fix other 2 cases of conflicting destdir files Cleanly fixed case where destdir file failed to be written because there was a directory with the same name. This can be detected with no extra system calls, and dealt with by finding all pages that wrote files inside the directory, and removing them and the directory. The other, inverse case would be expensive to detect in will_render, since it would need to check each parent directory of the file to see if the directory is really a conflicting file. But prep_writefile already does a similar scan for symlinks in the path, so I added code there to remove the conflicting file. This fix assumes that the file is written using writefile, and not some other means (but using other means would be a security hole too, so hopefully nothing does). --- IkiWiki.pm | 29 +++++++++++++++++++++++++++-- debian/changelog | 2 ++ doc/bugs/conflicts.mdwn | 2 ++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/IkiWiki.pm b/IkiWiki.pm index 85b542486..fa49b2c34 100644 --- a/IkiWiki.pm +++ b/IkiWiki.pm @@ -823,6 +823,17 @@ sub prep_writefile ($$) { if (-l "$destdir/$test") { error("cannot write to a symlink ($test)"); } + if (-f _ && $test ne $file) { + # Remove conflicting file. + foreach my $p (keys %renderedfiles, keys %oldrenderedfiles) { + foreach my $f (@{$renderedfiles{$p}}, @{$oldrenderedfiles{$p}}) { + if ($f eq $test) { + unlink("$destdir/$test"); + last; + } + } + } + } $test=dirname($test); } @@ -876,11 +887,12 @@ sub will_render ($$;$) { my $dest=shift; my $clear=shift; - # Important security check. + # Important security check for independently created files. if (-e "$config{destdir}/$dest" && ! $config{rebuild} && ! grep { $_ eq $dest } (@{$renderedfiles{$page}}, @{$oldrenderedfiles{$page}}, @{$wikistate{editpage}{previews}})) { my $from_other_page=0; - foreach my $p (keys %renderedfiles) { + # Expensive, but rarely runs. + foreach my $p (keys %renderedfiles, keys %oldrenderedfiles) { if (grep { $_ eq $dest || dirname($_) eq $dest @@ -894,6 +906,19 @@ sub will_render ($$;$) { unless $from_other_page; } + # If $dest exists as a directory, remove conflicting files in it + # rendered from other pages. + if (-d _) { + foreach my $p (keys %renderedfiles, keys %oldrenderedfiles) { + foreach my $f (@{$renderedfiles{$p}}, @{$oldrenderedfiles{$p}}) { + if ($f eq dirname($dest) || dirname($f) eq $dest) { + unlink("$config{destdir}/$f"); + rmdir(dirname("$config{destdir}/$f")); + } + } + } + } + if (! $clear || $cleared{$page}) { $renderedfiles{$page}=[$dest, grep { $_ ne $dest } @{$renderedfiles{$page}}]; } diff --git a/debian/changelog b/debian/changelog index bb9a43692..21b5d01fa 100644 --- a/debian/changelog +++ b/debian/changelog @@ -18,6 +18,8 @@ ikiwiki (3.20100705) UNRELEASED; urgency=low * po: needstranslation() pagespec can have a percent specified. * Drop Cache-Control must-revalidate (Firefox 3.5.10 does not seem to have the caching problem that was added to work around). Closes: #588623 + * Made much more robust in cases where multiple source files produce + conflicting files/directories in the destdir. -- Joey Hess Mon, 05 Jul 2010 13:59:42 -0400 diff --git a/doc/bugs/conflicts.mdwn b/doc/bugs/conflicts.mdwn index a67450290..bef0f54cd 100644 --- a/doc/bugs/conflicts.mdwn +++ b/doc/bugs/conflicts.mdwn @@ -28,3 +28,5 @@ destination file is rendered by multiple pages. Or when one page renders a file that is a parent directory of the rendered file of another page. It could warn, rather than erroring. The last page rendered would "win"; generating the destdir file. + +[[done]] -- 2.26.2