WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 8 weeks ago

#38709 new defect (bug)

Unlimited query for invalid post names

Reported by: david.binda Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.6.1
Component: Query Keywords: needs-patch
Focuses: performance Cc:

Description

In case a request which is being parsed as one which sets a "name" query var (eg.: http://localhost/wordpress-latest/2016/11/08/[ ) includes an invalid name ( the [ ), a query without any limit with empty post_name is triggered:

SELECT wp_posts.*
FROM wp_posts 
WHERE 1=1 
AND ( ( YEAR( wp_posts.post_date ) = 2016
AND MONTH( wp_posts.post_date ) = 11
AND DAYOFMONTH( wp_posts.post_date ) = 8 ) )
AND wp_posts.post_name = ''
AND wp_posts.post_type = 'post' 
ORDER BY wp_posts.post_date DESC

The problem with the query is that it matches all the drafts in the database, since they don't have the post_name set (is empty). This may have a performance implications for the database in case it contains a lot of drafts (or posts without the post_name set for any reason) as it queries all the posts.

The post_name is being set as empty due to sanitize_title_for_query in https://core.trac.wordpress.org/browser/tags/4.6.1/src/wp-includes/query.php#L2717

I have tested a more suitable validation function than trim ( eg.: sanitize_title ) in https://core.trac.wordpress.org/browser/tags/4.6.1/src/wp-includes/query.php#L1609 but it leads to archives being served on those invalid URLs, which does not seem like good fix.

Trying to set posts_per_page to 1 in case the name is parsed based on the rewrite rule (which seems like a good thing since we really should be interested in a single and unique post_name in such cases) hits a wall as the is_singular() returns true and the limit is not set even if the posts_per_page is present ( due to https://core.trac.wordpress.org/browser/tags/4.6.1/src/wp-includes/query.php#L3215 ).

So the fix might need to be more complicated than.

Change History (2)

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


8 weeks ago

#2 @mikeschroder
8 weeks ago

  • Component changed from Permalinks to Query
  • Keywords needs-patch added

Hi @davidbinda!

Thanks so much for the report, and apologies for the extremely long time waiting for a response.

This ticket was discussed in a triage session today.

Investigating, @peterwilsoncc found that this does still seem to be an issue, and an EXPLAIN on that indicates it's using filesort so the indexes are missed.

Since this seems to be an issue coming from the Query component, going to move it there for further attention, and mark this as needs-patch.

Note: See TracTickets for help on using tickets.