WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#18086 closed defect (bug) (fixed)

Taxonomy Rewrite Strips Spaces from Querystring Vars

Reported by: MathSmath Owned by: dd32
Milestone: 3.3 Priority: normal
Severity: normal Version:
Component: Canonical Keywords: has-patch
Focuses: Cc:

Description

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.

Attachments (1)

18086.patch (398 bytes) - added by SergeyBiryukov 3 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 MathSmath3 years ago

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.

SergeyBiryukov3 years ago

comment:2 SergeyBiryukov3 years ago

  • Keywords has-patch added

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.

comment:3 dd323 years ago

  • Component changed from General to Canonical
  • Milestone changed from Awaiting Review to 3.3
  • Owner set to dd32
  • Status changed from new to accepted

Moving for review

comment:4 dd323 years ago

  • Keywords needs-unit-tests added

comment:5 dd323 years ago

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.

comment:6 dd323 years ago

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)

comment:7 dd323 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In [18884]:

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

comment:8 dd323 years ago

In [18885]:

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

comment:9 SergeyBiryukov2 years ago

  • Keywords needs-unit-tests removed
Note: See TracTickets for help on using tickets.