Query: Fix redirect_guess_404_permalink to only query viewable post types when given an array. - #12755
Query: Fix redirect_guess_404_permalink to only query viewable post types when given an array.#12755SainathPoojary wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes redirect_guess_404_permalink() so that when post_type is provided as an array, the generated SQL IN (...) clause is built from the intersected list of publicly viewable post types (rather than the raw query var), preventing non-viewable post types from being queried.
Changes:
- Use the already-filtered
$post_typesintersection when building thepost_type IN (...)SQL clause for array-formpost_typequery vars.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -986,7 +986,7 @@ function redirect_guess_404_permalink() { | |||
| if ( empty( $post_types ) ) { | |||
| return false; | |||
| } | |||
| $where .= " AND post_type IN ('" . join( "', '", esc_sql( get_query_var( 'post_type' ) ) ) . "')"; | |||
| $where .= " AND post_type IN ('" . join( "', '", esc_sql( $post_types ) ) . "')"; | |||
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
There was a problem hiding this comment.
🟡 Not ready to approve
The newly added unit test can be non-deterministic and may not reliably fail against the original bug without explicitly asserting the generated SQL/query behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (1)
tests/phpunit/tests/canonical.php:453
- The new test asserts the redirect target, but it may not reliably catch the original bug because the underlying
SELECT ID FROM $wpdb->posts WHERE ...has noORDER BY, so with both post types in theIN (...)clause the DB is free to return either matching row. To make the regression test deterministic and directly verify the fix, capture the SQL via thequeryfilter and assert it does not includewp_tests_private(and that a matching query was actually executed).
$this->go_to( '/?name=viewable-intersection-tes' );
set_query_var( 'post_type', array( 'page', 'wp_tests_private' ) );
$this->assertSame( get_permalink( $public_post ), redirect_guess_404_permalink() );
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟢 Ready to approve
The fix is narrowly scoped, matches the stated bug, and is covered by targeted unit tests for the relevant array post_type behavior.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
d0325df to
472033a
Compare
When a user passes an array of post types in a 404 URL, redirect_guess_404_permalink() correctly intersects them with publicly viewable post types to determine the valid list. However, it had a bug where it constructed the SQL IN (...) clause using the raw, unfiltered query variable (get_query_var( 'post_type' )) instead of the intersected list ($post_types).
This PR fixes the SQL clause to use the $post_types intersection variable, ensuring that non-public post types are never included in the database query. It also introduces unit tests to verify that a private post is correctly ignored when a public post with the same name exists and an array of their post types is requested.
Trac ticket: #44964
Use of AI Tools
AI assistance: Yes
Tool(s): GitHub Copilot
Model(s): Gemini, Claude
Used for: Checking for potential edge cases, drafting the PR description, and assisting with local code review. The final implementation and testing were written and executed manually by me.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.