WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 6 weeks ago

#29204 reviewing defect (bug)

SimplePie non-static WP_Feed_Cache::create error in fetch_feed()

Reported by: useStrict Owned by: SergeyBiryukov
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.8
Component: Feeds Keywords: has-patch 4.7-early
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 (5)

29204.patch (579 bytes) - added by Senning 3 years ago.
Fix WP_Feed_Cache::create being non-static
29204.diff (1.6 KB) - added by dd32 3 years ago.
29204.2.diff (1.6 KB) - added by markoheijnen 2 years ago.
0001-29204-Fix-errors-about-non-static-WP_Feed_Cache-crea.patch (1.3 KB) - added by ComputerGuru 8 weeks ago.
Updated patch for error
0001-29204-Fix-errors-about-non-static-WP_Feed_Cache-crea.2.patch (2.1 KB) - added by ComputerGuru 8 weeks ago.
Updated patch and validated against trunk.

Download all attachments as: .zip

Change History (37)

#1 @Ipstenu
4 years ago

  • Version changed from trunk to 3.8

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

#2 @Senning
3 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.

@Senning
3 years ago

Fix WP_Feed_Cache::create being non-static

This ticket was mentioned in Slack in #core by dd32. View the logs.


3 years ago

#4 @dd32
3 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?

@dd32
3 years ago

#5 @nacin
3 years ago

Fun history here: [21652], #21183.

#6 @l3rady
3 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 @HoaSi
2 years ago

  • Keywords has-screenshots added

Reproducing the original issue using Bones on local install, PHP7:

https://i.imgur.com/GE2RTOA.png

https://i.imgur.com/NKjWjKL.png

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.

https://i.imgur.com/ypXD4FT.png

Last edited 2 years ago by HoaSi (previous) (diff)

#8 @jdgrimes
2 years ago

#37121 was marked as a duplicate.

@markoheijnen
2 years ago

#9 @markoheijnen
2 years ago

  • Keywords has-patch added

I just created an alternative solution. Could this be something to fix it?

#10 @NathanAtmoz
2 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

#11 @markoheijnen
2 years ago

@rmccue @ocean90: Any change this can be looked at for 4.6?

This ticket was mentioned in Slack in #core by markoheijnen. View the logs.


2 years ago

#13 @SergeyBiryukov
2 years ago

  • Keywords 4.7-early added

This ticket was mentioned in Slack in #core by helen. View the logs.


22 months ago

#15 @helen
22 months ago

  • Keywords needs-refresh added; has-screenshots removed

#16 @SergeyBiryukov
22 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#17 @jeremyfelt
19 months ago

#38069 was marked as a duplicate.

#18 @jfoulquier
19 months 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 @thefarlilacfield
18 months 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.

#20 @austinpray
12 months ago

What's the word on this one?

#21 @Shelob9
8 months ago

I'm plus oneing this ticket. I'm not supposed to, but tired of seeing this in logs.

#23 @subscriptiongroup
8 months 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 @ComputerGuru
7 months 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 @rogerlos
5 months 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 @Mte90
5 months 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.

Last edited 5 months ago by Mte90 (previous) (diff)

This ticket was mentioned in Slack in #core by mte90. View the logs.


4 months ago

#30 @mopsyd
2 months 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.

Source: https://bitbucket.org/oroborosframework/oroboros-core/src/9e77876320ff55a4a8d1409839447036603e4f0a/src/core/traits/utilities/core/CallableUtilityTrait.php?at=master&fileviewer=file-view-default

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.

Last edited 2 months ago by mopsyd (previous) (diff)

@ComputerGuru
8 weeks ago

Updated patch and validated against trunk.

#31 follow-up: @ComputerGuru
8 weeks 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.

https://core.trac.wordpress.org/attachment/ticket/29204/0001-29204-Fix-errors-about-non-static-WP_Feed_Cache-crea.2.patch

#32 in reply to: ↑ 31 @ksbsb
6 weeks 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 declare create as static, and so we can't just redeclare it as such in our derived WP_Feed_Cache class.

https://core.trac.wordpress.org/attachment/ticket/29204/0001-29204-Fix-errors-about-non-static-WP_Feed_Cache-crea.2.patch

Note: See TracTickets for help on using tickets.