WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 3 years ago

#16839 reviewing defect (bug)

Category Base Should be Slugified

Reported by: miklb Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.1
Component: Rewrite Rules Keywords: 3.2-early needs-patch
Focuses: Cc:

Description

Vanilla install of 3.1. Change category base to Foo Bar. Link generated is example.com/Foo Bar/cat (note the %20/space). Clicking link tries to access /FooBar/cat and 404's.

I see there are a few other tickets regarding categories, including #16662 but no specific mention of custom category base.

Attachments (2)

16839.diff (640 bytes) - added by garyc40 3 years ago.
sanitize category_base before saving to the db
16839.2.diff (1.2 KB) - added by garyc40 3 years ago.
category base can be 'foo/bar' as well, sanitize both category base and tag base this time

Download all attachments as: .zip

Change History (9)

comment:1 scribu3 years ago

  • Component changed from Taxonomy to Rewrite Rules
  • Keywords early needs-patch added
  • Milestone changed from Awaiting Review to Future Release

I'm surprised this wasn't caught before.

comment:2 garyc403 years ago

What's the expected behavior here? Should we sanitize the whole thing (making it foo-bar), or just make sure the generated link and the redirection points to the same URL?

comment:3 scribu3 years ago

I would say both (the first might take care of the other, though).

Version 0, edited 3 years ago by scribu (next)

garyc403 years ago

sanitize category_base before saving to the db

comment:4 garyc403 years ago

  • Keywords has-patch added; needs-patch removed

comment:5 scribu3 years ago

  • Status changed from new to reviewing

I think it would be safe to remove the following two lines as well:

if ( ! empty( $category_base ) ) 
  $category_base = $blog_prefix . preg_replace('#/+#', '/', '/' . str_replace( '#', '', $category_base ) );

since it's already taken care of.

garyc403 years ago

category base can be 'foo/bar' as well, sanitize both category base and tag base this time

comment:6 garyc403 years ago

  • Keywords 3.2-early added; early removed

Seems like slashes are permitted in category_base and tag_base. Patch attached to take this into account when sanitizing slugs.

comment:7 SergeyBiryukov3 years ago

  • Keywords needs-patch added; has-patch removed

Stumbled upon this while debugging #15256.

16839.2.diff works with the example of "Foo Bar". However using Cyrillic characters in category base gives some strange results.

Currently Cyrillic category base is not urlencoded, but seems to work in most places, except for canonical redirect (where it's just cut, resulting in incorrect URL).

With 16839.2.diff, "рубрика" is correctly sanitized to %d1%80%d1%83%d0%b1%d1%80%d0%b8%d0%ba%d0%b0, however any category request returns *all* published posts.

This is what Debug Bar shows:

Request:
%d1%80%d1%83%d0%b1%d1%80%d0%b8%d0%ba%d0%b0/gallery

Matched Rewrite Rule:
%d1%80%d1%83%d0%b1%d1%80%d0%b8%d0%ba%d0%b0/(.+?)/?$

Matched Rewrite Query:
%d1%gallery&%d1%&%d0%&%d1%&%d0%&%d0%&%d0%&category_name=

Note: See TracTickets for help on using tickets.