Opened 4 years ago
Last modified 5 months 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 | Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 2.7 |
Component: | Options, Meta APIs | Keywords: | has-patch needs-unit-tests early |
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
SEToption_value
= NULL WHEREoption_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 (9)
This ticket was mentioned in PR #1074 on WordPress/wordpress-develop by pinoceniccola.
4 years ago
#1
- Keywords has-patch added
#2
@
10 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
@
9 months 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.
This ticket was mentioned in PR #6704 on WordPress/wordpress-develop by @shailu25.
5 months ago
#4
Trac Ticket: 52723
#6
in reply to:
↑ description
@
5 months ago
Replying to pinoceniccola:
A simple solution would be to set a different default value (
0
or even an empty string) inwp-admin/options.php
and, better yet, cast anyNULL
value to the same different default value in bothupdate_option
andadd_option
.
Hi there, welcome to WordPress Trac! Thanks for the ticket.
As previously noted in comment:6:ticket:57728, it appears that changing this default might have unintended consequences, as some options, e.g. blog_public
, actually rely on null
being passed to update_option()
if the option was not sent to options.php
, see [21849] and [21851] / #16416.
#7
@
5 months ago
Thanks @SergeyBiryukov
We may want to remove the reliance on the null
check for the option/s, and instead use empty()
to check if the value is either null
or an empty string ''
.
That way, it becomes clearer to check for an empty value without reliance on a specific type.
Any thoughts?
<?php $a = empty(null); echo var_export($a); // true $a = empty(''); echo var_export($a); // true
This ticket was mentioned in Slack in #core by oglekler. View the logs.
5 months ago
#9
@
5 months ago
- Keywords early added; needs-testing removed
- Milestone changed from 6.6 to Future Release
This ticket was discussed during bug scrub.
It needs consensus on a way forward first and then likely needs to happen earlier in the cycle.
I am moving it to the Future Release until there will be a clear way forward.
Add props @hellofromTonya and @costdev
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