Taxonomy Rewrite Strips Spaces from Querystring Vars

[MathSmath]

Overview
On custom taxonomy listings pages, adding pagination vars will cause any custom querystring vars to be stripped of all spaces on rewrite.


For example, the request

/my-categories/test-one/page/2/?something=one+two

is rewritten to

/my-categories/test-one/page/2/?something=onetwo

This does not happen on the initial results page. For example, if you visit

/my-categories/test-one/?something=one+two

The space stays intact in the custom querystring var.

Steps to reproduce

1) Clean install of trunk, with no plugins enabled, using the default theme.
2) Create a custom taxonomy and apply it to any content type (I used posts for my test, but this problem also occurs with custom content types). Does not seem to matter whether you use a custom slug or not when defining the taxonomy.

register_taxonomy(
	'my-categories',
	array(
		'post',
	),
	array(
		'hierarchical' => true,
		'label' => 'My Categories'
	)
);

3) Add a term to the new taxonomy using the admin, and assign it to an item or series of items.
4) Visit the archive page for that term, adding any custom variable to the querystring. This custom variable should be two words (for example, "?something=one+two")
5) Either manually, or using next/previous links, navigate to a secondary results page (ex. /page/2/), keeping your custom variable intact.

Any spaces in the custom variable are stripped. This happens no matter how the spaces are urlencoded (as a plus, as %20, or as actual spaces). Adding the custom variable to the vars list using the query_vars filter has no effect.

[MathSmath]

I should also note that this doesn't happen on standard "category" pages, or anywhere else that I can see. It only appears to happen on listings pages for custom taxonomy terms. Which makes me think it has something to do with the way that rewrites are defined in the core register_taxonomy() function.

[SergeyBiryukov]

In 3.3-trunk, this happens on regular category archives too.

The problem is in redirect_canonical():
​http://core.trac.wordpress.org/browser/tags/3.2.1/wp-includes/canonical.php#L253

$_parsed_query is Array( [something] => one two ), and add_query_arg() makes it ?something=onetwo.

18086.patch​ fixes this. Not sure if add_query_arg() should be fixed instead.

[dd32]

Moving for review

[dd32]

In core we pass encoded data into add_query_arg() in a few places, and given that's the way it's been for quite awhile, what's needed there is some documentation that it expects encoded data.

the patch looks to be the way forward here.

[dd32]

​http://unit-tests.trac.wordpress.org/changeset/446

Going with rawurlencode() here as it seems like the correct function (urlencode does spaces as + "for historical reasons", rawurlencode does it as %20)

[dd32]

In [18884]:

Encode extra query vars in Canonical Taxonomy redirections. Props SergeyBiryukov. Fixes #18086

[dd32]

In [18885]:

Document that add_query_arg() expects encoded data. See #18086