Skip to content

Feeds: Guard against empty array passed to max() in get_feed_build_date() #11387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chubes4 wants to merge 8 commits into
base: trunk
Choose a base branch
from
+118 −5
Open

Feeds: Guard against empty array passed to max() in get_feed_build_date() #11387

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
beb5070
Feeds: Guard against empty array passed to max() in get_feed_build_da…
chubes4 Mar 29, 2026
eeb7bd5
Fix PHPCS: align equals signs in test assignments.
chubes4 Mar 29, 2026
5b79996
Tests: Fix test to match actual production trigger for empty modified…
chubes4 Mar 29, 2026
cdfa9f5
Merge branch 'trunk' into fix/59956-feed-build-date-empty-array
chubes4 Apr 10, 2026
c0fe548
Merge branch 'trunk' into fix/59956-feed-build-date-empty-array
westonruter Apr 23, 2026
6b4c4c3
Feeds: Address review feedback on get_feed_build_date() empty array fix.
chubes4 Apr 24, 2026
7962e11
Tests: Use multi-line block comment style in get_feed_build_date test.
chubes4 Apr 24, 2026
ec8dfad
Feeds: Resolve scalar post IDs in get_feed_build_date() to WP_Post ob…
chubes4 Apr 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/wp-includes/feed.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,9 @@ function get_feed_build_date( $format ) {
}

// Determine the maximum modified time.
$datetime = date_create_immutable_from_format( 'Y-m-d H:i:s', max( $modified_times ), $utc );
if ( ! empty( $modified_times ) ) {
Copy link
Copy Markdown
Member

@westonruter westonruter Apr 23, 2026

Choose a reason for hiding this comment

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

While core uses empty() extensively, we should avoid it if possible. This can be more simply:

Suggested change
if ( ! empty( $modified_times ) ) {
if ( $modified_times ) {

Copy link
Copy Markdown
Member

@westonruter westonruter Apr 23, 2026

Choose a reason for hiding this comment

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

Or more explicitly:

Suggested change
if ( ! empty( $modified_times ) ) {
if ( count( $modified_times ) > 0 ) {

$datetime = date_create_immutable_from_format( 'Y-m-d H:i:s', max( $modified_times ), $utc );
}
}

if ( false === $datetime ) {
Expand Down
41 changes: 41 additions & 0 deletions tests/phpunit/tests/date/getFeedBuildDate.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,47 @@ public function test_should_return_correct_feed_build_date() {
$this->assertSame( '2018-07-23T03:13:23+00:00', get_feed_build_date( DATE_RFC3339 ) );
}

/**
* Test that get_feed_build_date() does not throw a ValueError
* when wp_list_pluck() returns an empty array because the posts
* array contains non-object, non-array values.
*
* @ticket 59956
*/
public function test_should_not_error_when_modified_times_is_empty() {
global $wp_query;

Comment on lines +51 to +52
Copy link
Copy Markdown
Member

@westonruter westonruter Apr 23, 2026

Choose a reason for hiding this comment

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

Suggested change
global $wp_query;

$datetime = new DateTimeImmutable( 'now', wp_timezone() );
$datetime_utc = $datetime->setTimezone( new DateTimeZone( 'UTC' ) );

// Create a real post so the fallback has something to return.
self::factory()->post->create(
array(
'post_date' => $datetime->format( 'Y-m-d H:i:s' ),
)
);

// Simulate a query where have_posts() is true but wp_list_pluck()
// returns an empty array because the posts array contains scalar
// values (neither objects nor arrays). This triggers the _doing_it_wrong
// notice in WP_List_Util::pluck() and produces an empty result.
$wp_query = new WP_Query();

$wp_query->posts = array( 1 );
$wp_query->post_count = 1;
Comment on lines +56 to +68
Copy link
Copy Markdown
Member

@westonruter westonruter Apr 23, 2026

Choose a reason for hiding this comment

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

No need to simulate when we can do the real thing:

Suggested change
self::factory()->post->create(
array(
'post_date' => $datetime->format( 'Y-m-d H:i:s' ),
)
);
// Simulate a query where have_posts() is true but wp_list_pluck()
// returns an empty array because the posts array contains scalar
// values (neither objects nor arrays). This triggers the _doing_it_wrong
// notice in WP_List_Util::pluck() and produces an empty result.
$wp_query = new WP_Query();
$wp_query->posts = array( 1 );
$wp_query->post_count = 1;
$id = self::factory()->post->create(
array(
'post_date' => $datetime->format( 'Y-m-d H:i:s' ),
)
);
// Use a query where have_posts() is true but wp_list_pluck()
// returns an empty array because the posts array contains scalar
// values (neither objects nor arrays). This triggers the _doing_it_wrong
// notice in WP_List_Util::pluck() and produces an empty result.
query_posts(
array(
'posts__in' => array( $id ),
'fields' => 'ids',
)
);


$this->setExpectedIncorrectUsage( 'WP_List_Util::pluck' );

Comment on lines +53 to +70
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

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

The test currently uses query_posts( [ 'fields' => 'ids' ] ) to force wp_list_pluck() to return an empty array, which requires setExpectedIncorrectUsage( 'WP_List_Util::pluck' ) and depends on a DB query plus an unrelated _doing_it_wrong() path. You can simulate the reported inconsistency more directly (and avoid the incorrect-usage expectation) by setting global $wp_query to a fresh WP_Query with post_count = 1 (so have_posts() is true) but an empty posts array (so wp_list_pluck() returns []). This keeps the test focused on guarding max() against empty input.

Suggested change
$datetime = new DateTimeImmutable( 'now', wp_timezone() );
$datetime_utc = $datetime->setTimezone( new DateTimeZone( 'UTC' ) );
$id = self::factory()->post->create(
array(
'post_date' => $datetime->format( 'Y-m-d H:i:s' ),
)
);
// Use a query where have_posts() is true but wp_list_pluck()
// returns an empty array because the posts array contains scalar
// values (neither objects nor arrays). This triggers the _doing_it_wrong
// notice in WP_List_Util::pluck() and produces an empty result.
query_posts(
array(
'posts__in' => array( $id ),
'fields' => 'ids',
)
);
$this->setExpectedIncorrectUsage( 'WP_List_Util::pluck' );
global $wp_query;
$datetime = new DateTimeImmutable( 'now', wp_timezone() );
$datetime_utc = $datetime->setTimezone( new DateTimeZone( 'UTC' ) );
self::factory()->post->create(
array(
'post_date' => $datetime->format( 'Y-m-d H:i:s' ),
)
);
// Simulate a query where have_posts() is true but wp_list_pluck()
// receives an empty posts array, so modified_times is empty.
$wp_query = new WP_Query();
$wp_query->post_count = 1;
$wp_query->posts = array();

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@westonruter westonruter Apr 24, 2026

Choose a reason for hiding this comment

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

@chubes4 Would this replicate the original scenario that you've encountered? Curious what your WP_Query has in it which caused this issue in the first place.

Copy link
Copy Markdown
Author

@chubes4 chubes4 Apr 24, 2026

Choose a reason for hiding this comment

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

No, that does not replicate the original scenario.

The specific query that triggers it can be observed here: https://github.com/Extra-Chill/data-machine-events/blob/main/inc/Core/Event_Post_Type.php#L153-L180

We use a custom artist taxonomy, and a custom data_machine_events post type.

When there are existing data_machine_events posts using a given artist term, but 0 regular posts, and we query a feed URL with fields => 'ids' set, WP_Query returns an array of scalar ids instead of WP_Post objects.

wp_list_pluck() requires each element to be an object or array, so it hits _doing_it_wrong and returns an empty array. max([]) then fatals.

Copy link
Copy Markdown
Member

@westonruter westonruter Apr 24, 2026

Choose a reason for hiding this comment

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

Is this masking maybe a lower level issue? Shouldn't this also ensure that get_feed_build_date() works when posts is a list of post IDs and not WP_Post objects? This would fix the underlying issue.

Now, it would sense still to include the check added in this PR to prevent passing an empty array to max(), but I see this as additional hardening on top of a more fundamental issue that can be fixed.

Copy link
Copy Markdown
Member

@westonruter westonruter Apr 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I think this may be needed in both of these places:

wordpress-develop/src/wp-includes/feed.php

Line 724 in 59a9905

$modified_times = wp_list_pluck( $wp_query->posts, 'post_modified_gmt' );

wordpress-develop/src/wp-includes/feed.php

Line 729 in 59a9905

$comment_times = wp_list_pluck( $wp_query->comments, 'comment_date_gmt' );

Instead of using wp_list_pluck(), it should be using array_map() and just pass through the value if it is an integer. Otherwise, it can return the ID property of the respective object.

Copy link
Copy Markdown
Author

@chubes4 chubes4 Apr 25, 2026

Choose a reason for hiding this comment

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

You're right. The original fix only stopped the fatal, but it returned the global last modified time instead of the expected post. The bug was deeper down, and now it returns the timestamp for the correct post.

$result = get_feed_build_date( DATE_RFC3339 );
Comment thread
westonruter marked this conversation as resolved.

$this->assertEqualsWithDelta(
strtotime( $datetime_utc->format( DATE_RFC3339 ) ),
strtotime( $result ),
2,
'Should fall back to last post modified when modified_times is empty.'
);
}

/**
* Test that get_feed_build_date() works with invalid post dates.
*
Expand Down
Loading