#43056 closed defect (bug) (fixed)
Notice in redirect_guess_404_permalink() when post type is an array
Reported by: | junaidbhura | Owned by: | peterwilsoncc |
---|---|---|---|
Milestone: | 5.6 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Canonical | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
When manipulating URLs, it sometimes becomes necessary to update the query using the pre_get_posts
hook.
If we set the post type to an array in the pre_get_posts
hook like so:
$query->set( 'post_type', array( 'post', 'page', 'my_cpt' ) );
On a 404 page, we get a notice when WP_DEBUG
is set to true
:
Notice: wpdb::prepare was called incorrectly. The query only expected one placeholder, but an array of multiple placeholders was sent.
This is caused by the following code in redirect_guess_404_permalink()
:
$where .= $wpdb->prepare(" AND post_type = %s", get_query_var('post_type'));
This can be fixed by looking for an array and updating the query.
Attachments (5)
Change History (25)
This ticket was mentioned in Slack in #core by junaidbhura. View the logs.
7 years ago
#5
@
6 years ago
- Keywords needs-refresh added
- Milestone changed from 5.1 to Future Release
- Version trunk deleted
This ticket was mentioned in Slack in #core by enchiridion. View the logs.
6 years ago
This ticket was mentioned in Slack in #core by junaidbhura. View the logs.
4 years ago
#12
@
4 years ago
I've taken a look at 43056.2.diff
The escaping looks good with esc_url
, the post types are verified as legitimate in class-wp.php
.
You'll notice that at the moment, the canonical guess function uses get_query_var( 'post_type' )
calls multiple times. This is to act as a safety switch so that the value of a variable isn't changed by mistake -- this can lead to problems ;)
Rather than putting the value in a variable, I think calling get_query_var( 'post_type' )
each time would an improvement.
#13
@
4 years ago
Nice catch @peterwilsoncc ! I've updated and attached a new diff, could you please take a look?
This ticket was mentioned in PR #480 on WordPress/wordpress-develop by peterwilsoncc.
4 years ago
#14
- Keywords has-unit-tests added
Trac ticket: https://core.trac.wordpress.org/ticket/43056
#15
@
4 years ago
- Owner set to peterwilsoncc
- Status changed from new to assigned
@junaidbhura I've modified your patch slightly in PR 480 on the GitHub repo:
- The escaping was a little out,
$wpdb->prepare()
can't be used forIN
so the query would have being incorrect, it would have looked for a post type of an apparently random concatenation of the post types. I've replaced that with the counter-intuitive method of correctly escapingLIKE
s. - I've added some unit tests too for array formatted post types. I've used the publicly queryable post types in a query string rather than the
pre_get_posts
filter example in your initial report.
#16
@
4 years ago
Thanks @peterwilsoncc ! I've added a couple of comments on your PR. Could you please take a look?
#17
@
4 years ago
43056.4.diff is the final state of the PR with tests and @junaidbhura's code
dream-encode commented on PR #480:
4 years ago
#19
Merged into WP Core in https://core.trac.wordpress.org/changeset/49200
I suspect 43056.diff will introduce SQL injection issues.
wpdb:prepare()
won't put quotes around each element of the array being sent to it when replacing into the%s
.