Skip to content

Harden lightweight popup selector queries #1356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danieliser merged 4 commits into from
Aug 13, 2026
Merged

Harden lightweight popup selector queries #1356

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
16fdf21
Harden lightweight popup selector queries
danieliser Aug 13, 2026
e6fb392
Constrain filtered popups to query results
danieliser Aug 13, 2026
f1b3fa6
Handle selector capability and nested queries
danieliser Aug 13, 2026
47c2e26
Identify selector query by marker
danieliser Aug 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 53 additions & 7 deletions classes/Helpers.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,12 +374,18 @@ public static function popup_selectlist( $args = [] ) {
return [];
}

$post_status = 'publish';

if ( isset( $args['post_status'] ) ) {
$statuses = array_filter( (array) $args['post_status'] );
$statuses = array_values( array_unique( array_filter( array_map( 'sanitize_key', (array) $args['post_status'] ) ) ) );

if ( ! empty( $statuses ) && ! in_array( 'publish', $statuses, true ) && ! in_array( 'any', $statuses, true ) ) {
return [];
}

if ( in_array( 'publish', $statuses, true ) && in_array( 'private', $statuses, true ) ) {
$post_status = [ 'publish', 'private' ];
}
}

if ( isset( $args['popups'] ) ) {
Expand All @@ -402,7 +408,7 @@ public static function popup_selectlist( $args = [] ) {
$args,
[
'post_type' => 'popup',
'post_status' => 'publish',
'post_status' => $post_status,
'posts_per_page' => -1,
'fields' => 'ids',
'no_found_rows' => true,
Expand All @@ -411,28 +417,68 @@ public static function popup_selectlist( $args = [] ) {
]
);

if ( is_array( $post_status ) ) {
$query_args['perm'] = 'readable';
}

$use_filtered_posts = ! $query_args['suppress_filters'] && self::popup_query_result_filters_active();

if ( $use_filtered_posts ) {
$query_args['fields'] = 'all';
$query_args['update_post_meta_cache'] = false;
$query_args['update_post_term_cache'] = false;
$queried_popup_ids = [];
$popup_list = [];
$capture_queried_ids = static function ( $posts ) use ( &$queried_popup_ids ) {
foreach ( $posts as $post ) {
if ( $post instanceof WP_Post ) {
$queried_popup_ids[] = (int) $post->ID;
}
}

return $posts;
};

foreach ( get_posts( $query_args ) as $popup ) {
if ( $popup instanceof WP_Post && 'publish' === get_post_status( $popup->ID ) ) {
$popup_list[ (string) $popup->ID ] = (string) $popup->post_title;
add_filter( 'posts_results', $capture_queried_ids, PHP_INT_MIN );
Comment thread
danieliser marked this conversation as resolved.
Outdated

try {
$filtered_posts = get_posts( $query_args );
} finally {
remove_filter( 'posts_results', $capture_queried_ids, PHP_INT_MIN );
}

foreach ( $filtered_posts as $popup ) {
if ( ! $popup instanceof WP_Post || ! in_array( (int) $popup->ID, $queried_popup_ids, true ) || 'popup' !== $popup->post_type || ! in_array( $popup->post_status, (array) $post_status, true ) ) {
Comment thread
danieliser marked this conversation as resolved.
Outdated
continue;
}

if ( 'private' === $popup->post_status && ! current_user_can( 'read_post', $popup->ID ) ) {
continue;
}

$popup_list[ (string) $popup->ID ] = (string) $popup->post_title;
}

$filtered_popup_list = apply_filters( 'popup_maker/popup_title_choices', $popup_list );

return is_array( $filtered_popup_list ) ? $filtered_popup_list : $popup_list;
if ( ! is_array( $filtered_popup_list ) ) {
return $popup_list;
}

foreach ( $popup_list as $popup_id => $popup_title ) {
if ( array_key_exists( $popup_id, $filtered_popup_list ) ) {
$popup_list[ $popup_id ] = (string) $filtered_popup_list[ $popup_id ];
} else {
unset( $popup_list[ $popup_id ] );
}
}

return $popup_list;
}

static $queries = [];

$query_key = md5( wp_json_encode( $query_args ) );
$query_key = md5( wp_json_encode( $query_args ) . ':' . wp_cache_get_last_changed( 'posts' ) . ':' . get_current_user_id() );
Comment thread
danieliser marked this conversation as resolved.

if ( isset( $queries[ $query_key ] ) ) {
$popup_ids = $queries[ $query_key ];
Expand Down
210 changes: 210 additions & 0 deletions tests/php/tests/PUM_Helpers_Test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,36 @@ public function test_popup_selectlist_treats_empty_post_status_as_default() {
}
}

/**
* Explicit private choices remain capability-gated.
*
* @return void
*/
public function test_popup_selectlist_includes_only_readable_private_popups() {
$previous_user_id = get_current_user_id();
$admin_user_id = self::factory()->user->create( [ 'role' => 'administrator' ] );
$subscriber_id = self::factory()->user->create( [ 'role' => 'subscriber' ] );
$private_id = self::factory()->post->create(
[
'post_author' => $admin_user_id,
'post_type' => 'popup',
'post_status' => 'private',
'post_title' => 'Private popup',
]
);
$args = [ 'post_status' => [ 'publish', 'private' ] ];

try {
wp_set_current_user( $subscriber_id );
$this->assertArrayNotHasKey( $private_id, PUM_Helpers::popup_selectlist( $args ) );

wp_set_current_user( $admin_user_id );
$this->assertSame( [ $private_id => 'Private popup' ], PUM_Helpers::popup_selectlist( $args ) );
} finally {
wp_set_current_user( $previous_user_id );
}
}

/**
* Legacy name ordering remains ascending unless explicitly overridden.
*
Expand Down Expand Up @@ -165,6 +195,110 @@ public function query_result_filter_provider() {
];
}

/**
* Filtered results cannot inject posts outside the permitted popup set.
*
* @dataProvider invalid_filtered_post_provider
* @param string $post_type Injected post type.
* @param string $post_status Injected post status.
* @param bool $include_in_request Whether the injected post is requested.
* @return void
*/
public function test_popup_selectlist_rejects_invalid_filtered_posts( $post_type, $post_status, $include_in_request ) {
$popup_id = self::factory()->post->create(
[
'post_type' => 'popup',
'post_status' => 'publish',
'post_title' => 'Published popup',
]
);
$injected_args = [
'post_type' => $post_type,
'post_status' => $post_status,
'post_title' => 'Injected post',
];

if ( 'future' === $post_status ) {
$injected_args['post_date'] = '2035-01-01 00:00:00';
}

$injected_id = self::factory()->post->create( $injected_args );
$requested_ids = [ $popup_id ];

if ( $include_in_request ) {
$requested_ids[] = $injected_id;
}

$filter = static function ( $posts ) use ( $injected_id ) {
$posts[] = get_post( $injected_id );

return $posts;
};

add_filter( 'posts_results', $filter );

try {
$choices = PUM_Helpers::popup_selectlist( [ 'post__in' => $requested_ids ] );
} finally {
remove_filter( 'posts_results', $filter );
}

$this->assertSame( [ $popup_id => 'Published popup' ], $choices );
}

/**
* Invalid posts that a query-result filter may inject.
*
* @return array<string,array{string,string,bool}>
*/
public function invalid_filtered_post_provider() {
return [
'published page' => [ 'page', 'publish', true ],
'draft popup' => [ 'popup', 'draft', true ],
'future popup' => [ 'popup', 'future', true ],
'private popup' => [ 'popup', 'private', true ],
'unrequested published popup' => [ 'popup', 'publish', false ],
];
}

/**
* Filtered private choices remain capability-gated.
*
* @return void
*/
public function test_popup_selectlist_filters_private_posts_by_capability() {
$previous_user_id = get_current_user_id();
$admin_user_id = self::factory()->user->create( [ 'role' => 'administrator' ] );
$subscriber_id = self::factory()->user->create( [ 'role' => 'subscriber' ] );
$private_id = self::factory()->post->create(
[
'post_author' => $admin_user_id,
'post_type' => 'popup',
'post_status' => 'private',
'post_title' => 'Private popup',
]
);
$args = [ 'post_status' => [ 'publish', 'private' ] ];
$filter = static function ( $posts ) use ( $private_id ) {
$posts[] = get_post( $private_id );

return $posts;
};

add_filter( 'posts_results', $filter );

try {
wp_set_current_user( $subscriber_id );
$this->assertArrayNotHasKey( $private_id, PUM_Helpers::popup_selectlist( $args ) );

wp_set_current_user( $admin_user_id );
$this->assertSame( [ $private_id => 'Private popup' ], PUM_Helpers::popup_selectlist( $args ) );
} finally {
remove_filter( 'posts_results', $filter );
wp_set_current_user( $previous_user_id );
}
}

/**
* Suppressed query filters retain the normal raw-title fast path.
*
Expand Down Expand Up @@ -231,6 +365,40 @@ public function test_popup_selectlist_uses_two_cold_queries() {

$this->assertCount( 10, $choices );
$this->assertSame( 2, $wpdb->num_queries - $query_count );

foreach ( $popup_ids as $popup_id ) {
$this->assertFalse( wp_cache_get( $popup_id, 'posts' ) );
}
}

/**
* Same-request query caching refreshes when published popups change.
*
* @return void
*/
public function test_popup_selectlist_refreshes_cached_ids_after_post_changes() {
$args = [ 'orderby' => 'name' ];

$this->assertSame( [], PUM_Helpers::popup_selectlist( $args ) );

$popup_id = self::factory()->post->create(
[
'post_type' => 'popup',
'post_status' => 'publish',
'post_title' => 'New popup',
]
);

$this->assertSame( [ $popup_id => 'New popup' ], PUM_Helpers::popup_selectlist( $args ) );

wp_update_post(
[
'ID' => $popup_id,
'post_status' => 'draft',
]
);

$this->assertSame( [], PUM_Helpers::popup_selectlist( $args ) );
}

/**
Expand Down Expand Up @@ -297,4 +465,46 @@ public function test_popup_selectlist_reapplies_title_filter_to_cached_ids() {
remove_filter( 'popup_maker/popup_title_choices', $filter );
}
}

/**
* Dedicated title filters cannot inject unqueried choices.
*
* @return void
*/
public function test_popup_selectlist_rejects_title_filter_injections() {
$popup_id = self::factory()->post->create(
[
'post_type' => 'popup',
'post_status' => 'publish',
'post_title' => 'Published popup',
]
);
$draft_id = self::factory()->post->create(
[
'post_type' => 'popup',
'post_status' => 'draft',
'post_title' => 'Draft popup',
]
);
$results_filter = static function ( $posts ) {
return $posts;
};
$title_filter = static function ( $titles ) use ( $draft_id ) {
$titles[ $draft_id ] = 'Injected draft';

return $titles;
};

add_filter( 'posts_results', $results_filter );
add_filter( 'popup_maker/popup_title_choices', $title_filter );

try {
$choices = PUM_Helpers::popup_selectlist( [ 'post__in' => [ $popup_id ] ] );
} finally {
remove_filter( 'popup_maker/popup_title_choices', $title_filter );
remove_filter( 'posts_results', $results_filter );
}

$this->assertSame( [ $popup_id => 'Published popup' ], $choices );
}
}
Loading