WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 8 months ago

Last modified 8 months ago

#23383 closed defect (bug) (fixed)

High archive page numbers cause invalid database queries

Reported by: Viper007Bond Owned by: wonderboymusic
Milestone: 4.0 Priority: normal
Severity: normal Version: 3.6
Component: Query Keywords: has-patch commit
Focuses: Cc:

Description (last modified by Viper007Bond)

Visiting http://www.viper007bond.com/category/videos/page/6805063692754011230 on my site generates the following database query:

SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts INNER JOIN wp_term_relationships ON (wp_posts.ID = wp_term_relationships.object_id) WHERE 1=1 AND ( wp_term_relationships.term_taxonomy_id IN (21) ) AND wp_posts.post_type = 'post' AND (wp_posts.post_status = 'publish' OR wp_posts.post_status = 'private') GROUP BY wp_posts.ID ORDER BY wp_posts.post_date DESC LIMIT 6.805063692754E+19, 10

Note the scientific notation LIMIT value.

PHP: 5.3.15-pl0-gentoo
MySQL: 5.2.12
WordPress: r23386

Attachments (2)

23383.diff (689 bytes) - added by wonderboymusic 10 months ago.
23383.2.diff (533 bytes) - added by SergeyBiryukov 8 months ago.

Download all attachments as: .zip

Change History (22)

comment:1 @Viper007Bond2 years ago

  • Description modified (diff)

comment:2 @Viper007Bond2 years ago

Interestingly I cannot reproduce this locally, so it may be 32bit vs 64bit or something. The same paged value results in this query:

SELECT SQL_CALC_FOUND_ROWS wp_trunk_posts.ID FROM wp_trunk_posts INNER JOIN wp_trunk_term_relationships ON (wp_trunk_posts.ID = wp_trunk_term_relationships.object_id) WHERE 1=1 AND ( wp_trunk_term_relationships.term_taxonomy_id IN (1) ) AND wp_trunk_posts.post_type = 'post' AND (wp_trunk_posts.post_status = 'publish' OR wp_trunk_posts.post_status = 'private') GROUP BY wp_trunk_posts.ID ORDER BY wp_trunk_posts.post_date DESC LIMIT 21474836460, 10

No idea where this other LIMIT is coming from.

comment:3 @nacin2 years ago

2147483647 is the maximum integer (signed) on 32 bit systems.

comment:4 @SergeyBiryukov2 years ago

Version 0, edited 2 years ago by SergeyBiryukov (next)

comment:5 @markjaquith20 months ago

  • Milestone changed from Awaiting Review to Future Release

@wonderboymusic10 months ago

comment:6 @wonderboymusic10 months ago

  • Keywords has-patch added
  • Milestone changed from Future Release to 4.0

23383.diff adds an optional 2nd param to absint( $maybeint, $limit = false ) that, if set to true, will only return up to PHP_INT_MAX

comment:8 @pbearne9 months ago

I feel we should return 0 not max_int as this is more likely to tested for as an error this is what string / errors current return

Max_int will diff on 32 and 64 bit systems

paul

comment:9 @wonderboymusic8 months ago

In 28855:

Add a second optional parameter to absint() to limit the result to PHP_INT_MAX.

See #23383.

comment:10 @wonderboymusic8 months ago

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

In 28856:

absint() should always return PHP_INT_MAX if the resulting value exceeds it.

See [28855].
Fixes #23383.

comment:11 @nacin8 months ago

  • Keywords REVERT added; has-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

No. No, absolutely not. Why slow down this function?

An aside, does this even work? When can an integer ever be greater than PHP_INT_MAX?

comment:12 @wonderboymusic8 months ago

In 28857:

Revert [28856] at nacin's behest. See #23383.

comment:13 @nacin8 months ago

To be clear, I would like to see [28855] reverted too.

Incidentally, this has the potential to break things when absint() is attached to a filter that passes more than one argument. But my real objection is this is absurd to fix, at least in this way, inside this generic function.

comment:14 @SergeyBiryukov8 months ago

  • Keywords REVERT removed

[28858] missed the ticket.

comment:15 @wonderboymusic8 months ago

  • Milestone 4.0 deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

comment:16 @nacin8 months ago

  • Keywords 2nd-opinion added
  • Milestone set to Future Release
  • Resolution wontfix deleted
  • Status changed from closed to reopened

I don't think this can't be solved in some regard for some situations, I just don't think absint() is the right level.

Example: Convert $qpaged? from a string to a float, then see if it is greater than the maximum integer. (I believe this works.) Handle it specifically for pagination.

Another option is to look into handling this inside wpdb->prepare() for %d, though I'm not sure.

@SergeyBiryukov8 months ago

comment:17 @SergeyBiryukov8 months ago

  • Component changed from Database to Query
  • Keywords has-patch commit added; 2nd-opinion removed

I can reproduce the issue on a shared hosting, and 23383.2.diff fixes it for me.

I've tested other query vars, and they don't have this issue.

comment:18 @SergeyBiryukov8 months ago

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

In 28864:

Make sure the first number in LIMIT clause in WP_Query::get_posts() is always an integer.

fixes #23383.

comment:19 @pbearne8 months ago

Have look at the patch in #28559 to see the edge cases

Last edited 8 months ago by SergeyBiryukov (previous) (diff)

comment:20 @SergeyBiryukov8 months ago

  • Milestone changed from Future Release to 4.0
Note: See TracTickets for help on using tickets.