Disable statistics by default

[telyn]

I've replaced the no-stats file with a 'stats' file so the default is now for statistics to be disabled.

This will need reflecting in the documentation which I can look into if you like? We should probably write a little recommendation that users ensure that statistics are only enabled when the directory has a .htaccess file with either password or IP based security, too.

Closes #51

[patch0]

I don't think this is required, as no-stats effectively becomes a no-op, in most cases, I suspect, apart from when someone has made it a double negative (as you've done testing for contents of false).

Using redirects is tricky, due to TOCTOU considerations. It is difficult to be certain that (say) $stats isn't a symlink pointing to /etc/passwd.

Then there are permissions issues -- postinst runs as root, so a chown to the domain's owner is needed.

Also the stats file name is just stats not stats-enabled.

I think this snipped should do the trick:

if [ -e "$no_stats" ] && ( grep -q false "$no_stats" ) ; then
  # Use cp to maintain permissions, and follow symlinks, removing any destination file first.
  cp -aL --remove-destination $no_stats $stats
  truncate -s 0 $stats
fi

# Optionally remove the old no-stats file
rm "$no_stats"

[telyn]

Yeah, that'll do it, though I have noticed (since you got me thinking about TOCTOU) that we can't guarantee that $stats isn't a symlink to /etc/passwd during the truncate run - and that truncate will merrily truncate it if it is.

Didn't know about -q on grep though, that's cool :-)

[telyn]

We should probably use sudo to drop permissions down to admin during the truncate I guess

[patch0]

The cp -L copies the data from the symlinked source to a new file, so no need to drop privs. Also you'd need to work out which user to drop do, since domains can be owned by any non-system user.

patch@dynamo:/tmp/user/1000/testing$ ls -la
total 16
drwxr-xr-x 2 patch patch 4096 Aug 31 11:30 .
drwx------ 8 patch patch 4096 Aug 31 11:30 ..
-rw-r--r-- 1 patch patch 2888 Aug 31 10:36 passwd
-rw-r--r-- 1 patch patch  187 Aug 31 10:47 test
patch@dynamo:/tmp/user/1000/testing$ ln -s passwd no-stats
patch@dynamo:/tmp/user/1000/testing$ ls -la
total 16
drwxr-xr-x 2 patch patch 4096 Aug 31 11:30 .
drwx------ 8 patch patch 4096 Aug 31 11:30 ..
lrwxrwxrwx 1 patch patch    6 Aug 31 11:30 no-stats -> passwd
-rw-r--r-- 1 patch patch 2888 Aug 31 10:36 passwd
-rw-r--r-- 1 patch patch  187 Aug 31 10:47 test
patch@dynamo:/tmp/user/1000/testing$ cp -L no-stats stats
patch@dynamo:/tmp/user/1000/testing$ ls -la
total 20
drwxr-xr-x 2 patch patch 4096 Aug 31 11:30 .
drwx------ 8 patch patch 4096 Aug 31 11:30 ..
lrwxrwxrwx 1 patch patch    6 Aug 31 11:30 no-stats -> passwd
-rw-r--r-- 1 patch patch 2888 Aug 31 10:36 passwd
-rw-r--r-- 1 patch patch 2888 Aug 31 11:30 stats

[patch0]

The unmentioned alternative is to still accept no-stats as a setting, but preferring stats if present.

[telyn]

Yeah, understood that. My point was something else, but it doesn't really matter.

I figured that keeping no-stats would be pretty gross though.

[patch0]

I guess there is a race between the cp and truncate commands.. yes.