Call to a member function format() on boolean in wp-includes/feed.php

[dd32]

[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

[dd32]

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

[Rarst]

Ugh. Updating from fuzzy inputs / fuzzy outputs hurts.

Will patch up a check there.

[azaozz]

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 :)

[dd32]

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' );
}

[tellyworth]

Unit test to reproduce the bug

[Rarst]

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.

[Rarst]

Ok, take on a patch. This ensures we do not error, we fall back in all necessary cases, we respect format when falling back, clarifies inline documentation on what it does and possibility of false on failure (which was already possible, but not documented).

[Rarst]

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.

[SergeyBiryukov]

In 46973:

Docs: Clarify that get_lastpostdate() and get_lastpostmodified() can return false on failure.

The both use _get_last_post_time() internally.

Props Rarst.
See #48957.

[SergeyBiryukov]

In 46974:

Date/Time: Ensure that get_feed_build_date() correctly handles a modified post object with invalid date.

• Clarify in the documentation that the function returns false on failure.
• Consistently pass the return value through the get_feed_build_date filter.

Props Rarst, dd32, azaozz, tellyworth.
Fixes #48957.

[SergeyBiryukov]

Reopening for 5.3.2.

[SergeyBiryukov]

In 46977:

Date/Time: Ensure that get_feed_build_date() correctly handles a modified post object with invalid date.

• Clarify in the documentation that the function returns false on failure.
• Consistently pass the return value through the get_feed_build_date filter.

Props Rarst, dd32, azaozz, tellyworth.
Merges [46974] and [46973] to the 5.3 branch.
Fixes #48957.

[SergeyBiryukov]

In 46981:

Tests: Use delta comparison in test_should_fall_back_to_last_post_modified() to avoid race conditions.

See #48957.

[SergeyBiryukov]

In 46982:

Tests: Use delta comparison in test_should_fall_back_to_last_post_modified() to avoid race conditions.

Merges [46981] to the 5.3 branch.
See #48957.