When a plugin makes use of the 'automatic_updater_disabled' filter, it causes a PHP Warning to be thrown.

[jamesros161]

When using a plugin that makes use of the 'automatic_updater_disabled' filter, the following PHP Warning is thrown:

PHP Warning:  Use of undefined constant AUTOMATIC_UPDATER_DISABLED - assumed 'AUTOMATIC_UPDATER_DISABLED' (this will throw an Error in a future version of PHP) in /var/www/html/wp56tu/wp-admin/update-core.php on line 327
PHP Stack trace:
PHP   1. {main}() /var/www/html/wp56tu/wp-admin/update-core.php:0
PHP   2. core_auto_updates_settings() /var/www/html/wp56tu/wp-admin/update-core.php:1008

Steps to Replicate.

1. Install a plugin that makes use of the 'automatic_updater_disabled' filter, or add the following line to your theme's functions.php:

   add_filter( 'automatic_updater_disabled', '__return_false' );
   

2. Set WP_DEBUG to true.
3. Go to the Dashboard > Updates page, and see the error presented.

Suggested Resolution:

If the following changes are made to lines 324-337, this issue should be resolved.

Change This:

if ( defined( 'AUTOMATIC_UPDATER_DISABLED' )
                || has_filter( 'automatic_updater_disabled' )
        ) {
                if ( true === AUTOMATIC_UPDATER_DISABLED
                        /** This filter is documented in wp-admin/includes/class-wp-automatic-updater.php */
                        || true === apply_filters( 'automatic_updater_disabled', false )
                ) {
                        $upgrade_dev   = false;
                        $upgrade_minor = false;
                        $upgrade_major = false;
                }
                // The UI is overridden by the AUTOMATIC_UPDATER_DISABLED constant.
                $can_set_update_option = false;
        }

to:

if ( defined( 'AUTOMATIC_UPDATER_DISABLED' ) && true === AUTOMATIC_UPDATER_DISABLED
                || has_filter( 'automatic_updater_disabled' ) && true === apply_filters( 'automatic_updater_disabled', false )
        ) {
                $upgrade_dev   = false;
                $upgrade_minor = false;
                $upgrade_major = false;
        } else if ( defined( 'AUTOMATIC_UPDATER_DISABLED' ) || has_filter( 'automatic_updater_disabled' ) ) {
                // The UI is overridden by the AUTOMATIC_UPDATER_DISABLED constant.
                $can_set_update_option = false;
        }

[jamesros161]

Screenshot of issue

[pbiron]

@jamesros161 Welcome to Trac and thank you very much for the report.

Are you able to create a patch with your proposed changes? You can find info on how to do that at ​https://make.wordpress.org/core/handbook/tutorials/working-with-patches.

[audrasjb]

Hi, welcome to WordPress Trac and thank you for reporting this,

I am able to reproduce the issue using the steps provided by @jamesros161.

But at a glance, I think the following logic should be enough:

if ( defined( 'AUTOMATIC_UPDATER_DISABLED' ) && true === AUTOMATIC_UPDATER_DISABLED || has_filter( 'automatic_updater_disabled' ) && true === apply_filters( 'automatic_updater_disabled', false ) ) {
    $upgrade_dev   = false;
    $upgrade_minor = false;
    $upgrade_major = false;

    // The UI is overridden using a constant or a filter.
    $can_set_update_option = false;
}

I should be able to test it deeper later today.

[jamesros161]

Replying to audrasjb:

Hi, welcome to WordPress Trac and thank you for reporting this,

I am able to reproduce the issue using the steps provided by @jamesros161.

But at a glance, I think the following logic should be enough:

if ( defined( 'AUTOMATIC_UPDATER_DISABLED' ) && true === AUTOMATIC_UPDATER_DISABLED || has_filter( 'automatic_updater_disabled' ) && true === apply_filters( 'automatic_updater_disabled', false ) ) {
    $upgrade_dev   = false;
    $upgrade_minor = false;
    $upgrade_major = false;

    // The UI is overridden using a constant or a filter.
    $can_set_update_option = false;
}

I should be able to test it deeper later today.

The issue is, that in the original code, the $can_set_update_option is executed regardless of whether or not AUTOMATIC_UPDATER_DISABLED or apply_filters( 'automatic_updater_disabled') are true, as long as the constant is defined, and the filter exists.

[audrasjb]

OK, got it. Thanks for clarifying, you're right.

[jamesros161]

Proposed patch

[audrasjb]

Hey thanks for the patch @jamesros161.
Would be nice to replace else if with elseif for consistency with WordPress Sourcecode :)

[pbiron]

Thanx for the patch James.

Taking a quick look, I think the original logic is incorrect regarding setting $can_set_update_option = false; when AUTOMATIC_UPDATER_DISABLED is defined but false.

That is, if I set that constant to false (and don't return true from automatic_updater_disabled) I think the enable/disable action links should be displayed.

So, the proposal by @audrasjb might be a better fix. @helen what do you think?

[azaozz]

Replying to pbiron:

Taking a quick look, I think the original logic is incorrect regarding setting $can_set_update_option = false; when AUTOMATIC_UPDATER_DISABLED is defined but false.

Right. The logic there is two-fold (afaik):

1. If AUTOMATIC_UPDATER_DISABLED is defined OR if the 'automatic_updater_disabled' filter is used, the UI to set user preference should be disabled.
2. If true === AUTOMATIC_UPDATER_DISABLED OR true === apply_filters( 'automatic_updater_disabled', false ), set $upgrade_dev = false; $upgrade_minor = false; $upgrade_major = false;.

[azaozz]

Replying to azaozz:

Replying to pbiron:

Taking a quick look, I think the original logic is incorrect regarding setting $can_set_update_option = false; when AUTOMATIC_UPDATER_DISABLED is defined but false.

Right. The logic there is two-fold (afaik):

Hmm, actually no, that's incorrect (double negatives, lol).

The logic is/should be that if AUTOMATIC_UPDATER_DISABLED is set and false, it is equivalent to not being set, i.e. the updater is not disabled. Same for the 'automatic_updater_disabled' filter. So it should be checking only if any of the two is true (updater is disabled). Also the filter overrides the constant.

In addition, seems that wp_is_file_mod_allowed( 'automatic_updater' ) is not checked.

Thinking perhaps this should use WP_Automatic_Updater::is_disabled() instead of trying to duplicate the logic? It already uses the WP_Automatic_Updater to check for version control.

[azaozz]

In 51827.2.patch:

• Replace the conditionals that check AUTOMATIC_UPDATER_DISABLED and automatic_updater_disabled filter in update-core.php with a call to WP_Automatic_Updater::is_disabled().
• Also check wp_is_file_mod_allowed( 'automatic_updater' ) when disabling the UI.

Please test! :)

[pbiron]

Replying to azaozz:

The logic is/should be that if AUTOMATIC_UPDATER_DISABLED is set and false, it is equivalent to not being set

That's what I meant by "the original logic is incorrect" because it does not do act like that.

Thinking perhaps this should use WP_Automatic_Updater::is_disabled() instead of trying to duplicate the logic? It already uses the WP_Automatic_Updater to check for version control.

YES: calling is_disabled() instead of (incorrectly) recreating most of it's logic is a great idea.

51827.2.patch​ tests correctly for me.

Note to others testing, if you have wp-admin/update-core.php loaded in your browser and you then make changes that cause wp_is_file_mod_allowed( 'automatic_updater' ) to return false and then reload in your browser you'll get a Sorry, you are not allowed to access this page. error from WP...that is to be expected, since the Updates screen is not allowed when file mods are disallowed :-)

[azaozz]

Replying to pbiron:

51827.2.patch​ tests correctly for me.

Thanks!

you'll get a Sorry, you are not allowed to access this page. error from WP...that is to be expected, since the Updates screen is not allowed when file mods are disallowed :-)

Right. Checking wp_is_file_mod_allowed() there is mostly for completeness. In theory a plugin would be able to use the file_mod_allowed filter and allow loading of the Update screen even when DISALLOW_FILE_MODS is defined and true, but disallow the actual update (see the #context param in ​https://developer.wordpress.org/reference/hooks/file_mod_allowed/).

[audrasjb]

51827.2.patch looks good on my side as well, thanks 👍

[azaozz]

In 49677:

Upgrade/Install: Replace the conditionals that check the AUTOMATIC_UPDATER_DISABLED constant and the automatic_updater_disabled filter in update-core.php with a call to WP_Automatic_Updater::is_disabled(). This prevents a PHP warning, fixes the logic, and considers wp_is_file_mod_allowed( 'automatic_updater' ) when determining the UI state.

Props jamesros161, pbiron, audrasjb, azaozz.
Fixes #51827.

[azaozz]

Reopen for the 5.6 branch.

[SergeyBiryukov]

In 49681:

Docs: Update syntax for multi-line comment in core_auto_updates_settings() per the documentation standards.

Follow-up to [49677].

See #51827.

[SergeyBiryukov]

[49677] looks good to backport.

[SergeyBiryukov]

In 49682:

Upgrade/Install: Replace the conditionals that check the AUTOMATIC_UPDATER_DISABLED constant and the automatic_updater_disabled filter in update-core.php with a call to WP_Automatic_Updater::is_disabled().

This prevents a PHP warning, fixes the logic, and considers wp_is_file_mod_allowed( 'automatic_updater' ) when determining the UI state.

Props jamesros161, pbiron, audrasjb, azaozz.
Merges [49677] and [49681] to the 5.6 branch.
Fixes #51827.

[jamesros161]

Nice work @azaozz . Wish I would have caught that in my patch so I could of gotten that 'Core Contributor' badge. lol

[azaozz]

Replying to jamesros161:

Nice work @azaozz

Thanks :)

I could of gotten that 'Core Contributor' badge.

You will, when WP 5.6 is released (I think). You're one of the contributors on this ticket and patch/changeset as mentioned in the "Props" on the commit messages.

Thanks again for opening this ticket, the nice, detailed description, and for the initial patch.