WordPress.org

Make WordPress Core

Opened 7 years ago

Last modified 4 months ago

#21682 reviewing defect (bug)

Rewrite endpoints are lost if a custom category or tag base is defined

Reported by: sanchothefat Owned by: DrewAPicture
Milestone: Priority: normal
Severity: normal Version: 3.4
Component: Rewrite Rules Keywords: dev-feedback has-patch needs-refresh
Focuses: Cc:
PR Number:

Description

Problem

So this little bug was winding me up for a while.

The standard approach according to the codex for adding rewrite endpoints is to call the add_rewrite_endpoint() function within the init hook. So far so good.

The problem occurs whenever WP_Rewrite::init() is called after the init hook. It resets the endpoints array and so when rewrite rules are subsequently generated through the options-permalink.php admin page the rewrite rules are unknown to the system and hence don't work.

WP_Rewrite::init() is called within WP_Rewrite::set_category_base(), WP_Rewrite::set_tag_base() and WP_Rewrite::set_permalink_structure().

In the latter it is only called if the permalink structure has changed so on first save of a change endpoints are lost. In the other 2 it is called every time if the slug doesn't match the default so rewrites are always lost.

Solutions:

  1. add an action hook to the start of WP_Rewrite::rewrite_rules() where endpoints should be added
  2. store the endpoints at the start of WP_Rewrite::init() and restore them at the end
  3. don't reset them at all

I think solution 3 would make sense, the endpoints could be defaulted to an empty array and I can't see any reason to want to reset them anyway.

I've attached a simple patch that works (for me at least).

NB. this problem may also affect the $extra_rules and $non_wp_rules but I haven't tested that theory yet.

Attachments (4)

rewrite.php.patch (741 bytes) - added by sanchothefat 7 years ago.
Patch for rewrite endpoints being wiped in certain situations
21682.diff (769 bytes) - added by ninnypants 4 years ago.
Refresh of rewrites.php.diff
21682.2.diff (817 bytes) - added by ninnypants 4 years ago.
Regenerating patch from root
21682.3.diff (1.9 KB) - added by ninnypants 4 years ago.

Download all attachments as: .zip

Change History (16)

@sanchothefat
7 years ago

Patch for rewrite endpoints being wiped in certain situations

#1 @SergeyBiryukov
7 years ago

  • Component changed from General to Rewrite Rules

#2 @scribu
7 years ago

  • Milestone changed from Awaiting Review to 3.6

#3 @DrewAPicture
7 years ago

  • Cc xoodrew@… added

#4 @l3rady
7 years ago

  • Cc scott@… added

This one just caught me out and cost me two hours. Looking forward to seeing this patch added to core soon.

#5 @nacin
6 years ago

  • Milestone changed from 3.6 to Future Release

I think this is likely dealt with in an indirect manner in [24060].

#6 @duck_
6 years ago

Related: #18450, #20171

#7 @chriscct7
4 years ago

  • Version changed from 3.4.1 to 3.4

#8 @wonderboymusic
4 years ago

  • Milestone changed from Future Release to 4.4

Let's take a glance - up for grabs

#9 @DrewAPicture
4 years ago

  • Keywords needs-refresh added

Patch needs to be regenerated. @wonderboymusic still want to pursue this for 4.4?

@ninnypants
4 years ago

Refresh of rewrites.php.diff

#10 @DrewAPicture
4 years ago

  • Owner set to DrewAPicture
  • Status changed from new to reviewing

@ninnypants
4 years ago

Regenerating patch from root

#11 @wonderboymusic
4 years ago

  • Milestone changed from 4.4 to Future Release

breaks unit tests

@ninnypants
4 years ago

#12 @ninnypants
4 years ago

21682.3.diff fixes the issues with the oldSlugRedirect.php tests by making sure items in the $endpoints array are unique fixing the issues with the endpoint slug being appended to generated urls multiple times.

The test that failed in addRewriteEndpoint.php looks like it was relying on this bug to pass. The previously added endpoint of foo add_rewrite_endpoint( 'foo', EP_ALL, true ); being higher up in the generated rewrite rules array causing it to match when the endpoint in the test was http://example.org/page_endpoint/foo/ like it would with a normal url. Changing the test url to http://example.org/page_endpoint/baz/ allows the test to pass without other endpoints interfering.

Note: See TracTickets for help on using tickets.