Opened 10 years ago
Closed 4 years ago
#29204 closed defect (bug) (fixed)
SimplePie non-static WP_Feed_Cache::create error in fetch_feed()
Reported by: | useStrict | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 5.6 | Priority: | normal |
Severity: | normal | Version: | 3.8 |
Component: | Feeds | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
There seems to be a bug in SimplePie that raises the following notice every time an RSS Feed Dashboard widget is added to the Dashboard.
call_user_func_array() expects parameter 1 to be a valid callback, non-static method WP_Feed_Cache::create() should not be called statically on line 215 in file /path/to/wp-includes/SimplePie/Registry.php
Calling fetch_feed()
is the best way to raise it. I'm using 'blackbox-debug-bar' plugin to capture the notices.
Examples to reproduce the error:
Install and use Dashboard Feed Widget
or follow one of the tuts below.
http://adamscottcreative.com/add-your-own-news-feed-to-wordpress-dashboard/
http://samglover.net/build-your-own-wordpress-dashboard-widget-for-any-rss-feed/
I suspect the reason we don't see it when "WordPress News" is displayed is because the fetch_feed() runs via ajax and does not get captured in the current screen.
Attachments (7)
Change History (65)
#2
@
10 years ago
I'm getting the same error, and adding the static keyword to WP_Feed_Cache::create fixes it.
Going by the DocBlock (and SimplePie_Cache::create, which it extends), it looks like it's supposed to be static anyway.
This ticket was mentioned in Slack in #core by dd32. View the logs.
10 years ago
#4
@
10 years ago
Adding the static
keyword might first seem right, but the upstream method isn't marked as a static, so doing so will cause PHP Fatal error: Cannot make non static method SimplePie_Cache::create() static in class WP_Feed_Cache
.
I guess the "best" way to fix this is to use the Registry system SP has. @rmccue is 29204.diff the correct way there?
#6
follow-up:
↓ 33
@
10 years ago
I would love to see this fixed as currently we are having to mute this error in Sentry every time we add a WP site.
#7
@
9 years ago
- Keywords has-screenshots added
Reproducing the original issue using Bones on local install, PHP7:
After removing $feed->set_cache_class( 'WP_Feed_Cache' );
currently at line 674 and adding SimplePie_Cache::register( 'wp_transient', 'WP_Feed_Cache_Transient' );
$feed->set_cache_location( 'wp_transient://' . $url );
I can confirm that the patch 29204.diff fixes the issue.
#9
@
8 years ago
- Keywords has-patch added
I just created an alternative solution. Could this be something to fix it?
#10
@
8 years ago
I'm guessing this is related to this ticket...
When I attempt to use the RSS Widget with PHP7 I get this deprecation notice on pages where the widget is displayed:
Non-static method WP_Feed_Cache::create() should not be called statically
This ticket was mentioned in Slack in #core by markoheijnen. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by helen. View the logs.
8 years ago
#18
@
8 years ago
I also used to get the error since a long time. As far as I can tell, 29204.2.diff from @markoheijnen fixes it.
#19
@
8 years ago
I'm writing to confirm that 29204.diff
fixes this.
For the sake of reference (search results) it's worth noting a few things.
This change removes WP_Feed_Cache as a dependency of the WordPress core.
The set_cache_class()
method that is being removed by the patch had already been depreciated as of the 2012 version of SimplePie (1.3.1) relied upon by WordPress (up to 4.7 at least).
My look at the code has been cursory, but it appears that WordPress is relying upon legacy features of the library that inherently require a dynamic method to be called as static. As of PHP 5 this has merited an an E_STRICT
warning. PHP 7 escalates this to an E_DEPRECATED
warning, and the current documentation notes that support may be removed in the future.
It also appears that the maintainers of SimplePie have specifically assumed the legacy burden of WordPress support all the way up to version 1.4.3, maintaining backwards compatibility "unless there's a really good reason not to" (mblaney being is the current maintainer), and that this consideration has overshadowed discussion of a 2.0.0 revision of their library.
#21
@
7 years ago
I'm plus oneing this ticket. I'm not supposed to, but tired of seeing this in logs.
#23
@
7 years ago
@SergeyBiryukov Looking at the error details, the solution proposed at https://core.trac.wordpress.org/attachment/ticket/29204/29204.patch seems valid and really simple.
Is there a reason why this is not merged, yet it's pushed back for the past three(!) years?
Array ( [type] => 8192 [message] => Non-static method WP_Feed_Cache::create() should not be called statically [file] => /home/xxxx/public_html/wp-includes/SimplePie/Registry.php [line] => 215 )
#24
@
7 years ago
Is there any work required for this that one could volunteer to do or is it just waiting on WP.org? It's marked as v3.8 and WordPress v4.9 just shipped....
#25
@
7 years ago
+1
I've been staring at
Non-static method WP_Feed_Cache::create() should not be called statically In \wp-includes\SimplePie\Registry.php [line 215]:
on my dashboard for a few years now...
#26
@
7 years ago
Any chance for a fix soon?
This problem happen with the widget with the wordpress events in the dashboard so fills the error log very often.
This ticket was mentioned in Slack in #core by mte90. View the logs.
7 years ago
#30
@
7 years ago
I'm going to offer up a chunk of functionality from one of my own projects to solve this to whoever is actually working on the ticket, as well as any related tickets. It's a trait that you can drop onto any class that does a performant, intelligent substitution for call_user_func_array(). It's unit tested at 100%, it's faster than call_user_func_array by miles, it handles both static and object calls natively without distinction, and it can be dropped onto any class. Whoever is working on this or any related issues is free to use it for this or any related bugs.
The only thing you need to change in this trait is the exception, which should be converted to a native one instead of one of the ones from the rest of the project this is from (line 97). This project has a custom reporting and error handling system that the custom exception ties into, but that is the only external thing it calls, which can be easily substituted. Other than that it has no other dependencies. It solves all static call bugs like this one if used consistently.
This works by avoiding the underlying overhead of call_user_func_array (which is slooooow), by first blocking non-callables with an exception so they do not generate an error, and then doing a set of matching checks that will handle up to 14 parameters and call the class, array, closure, or function natively using the correct syntax for whatever is passed as if it were being natively called. If you are familiar with PHP internals, you may know that call_user_func_array uses a regex detection pattern under the hood and does not perform all that well. This works identically without the overhead of regex. If you pass more than 14 parameters (you shouldn't ever need to do that), it defers to call_user_func_array normally, and otherwise is no different, except for that it will call anything with fewer than that natively with the correct "->" for objects and "::" for static calls, if either of those apply.
Feel free to use it as is, drop it on an existing class, or convert it to a non-object-oriented function. It solves pretty much all static call bugs like this one, and doesn't collide with anything. If anyone doing core development is reading this, feel free to knock out this as well as a large swath of related bugs with this if you like.
This will work on all versions of PHP from 5.4+, and it's MIT licensed, so it should not collide with the WordPress GPLv2 license, as per the GPL licensing compatibility spec.
I stumbled across this issue because I am still seeing this bug on a clean install of WordPress 4.9 with no plugins aside from Jetpack and Askimet, so it's seemingly still a thing.
#31
follow-up:
↓ 32
@
7 years ago
- Keywords needs-refresh removed
- Summary changed from SimplePie error with fetch_feed() to SimplePie non-static WP_Feed_Cache::create error in fetch_feed()
I've uploaded a new patch for this that works against the WP@latest with the currently used version of SimplePie. The workaround was necessitated as SimplePie's own SimplePie_Cache
class does not declare create
as static, and so we can't just redeclare it as such in our derived WP_Feed_Cache
class.
#32
in reply to:
↑ 31
@
7 years ago
Thanks A LOT! It works for me.
Replying to ComputerGuru:
I've uploaded a new patch for this that works against the WP@latest with the currently used version of SimplePie. The workaround was necessitated as SimplePie's own
SimplePie_Cache
class does not declarecreate
as static, and so we can't just redeclare it as such in our derivedWP_Feed_Cache
class.
#33
in reply to:
↑ 6
@
5 years ago
Replying to l3rady:
I would love to see this fixed as currently we are having to mute this error in Sentry every time we add a WP site.
Glad to know I'm not the only one who tells Sentry to ignore this error until it affects 100,000 more users. *laugh*
#34
@
5 years ago
this defect is still alive with the last WordPress version.
I Changed class-wp-feed-cache.php to add static function get_handler
<?php /** * Create a new SimplePie_Cache object * * @param string $location URL location (scheme is used to determine handler) * @param string $filename Unique identifier for cache object * @param string $extension 'spi' or 'spc' * @return SimplePie_Cache_Base Type of object depends on scheme of `$location` */ public static function get_handler($location, $filename, $extension) { return new WP_Feed_Cache_Transient($location, $filename, $extension); } /** * Creates a new SimplePie_Cache object. * * @since 2.8.0 * * @param string $location URL location (scheme is used to determine handler). * @param string $filename Unique identifier for cache object. * @param string $extension 'spi' or 'spc'. * @return WP_Feed_Cache_Transient Feed cache handler object that uses transients. */ public function create( $location, $filename, $extension ) { return self::get_handler($location, $filename, $extension); //return new WP_Feed_Cache_Transient( $location, $filename, $extension ); }
I had also to change SimplePie/Registry.php to stop the use of create function.
I did it today and it works well
#35
@
4 years ago
Yup, this is still present in 5.4.1; my errors logs are chock full of it.
Is there anything someone who doesn't have a ton of experience contributing to core can help to do see progress? Thanks!
#38
@
4 years ago
I'm in the process of testing out WP 5.5 Prior to rolling it out and the first thing we observed within WP admin was this Notice:
Deprecated Non-static method WP_Feed_Cache::create() should not be called statically /path/to/wordpress/wp-includes/SimplePie/Registry.php 214
I'm running WordPress version 5.5 using PHP 7.3.9 with all plugins disabled. The error message appears on the dashboard within the 'WordPress Events and News' box. It doesn't appear that the update to SimplePie has fixed the error.
#41
@
4 years ago
- Keywords dev-feedback added
29204.3.diff combines 29204.diff and 0001-29204-Fix-errors-about-non-static-WP_Feed_Cache-crea.2.patch
This solves the issue with calling a non-static method and setting the cache.
Possibly fixes #51629
This ticket was mentioned in Slack in #core by afragen. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by afragen. View the logs.
4 years ago
#49
@
4 years ago
tested the patch, can confirm it fixes the severe breakage (critical error) in PHP8 as well.
This ticket was mentioned in Slack in #core by afragen. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by sergey. View the logs.
4 years ago
#52
@
4 years ago
- Keywords dev-feedback removed
Ticket has consensus for the implementation. Removing dev-feedback
.
#53
@
4 years ago
@SergeyBiryukov with you latest patch I am seeing the error as follows in the dashboard.
( ! ) SCREAM: Error suppression ignored for ( ! ) Deprecated: Non-static method WP_Feed_Cache::create() should not be called statically in /Users/afragen/Local_Beta_Sites/lightning/app/public/wp-includes/SimplePie/Registry.php on line 214 Call Stack # Time Memory Function Location 1 0.0003 369960 {main}( ) .../admin-ajax.php:0 2 0.8773 7813640 do_action( ) .../admin-ajax.php:184 3 0.8773 7814016 WP_Hook->do_action( ) .../plugin.php:484 4 0.8773 7814016 WP_Hook->apply_filters( ) .../class-wp-hook.php:311 5 0.8773 7815144 wp_ajax_dashboard_widgets( ) .../class-wp-hook.php:287 6 0.8775 7816784 wp_dashboard_primary( ) .../ajax-actions.php:400 7 0.8779 7817912 wp_dashboard_cached_rss_widget( ) .../dashboard.php:1513 8 0.8795 7846488 wp_dashboard_primary_output( ) .../dashboard.php:1135 9 0.8795 7846864 wp_widget_rss_output( ) .../dashboard.php:1529 10 0.8796 7846864 fetch_feed( ) .../widgets.php:1513 11 0.8812 7870000 SimplePie->init( ) .../feed.php:821 12 0.8817 7872296 SimplePie_Registry->call( ) .../class-simplepie.php:1412 Variables in local scope (#12) $class = /Users/afragen/Local_Beta_Sites/lightning/app/public/wp-includes/SimplePie/Registry.php:214:string 'WP_Feed_Cache' (length=13) $method = /Users/afragen/Local_Beta_Sites/lightning/app/public/wp-includes/SimplePie/Registry.php:214:string 'get_handler' (length=11) $parameters = /Users/afragen/Local_Beta_Sites/lightning/app/public/wp-includes/SimplePie/Registry.php:214: array (size=3) 0 => string './cache' (length=7) 1 => string '9bbd59226dc36b9b26cd43f15694c5c3' (length=32) 2 => string 'spc' (length=3) $result = Undefined $type = /Users/afragen/Local_Beta_Sites/lightning/app/public/wp-includes/SimplePie/Registry.php:214:string 'Cache' (length=5)
This might be from cached transient.
#54
follow-up:
↓ 56
@
4 years ago
I have not been able to reproduce the above error after deleting the cached transient and testing the patch.
Looks good.
#55
@
4 years ago
For reference, here is a minimal snippet to reproduce the issue in question:
class My_Test_Class { public function create() { } } call_user_func( array( 'My_Test_Class', 'create' ) );
As noted in comment:19 and seen in test runs, this causes different results in different versions of PHP:
E_STRICT
notice in PHP 5:
Strict Standards: call_user_func() expects parameter 1 to be a valid callback, non-static method My_Test_Class::create() should not be called statically
E_DEPRECATED
notice in PHP 7:
Deprecated: Non-static method My_Test_Class::create() should not be called statically
- A fatal error in PHP 8:
Uncaught TypeError: call_user_func(): Argument #1 ($function) must be a valid callback, non-static method My_Test_Class::create() cannot be called statically
In case of SimplePie, this is a bit tricky to reproduce, because the affected call_user_func_array() call is silenced with the @
operator, but it becomes more obvious with the fatal error in PHP 8.
This appears to be a long-standing issue in SimplePie back from 1.2.x versions.
Some history on the related changes:
- [21644] / #21183 updated SimplePie to 1.3, including marking both
SimplePie_Cache::create()
andWP_Feed_Cache::create()
asstatic
. - [21652] / #21183 briefly used a newer approach to register the cache handler in the same way as suggested in 29204.diff. Per SimplePie documentation, registering a cache handler is indeed the recommended approach for SimplePie 1.3 or later, overriding the
SimplePie_Cache
class is no longer needed. - SimplePie 1.3.1 included a commit that reverted the addition of
static
keyword forSimplePie_Cache::create()
and introduced theSimplePie_Cache::get_handler()
method instead. - [22366] / #22321 updated SimplePie to 1.3.1 in core.
- [22599] / #22321 removed the compatibility code added in [21652], see comment:11:ticket:22321 for the reasoning. The concern was that plugins could include earlier versions, so the legacy approach seemed good enough at the time, but did not account for the silenced notices that became more severe in later PHP versions and now result in a fatal error on PHP 8.
Looking at 29204.3.diff, it combines both approaches suggested here earlier, which seems a bit redundant:
- Changing
WP_Feed_Cache
to no longer depend onSimplePie_Cache
, in order to add thestatic
keyword. Since that is a legacy method that should have only been used for SimplePie 1.2 or earlier, I don't think that is the correct path to take here. The wholeclass-wp-feed-cache.php
file should be deprecated instead. - Registering and using a cache handler, as suggested in 29204.diff. I believe that is the way to go here.
In 29204.4.diff:
- Use the recommended approach of registering a cache handler from SimplePie documentation.
- Mark the
wp-includes/class-wp-feed-cache.php
file as deprecated. - Add a back-compat check to avoid a fatal error in case there are still plugins out there including SimplePie 1.2.x.
- Tested on PHP 8.0.0RC4, 7.4.12, and 5.6.40.
#56
in reply to:
↑ 54
@
4 years ago
Replying to afragen:
I have not been able to reproduce the above error after deleting the cached transient and testing the patch.
Thanks for testing! Yeah, the legacy ::create()
method should not be called at all with the patch. A cached transient did not affect that in my testing, though I only tested by calling fetch_feed()
directly and displaying the results.
Using normal debug bar and wp_debug set to true, I don't have any errors on my site. I can reproduce the error with blackbox-debug-bar, however it shows on also 3.9.2 as well so this isn't 'new'
3.8 https://wordpress.org/support/topic/custom-dashboard-rss-feed-strict-error?replies=2
Looking at SimplePie, this may just be to support older versions of PHP: https://github.com/simplepie/simplepie/blob/master/library/SimplePie/Registry.php#L210