WordPress.org

Make WordPress Core

Opened 17 months ago

Last modified 10 months ago

#39535 new defect (bug)

Canonical redirects disallow tag named comments

Reported by: dwc Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Canonical Keywords: has-patch needs-testing needs-unit-tests
Focuses: Cc:

Description

The redirect_canonical function includes a regular expression replacement which effectively disallows tags named "comments" to have an RSS feed:

https://github.com/WordPress/WordPress/blob/master/wp-includes/canonical.php#L285

For example:

/blog1/tag/comments/feed/

This is redirected to:

/blog1/tag/feed/

Could / should the regular expression on line 285 could pay attention to the first placeholder it matched (comments/?) or check whether the page is a tag request?

Attachments (2)

39535.diff (1.4 KB) - added by asalce 17 months ago.
39535.unit-test.patch (1.8 KB) - added by asalce 16 months ago.

Download all attachments as: .zip

Change History (16)

@asalce
17 months ago

#1 @asalce
17 months ago

  • Keywords has-patch needs-testing added

@dwc I attached a patch which should work. The safest way was to check against the generated site-wide comments feed. Could you take look and let me know if that solves your issue?

#2 @asalce
17 months ago

@dd32 @markjaquith I looked a bit at the source code and this seems like it has been around for a while. Sorry to pull you in on this one, not sure who would be assigned :)

Here is the commit on Git: https://github.com/WordPress/WordPress/commit/ecffc4649f4cb90b43fe0b38ab0f4b18967cb3fd

I was able to reproduce the bug myself, real easy to do so. Just create a tag named "comments" and then try to load the rss feed for the tag: http://URL/tag/comments/feed/

Edit: tagged the canonical gods

Last edited 16 months ago by asalce (previous) (diff)

This ticket was mentioned in Slack in #core by asalce. View the logs.


17 months ago

#4 @swissspidy
17 months ago

  • Keywords needs-unit-tests added
  • Version 4.7 deleted

#5 @dwc
17 months ago

@asalce Thanks for the patch! I've confirmed it works in my setup.

#6 @asalce
16 months ago

@swissspidy I submitted the unit-test, The best way I could test this out was to test if the query vars were set correctly after a go_to("/tag/comments/feed/") the query vars should have a "feed" and "tag" should equal "comments" , I also added some other tests to make sure tests were running correctly.

@dwc perfect! :)

This ticket was mentioned in Slack in #core by asalce. View the logs.


16 months ago

This ticket was mentioned in Slack in #core by asalce. View the logs.


16 months ago

#9 follow-up: @SergeyBiryukov
16 months ago

  • Milestone changed from Awaiting Review to 4.8

#10 in reply to: ↑ 9 @asalce
15 months ago

Replying to SergeyBiryukov:

I ran into this bug with another site, Any chance this could be moved into 4.7.x ?

#11 @javix
13 months ago

I have tested this patch successfully, it works for me.

This ticket was mentioned in Slack in #core by obenland. View the logs.


12 months ago

#13 @obenland
12 months ago

  • Milestone changed from 4.8 to Future Release

This ticket was mentioned in Slack in #core by asalce. View the logs.


10 months ago

Note: See TracTickets for help on using tickets.