Merge lp:~louis/ubuntu/trusty/clamav/lp799623_fix_logrotate into lp:ubuntu/trusty/clamav

[Louis Bouchard]

[Martin Pitt (pitti)]

This looks like it could easily introduce a regression, especially on config files which look "normal", i. e. only use one space to separate words. Are you sure that instead of

  value=`grep ^$variable[[:space:]] $CLAMAVCONF | head -n1 | awk '{print $2}'`

you don't mean

  value=`grep ^$variable[[:space:]]* $CLAMAVCONF | head -n1 | awk '{print $2}'`

?

review: Needs Fixing

[Martin Pitt (pitti)]

Oh, the $CLAMAVCONF is not part of the search string indeed. This really ought to be quoted, like

value=`grep "^$variable " $CLAMAVCONF | head -n1 | awk '{print $2}'`

[Martin Pitt (pitti)]

review: Approve

[Colin Watson (cjwatson)]

On Wed, Feb 12, 2014 at 03:12:52PM -0000, Martin Pitt wrote:
> value=`grep ^$variable[[:space:]]* $CLAMAVCONF | head -n1 | awk '{print $2}'`

Also surely that regex should be double-quoted? Otherwise that might
happen to expand to something in the current working directory and weird
stuff would ensue.

[Louis Bouchard (louis)]

Makes perfect sense to me. I was too trusty of the patch proposed in the bug.

I will rework according to the suggestions

[Martin Pitt (pitti)]

Uploaded with

- value=`grep ^$variable $CLAMAVCONF | head -n1 | awk '{print $2}'`
+ value=`grep "^$variable[[:space:]]" $CLAMAVCONF | head -n1 | awk '{print $2}'`

now. I didn't actually use UDD, that branch takes abysmally long to check out (I killed it after 10 mins or so).

Thanks!

[Louis Bouchard (louis)]

Before I go any further, the addition of [[:space:]] was to discriminate on word boundary. When the CLAMAVCONF file has entries like this :

StreamMaxLength 25M
LogFileMaxSize 0
LogFile /var/log/clamav/clamav.log

the grep could not discriminate b/w LogFileMaxSize & LogFile, so the LogFile variable ended up with 0.

Revisiting the expression, I think that using \b would be more appropriate to discriminate on word boudaries so the expression would be :

value=`grep "^$variable\b" $CLAMAVCONF | head -n1 | awk '{print $2}'`

Unless you see an issue with this, I will go ahead with that fix, since the [[:space:]] was not intended to identify single space, but word boundaries

=== modified file 'debian/changelog'
--- debian/changelog	2014-02-03 17:04:49 +0000
+++ debian/changelog	2014-02-10 17:01:04 +0000
@@ -1,3 +1,11 @@
+clamav (0.98.1+dfsg-1ubuntu5) trusty; urgency=low
+
+  * debian/common_functions
+    - Fix slurp_config() that is sometimes not discriminative
+    enough which leads to invalid configuration files (LP: #799623)
+
+ -- Louis Bouchard <[e-mail]>  Mon, 10 Feb 2014 17:56:52 +0100
+
 clamav (0.98.1+dfsg-1ubuntu4) trusty; urgency=medium
 
   * Stop using a cargo-culted syscall table and trust the glibc headers.

=== modified file 'debian/common_functions'
--- debian/common_functions	2012-06-19 21:39:28 +0000
+++ debian/common_functions	2014-02-10 17:01:04 +0000
@@ -147,7 +147,7 @@
         value=`grep ^$variable $CLAMAVCONF | head -n1 | sed -e s/$variable\ //`
         ;;
         *)
-        value=`grep ^$variable $CLAMAVCONF | head -n1 | awk '{print $2}'`
+        value=`grep ^$variable[[:space:]] $CLAMAVCONF | head -n1 | awk '{print $2}'`
         ;;
       esac
       if [ -z "$value" ]; then