Make WordPress Core

Opened 4 weeks ago

Closed 3 weeks ago

Last modified 7 days ago

#63307 closed defect (bug) (fixed)

REST API: Returns incorrect post when querying by slug if sticky posts exist

Reported by: wpmudevsupport13's profile wpmudevsupport13 Owned by: jorbin's profile jorbin
Milestone: 6.8.1 Priority: normal
Severity: critical Version: 6.8
Component: REST API Keywords: changes-requested has-test-info has-unit-tests has-patch fixed-major dev-reviewed commit
Focuses: rest-api Cc:

Description

It was mentioned here by one of the reddit users https://www.reddit.com/r/Wordpress/comments/1k106qz/wordpress_68_bug_with_rest_api_posts_pulling/
And it seems the issue can be easily replicated:

  1. Create new post: /test-post
  2. Open this in a browser: https://example.com/wp-json/wp/v2/posts?slug=test-post

At this point, multiple values will get the post slug and ID.

  1. Create another new post: /second-test and make that post sticky

Visit again https://example.com/wp-json/wp/v2/posts?slug=test-post, and this time, many values will get that /second-test slug and ID, where it should still show /test-post.

If you create multiple sticky posts, the https://example.com/wp-json/wp/v2/posts?slug=test-post will get the slug and ID from the sticky post that has the newest published date.

Attachments (4)

wp6.8-post.png (41.6 KB) - added by nikunj8866 4 weeks ago.
wp6.8-rest-api.png (7.1 KB) - added by nikunj8866 4 weeks ago.
I've verified this issue and can confirm it is reproducible in WordPress 6.8. When querying /wp-json/wp/v2/posts?slug=test-post, the response includes data from newer sticky posts, which should not happen.
wp6.7.2-post.png (43.8 KB) - added by nikunj8866 4 weeks ago.
wp6.7.2-rest-api.png (8.0 KB) - added by nikunj8866 4 weeks ago.
I tested the same steps on WordPress 6.7.2, and the issue does not occur there. The correct post is returned when querying by slug, even with sticky posts present.

Download all attachments as: .zip

Change History (41)

@nikunj8866
4 weeks ago

@nikunj8866
4 weeks ago

I've verified this issue and can confirm it is reproducible in WordPress 6.8. When querying /wp-json/wp/v2/posts?slug=test-post, the response includes data from newer sticky posts, which should not happen.

@nikunj8866
4 weeks ago

I tested the same steps on WordPress 6.7.2, and the issue does not occur there. The correct post is returned when querying by slug, even with sticky posts present.

This ticket was mentioned in PR #8713 on WordPress/wordpress-develop by @nikunj8866.


4 weeks ago
#2

  • Keywords has-patch added

…osts exist issue

Trac ticket: https://core.trac.wordpress.org/ticket/63307

@nikunj8866 commented on PR #8713:


4 weeks ago
#3

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

## Unlinked Accounts
The following contributors have not linked their GitHub and WordPress.org accounts: @nhatkar@….

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Please use @nikunj8866 WordPress profile.

#4 @nikunj8866
4 weeks ago

  • Keywords needs-testing added
  • Summary changed from REST API posts return 2 posts to REST API: Returns incorrect post when querying by slug if sticky posts exist

#5 @SirLouen
4 weeks ago

  • Keywords changes-requested has-testing-info added; has-patch needs-testing removed

Patch Test Report

Description

❌ This report can't fully validate that the tested patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/8713.diff

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.4.6
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.4.6)
  • Browser: Chrome 135.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Actual Results

  1. 🟠 The issue is resolved with the patch but new issues arise

Additional Notes

The intention of that parameter is to keep sticky on top by default in the main REST json result
http://localhost:8889/wp-json/wp/v2/posts
By changing this param, you solve this issue, but now the sticky post won't appear on top of the full post list, which is the intention of such.

#6 follow-up: @nikunj8866
4 weeks ago

@SirLouen Thank you for the detailed test report and for highlighting the side effect introduced by the patch.

It does change the default behavior of sticky posts being prioritized at the top of the main REST response (/wp-json/wp/v2/posts), which was an intentional feature.

So for users who are specifically querying posts by slug and need consistent results regardless of sticky status, the recommended approach is to use the ignore_sticky=true parameter. For example: /wp-json/wp/v2/posts?slug=test-post&ignore_sticky=true

#7 in reply to: ↑ 6 @SirLouen
4 weeks ago

Replying to nikunj8866:

It does change the default behavior of sticky posts being prioritized at the top of the main REST response (/wp-json/wp/v2/posts), which was an intentional feature.

In my test it was the other way around. With the patch, the sticky posts disappear from top positions in /posts

#8 follow-up: @nikunj8866
4 weeks ago

  • Keywords 2nd-opinion added

@SirLouen When someone uses specific filters like slug, author, status, include, etc., they usually expect to get only the exact matching posts. Right now, if there's a sticky post with a newer date, it can wrongly appear instead of the correct post—for example, when using ?slug=test-post.

We suggest that when these kinds of filters are used, API should automatically ignore sticky posts (unless the user specifically wants them). This would help make the results more accurate and avoid confusion for developers using the API.

Let us know what you think about this. We’re happy to help with a patch or more suggestions if needed.

This ticket was mentioned in PR #8730 on WordPress/wordpress-develop by @ankitmaru.


4 weeks ago
#9

  • Keywords has-patch added

### Summary
This PR updates the WordPress REST API to ignore sticky posts when specific filters are used (like slug, include, author, etc.), unless explicitly overridden.

By default, /wp-json/wp/v2/posts continues to return all posts, including sticky ones, as expected.

### Trac Ticket
https://core.trac.wordpress.org/ticket/63307

### Testing

  1. Create a sticky post (sticky-post) and a non-sticky post (test-post).
  2. Request: /wp-json/wp/v2/posts?slug=test-post Only test-post should be returned.
  3. Request: /wp-json/wp/v2/posts?slug=test-post&ignore_sticky=false Still only test-post is returned.
  4. Request: /wp-json/wp/v2/posts All posts are returned, sticky posts appear first (default behavior).

Let me know if you'd like changes or additional tests.

#10 @ankitmaru
4 weeks ago

Hello,
PR submitted: https://github.com/WordPress/wordpress-develop/pull/8730

Fixes an issue where sticky posts could appear in REST API results even when using specific filters like slug.
Adds ignore_sticky support and defaults to ignoring sticky posts when exact filters are used.

Let me know if any changes are needed.

#11 in reply to: ↑ 8 @SirLouen
4 weeks ago

  • Keywords needs-testing added

Replying to nikunj8866:

Let us know what you think about this. We’re happy to help with a patch or more suggestions if needed.

As I commented in my Test Report, you are not solving the issue, you are just hindering it.

If you do this, for example, with your patch:

http://localhost:8889/wp-json/wp/v2/posts?slug=test-post&ignore_sticky=false

It will return the post "test-post" and all the sticky posts. This is not the expected behavior, because you are not addressing the real problem that is being caused.

Sticky posts are meant to be shown just on listing queries, not on individual records queries.
And sticky posts should be displayed by default on listings, unless explicitly specified with the given filter, not the opposite.

Last edited 4 weeks ago by SirLouen (previous) (diff)

@SirLouen commented on PR #8730:


4 weeks ago
#12

@wpankit this is a little refactored verrsion including PHPCS standards:

// Automatically ignore sticky posts when specific filters are used, unless explicitly requested.
$specific_filters = array( 'slug', 'author', 'author_exclude', 'include', 'exclude', 'parent', 'parent_exclude' );

foreach ( $specific_filters as $filter ) {
        if ( isset( $request[ $filter ] ) && ! empty( $request[ $filter ] ) ) {
                if ( ! ( isset( $request['ignore_sticky'] ) && $request['ignore_sticky'] ) ) {
                        $args['ignore_sticky_posts'] = true;
                }
                break;
        }
}

Although, I'm not convinced it needs a little more work, I will provide a test report in the Ticket

#13 @SirLouen
4 weeks ago

  • Keywords 2nd-opinion has-patch needs-testing removed

Patch Test Report

Description

❌ This report can't validate that the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/8730.diff

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 135.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Fifteen 4.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Bug Reproduction Steps

  • Using URL like:
  1. ✅ Single post: https://example.com/wp-json/wp/v2/posts?slug=test-post
  2. ✅ All posts: https://example.com/wp-json/wp/v2/posts
  3. ❌ Template listings (i.e., Author): https://example.com/wp-json/wp/v2/posts?author=1 => If the author has an sticky post, it should appear in first position

Actual Results

  1. ✅ For the condition in the reporting post, this patch solves
  2. ❌ Some other scenarios are not solved.

Additional Notes

I think that the only affecting filter is slug. The other filters should return the "stuck" post if there is a sticky post among the filtered results. Examples: If there are 10 posts, 2 of which are sticky

  1. If the user is using "exclude" with one of the sticky posts, it should return, the other sticky post on top, and the other non-sticky posts in the bottom
  2. If the user is using "include" with one sticky post and one non-sticky post, it should return, first the sticky one and second the non-sticky one, regardless of the ordering selected.

Conclusion for the correct behavior:
For anything listing ⇒ If the listing has sticky posts the should go first
For anything single ⇒ Just show the single post.

I'm going to be adding some unit tests so whoever builds the patch, takes all this into consideration.

This ticket was mentioned in PR #8731 on WordPress/wordpress-develop by @SirLouen.


4 weeks ago
#14

  • Keywords has-patch has-unit-tests added

I'm adding some unit tests for whoever is willing to this patch they most be compliant

Add these tests and run: npm run test:php -- --group 63307

#15 @SirLouen
4 weeks ago

  • Keywords has-patch removed

Removing this auto-keyword

#16 @SirLouen
4 weeks ago

#63339 was marked as a duplicate.

#17 @SirLouen
4 weeks ago

  • Keywords dev-feedback added
  • Severity changed from normal to critical

@audrasjb (or any other committer that reads this)
Can you add priority Milestone 6.8.1 to this regression?
It has been reported in other posts like #63339 as a regression of [59801]

Last edited 3 weeks ago by SirLouen (previous) (diff)

#18 @wildworks
4 weeks ago

  • Milestone changed from Awaiting Review to 6.8.1

This ticket was mentioned in PR #8737 on WordPress/wordpress-develop by @jorbin.


3 weeks ago
#19

  • Keywords has-patch added

This is to:
1) prepare a revert as a potential option
2) Demonstrate that [59801] is the root cause.

Includes unit tests from #8731
Reverts [59801].

Core Trac: https://core.trac.wordpress.org/ticket/63307

#20 follow-up: @jorbin
3 weeks ago

Confirmed using @SirLouen's tests that this is caused by [59801] so cc @peterwilsoncc, @audrasjb, @danielbachhuber, @joemcgill, @johnbillion, @jorbin (it's a me!), @mamaduka, @rmccue who all worked on that along with @spacedmonkey and @TimothyBlynJacobs who worked on the related GB PR

It should absolutely be fixed in 6.8.1, even if that means reverting [59801] which I would like to avoid if possible.

RE: The two proposed solutions -

https://github.com/WordPress/wordpress-develop/pull/8713 - I don't think this is feasible since it will cause sticky posts to need to be explictly requested which is a change in behavior (it's a bit concerning that no tests failed though)

https://github.com/WordPress/wordpress-develop/pull/8730 - Something along these lines makes sense to me, though I would like for each of them to have some sort of test. I also think that this should use the WP_Query params, not the rest API version.

#21 in reply to: ↑ 20 @SirLouen
3 weeks ago

Replying to jorbin:

Confirmed using @SirLouen's tests that this is caused by [59801] so cc @peterwilsoncc, @audrasjb, @danielbachhuber, @joemcgill, @johnbillion, @jorbin (it's a me!), @mamaduka, @rmccue who all worked on that along with @spacedmonkey and @TimothyBlynJacobs who worked on the related GB PR

It should absolutely be fixed in 6.8.1, even if that means reverting [59801] which I would like to avoid if possible.

RE: The two proposed solutions -

https://github.com/WordPress/wordpress-develop/pull/8713 - I don't think this is feasible since it will cause sticky posts to need to be explictly requested which is a change in behavior (it's a bit concerning that no tests failed though)

https://github.com/WordPress/wordpress-develop/pull/8730 - Something along these lines makes sense to me, though I would like for each of them to have some sort of test. I also think that this should use the WP_Query params, not the rest API version.

I would need to recheck but from what I remember neither of these two solutions worked with my test (and the main reason of why I did such test, to help the guys to build the right fix)

#22 @karthikeya01
3 weeks ago

https://github.com/WordPress/wordpress-develop/pull/8713 - I don't think this is feasible since it will cause sticky posts to need to be explictly requested which is a change in behavior (it's a bit concerning that no tests failed though)

I want to add that until 6.7.2 the REST API doesn't include sticky posts on top. It used ignore_sticky_posts=1 for all get_items requests.

Introducing sticky posts also causes issues in pagination mentioned in the ticket https://core.trac.wordpress.org/ticket/63339

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


3 weeks ago

#24 @Mamaduka
3 weeks ago

Thanks for the ping, @jorbin!

If I'm not mistaken, the actual behavior is coming from WP_Query. It's an old and odd behavior that we have to maintain for backward compatibility. These "gotchas" are also mentioned in the documentation.

Now that we have fixed the sticky post display for the REST API, it also inherits the same query behavior.

The resolution should honor the original REST API behavior for posts collection, while maintaining support for sticky posts. I've included a similar case for the post__in filter, but unfortunately, other cases slipped my mind. See: https://github.com/Mamaduka/wordpress-develop/blob/f01ff83ac372867253e4e757c5843add4ef83bd9/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php#L341-L347.

Regarding #63339. This is expected behavior; unless ignored, the sticky posts are prepended to the first page results, ignoring the per_page parameter. It was a bug stopping the Query block from working correctly in the editors.

#25 follow-ups: @jorbin
3 weeks ago

Thanks @Mamaduka!

I think the challenge is that the wp/v2/posts endpoint has now also been around for a long time with it's own "old and odd behavior" that we need to be careful with.

I think the best path is that in 6.8.1, keep the ignore_sticky_posts param that was added, but change the default in the rest API to true which will restore the behavior that the endpoint had from 4.7 -> 6.7. The Query Block can be modified to call the endpoint with ignore_sticky_posts set to false. We can then look at adding a new endpoint (wp/v3/posts? wp/v2/wp_query?) in 6.9 that more closely resembles WP_Query. This would honor the original REST API behavior for posts collection, while maintaining support for sticky posts.

This ticket was mentioned in PR #8743 on WordPress/wordpress-develop by @jorbin.


3 weeks ago
#26

This builds off the work in #8713 and #8731.

Core Trac: https://core.trac.wordpress.org/ticket/63307

#27 in reply to: ↑ 25 @spacedmonkey
3 weeks ago

Replying to jorbin:

Thanks @Mamaduka!

I think the challenge is that the wp/v2/posts endpoint has now also been around for a long time with it's own "old and odd behavior" that we need to be careful with.

I think the best path is that in 6.8.1, keep the ignore_sticky_posts param that was added, but change the default in the rest API to true which will restore the behavior that the endpoint had from 4.7 -> 6.7. The Query Block can be modified to call the endpoint with ignore_sticky_posts set to false. We can then look at adding a new endpoint (wp/v3/posts? wp/v2/wp_query?) in 6.9 that more closely resembles WP_Query. This would honor the original REST API behavior for posts collection, while maintaining support for sticky posts.

+1 to this.

#28 in reply to: ↑ 25 @SirLouen
3 weeks ago

Replying to jorbin:

Thanks @Mamaduka!

I think the challenge is that the wp/v2/posts endpoint has now also been around for a long time with it's own "old and odd behavior" that we need to be careful with.

I think the best path is that in 6.8.1, keep the ignore_sticky_posts param that was added, but change the default in the rest API to true which will restore the behavior that the endpoint had from 4.7 -> 6.7. The Query Block can be modified to call the endpoint with ignore_sticky_posts set to false. We can then look at adding a new endpoint (wp/v3/posts? wp/v2/wp_query?) in 6.9 that more closely resembles WP_Query. This would honor the original REST API behavior for posts collection, while maintaining support for sticky posts.

I don't think this is a good idea entirely: [59801] should be reverted, as It's still buggy and needs more development. I've been looking into the code to find a quick fix, but it's not going to be as quick as I thought.

The solution of setting the default ignore_sticky to true keeps the added functionality troubles provided by [59801] alive.

Basically, if you change the URL param behavior, all sticky posts will return (regardless of whether they match the query or not). We could argue that it's weird to add "ignore_sticky=false" to the slug param, but for any other param it will also break the results as it does with slug and it's not that weird to use this. The sole scenario where "ignore_sticky" makes sense (and to which it seemed to be tested), was against the regular no-parameters query. Probably not defaulting ignore_sticky and working from there could be the solution code-wise. But many extra tweaks have to be made to avoid delivering a half-cooked buggy solution.

So basically, my argument is that the "ignore_sticky" is partially broken, and it should be reverted for now.

Last edited 3 weeks ago by SirLouen (previous) (diff)

#29 @SirLouen
3 weeks ago

After bumping my head with the wall to death yesterday for a good while, suddenly, a thought came to my mind: How WP_Query is actually handling ignore_sticky_posts?

And I got to this conclusion: #63360

The big problem here is related with WP_Query itself when using the __in query args (which happen to be the main query args that the REST API is using). For example, the name query arg works great, but the post_name__in generates this problem.

Maybe I'm wrong at some point, but reverting [59801] feels definitely the best option and for now, eliminate the use of stickiness, to have time for 6.8.2 not only looking into this, but looking into WP_Query in general, over ignore_sticky_posts behavior feels required.

Last edited 3 weeks ago by SergeyBiryukov (previous) (diff)

#30 follow-up: @jorbin
3 weeks ago

  • Keywords commit added

Unless you can demonstrate a way that the behavior of the rest endpoint and WP_Query are returning different results besides the fact that there are different defaults for sticky posts, it is working as expected. I've pushed up a few test changes that show they are working the same.

#31 in reply to: ↑ 30 @SirLouen
3 weeks ago

Replying to jorbin:

Unless you can demonstrate a way that the behavior of the rest endpoint and WP_Query are returning different results besides the fact that there are different defaults for sticky posts, it is working as expected. I've pushed up a few test changes that show they are working the same.

If you go to #63360 you can see a plugin I've uploaded, that clearly demonstrates this. I've separated into a different ticket, because the real issue is WP_Query not this. Current unit tests are incomplete. Before adding new unit tests that contemplate this, I have uploaded that plugin for first-line testing.

But still, I left this ticket because you need to do something before 6.8.1 RC1 and as I say, the safest bet is a regression here.

#32 @jorbin
3 weeks ago

  • Owner set to jorbin
  • Resolution set to fixed
  • Status changed from new to closed

In 60197:

REST API: Change posts endpoint to ignore_sticky=true by default

This restores the 6.7 and below behavior for the posts endpoint which did not include sticky posts by default.

Follow-up to [59801].

Props nikunj8866, SirLouen, ankitmaru, wildworks, karthikeya01, Mamaduka, spacedmonkey, jorbin.
Fixes #63307. See #35907.

#33 @jorbin
3 weeks ago

  • Keywords fixed-major added; commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

#34 @desrosj
3 weeks ago

  • Keywords dev-reviewed added; dev-feedback removed

[60197] looks good for backport.

#35 @desrosj
3 weeks ago

  • Keywords commit added

#36 @jorbin
3 weeks ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 60200:

REST API: Change posts endpoint to ignore_sticky=true by default

This restores the 6.7 and below behavior for the posts endpoint which did not include sticky posts by default.

Follow-up to [59801].

Reviewed by desrosj.
Merges [60197] to the 6.8 branch.

Props nikunj8866, SirLouen, ankitmaru, wildworks, karthikeya01, Mamaduka, spacedmonkey, jorbin.
Fixes #63307. See #35907.

#37 @wordpressdotorg
7 days ago

  • Keywords has-test-info added; has-testing-info removed
Note: See TracTickets for help on using tickets.