#48957 closed defect (bug) (fixed)
Call to a member function format() on boolean in wp-includes/feed.php
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.3.2 | Priority: | high |
Severity: | normal | Version: | 5.3.1 |
Component: | Date/Time | Keywords: | has-patch has-unit-tests fixed-major |
Focuses: | Cc: |
Description
[46774] / #48675 is causing a PHP fatal on buddypress.org, that may ultimately be caused by BuddyPress, but either way is a new core fatal condition as of 5.3.1.
Fatal error: Uncaught Error: Call to a member function format() on boolean in wp-includes/feed.php on line 692
The root cause of the issue is that the WP_Post
object appears to have been emptied by BP similar to this: https://github.com/buddypress/BuddyPress/blob/fe47890c1ab7e2cb24c005f1c35fb9c6c8c8ab7c/src/bp-members/classes/class-bp-members-theme-compat.php#L119-L136
That ultimately means that $modified_times
is [ 0 ]
which causes date_create_immutable_from_format()
to return false instead of the expected DateTime
object.
cc @SergeyBiryukov @johnjamesjacoby
Attachments (2)
Change History (19)
#2
@
5 years ago
Ugh. Updating from fuzzy inputs / fuzzy outputs hurts.
Will patch up a check there.
#4
@
5 years ago
- Priority changed from normal to high
What would be the best fix here? I can see this happening in other cases/for CPTs perhaps. Would it be enough to check the returned value from date_create_immutable_from_format()
or perhaps have a wrapper for it that can handle ...other exceptions/errors?
Looking at the PHP docs (for date_create_from_format which is like date_create_immutable_from_format), specifically at https://www.php.net/manual/en/datetime.createfromformat.php#refsect1-datetime.createfromformat-parameters, it implies some possible errors when the same strings like for date()
are used.
Setting this to high priority as a fatal is a fatal, needs fixing asap :)
#5
@
5 years ago
Would it be enough to check the returned value from date_create_immutable_from_format()
I think checking the return value is in general probably enough, with the expectation that it'll take appropriate action to try to make sure it's valid.
In this case, max( $modified_times )
is probably enough to attempt to get a valid date, avoiding the 0
and 0000-00-00 00:00:00
dates if there's valid data available.
The question just remains - what kind of output should this function have in the event of this error condition?
It looks like the fallback higher up in the function for get_lastpostmodified()
is probably a good option, so:
if ( ! $datetime ) { // Fallback to last time any post was modified or published. return get_lastpostmodified( 'GMT' ); }
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
5 years ago
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
5 years ago
#9
@
5 years ago
Working on it, but turns out existing fallback is incorrect because it doesn't respect the $format
argument at all.
Writing out the test a bit for that and looking to adjust the fallback implementation.
#10
@
5 years ago
- Keywords has-patch has-unit-tests added
Ok, take on a patch. This ensures:
- we do not error
- we fall back in all necessary cases
- we respect format when falling back
- we pass all cases through the filter
- inline documentation is clear on what it does and possibility of false on failure (which was already possible, but not documented)
#11
@
5 years ago
I am not 100% confident on passing false
case through the filter though. I hate it when return filter randomly doesn't apply to some of the branches, but in modern PHP I would not add a different type to something established. Previously fallback case could return false
but w/o passing through the filter.
Intuitively I think passing everything and updating documentation is the way this time, but open to feedback.
This was triggered on the
/members/feed/
buddypress feed, and while buddypress shouldn't be messing with the globals like that this much, core should still be defensive here