Wrong page loaded requesting custom registered query_vars when correcting is_* for page_on_front requests

[jondavis]

• Install vanilla WP 3.0.4.
• Register a new 'qv_test' query var in the default theme's function.php file.

	function custom_query_vars_test ($vars) {
		$vars[] = 'qv_test';
		return $vars;
	}
	add_filter('query_vars','custom_query_vars_test');

• Create a 'Home' page.
• Under Settings → Reading set the 'Front page displays' setting to 'A static page (select below' and set the 'Front page:' setting to the 'Home' page.
• Load the front end page.
• Add the 'qv_test' query var to the request (e.g. ​http://blogurl.com/?qv_test=test)

The wrong page is loaded.

Adding an invalid query_var (one that is not registered) continues to correctly load the page.

The issue occurs regardless of permalink configuration.

This issue appears related to #12047 and is either a regression from the fixes applied to that issue, or is simply case not covered by the fixes. Changes in revision [14445] also relate to #12047.

This issue still exists as of at least 3.0.4 up to 3.1-RC3.

[jondavis]

Suggested fix (works for me)

[markjaquith]

Can reproduce the issue on 3.1-RC3, 3.0.4, and 3.0. As it is not a regression from 3.0, it'll have to wait until 3.2.

[Otto42]

This looks like it got forgotten. Punting to 3.3-early, perhaps? Seems simple enough.

[Otto42]

Whoops, changing the version was not by design.

[nacin]

if ( empty($_query) || !array_diff( array_keys($_query), array('preview', 'page', 'paged', 'cpage') ) ) {

Maybe someone with more knowledge of the history here can shed some light on what this is supposed to do, and why it avoids normal query vars.

[dd32]

Looking at the changes I made there, This case is simply a use case which was missed. It would cause the issue before the changes were made, and after. That code block is simply checking if any public query var other than those listed in array('preview', 'page', 'paged', 'cpage') are set, and if so, decides it's not the front page being viewed (as those query vars most likely map somewhere).

(also note, for anyone playing along at home $_query only contains registered query vars, /?something_not_registered=1 would still be mapped to the front page)

Jumping back before the changesets mentioned here, to [4956] where the last major change was, the bug at that point was that only / and /?preview=true would map to the front page, paging(of the page, and of comments) wouldn't work on static front page, and any public query vars being appended would load the Blog posts.

---

Fixing this bug however, The attached patch is basically ( empty($_query) || true ) as that array_diff should *always* return empty. I can't seem to find a use-case (with pretty permalinks enabled) which requires that if() branch to be there at all anymore, meaning, I think something else has changed in the query parsing in the last 4 years since everything -seems- to be working for me without the if. Things to check that still work with it removed: Endpoints, particually Feeds(posts, and comments)

[jondavis]

The comments above being accurate, the issue still remains. After discussion with dd32 I will work on and submit unit tests to prove the problem, then follow-up with a more appropriate fix.

[dd32]

So the main issues here are:

• We assume that if a Query var (other than the whitelisted vars: preview, page, paged, cpage) is set, then it's a archive/loop modifying query var
• Query Vars do not directly map to a Query modification, It can affect plugins/Themes instead
• We have no filters available to say that a query var is a non-query modifying query var
• Changing this could have backwards compatibility issues -

To expand on that last one, If we simply wrapped a filter around the array of whitelisted vars, a plugin could do something such as this:

add_filter( 'singular_query_vars', function( $vars ) {
    $vars[] = 'qv_test';
    return $vars;
} );

but, that would still require multiple filters (One to add the query var, and one to set the 'context' of the query var.

Perhaps what we really need here, Is a better way to register query vars, and provide some kind of context at the same time (ie. "This query var maps to this WP_Query tag", "This query var only modifies content on a singular view", etc)

[jondavis]

test/query.php unit test for page_on_front query vars

[jondavis]

It seems to me that creating a formal API for this situation is a bit overkill. Seems by designthat query vars shouldn't be used as a registry for plugins/themes to use as custom query string arguments. My own confusion on the issue was in my rush to move to doing things the "WordPress way" I was attempting to use the query vars as a whitelist of registered $_GET variables unnecessarily.

That said, there may be edge-cases I (or apparently anyone else) are not aware of where plugins/themes do need to use registered query vars but still expect to get page_on_front. I can't even imagine why that might be. In any event, for that use case I submitted test_page_on_front_query_var.diff​ as a use case to test against should the powers-that-be decide it is a worthwhile design consideration.

Given my new understanding of the design intent, I will be changing our plugin code to just use $_GET variables and ensure proper input sanitization is occuring.

As an action item, perhaps the result from this ticket is to update the codex to explicitly direct developers in the proper/expected/intended use-case strictly for modifying the WP_Query, and avoid any possible misunderstanding that WP_Query query vars should not be used for general purpose QUERY_STRING "query vars".

[jondavis]

Closing as invalid.

[chipbennett]

Reopening, as I found what I believe is a perfectly valid use-case for registering query variables on the front page, where the query variables aren't intended to modify the loop query.

While developing a Plugin to allow for front-end modification of Theme settings, as a Theme options "demo", I ran into this issue. The Plugin properly registers the query variables, and then sanitizes/validates and uses them, if set, to filter the front-end-modified Theme settings. Everything works as expected, unless the front page is set to display a static page. In that case, when the form is submitted, the page is refreshed and instead of displaying page_on_front, displays the blog posts index.

If this is too localized, feel free to close again. I just thought it worthy of consideration. I'm sure there are other, related use cases.

Note, should anyone else encounter this issue, the following work-around works for me:

function themeslug_force_static_front_page( $query ) {
	if ( $query->is_main_query() ) {
		if ( 'page' == get_option( 'show_on_front' ) ) {
			if ( '' != get_query_var( 'foobar' ) ) { // Registered custom query var
				$query->set( 'page_id', get_option( 'page_on_front' ) );
				$query->is_page = true;
				$query->queried_object = get_post(get_option('page_on_front') );
			}
		}
	}
}
add_action( 'pre_get_posts', 'themeslug_force_static_front_page' );

Edit (2)

Edited to add a functional workaround, that results in is_front_page() returning true.

[Otto42]

I'm kinda confused here with this:

where the query variables aren't intended to modify the loop query

If the variables aren't intended to modify the main Loop, then they don't actually need to be registered as query_vars, do they?

The only thing registering a query_var does is to make the main Query pick it up and store it and have it available via get_query_var and such. Not registering it doesn't make a variable in the URL unavailable, as it's still there via the GET global and so forth.

So, the only reason to register a query_var is to have it available to the main query in some way, which would then be used to change the results of the main query, thus affecting the main Loop.

I think that this problem is primarily caused by people using the query_var system as an general means of grabbing data from the URL, when that isn't really the main intention of it. PHP already has ways to grab data from the URL, the query_var stuff is there to affect the query, not for any other purpose.

Try simply not registering your input URL variables as query_vars, unless they are affecting the Loop. Then they won't impact anything.