Skip to content

Set update plugins transient to object instead of null#622

Merged
Crabcyborg merged 1 commit into
masterfrom
set_update_plugins_transient_to_object_instead_of_null
Nov 12, 2021
Merged

Set update plugins transient to object instead of null#622
Crabcyborg merged 1 commit into
masterfrom
set_update_plugins_transient_to_object_instead_of_null

Conversation

@Crabcyborg
Copy link
Copy Markdown
Contributor

@Crabcyborg Crabcyborg commented Nov 12, 2021

I don't really know how to run this directly, this is pretty deep in EDD code, but I believe this shouldn't have any issues.

Related to https://secure.helpscout.net/conversation/1694625984/85305?folderId=4324209

If I have woocommerce installed and I run the following snippet:

set_site_transient( 'update_plugins', null );

PHP Fatal error: Uncaught Error: Attempt to assign property "translations" on null in /var/www/src/wp-content/plugins/woocommerce/includes/admin/helper/class-wc-helper-updater.php:81
Stack trace:
#0 /var/www/src/wp-includes/class-wp-hook.php(305): WC_Helper_Updater::transient_update_plugins(NULL)
#1 /var/www/src/wp-includes/plugin.php(189): WP_Hook->apply_filters(NULL, Array)
#2 /var/www/src/wp-includes/option.php(1977): apply_filters('pre_set_site_tr...', NULL, 'update_plugins')
#3 /var/www/src/wp-content/plugins/code-snippets/php/snippet-ops.php(446) : eval()'d code(12): set_site_transient('update_plugins', NULL)
#4 /var/www/src/wp-content/plugins/code-snippets/php/snippet-ops.php(446): eval()
#5 /var/www/src/wp-content/plugins/code-snippets/php/snippet-ops.php(534): execute_snippet('/*\r\n$updates ...', 185)
#6 /var/www/src/wp-includes/class-wp-hook.php(03): execute_active_snippets('')
#7 /var/www/src/wp-includes/class-wp-hook.php(327): WP_Hook->apply_filters(NULL, Array)
#8 /var/www/src/wp-includes/plugin.php(470): WP_Hook->do_action(Array)
#9 /var/www/src/wp-settings.php(441): do_action('plugins_loaded')
#10 /var/www/wp-config.php(73): require_once('/var/www/src/wp...')
#11 /var/www/src/wp-load.php(55): require_once('/var/www/wp-con...')
#12 /var/www/src/wp-admin/admin.php(34): require_once('/var/www/src/wp...')
#13 {main}
thrown in /var/www/src/wp-content/plugins/woocommerce/includes/admin/helper/class-wc-helper-updater.php on line 81
[12-Nov-2021 14:28:03 UTC] PHP Notice: is_embed was called incorrectly. Conditional query tags do not work before the query is run. Before then, they always return false. Please see Debugging in WordPress for more information. (This message was added in version 3.1.0.) in /var/www/src/wp-includes/functions.php on line 5663
[12-Nov-2021 14:28:03 UTC] PHP Notice: is_search was called incorrectly. Conditional query tags do not work before the query is run. Before then, they always return false. Please see Debugging in WordPress for more information. (This message was added in version 3.1.0.) in /var/www/src/wp-includes/functions.php on line 5663

However this snippet:

$updates               = new stdClass();
$updates->last_checked = 0;
$updates->response     = array();
$updates->translations = array();
$updates->no_update    = array();
$updates->checked      = array();
set_site_transient( 'update_plugins', $updates );

Runs just fine.

In woocommerce, in WC_Helper_Updater::transient_update_plugins it has this line:

$transient->translations = array_merge( isset( $transient->translations ) ? $transient->translations : array(), $translations );

There are also a few lines above that could be problematic as well:

if ( version_compare( $plugin['Version'], $data['version'], '<' ) ) {
	$transient->response[ $filename ] = (object) $item;
	unset( $transient->no_update[ $filename ] );
} else {
	$transient->no_update[ $filename ] = (object) $item;
	unset( $transient->response[ $filename ] );
}

It really expects that the transient is an object. This is really more of a bug with woocommerce expecting data that could definitely be something else, but it's something we can fix on our end.

I borrowed most of this code from WordPress, see https://developer.wordpress.org/reference/functions/wp_update_plugins/

@Crabcyborg Crabcyborg added this to the 5.0.13 milestone Nov 12, 2021
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 12, 2021

Codecov Report

Merging #622 (5692848) into master (be4a777) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #622      +/-   ##
============================================
- Coverage     27.40%   27.40%   -0.01%     
  Complexity     6292     6292              
============================================
  Files            90       90              
  Lines         17364    17370       +6     
============================================
+ Hits           4759     4760       +1     
- Misses        12605    12610       +5     
Impacted Files Coverage Δ
classes/models/FrmAddon.php 5.91% <0.00%> (-0.11%) ⬇️
classes/helpers/FrmAppHelper.php 29.74% <0.00%> (+0.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be4a777...5692848. Read the comment docs.

Copy link
Copy Markdown
Contributor

@stephywells stephywells left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find! Weird that WooCommerce has this issue when it would conflict with so many plugins. I'm seeing the "null" all over the place when I do a Google search...

@Crabcyborg Crabcyborg merged commit ca04298 into master Nov 12, 2021
@Crabcyborg Crabcyborg deleted the set_update_plugins_transient_to_object_instead_of_null branch November 12, 2021 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants