Validation and context parameter for query filter application

[kevinB]

One improperly implemented query filter can easily crash a site, and this risk only increases with use of filterable get_posts() calls throughout wp-admin.

In the most common scenario, a posts_where or posts_request filter fails to return a value. Even if the filter was intended for wp-admin but was not added selectively based on is_admin(), the site front end is crashed. It's the plugin's fault, but leaves the end user wondering why WP can't be more robust.

The associated patch eliminates that scenario by checking is_null() before accepting a filtered value. If null, the filter is ignored, with a warning triggered if WP_DEBUG is defined. Since this validation is performed in apply_query_filters() rather than the heavily-used apply_filters(), new overhead is minimal.

Another benefit of a dedicated apply_query_filters() function is the ability to apply filters selectively based on a query_context specified by supplemental get_posts() calls. This will also tend to mitigate the scope of query filtering errors if use of the context parameter becomes standard practice for get_posts() execution and filtering.

[kevinB]

new plugin API for query filtering :add_query_filter() and apply_query_filters()

[kevinB]

Related: #16661, #17019

[kevinB]

Note: add_query_filter() and apply_query_filters() are a fully equivalent to add_filter() and apply_filters(), with the validation and context check tacked on.

Query filters added via add_filter() still work and also get return value validation.

[nacin]

I'm not sure, in terms of processing, how this is much different from a callback checking context on their own. I could imagine many use cases would be to do different things on different contexts. I think better effort would be spent tagging every query in core with a context, than to come up with a new (and strict) API.

[kevinB]

Replying to nacin:

I'm not sure, in terms of processing, how this is much different from a callback checking context on their own. I could imagine many use cases would be to do different things on different contexts. I think better effort would be spent tagging every query in core with a context, than to come up with a new (and strict) API.

We're in agreement on tagging core queries with a context. Do I hear a task blessing there?

Agreed that context checking by the caller is a lesser concern. But if you look at it in terms of risk mitigation or damage control, a filter added with a specified context is immediately safer. If the added overhead is minimal, isn't that better for everyone? Otherwise every query in the request depends on the callback's successful returning of an unfiltered value in its out-of-context executions - doable but often unnecessary.

If strictness is a concern, context could be implemented as an array to let filters specify multiple contexts. And filters are always free to implement their own context check by leaving it unspecified.

Anyway, I thought the null validation issue might be compelling enough to bring caller-side context checking in on its coattails :)

[scribu]

I'm ok with a safe_apply_filters() function that does null checks, but don't think moving the context check there is a good idea.

[wonderboymusic]

The patch on #17019 adds the query_context var, I am going to suggest close here