Make WordPress Core

Opened 5 years ago

Last modified 2 weeks ago

#20383 accepted defect (bug)

Strip trailing punctuation with canonical URLs

Reported by: nacin Owned by: SergeyBiryukov
Milestone: 4.8 Priority: normal
Severity: normal Version:
Component: Canonical Keywords: has-patch dev-feedback has-unit-tests
Focuses: Cc:


A follow-up to #7537 where we removed %20 and " " from the end of URLs, we should try to remove all sorts of punctuation from the end of a URL, both URL-encoded and decoded.

Example bad URLs we should surely be able to resolve:





More difficult when there is no trailing slash in the permalink structure (or the link requested, regardless of permalink structure), but if is_404() is taken into account, it should be doable to trail various pieces of punctuation and see if we can get things to resolve.

Attachments (2)

20383.patch (659 bytes) - added by joostdevalk 3 years ago.
20383.2.patch (1.4 KB) - added by lancewillett 3 months ago.
Updated patch to catch more punctuation; adds unit tests

Download all attachments as: .zip

Change History (9)

#1 @n3k4
5 years ago

  • Cc hellokane@… added

3 years ago

#2 @joostdevalk
3 years ago

  • Keywords has-patch reporter-feedback added
  • Owner set to joostdevalk
  • Status changed from new to accepted

I added the suggested ones and a couple more into the same rewrite in which we fixed the trailing space. WordPress doesn't allow for these characters in the URL so I think it's safe to assume that if they're in there, at the end of the URL, it's a mistake.

Please do tell me if you disagree, if so I could move this to redirect_guess_404_permalink without issue of course.

#3 @chriscct7
18 months ago

  • Keywords dev-feedback added; reporter-feedback removed

3 months ago

Updated patch to catch more punctuation; adds unit tests

#4 @lancewillett
3 months ago

  • Keywords has-unit-tests added

.2 patch adds a bit more cases for trailing punctuation, and adds unit tests.

#5 @joostdevalk
3 months ago

That looks like what I meant. @SergeyBiryukov could you have a look?

#6 @lancewillett
2 weeks ago

@SergeyBiryukov Can you review and test the patch, please?

#7 @SergeyBiryukov
2 weeks ago

  • Milestone changed from Future Release to 4.8
  • Owner changed from joostdevalk to SergeyBiryukov
Note: See TracTickets for help on using tickets.