HyperDB on PHP 8.1+ causes "Duplicate entry 'lastNotificationID' for key 'PRIMARY'" error in some cases (e.g. Wordfence)

My Wordfence scans have been consistently failing recently (yes, I know this isn't the Wordfence repo), and I finally had time to look into why. The error shown in the scan report is this: Duplicate entry 'lastNotificationID' for key 'PRIMARY'.

After some research, I've found a few things that have coalesced to make this a breaking issue, and I think HyperDB might be the thing in need of a fix. I have code that has solved the problem, but I'm really not certain whether this is the best way to fix it long term, so I'm coming here for discussion instead of opening a PR.

It turns out that the original wpdb class has a method called suppress_errors(). Wordfence relies on the error suppression to do a weird thing during their scans. Basically, there's a routine that tries to add some notification data to a Wordfence table (wp_wfconfig in the default case). I don't understand exactly why, but this routine first tries to do a MySQL INSERT with the notification data, and then upon failure (because of a duplicate entry), it falls back to an UPDATE. Here's the code, with the Fatal Error coming from line 462.

(File found at wordfence/lib/wfConfig.php)

The main assumption that this code depends on is that the wpdb->query() method returns false upon error. However, in the case of HyperDB, that's not true. I don't know why HyperDB was built this way, but it is probably not great for the query() method signature to be different from the core wpdb class it's extending. I also don't know why Wordfence was built this way. It seems like it would be better to use the INSERT … ON DUPLICATE KEY UPDATE pattern instead of waiting for the INSERT to fail.

In any case, even the mysqli_query() function that's eventually called by HyperDB says it returns false on error, so you would think the effect of the HyperDB setup would be the same. That brings us to the next piece of the puzzle: the mysqli PHP extension.

From the PHP.net mysqli report_mode documentation:

As of PHP 8.1.0, the default setting is MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT. Previously, it was MYSQLI_REPORT_OFF.

So, since PHP 8.1, when there are MySQL errors during HyperDB's mysqli_query() call, it throws an exception that causes a PHP Fatal Error, instead of returning false, as expected. This actually seems fine as a default behavior, provided we still have a way to suppress errors. But, HyperDB currently has nothing that respects the wpdb->suppress_errors property.

So, the solution that is working for me is to refactor the ex_mysql_query() method of HyperDB to set the driver's error handling settings based on the suppress_errors property:

public function ex_mysql_query( $query, $dbh ) {
    if ( ! $this->use_mysqli ) {
        return mysql_query( $query, $dbh );
    }
    $driver = new mysqli_driver();
    if ($this->suppress_errors) {
        $driver->report_mode = MYSQLI_REPORT_OFF;
    } else {
        $driver->report_mode = MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT;
    }
    return mysqli_query( $dbh, $query );
}

As I mentioned before, I'm not certain this is the best way to handle this change. Perhaps suppress_errors was intended to just suppress the printing of the errors, and not the actual handling of the errors in PHP. But, this change is deployed to our production environment, it does allow the Wordfence code to proceed as normal, and is not causing issues elsewhere, to the best of my knowledge.

There also might be other things needed to fully, properly support the suppress_errors functionality intended by the parent wpdb class. I don't have a lot of background knowledge about the original wpdb class, so this should definitely be looked at by someone with a bit more domain expertise.

I hope this prompts a useful discussion, or at least gives someone else a solution, if they run into this problem. Cheers.

Automattic / HyperDB

HyperDB on PHP 8.1+ causes "Duplicate entry 'lastNotificationID' for key 'PRIMARY'" error in some cases (e.g. Wordfence) #155