WordPress.org

Make WordPress Core

Opened 22 months ago

Last modified 18 months ago

#21167 reopened enhancement

Problem with custom permalinks

Reported by: kaffeeringe Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.4
Component: Permalinks Keywords:
Focuses: Cc:

Description

I am currently moving my blog (http://kaffeeringe.de/) to Wordpress and I don't want to break my old URLs (Cool URLs don't change). The problem is, that these custom URLs don't work in Wordpress 3.4.1:

/blog/%postname%/%post_id%.html works
/blog/%postname%.%post_id%.html (which I need) doesn't work.

It doesn't work with the . in between these parameters…

I hope somebody is able to help me.

Attachments (3)

21167.patch (667 bytes) - added by SergeyBiryukov 22 months ago.
21167.2.patch (502 bytes) - added by SergeyBiryukov 22 months ago.
21167-unit-test.patch (3.1 KB) - added by kurtpayne 21 months ago.
Unit test

Download all attachments as: .zip

Change History (28)

comment:1 Ipstenu22 months ago

  • Keywords close added
  • Resolution set to invalid
  • Status changed from new to closed

This isn't a bug - Trac is for submitted bugs in WordPress :)

You should post in the WordPress forums instead (and I would in this case use a .htaccess to parse /blog/%postname%.%post_id%.html into /blog/%postname%/%post_id%/ (or anything else you wanted)

http://wordpress.org/support if you need more help.

comment:2 helenyhou22 months ago

  • Milestone Awaiting Review deleted

comment:3 nacin22 months ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

Sounds like a bug to me. We allow random characters in a URL permalink structure, like ".html". Would be interesting to see where and why this fails.

comment:4 helenyhou22 months ago

  • Keywords close removed
  • Milestone set to Awaiting Review

comment:5 nacin22 months ago

  • Type changed from defect (bug) to enhancement

Looks like our support for this is quite tenuous. Many combinations work, but mostly by happenstance.

comment:6 Ipstenu22 months ago

/%postname%.%post_id%/ fails

/%year%.%monthnum%.%postname%/ works
/%post_id%.%postname%/ works
/%postname%.%year%/ works

Well now that's just odd...

Version 0, edited 22 months ago by Ipstenu (next)

comment:7 sirzooro22 months ago

  • Cc sirzooro added

I checked this and found that dot is not escaped when generating rules, e.g.:

  '([^/]+).([0-9]+)(/[0-9]+)?/?$' => 'index.php?name=$1&p=$2&page=$3'

Other special RX chars may also cause problems here - need to check if patch will work with them too.

SergeyBiryukov22 months ago

comment:8 SergeyBiryukov22 months ago

  • Keywords has-patch added

21167.patch is an attempt to escape the special characters which are currently allowed in permalink structure and are acceptable in terms of valid URLs.

Seems to work in the following cases:

/%postname%.%post_id%/
/%postname%+%post_id%/
/%postname%$%post_id%/
/%postname%(%post_id%)/

comment:9 follow-up: dd3222 months ago

Is there any reason why we can't just run the permastructure through preg_quote()?

> preg_quote( '.+/()$[]%' );
\.\+/\(\)\$\[\]%

comment:10 in reply to: ↑ 9 SergeyBiryukov22 months ago

Replying to dd32:

Is there any reason why we can't just run the permastructure through preg_quote()?

Thanks, that indeed makes more sense.

comment:11 SergeyBiryukov22 months ago

  • Keywords needs-unit-tests added
  • Milestone changed from Awaiting Review to 3.5

SergeyBiryukov22 months ago

kurtpayne21 months ago

Unit test

comment:12 kurtpayne21 months ago

  • Cc kpayne@… added

Unit test added, 21167.2.patch looks good. Let me know if there are any test cases missing.

comment:13 tlovett121 months ago

  • Owner set to tlovett1
  • Status changed from reopened to accepted

comment:14 tlovett121 months ago

  • Owner tlovett1 deleted
  • Status changed from accepted to assigned

comment:15 nacin19 months ago

  • Keywords commit added

comment:16 wonderboymusic18 months ago

@kurtpayne - was that unit test patch committed?

comment:17 SergeyBiryukov18 months ago

  • Keywords needs-unit-tests removed

comment:19 ryan18 months ago

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

comment:20 xknown18 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

The fix is probably incomplete. We should be passing the second argument on preg_quote and use the same delimiter where rewrite rules are being used.

For example, having the following permalink structure "/%post_id%#foo" (yes, probably unrealistic) would produce an invalid regular expression in wp-includes/class-wp.php, where # is used as a delimiter.
http://core.trac.wordpress.org/browser/trunk/wp-includes/rewrite.php#L344
http://core.trac.wordpress.org/browser/trunk/wp-includes/class-wp.php#L204

comment:21 ryan18 months ago

  • Keywords has-patch commit removed

comment:22 nacin18 months ago

Seems to me like this should be preg_quote()'d before getting used in a regexp, rather than on save.

comment:23 jeremyfelt18 months ago

Using preg_quote() breaks rewrites for post types with hyphens. I'm not entirely sure how supported that is to begin with, but I know I need to convert a few now. :)

comment:24 ryan18 months ago

In 22403:

Revert [22365]. see #21167

comment:25 ryan18 months ago

  • Milestone changed from 3.5 to Future Release
Note: See TracTickets for help on using tickets.