Fatal Errors under PHP 8.1

  • Resolved mpemburn

    (@mpemburn)


    1 year, 2 months ago

    While preparing our websites for the upgrade to PHP 8.x, I discovered a handful of errors in the 3.6.1 version of the Authorizer plugin–something we depend on heavily.

    The first of these was:

    PHP Fatal error: Uncaught TypeError: Unsupported operand types: string * int in /dom25405/wp-content/plugins/authorizer/src/authorizer/class-authentication.php:86

    I was able to repair this and the related code with a series of casts:

    $reset_duration = (int)$lockouts['reset_duration'] * 60; // minutes to seconds.
$num_attempts_long_lockout = (int)$lockouts['attempts_1'] + (int)$lockouts['attempts_2'];
$num_attempts_short_lockout = (int)$lockouts['attempts_1'];
$seconds_remaining_long_lockout = (int)$lockouts['duration_2'] * 60 - $time_since_last_fail;
$seconds_remaining_short_lockout = (int)$lockouts['duration_1'] * 60 - $time_since_last_fail;

    The next error was:

    PHP Fatal error:  Uncaught TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in /dom25405/wp-content/plugins/authorizer/src/authorizer/class-wp-plugin-authorizer.php:222

    The fix for this is a test for is_array():

    'role' => is_array( $user->roles ) && count( $user->roles ) > 0 ? $user->roles[0] : 'administrator',

    The fixes to the plugin appear to be working in our staging environment, but I’m hoping you can have an updated version ready soon. Thanks!

    Regards,

    Mark Pemburn
    Web Application Administrator
    Clark University

    • This topic was modified 1 year, 2 months ago by mpemburn.
Viewing 15 replies - 1 through 15 (of 15 total)
  • Plugin Author Paul Ryan

    (@figureone)

    1 year, 2 months ago

    Thanks! We’ll get these fixed this week along with some more general php 8.x testing. Also planning to upgrade ourselves in March, so we appreciate you flagging this.

    Plugin Author Paul Ryan

    (@figureone)

    1 year, 2 months ago

    Question for you, I’m testing on PHP 8.2 and I can’t get the first error you mentioned to trigger:

    PHP Fatal error: Uncaught TypeError: Unsupported operand types: string * int in /dom25405/wp-content/plugins/authorizer/src/authorizer/class-authentication.php:86

    What is the value of $lockouts['reset_duration'] that’s triggering this? (In Authorizer Settings it’s the value in: “Reset the delays after X minutes with no invalid attempts”)

    If that value is a number stored as a string, PHP should silently cast it to an int. It should only fail if the value is a string that doesn’t cast (like an empty string, or alphabetic characters). I’m unable to get one of those values because the sanitizer will replace invalid strings with (int) 0 when saving:

    https://github.com/uhm-coe/authorizer/blob/master/src/authorizer/class-options.php#L829-L836

    Plugin Author Paul Ryan

    (@figureone)

    1 year, 2 months ago

    Regarding the next error (user roles is not an array), that one is curious since the core WP_User object defines that attribute as an array:

    https://github.com/WordPress/WordPress/blob/master/wp-includes/class-wp-user.php#L74-L80

    I wonder if maybe you have a 3rd party plugin that is altering the core WP_User attributes?

    Regardless, we will add sanity checks as suggested.

    Thread Starter mpemburn

    (@mpemburn)

    1 year, 2 months ago

    Hi Paul,
    I’m afraid that I can only report what I was getting from the error logs. Our system logins are managed by ADFS and CAS, and I’m not as conversant in the complexities of this as I might be.

    All I can say is that given how fussy PHP has become in version 8–and will only become more so in future versions–it’s not safe to make assumptions about data types anymore. Clark University has vast multisite systems with numerous plugins, and it has been a real headache ferreting out the incompatible code in preparation for this version upgrade.

    –Mark

    Thread Starter mpemburn

    (@mpemburn)

    1 year, 2 months ago

    I just received this from our hosting provider, who is trying to track down the reason for timeouts we’ve been experiencing of late. I don’t know if this is useful to you:
    “We were alerted a few minutes ago to a slow login on wp-login.php that took nearly 30 seconds. I was able to find a trace in New Relic. It looks like the bulk of the slowness is coming from the Authorizer plugin performing update checks or ‘set_default_options’ calls.”

    Plugin Author Paul Ryan

    (@figureone)

    1 year, 2 months ago

    No worries, we’re just trying to reproduce the issue so we can confirm and then see if there are other spots in the codebase that need updating also.

    Also there was another post on multisite performance so we’ll be trying to address that in the next update too. Seems like your timeout issue is related if you have a very large multisite:

    https://wordpress.org/support/topic/lots-of-queries-from-class-updates-php-when-using-multisite/

    Thread Starter mpemburn

    (@mpemburn)

    1 year, 2 months ago

    Thanks Paul. That other ticket is from a coworker. We’re trying to nail down a bunch of issues before our hosting provider throws us off the PHP 8 cliff.

    Do you have a timeline for the next update?

    Plugin Author Paul Ryan

    (@figureone)

    1 year, 2 months ago

    We’ve got a backlog of smaller tickets we’re working on for the next release (and of course everyone is crazy busy).

    But we can push out a small release with just these fixes if your upgrade is looming close, just let us know!

    Thread Starter mpemburn

    (@mpemburn)

    1 year, 2 months ago

    Hi Paul,

    If you can make that small release, it’d be fantastic!

    Beyond the items I mention here, the bug that’s described here: https://wordpress.org/support/topic/lots-of-queries-from-class-updates-php-when-using-multisite/ has caused some real headaches. I had our hosting provider research an issue on particular subsite, and they found that Authorizer\Updates->auth_update_check()
    took over 27 seconds to complete.

    -Mark

    Plugin Author Paul Ryan

    (@figureone)

    1 year, 1 month ago

    Aloha, version 3.6.2 is released with these changes, let us know if you run into any issues!

    Thread Starter mpemburn

    (@mpemburn)

    1 year, 1 month ago

    Thanks Paul–it works like a champ!

    Thread Starter mpemburn

    (@mpemburn)

    1 year, 1 month ago

    Ugh, sorry… I spoke too soon. It seems that my original test page was not sufficient to vet the problem. Other pages on my staging site are now doing the same thing: taking ~30 seconds to load (or timing out before they do) because they are writing updates to each of nearly 200 wp_{n}_options table on every page load. This is what I did to circumvent this: 

    $network_active = is_plugin_active_for_network('authorizer');

if ( is_multisite()  && ! $network_active) {
    $auth_version = get_blog_option( get_network()->blog_id, 'auth_version' );
} else {
    $auth_version = get_option( 'auth_version' );
}

    and:

    if ( $needs_updating ) {
    if ( is_multisite() && ! $network_active) {
        // phpcs:ignore WordPress.WP.DeprecatedFunctions.wp_get_sitesFound
        $sites = function_exists( 'get_sites' ) ? get_sites() : wp_get_sites( array( 'limit' => PHP_INT_MAX ) );
        foreach ( $sites as $site ) {
            $blog_id = function_exists( 'get_sites' ) ? $site->blog_id : $site['blog_id'];
            update_blog_option( get_network()->blog_id, 'auth_version', $auth_version );
        }
    } else {
        update_option( 'auth_version', $auth_version );
    }
}

    Unless there is some compelling reason not to do this, please incorporate these changes into your next release.

    –Mark

    Plugin Author Paul Ryan

    (@figureone)

    1 year, 1 month ago

    Are you sure you’re running the latest version? Your 2nd code sample includes the loop over all subsites to update the version string, but that doesn’t exist anymore: https://github.com/uhm-coe/authorizer/blob/master/src/authorizer/class-updates.php#L548-L555

    The last database migration in that class-updates.php file is actually the deleting of all the auth_version options in each subsite (leaving only the single one on the network wp_options table). So it will take some time to complete. That’s kind of the problem here: some plugin updates will include database migrations that need to update every subsite. Is it possible for you to increase the max_execution_time in PHP on your server? If you are experiencing timeouts, then the version number is not getting updated (it’s the last step of the process), so the migrations are probably re-running because the version number is still out of date. You can check your wp_{id}_options tables for the auth_version option, it should be 20230222 (Feb 22, 2023). If the version number is up to date, then nothing in that file runs at all, so you shouldn’t be seeing any performance issues.

    Lastly, I’m hesitating to make your change because the logic doesn’t make sense to me. You’re introducing the concept of whether Authorizer is network activated or not, but then choosing to look up the auth_version from the current blog ID instead of the main site of the network if the plugin is in fact network activated. The update check happens in all contexts so if the current blog is not the main site in the network, it won’t have an auth_version option. Basically we’re just storing a single auth_version in the wp_options table, not in any of the wp_{id}_options tables, so in a network environment we always want to look up the option from the main site in the network, not a subsite, regardless of whether Authorizer is network activated or not.

    Thread Starter mpemburn

    (@mpemburn)

    1 year, 1 month ago

    Paul,

    We do have version 3.6.2 installed now. What I reported above was an issue yesterday, but has not been a problem since. I can see that it might have to do the sweep through the many, many subsites once, but apparently it’s settled down now.

    The only question now is: will it do this the next time there’s an update to Authorizer? If so, we should make sure to access the problem subsites immediately after an update.

    Thanks,

    Mark

    • This reply was modified 1 year, 1 month ago by mpemburn.
    Plugin Author Paul Ryan

    (@figureone)

    1 year, 1 month ago

    The latest migration we just added to class-updates.php was to remove all the redundant auth_settings options in the database, so that should help future performance whenever database migrations are necessary. We also added a performance tweak so auth_multisite_settings isn’t updated redundantly for each subsite in the network when one of these migrations is running.

    Generally though there are very few database migrations (there’s been 19 since 2014, you can browse them in class-updates.php): https://github.com/uhm-coe/authorizer/blob/master/src/authorizer/class-updates.php

    If there is one, it is intended to run immediately after updating the plugin at /wp-admin/network/update-core.php, and only once. So if there are any timeout issues, your administrator performing the updates should see evidence of it failing. It’s also possible that your particular upgrade methods are interfering here. For example, if you load a bunch of tabs of all subsites immediately after updating, each of those requests may try to perform the database migration because the auth_version string hasn’t been updated yet, signaling that the migration has been completed. Or if you update via wp-cli, maybe the migration doesn’t run until another user visits the site (we haven’t tested that scenario). In those cases, minimizing traffic during the update is advisable.

    We could also consider only queueing the migrations when an administrator is making a request, but that seems like a bad idea because we want to ensure these migrations run as soon as possible since authentication is such an important part of security and we can’t predict what future migrations might cover.

Viewing 15 replies - 1 through 15 (of 15 total)
  • The topic ‘Fatal Errors under PHP 8.1’ is closed to new replies.