Make WordPress Core

Opened 3 years ago

Last modified 7 weeks ago

#52723 new defect (bug)

Admin options.php default value to NULL for option_value may lead to MySQL Integrity constraint violation error, potential other bugs

Reported by: pinoceniccola's profile pinoceniccola Owned by:
Milestone: 6.6 Priority: normal
Severity: normal Version: 2.7
Component: Options, Meta APIs Keywords: has-patch needs-unit-tests needs-testing
Focuses: administration Cc:

Description

It looks like wp-admin/options.php set a null value by default for any unchecked option:

https://core.trac.wordpress.org/browser/trunk/src/wp-admin/options.php#L306

Now, this leads to execute queries like this by update_option:

UPDATE wp_options SET option_value = NULL WHERE option_name = 'default_pingback_flag'

Which is wrong, given the schema explicitly set option_value to NOT NULL:

https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/schema.php#L144

This would trigger an integrity constraint violation error by MySQL when in (default) strict mode:

Error! SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'option_value' cannot be null

To get around this (and for other reasons too, I presume), WordPress currently tries to disable any MySQL strict mode in the $wpdb class, with the effect that MySQL silently "fix" the error itself:

https://core.trac.wordpress.org/browser/trunk/src/wp-includes/wp-db.php#L567

https://core.trac.wordpress.org/browser/trunk/src/wp-includes/wp-db.php#L826

But not every environment support this, so there are people out there who cannot save options and they are confused about the reason why, for example:

https://www.reddit.com/r/Wordpress/comments/l61rvs/cannot_disable_avatars/
https://wordpress.org/support/topic/discussion-comment-settings-saved-changes-are-not-taking-effect-at-all/
https://wordpress.org/support/topic/wordpress-database-error-column-option_value-cannot-be-null/

A simple solution would be to set a different default value (0 or even an empty string) in wp-admin/options.php and, better yet, cast any NULL value to the same different default value in both update_option and add_option.

Please note that, without a fix, this bug may also lead to other nasty side effects.

As a quick fix/test, I successful got around this with this simple filter:

<?php
add_action( 'init', 'p_options_fix' );

function p_options_fix() {

        add_filter( 'pre_update_option', 'p_options_fix_not_null', 10, 3 );

        function p_options_fix_not_null( $value, $option, $old_value ) {

                // The fix: cast NULL values to 0
                $value = ( is_null($value) ) ? 0 : $value;

                return $value;
        }
}


But I think this is something that should be really fixed in the core.

Thank you.

Change History (3)

This ticket was mentioned in PR #1074 on WordPress/wordpress-develop by pinoceniccola.


3 years ago
#1

  • Keywords has-patch added

Replaces null value as default admin option value to avoid bugs and side-effects. See ticket for details.

Trac ticket: https://core.trac.wordpress.org/ticket/52723

#2 @xmic
3 months ago

  • Component changed from General to Options, Meta APIs
  • Version set to trunk

A very warm welcome @pinoceniccola ! 👋

We've also encountered this issue on HyperDB since it didn't seem to save NULL option values (see the HyperDB 1.4 changelog).

Therefore, we have followed the solution to "cast NULL to zero" and it worked as intended.

<?php

/**
 * Casts NULL values to 0 for HyperDB.
 *
 * HyperDB currently doesn't support NULL values, so this function is used to cast any NULL values to 0.
 *
 * @param  mixed  $value     The value to be checked and possibly cast to 0.
 * @param  string $option    The name of the option being updated.
 * @param  mixed  $old_value The old value of the option.
 * @return mixed  The new, possibly cast, value.
 *
 * @see https://core.trac.wordpress.org/ticket/52723
 * @see HyperDB Changelog 1.4 here: https://github.com/Automattic/HyperDB
 * 
 * @since HyperDB 1.9
 */
function cast_null_option_to_zero( $value, $option, $old_value ) {

        // Bail early if HyperDB isn't installed.
        if ( ! class_exists( 'hyperdb' ) ) {
                return;
        }

        // Cast NULL values to 0.
        $value = ( is_null( $value ) ) ? 0 : $value;

        return $value;
}

add_filter( 'pre_update_option', 'cast_null_option_to_zero', 10, 3 );

Kind regards,
Michael

#3 @swissspidy
7 weeks ago

  • Keywords needs-unit-tests needs-testing added
  • Milestone changed from Awaiting Review to 6.6
  • Version changed from trunk to 2.7

Introduced in [9506].

Casting to 0 (zero) sounds odd to me, an empty string seems to make more sense.

Note: See TracTickets for help on using tickets.