Opened 8 years ago

Last modified 8 years ago

#565 testing defect (fixed)

MySQL and SA: tokens don't update correctly in some cases

Reported by: rjl@… Owned by: rjl@…
Priority: normal Milestone: 1.0.3
Component: SQL scripts Version: 1.0.2
Severity: normal Keywords: mysql spamassassin bayes awl
Cc:

Description

The SpamAssassin developers report in SA:6624 and MySQL:46675 that a subtle bug in MySQL 5.1.20 and newer causes it to incorrectly report the number of rows returned from INSERT ... ON DUPLICATE KEY operations.

With ON DUPLICATE KEY UPDATE, the affected-rows value per row is 1 if
the row is inserted as a new row and 2 if an existing row is updated.

Due to a MySQL server bug a value of 3 can be seen.
See: http://bugs.mysql.com/bug.php?id=46675

When executing the INSERT ... ON DUPLICATE KEY UPDATE statement
and checking the rows return count:

mysql_client_found_rows = 0: The second INSERT returns a row count
                             of 2 in all MySQL versions.

mysql_client_found_rows = 1: The second INSERT returns this row count:

   Before MySQL 5.1.20: 2
   MySQL 5.1.20: undef on Mac OS X, 139775481 on Linux (garbage?)
   MySQL 5.1.21 and up: 3

The Bayes database uses this mechanism extensively to update its tokens, so as they explain:

The effect of the bug with SpamAssassin is that tokens are only able
to be inserted once, but their counts cannot increase, leading to
terrible bayes results if the bug is not noticed.

They have committed a patch for BayesStore/MySQL.pm which detects and works around the MySQL bug, but since this was detected hours after the 3.3.2 release it will not be visible to most users until 3.3.3 at some point in the future.

We can use configtest.pl to warn users of MySQL >= 5.1.20 about the bug and point them to this patch until SA 3.3.3 is released. There is also another workaround being suggested on SpamAssassin-Dev as a temporary solution, which is to just add a connect-time parameter to the bayes_sql_dsn in local.cf, i.e.

bayes_sql_dsn DBI:mysql:maia;host=127.0.0.1;port=3306;mysql_client_found_rows=0

This apparently "avoids this particular count bug problem (but may cause others, as mentioned in the MySQL bug report referenced above and indicated in the DBD::mysql documentation)."

Attachments (1)

mysql-bug-46675.patch (3.2 KB) - added by rjl@… 8 years ago.
Patch for BayesStore?/MySQL.pm

Download all attachments as: .zip

Change History (3)

comment:1 Changed 8 years ago by rjl@…

  • Owner changed from rjl to rjl@…
  • Status changed from new to accepted

In [1557] I've committed the changes to configtest.pl to direct people to this ticket if they're running vulnerable versions of MySQL. If you arrived here as a result of such a warning, the fix, such as it is for the moment, is to apply the patch I linked to earlier in this ticket. The only affected file is BayesStore/MySQL.pm, which you should be able to find at /usr/{lib,share}/{perl,perl5}/Mail/SpamAssassin/BayesStore/MySQL.pm. If you're having trouble finding it, try:

find /usr -name MySQL.pm | grep BayesStore

Download the patch to the directory where your MySQL.pm file lives, name it something like mysql-bugfix.patch and then do the following:

patch -p0 MySQL.pm < mysql-bugfix.patch

You should see something like this:

patching file MySQL.pm

If that's all you see, then the patch was successful; restart maiad (amavisd-maia in older releases) and you're done. If you see messages about errors during the patching process (e.g. "hunk failed", etc.) then you likely have an outdated version of SpamAssassin (i.e. pre-3.3) and should be upgrading that first.

Given how vital this patch is, we might consider going a step further to have configtest.pl detect the presence of patched/unpatched MySQL.pm and performing the patching automatically if necessary. I'll leave this ticket open in the meantime.

Changed 8 years ago by rjl@…

Patch for BayesStore?/MySQL.pm

comment:2 Changed 8 years ago by rjl@…

  • Resolution set to fixed
  • Status changed from accepted to testing

In [1558] I added an automated patching facility to configtest.pl which uses an arbitrary number of conditions to determine whether a patch is needed, then tests the contents of the target file to determine whether it has already been patched. If patching is necessary, it tries to find the patch file in the current directory first, then along a path that includes %%_MAIA_DOCDIR_%%/patches.

This should solve the problems related to this ticket. I've removed the warning message, now that the script can detect and handle the problem on its own.

Note that the mysql-bug-46675.patch file the script is looking for will be shipped with 1.0.3, but in the meantime people will need to get it from this ticket.

Note: See TracTickets for help on using tickets.