summaryrefslogtreecommitdiff
path: root/doc/patchqueue/various_fixes.mdwn
blob: 318e7e941c7c7d5b1eb04ae769296786d543bbf7 (plain)

Sorry if you don't appreciate me lumping all of these patches together.

These are various fixes I had to make when installing Ikiwiki. Some are due it being a non-Debian system, the others are actual bugs.

--- upstream/IkiWiki/Rcs/svn.pm 2006-09-16 01:11:55.000000000 +0100
+++ main/IkiWiki/Rcs/svn.pm     2006-09-16 01:12:50.000000000 +0100
@@ -34,7 +34,7 @@
        my $field=shift;
        my $file=shift;

-       my $info=`LANG=C svn info $file`;
+       my $info=`svn info $file`;
        my ($ret)=$info=~/^$field: (.*)$/m;
        return $ret;
 } #}}}
@@ -140,7 +140,7 @@
                if $svn_version =~ /\d\.(\d)\.\d/ && $1 >= 2;

        my $svn_url=svn_info("URL", $config{srcdir});
-       my $xml = XMLin(scalar `svn $svn_limit --xml -v log '$svn_url'`,
+       my $xml = XMLin(scalar `svn $svn_limit --xml -v log '$svn_url' --config-dir /tmp`,
                ForceArray => [ 'logentry', 'path' ],
                GroupTags => { paths => 'path' },
                KeyAttr => { path => 'content' },

The first hunk of this patch is strange. It just failed to work with this in place, and it took me a long time to figure it out. I realise what you are trying to do, and it still works here as I use a similar LANG anyway.

For reference svn version 1.3.1 (r19032), my $LANG=en_GB.utf8, but I'm not sure what the CGI was running under.

That's strange. Is the problem to do with setting LANG=C or to do with the way it's set and exported on the same line as the svn info call? Can you reproduce the problem running svn info outside of ikiwiki? --[[Joey]]

I've now managed to reproduce the problem. I'll try and give some more information. When going to the Recent Changes link I get

[Sat Sep 16 15:16:08 2006] [error] [client xxxx] svn: Can't check path '/home/jw2328/.subversion': Permission denied, referer: http://xxxxx/test/sandbox.html
[Sat Sep 16 15:16:08 2006] [error] [client xxxx] Use of uninitialized value in concatenation (.) or string at /usr/lib/perl5/site_perl/5.8.3/IkiWiki/Rcs/svn.pm line 145., referer: http://xxxx/test/sandbox.html
[Sat Sep 16 15:16:08 2006] [error] [client xxxxx] svn: Can't check path '/home/jw2328/.subversion': Permission denied, referer: http://xxxx/test/sandbox.html
[Sat Sep 16 15:16:09 2006] [error] [client xxxx] File does not exist:  at /usr/lib/perl5/site_perl/5.8.3/IkiWiki/Rcs/svn.pm line 145, referer: http://xxxx/test/sandbox.html
[Sat Sep 16 15:16:09 2006] [error] [client xxxx] Premature end of script headers: ikitest, referer: http://xxxx/test/sandbox.html

which the $svn_url is causing the uninitialised value, due to the LANG=C it seems, as if I remove it it goes away. The file does not exist is due to the unreadable .subversion. echoing the LANG before it is set shows that the variable is normally empty for the user that is running it.

The second removes problems with cannot access /home/$user/.svnsomething in the logs. I think this problem was also fatal (I should have reported these sooner).

I can try and debug these problems if you could suggest some way to do so, but I am probably losing the server in a couple of days, so I can't be of too much help I'm afraid.

I imagine that passing --config-dir /tmp would be either insecure or would limit ikiwiki use to one user per machine. --config-dir /etc/subversion might be better, though still a hack, since a user's real ~/.subversion might be needed to access some repos.

Maybe I didn't notice this problem since I have a ~/.subversion everywhere that I've used ikiwiki. Hmm, no, I don't reproduce it, svn happily recreated the directory during an ikiwiki run after I moved it out of the way here. Maybe an issue with old versions of svn? Although AFIACR, svn has auto-created ~/.subversion for years.

What's the error message? --[[Joey]]

svn: Can't check path '/home/jw2328/.subversion': Permission denied, where jw2328 is my usual user. I have restrictive permissions of 0700 on home dirs on the server, and the CGI is running as uid apache, euid root. (Not my setup anymore). The way I had it set up, was jw2328 owning thesource dir, and the svn repo, with g+sw on them both. I ran sudo ikiwiki --setup though, as I was reluctant to adjust permissions on my cgi-dir. This seems to be the root of the problem.

Ah, I think it's better to keep the permissions of the repository and source directory sane (755) and make the cgi suid to your user, which is how it's designed to work.

I realise that now, and I now have a much more sane setup that works.


--- IkiWiki.pm
+++ IkiWiki.pm
@@ -734,7 +734,18 @@
        my $page=shift;
        my $spec=shift;

-       return eval pagespec_translate($spec);
+        my $pagespec = pagespec_translate($spec);
+
+        my $newpagespec;
+
+       local($1);
+       if ($pagespec =~ /(.*)/) {
+               $newpagespec = $1;
+       } else {
+               die "oh";
+       }
+
+       return eval $newpagespec;
 } #}}}

 sub match_glob ($$) { #{{{

This works around a silly, but extremely annoying, taint bug in older versions of perl. I'm not sure of the details, but it means that either values become tainted from nowhere, or they don't get untainted possibly. This also affects backports to sarge. "oh" is not going to be very informative if that code path is ever taken, but I hope that it never is.

You're not the first person to report a problem here with older versions of perl and pagespec tainting. I suspect that this would work around it: return eval possibly_foolish_untaint(pagespec_translate($spec)); I'm very uncomfortable putting that in the shipping version of ikiwiki, because pagespecs are potentially insanely dangerous, given how they're evaled and all. The tainting is the only sanity check there is that pagespec_translate manages to clean up any possibly harmful perl code in a pagespec. It's good to have belt and suspenders here.

For all I know, older versions of perl are keeping it tainted because pagespec_translate is somehow broken under old versions of perl and is in fact not fully untainting the pagespec. Ok, probably not, it's more likely that some of the regexps in there don't manage to clear the taint flag with old versions of perl, while still doing a perfectly ok job of sanitising the pagespec.

I suppose that the version of perl ($^V) could be checked and the untaint only be called for the old version. Though it seems it would be better to try to debug this some first. Maybe instrumenting pagespec_translate with calls to Scalar::Utils's tainted() function and seeing which parts of pagespecs arn't getting untainted would be a good start.

--[[Joey]]

It seems like it is always the (with instrumentation)

 elsif ($word =~ /^(link|backlink|created_before|created_after|creation_month|creation_year|creation_day)\((.+)\)$/) {
      warn("\$1 tainted=".tainted($1).", \$2 tainted=".tainted($2)." \$code tainted=".tainted($code));
      $code.=" match_$1(\$page, ".safequote($2).")";
      warn("\$1 tainted=".tainted($1).", \$2 tainted=".tainted($2)." \$code tainted=".tainted($code));
      warn("safequote tainted=".tainted(safequote($2)));
 }

bit that causes it. With the following trace:

 $1 tainted=0, $2 tainted=0 $code tainted=0 at IkiWiki.pm line 718.
 $1 tainted=0, $2 tainted=0 $code tainted=1 at IkiWiki.pm line 720.
 safequote tainted=0 at IkiWiki.pm line 721.

which shows that $code appears to become tainted from nowhere. http://mail-archives.apache.org/mod_mbox/spamassassin-dev/200509.mbox/%3C3838.431C7D9B.5F152B8F.dev@spamassassin.apache.org%3E is what pointed me to find the problem/workaround.

Given that verification, an untaint contingent on the value of $^V sounds reasonable and I'd accept such a patch. I'm not quite sure which version(s) of perl it should check for.

I'm not going to write one though. I don't know what versions either, but I think the evil of the special case is too much in this case. If you are happy to insist on a newer version of perl then I will leave it at that and sort something out locally. If you want the patch I will code it though, as I realise you may want to support sarge installs.