WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#20628 closed defect (bug) (fixed)

Splitting the main query can result in long query strings that segfault

Reported by: ryan Owned by: ryan
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.4
Component: Query Keywords:
Focuses: Cc:

Description

When paging hierarchical post types, edit.php via wp_edit_posts_query() can create very long query strings that enumerate every post ID to fetch. If a limit is not present or very large, the query may beed to fallback to the old single query style.

See #20621

Attachments (3)

20628.diff (674 bytes) - added by ryan 3 years ago.
split_the_query filter. Don't split if no limit.
20628.2.diff (720 bytes) - added by SergeyBiryukov 3 years ago.
Check $limits (potentially filtered, e.g. via post_limits)
20628.2.alt.diff (690 bytes) - added by SergeyBiryukov 3 years ago.
Just check posts_per_page

Download all attachments as: .zip

Change History (12)

@ryan3 years ago

split_the_query filter. Don't split if no limit.

comment:1 @nacin3 years ago

Looks good. Should probably also pick an arbitrary limit like 500 at which point we no longer try to split the query.

comment:2 @ryan3 years ago

In [20740]:

Don't split the main query if there is no limit. This helps avoid extremely long query strings that could cause segfaults. Allow plugins to control splitting via split_the_query filter. see #20628

comment:3 @ryan3 years ago

Still need to check the limit if anyone cares to patch that.

comment:4 @scribu3 years ago

+1 for 'split_the_query'.

Still need to check the limit if anyone cares to patch that.

I already see !empty( $limits ) in there, so what's left?

comment:5 @scribu3 years ago

Oh, to not split the query if 'posts_per_page' => 500+? I don't know, that seems an unlikely edge case.

comment:6 follow-up: @scribu3 years ago

You usually either want paging (i.e a reasonable number), or you don't. Plus, it can be handled with 'split_the_query'.

comment:7 in reply to: ↑ 6 @nacin3 years ago

Replying to scribu:

You usually either want paging (i.e a reasonable number), or you don't. Plus, it can be handled with 'split_the_query'.

While it *can* be handled with split the query, surely we've all seen posts_per_page = 99999 before — we should try to catch this by default.

comment:8 @scribu3 years ago

How about we catch it earlier on, say in parse_query()?

if ( $q['posts_per_page'] >= 9999 )
  $q['posts_per_page'] = -1;

@SergeyBiryukov3 years ago

Check $limits (potentially filtered, e.g. via post_limits)

@SergeyBiryukov3 years ago

Just check posts_per_page

comment:9 @ryan3 years ago

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

In [20756]:

Set split_the_query to false if more than 500 rows requested. Props SergeyBiryukov. fixes #20628

Note: See TracTickets for help on using tickets.