fix(2meet-data-optimizer): rewrite only standalone posts.ID tie-breaks in ORDER BY

- flat_first and the copy route share ties_on_post_id(): a posts.ID term
  alone (optionally with a direction) reads the flat post_id; posts.ID inside
  an expression a plugin added (FIELD(), IN) is left alone — inserting a
  direction there made the SQL invalid and the page empty
- PCRE failures return the text given instead of '' (the query kept its
  shape rather than becoming an empty request)
- Found in the 2.4 correctness review (Medium) and security review (Low L1)
- Verify: 2 new integration tests (the flat-first one runs the SQL), router suites 69 OK
- PR: PR-131
This commit is contained in:
2026-09-28 00:29:30 +08:00
parent bed3f906da
commit 33addd1e49
3 changed files with 79 additions and 24 deletions
+49 -20
View File
@@ -778,22 +778,58 @@ class TMDO_Post_Query_Router {
if ( empty( $copy ) ) { if ( empty( $copy ) ) {
return $request; return $request;
} }
return self::ties_on_post_id( self::without_group_by( $request ), self::table_alias( $copy['post_type'], $copy['group'] ), '' );
}
/**
* ORDER BY terms that are posts.ID alone (with or without a direction)
* read as the equal flat post_id — in $direction, or each in its own when
* $direction is '' — and only once. posts.ID inside an expression a plugin
* added (FIELD(), IN …) is left alone: a direction inserted there breaks
* the SQL.
*
* @param string $request SELECT statement.
* @param string $alias Flat table alias.
* @param string $direction 'ASC', 'DESC' or '' (keep each term's own).
* @return string
*/
private static function ties_on_post_id( string $request, string $alias, string $direction ): string {
global $wpdb; global $wpdb;
$tie = '`' . self::table_alias( $copy['post_type'], $copy['group'] ) . '`.`post_id` '; $tie = "`{$alias}`.`post_id` ";
return (string) preg_replace_callback( $id = '`?' . preg_quote( $wpdb->posts, '/' ) . '`?\.`?ID`?';
return self::replaced(
preg_replace_callback(
'/\bORDER BY\b(?!.*\bORDER BY\b).*$/s', '/\bORDER BY\b(?!.*\bORDER BY\b).*$/s',
static function ( $m ) use ( $wpdb, $tie ) { static function ( $m ) use ( $id, $tie, $direction ) {
$order = (string) preg_replace_callback( $order = self::replaced(
'/`?' . preg_quote( $wpdb->posts, '/' ) . '`?\.`?ID`?(?!\w)(?:\s+(ASC|DESC)\b)?/i', preg_replace_callback(
static fn( $id ) => $tie . ( 'DESC' === strtoupper( $id[1] ?? '' ) ? 'DESC' : 'ASC' ), '/(?<=ORDER BY |,)(\s*)' . $id . '(?:\s+(ASC|DESC)\b)?(?=\s*(?:,|\bLIMIT\b|$))/i',
static fn( $t ) => $t[1] . $tie . ( '' !== $direction ? $direction : ( 'DESC' === strtoupper( $t[2] ?? '' ) ? 'DESC' : 'ASC' ) ),
$m[0]
),
$m[0] $m[0]
); );
return (string) preg_replace( '/(,\s*' . preg_quote( $tie, '/' ) . '(ASC|DESC))(\s*,\s*' . preg_quote( $tie, '/' ) . '\2)+/', '$1', $order ); return self::replaced( preg_replace( '/(,\s*' . preg_quote( $tie, '/' ) . '(ASC|DESC))(\s*,\s*' . preg_quote( $tie, '/' ) . '\2)+/', '$1', $order ), $order );
}, },
self::without_group_by( $request ) $request
),
$request
); );
} }
/**
* A preg_* result, or the text it was given when PCRE failed (backtrack
* or JIT stack limit on a very long request): the query stays unrouted
* in shape rather than empty.
*
* @param string|null $result preg_* return value.
* @param string $fallback Input text.
* @return string
*/
private static function replaced( ?string $result, string $fallback ): string {
return $result ?? $fallback;
}
/** /**
* WordPress groups by posts.ID for any tax or meta query. With a flat row * WordPress groups by posts.ID for any tax or meta query. With a flat row
* (one per post) as the only join there is nothing to collapse, and the * (one per post) as the only join there is nothing to collapse, and the
@@ -808,7 +844,7 @@ class TMDO_Post_Query_Router {
if ( 1 !== preg_match_all( '/\bJOIN\b/i', $request ) ) { if ( 1 !== preg_match_all( '/\bJOIN\b/i', $request ) ) {
return $request; return $request;
} }
return (string) preg_replace( '/\s+GROUP BY\s+`?' . preg_quote( $wpdb->posts, '/' ) . '`?\.`?ID`?(?=\s)/', ' ', $request, 1 ); return self::replaced( preg_replace( '/\s+GROUP BY\s+`?' . preg_quote( $wpdb->posts, '/' ) . '`?\.`?ID`?(?=\s)/', ' ', $request, 1 ), $request );
} }
/** /**
@@ -882,16 +918,9 @@ class TMDO_Post_Query_Router {
// post_id, once, in the sort's direction: a (column, post_id) index then // post_id, once, in the sort's direction: a (column, post_id) index then
// returns rows in order with no filesort. Ties are ordered by ID the same // returns rows in order with no filesort. Ties are ordered by ID the same
// way as the column (natively ID DESC); pagination stays stable. // way as the column (natively ID DESC); pagination stays stable.
$tie = "`{$alias}`.`post_id` " . ( 'ASC' === strtoupper( (string) $sort['order'] ) ? 'ASC' : 'DESC' ); $direction = 'ASC' === strtoupper( (string) $sort['order'] ) ? 'ASC' : 'DESC';
$flat_first = preg_replace_callback( $flat_first = self::ties_on_post_id( $flat_first, $alias, $direction );
'/\bORDER BY\b(?!.*\bORDER BY\b).*$/s', return self::replaced( preg_replace( '/^\s*SELECT\s/', 'SELECT STRAIGHT_JOIN ', $flat_first, 1 ), $request );
static function ( $m ) use ( $wpdb, $tie ) {
$order = (string) preg_replace( '/`?' . preg_quote( $wpdb->posts, '/' ) . '`?\.`?ID`?(?!\w)(\s+(ASC|DESC)\b)?/i', $tie, $m[0] );
return (string) preg_replace( '/(,\s*' . preg_quote( $tie, '/' ) . ')(\s*,\s*' . preg_quote( $tie, '/' ) . ')+/', '$1', $order );
},
(string) $flat_first
);
return (string) preg_replace( '/^\s*SELECT\s/', 'SELECT STRAIGHT_JOIN ', (string) $flat_first, 1 );
} }
/** /**
@@ -901,7 +930,7 @@ class TMDO_Post_Query_Router {
* @return string * @return string
*/ */
private static function without_found_rows( string $request ): string { private static function without_found_rows( string $request ): string {
return (string) preg_replace( '/\bSQL_CALC_FOUND_ROWS\s+/', '', $request, 1 ); return self::replaced( preg_replace( '/\bSQL_CALC_FOUND_ROWS\s+/', '', $request, 1 ), $request );
} }
/** /**
@@ -100,6 +100,13 @@ class PostQueryRouterCopyTest extends TestCase {
$this->assertStringEndsWith( 'ORDER BY ' . self::ALIAS . '.`post_date` DESC LIMIT 0, 10', $this->request( '', '' ) ); $this->assertStringEndsWith( 'ORDER BY ' . self::ALIAS . '.`post_date` DESC LIMIT 0, 10', $this->request( '', '' ) );
} }
public function test_copy_route_leaves_posts_id_inside_expressions_alone(): void {
global $wpdb;
$sql = $this->request( '', ', FIELD({posts}.ID,5,9) DESC, ({posts}.ID IN (3,4)) DESC, {posts}.ID DESC' );
$this->assertStringEndsWith( "`post_date` DESC, FIELD({$wpdb->posts}.ID,5,9) DESC, ({$wpdb->posts}.ID IN (3,4)) DESC, " . self::ALIAS . '.`post_id` DESC LIMIT 0, 10', $sql );
}
public function test_copy_route_keeps_group_by_with_another_join(): void { public function test_copy_route_keeps_group_by_with_another_join(): void {
global $wpdb; global $wpdb;
$sql = $this->request( " LEFT JOIN wp_term_relationships ON ({$wpdb->posts}.ID = wp_term_relationships.object_id)" ); $sql = $this->request( " LEFT JOIN wp_term_relationships ON ({$wpdb->posts}.ID = wp_term_relationships.object_id)" );
@@ -272,6 +272,25 @@ class PostQueryRouterFoundRowsTest extends TestCase {
$this->assertSame( array( '2', '3' ), $wpdb->get_col( $sql ) ); $this->assertSame( array( '2', '3' ), $wpdb->get_col( $sql ) );
} }
/**
* Only standalone posts.ID terms are tie-breaks: inside an expression a
* plugin added (pinning posts with FIELD() or IN) the direction must not be
* inserted, or the SQL breaks (2.4 correctness review).
*/
public function test_flat_first_leaves_posts_id_inside_expressions_alone(): void {
global $wpdb;
list( $router, $query, $request ) = $this->sorted();
$pinned = str_replace( ' LIMIT 0, 10', ", FIELD({$wpdb->posts}.ID,3,2) DESC, ({$wpdb->posts}.ID IN (2)) DESC LIMIT 0, 10", $request );
$sql = $router->posts_request( $pinned, $query );
$this->assertStringStartsWith( 'SELECT STRAIGHT_JOIN ', $sql );
$this->assertStringContainsString( "FIELD({$wpdb->posts}.ID,3,2) DESC, ({$wpdb->posts}.ID IN (2)) DESC LIMIT", $sql );
$wpdb->last_error = '';
$this->assertSame( $wpdb->get_col( str_replace( 'SQL_CALC_FOUND_ROWS ', '', $pinned ) ), $wpdb->get_col( $sql ) );
$this->assertSame( '', $wpdb->last_error );
}
public function test_date_ordered_filtered_request_keeps_wordpress_join_order(): void { public function test_date_ordered_filtered_request_keeps_wordpress_join_order(): void {
list( $router, $query, $request ) = $this->routed(); list( $router, $query, $request ) = $this->routed();