[wp-trac] [WordPress Trac] #48223: parse_request(): When request has multiple matching rewrite rules, and matched rule returns 404 - iterate to next rewrite rule

[WordPress Trac]

#48223: parse_request(): When request has multiple matching rewrite rules, and
matched rule returns 404 - iterate to next rewrite rule
------------------------------------+------------------------------
 Reporter:  apedog                  |       Owner:  (none)
     Type:  defect (bug)            |      Status:  new
 Priority:  normal                  |   Milestone:  Awaiting Review
Component:  Rewrite Rules           |     Version:
 Severity:  normal                  |  Resolution:
 Keywords:  has-patch dev-feedback  |     Focuses:
------------------------------------+------------------------------

Comment (by apedog):

 So in attempting to write a patch for this ticket I ended up having to re-
 factor class WP. So the attached patch is a little "bigger" than just a
 patch for this ticket and might justify opening a separate ticket for.

 -----

 I've added a patch (48223.wp-refactor.patch) that does multiple things.
 - Refactors class WP
 - Introduced a new class Request_Environment
 - Added code to allow parsing multiple rules if the first one returns
 empty/404

 == Request_Environment
 Request_Environment is the same as the old WP environment class but is
 decoupled from {{{$_SERVER}}}, {{{$_GET}}}, {{{$_POST}}}, {{{global
 $wp_the_query}}} etc.
 It does not send headers. It does not set globals.

 Request_Environment can ideally be used within a virtual WordPress
 environment, to parse any request string, without setting globals, without
 sending headers and using global $wp_query.

 It does not accesses {{{$_SERVER}}} and {{{global $wp_query}}} directly,
 but instead uses instance variables {{{$this->server}}},
 {{{$this->wp_query}}} etc.

 It introduces a new method {{{setup_request_environment()}}} that accepts
 {{{$_SERVER}}} (or a similarly formatted array), {{{$_GET}}}, {{{$_POST}}}
 (or equivalent array), a {{{WP_Query}}} object and a {{{WP_Rewrite}}}
 instance. And stores them to local/instance variables.

 == Refactor class WP
 WP is now an extension of Request_Environment.
 It uses the new {{{setup_request_environment()}}} method to populate
 instance variables with actual globals {{{$_SERVER}}}, {{{$_GET}}}, global
 {{{$wp_query}}}, {{{$wp_rewrites}}}.
 It also sends headers and sets globals after parsing is done as before.

 All methods that were available before are still available now. Either in
 {{{class WP}}} or in {{{class Request_Environment}}}.

 All hooks that were available before are still available now. Except some
 minor changes detailed below.

 == parse_request()
 In order to better work with the WP environment object and
 {{{parse_request()}}} method, I've broken it up into 3 separate
 chunks/methods - detailed in the section below.

 == Multiple Rules Matching
 In order to accomodate multiple rule matching (the main impetus behind
 this exercise), I've had to break up {{{parse_request()}}} into smaller
 more managable chunks:
 - {{{_match_rewrite_rules()}}}: gets all rules that match request. Does
 not break after first match and keeps them in an array.
 - {{{_match_single_rule()}}}: same algorithm as before.
 - {{{_parse_request()}}}: actual parsing of the current rule being
 matched. same as before.

 I've added a hook {{{'match_multiple_rules'}}} and a new function
 _reparse_request().
 If {{{match_multiple_rules}}} is set to {{{true}}}, and the first
 {{{query_posts()}}} returns empty or 404, {{{_reparse_request()}}} will
 clear the WP_Query object, will parse the next matching rule and
 query_posts again.

 == Notable Changes
 I've changed the order of query_posts() and send_headers(). So headers are
 only sent after query_posts ( and reparse_request ) have run. This means
 some hooks will now fire in a slightly different order. This might cause
 problems in some rare edge-cases. But I believe this is a minor change
 that should not break any existing installaiton or plugin.
 But if testing proves this assumption wrong - reverting it shouldn't be
 too hard to do. It will just require slightly repetitive code.

 {{{unset($_GET['error'])}}} now executes inside {{{setup_globals()}}} -
 after {{{_reparse_request()}}} finishes  - and not during
 {{{parse_request()}}} as before.

 ----

 This might need some further iterations. But it generally works. I'd love
 to get some feedback on this. And am willing to shepherd this ticket and
 make changes if necessary. I know this might be/seem to be a big change so
 any and all feedback is appreciated.


 @SergeyBiryukov I know you've made some changes to class-wp recently. This
 applies cleanly to that AFAICT. But obviously this needs scrutiny.
 @johnbillion - You've expressed some interest in this type of
 functionality. This is what I came up with. I'd greatly appreciate your
 input as well.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/48223#comment:4>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform