Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 20 months ago

#12271 closed defect (bug) (fixed)

Using # in permalinks breaks preg_match in parse_request

Reported by: timsogard Owned by: ryan
Milestone: 3.0 Priority: normal
Severity: normal Version: 2.9.2
Component: Permalinks Keywords:
Focuses: Cc:


When using a hash symbol (#) in a custom permalink structure the parse_request function fails with the error "Unmatched modifier". This is due to the pattern being delimited by hash symbols (#s).

The call to preg_match is on lines 210 and 211 in wp-includes/classes.php:

   if (preg_match("#^$match#", $request_match, $matches) ||
      preg_match("#^$match#", urldecode($request_match), $matches)) {
      // Got a match.

Suggested, probably weak as it's been a while for me and PHP, fix:

Using str_replace on '#'s into '\#'s.

   if (preg_match("#^" . str_replace("#","\#",$match) . "#", $request_match, $matches) ||
      preg_match("#^" . str_replace("#","\#",$match) . "#", urldecode($request_match), $matches)) {
      // Got a match.

I am using hashes in my URLs because I am loading my WP blog with an ajax loader (it loads in a div).

Other possible fixes I don't have the werewithall to test(/benchmark) at the moment:

use a symbol from the intersection of the sets of "may show up in a URL" and "is a valid PCRE pattern delimeter" as the pattern delimeter.

wrap $match in preg_quote, which will properly escape any of the regular expression syntax chars ($, *, ?, etc.)

Change History (10)

#1 @timsogard
6 years ago

  • Cc timsogard added

#2 @miqrogroove
6 years ago

My first impression is that this is a permastruct bug and should not be hacked into the rewrite system. The cure would be worse than the disease. I recommend someone should find out if permastructs are aware of the need to sometimes-escape for the rewrite rules.

#3 @nacin
6 years ago

The bug here is that WordPress is allowing you to include an anchor in your permastruct.

The permalink structures are used in two places: to generate links, and to generate rewrite rules. To do what you're trying to do, you'd need to sever them. (Why in a moment.) The best way to sever them I think would be to use a typical permastruct (i.e. without /#/) and then use filters to modify the links to use /#/. The latter could also be done via a JS bind (it really all depends how you are planning out your AJAX architecture).

Now, here's why you have to sever them. URI fragments are handled browser-side. They are never passed to the server. If this change was made, you'd see that no rewrite rules would ever match, as WP would literally see no URL. Via ajax you have to send a sound request to WP, but you're probably not getting that far due to the error. Of course, your AJAX application could already be set up to pass WP the right string.

My point is, try to think of the permalink structure as what generates the rewrite rules, not the links, then fix the links. The other way around doesn't really apply because rewrite rules should never expect an anchor.

So it sounds like we should be fixing options-permalink.php to reject anchors in structures. (Again, think rewriting, not links.)

Does that sound right, or have I not thought this through entirely? (More coffee?)

#4 @nacin
6 years ago

(Also, you won't be getting CC e-mails. To do that, you have to add your e-mail address to your Trac profile, then check the box on a ticket.)

#5 @dd32
6 years ago

preg_quote($match, '#'); is your friend in this case, Anything that takes a variable and places it within the match should be run through preg_quote at one stage or another.

nacin: I do believe that simply adding your username to the CC should be adequate, Theres no need for the full email address if its tied to the user account correctly.

#6 @dd32
6 years ago

  • Milestone changed from Unassigned to 3.0

#7 @dd32
6 years ago

nacin just pointed out to me on IRC why preg_quote wont work here.. Escaping # is the easy solution here, Unless theres a better delimiter which isnt potentialy used within a url.

# should never be sent to the server directly, but perhaps it could be sent urlencoded? which would then work in the rewrite code.. I'm not sure.

#8 @timsogard
6 years ago

nacin, thanks for the explanation about parse_request as well as reminding me why it was my rewrite rules weren't working (specifically that a request was never making it as far as apache for an anchor). I was having some trouble with rewrites and had been editing them by hand and it makes some sense as to why now.

Also in the spirit of properly "building up" my page by layering the ajax functionality over a working non-javascript-requiring base, it does make more sense as you said to modify the links at the point of actual load.

This now makes this a non-issue for me as I will be going back to a permalink struct that doesn't include a hash, though I hope this ticket still does have some benefit to the project.

Thank you

#9 @nacin
6 years ago

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

(In [13980]) Don't allow # in custom permalink structures and cat/tag bases. Improve some MS branching. fixes #12271

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.

20 months ago

Note: See TracTickets for help on using tickets.