Opened 18 years ago
Closed 18 years ago
#3964 closed defect (bug) (fixed)
Aggressive matching in rewrite.php leads to "nothing matches" errors with custom rewrite endpoints
Reported by: | jhodgdon | Owned by: | westi |
---|---|---|---|
Milestone: | 2.2 | Priority: | normal |
Severity: | normal | Version: | 2.1.2 |
Component: | General | Keywords: | rewrite reporter-feedback has-patch 2nd-opinion |
Focuses: | Cc: |
Description (last modified by )
In a plugin, I call add_rewrite_endpoint to add a custom endpoint to the rewrite rules, and then cause a refresh of the rules by calling $wp_rewrite->flush_rules(). I add a suffix endpoint called "foo" to the rewrite rules, for a variety of situations, including post permalinks (in my case, "foo" is actually a language choosing parameter).
My permalink structure is set to /%category%/%postname%/
So, it's working fine on a post, category, page, etc. -- my suffixes are recognized and I can set the language and then find the post. But when I go to the home page and add suffixes, to get a permalink like:
http://www.example.com/blogdir/foo/param
the query fails, because the above it matches the post-with-page-suffix rewrite rule:
(.+?)/([^/]+)(/[0-9]+)?/?$
and WordPress thinks it is the post in category "foo" with slug "param" (which doesn't exist).
It's also a problem on feeds, where
http://www.example.com/blogdir/feed/foo/param
matches the same matching rule, and WordPress thinks it is the post in category "feed/foo", with slug "param". That .+ at the beginning of the rule is pretty permissive.
I do have a rule:
foo(/(.*))?/?$
but it is farther down in the list than the rule above, and so the rule above takes precidence.
I am not sure how to fix this... any ideas? The API doesn't allow for upping the precedence of rules. For now I have just made my plugin use a GET instead of permalink endpoint for the blog's home page and for feeds, but it is rather annoying.
Attachments (2)
Change History (22)
#1
@
18 years ago
- Component changed from Administration to General
- Priority changed from low to normal
- Version set to 2.1.2
#4
@
18 years ago
- Owner changed from anonymous to westi
- Status changed from new to assigned
Could you attach some example code and/or a print_r of the generated rules for me to review what you are trying to achieve.
Reading through the rewrite code the permalink rules should come last in the array and so the extra endpoints should be matched earlier (or so i thought).
#6
@
18 years ago
It's not a solution to the underlying problem you describe, but you can use the rewrite_rules_array filter and append the permalinks that way. That was the approach I took with Gengo, which was written before the new endpoint stuff was around. You can control the order of the rules that you return, as well as choosing which ones to append. It's probably not quite as easy as using the new API, but it's been working for a long time...
#7
@
18 years ago
Actually, I was wrong, and I don't think my endpoints created a matching rule that would match my special endpoint when it is appended directly to the blog home URL.
#9
follow-up:
↓ 10
@
18 years ago
One furthere note. Even with the call to add_rewrite_endpoints changed to
add_rewrite_endpoint( $langSwitchPermalink, 4095 );
(which should add all possible endpoints) I still do not get a matching rule that will match
http://www.example.com/blogdir/foo/param
(except for the erroneous match on category/postslug) So maybe that is all there is to the bug, that the endpoints generated are not sufficient to match the main blog URL or the feed with my endpoint suffixes. Should I just add them manually with a different function call? Should I have to?
#10
in reply to:
↑ 9
@
18 years ago
Replying to jhodgdon:
One furthere note. Even with the call to add_rewrite_endpoints changed to
add_rewrite_endpoint( $langSwitchPermalink, 4095 );
(which should add all possible endpoints) I still do not get a matching rule that will match
http://www.example.com/blogdir/foo/param
(except for the erroneous match on category/postslug) So maybe that is all there is to the bug, that the endpoints generated are not sufficient to match the main blog URL or the feed with my endpoint suffixes. Should I just add them manually with a different function call? Should I have to?
Looking through your generated links this looks like a definete bug.
If I read right, it seems the only rules generated for EP_ROOT are:
feed/(feed|rdf|rss|rss2|atom)/?$ (feed|rdf|rss|rss2|atom)/?$ page/?([0-9]{1,})/?$
Which is definetly wrong from the example code you attached.
I will have a look into working out what is going wrong in generate_rewrite_rules.
#11
@
18 years ago
Now that we have narrowed it down to this (thanks!), I can try to patch it myself. Assign it to me if you like; if not I will assume you prefer to do it.
#12
@
18 years ago
- Keywords reporter-feedback 2nd-opinion added
Right I've had a look at this and EP_ROOT is definetly broken.
However, I think there is a mismatch between what is available with the addition of extra endpoints and what you expect to happen.
From reading the code I believe the following is true:
- The are a number of builtin endpoints = feeds, trackback, paging
- You can add you own endpoints at the same level.
- These endpoints are then added to the places you specify e.g. EP_ROOT - Therefore you would only get the following rules with EP_ROOT fixed:
feed/(feed|rdf|rss|rss2|atom)/?$ (feed|rdf|rss|rss2|atom)/?$ page/?([0-9]{1,})/?$ new_endpoint(/(.*))?/?$
I am not sure that a new endpoint is what you want to use in order to be able to add language tags to every url.
Do you not want a new rewrite tag? - e.g. %lang% which maps to ?lang= and is used in the permalink structure. This would then get added for all permalink urls before the endpoints.
Otherwise you need to be able to add endpoints to endpoints e.g. we would need EP_FEED, EP_PAGING and the ability to extend EP_ for new endpoints that wanted to support sub endpoints.
@
18 years ago
Fix EP_ROOT by moving the endpoint code outside of the check for any sub directories.
#13
@
18 years ago
- Keywords has-patch added
The above patch fixes EP_ROOT to do what it should do.
Test Plugin:
<?php /* Plugin Name: Rewrite Description: Tests rewrite code. Version: 0.1 */ function test_add_endpoint() { global $wp_rewrite; add_rewrite_endpoint('test',EP_ALL); ?><!--<?php print_r($wp_rewrite->generate_rewrite_rules($wp_rewrite->root . '/', EP_ROOT)); print_r($wp_rewrite->generate_rewrite_rules($wp_rewrite->permalink_structure, EP_PERMALINK)); ?>--><?php } add_action('init', 'test_add_endpoint'); ?>
Output:
( [feed/(feed|rdf|rss|rss2|atom)/?$] => index.php?&feed=$1 [(feed|rdf|rss|rss2|atom)/?$] => index.php?&feed=$1 [page/?([0-9]{1,})/?$] => index.php?&paged=$1 [test(/(.*))?/?$] => index.php?&test=$2 ) Array ( [(.+?)/([0-9]{4})/([^/]+)/trackback/?$] => index.php?category_name=$1&year=$2&name=$3&tb=1 [(.+?)/([0-9]{4})/([^/]+)/feed/(feed|rdf|rss|rss2|atom)/?$] => index.php?category_name=$1&year=$2&name=$3&feed=$4 [(.+?)/([0-9]{4})/([^/]+)/(feed|rdf|rss|rss2|atom)/?$] => index.php?category_name=$1&year=$2&name=$3&feed=$4 [(.+?)/([0-9]{4})/([^/]+)/page/?([0-9]{1,})/?$] => index.php?category_name=$1&year=$2&name=$3&paged=$4 [(.+?)/([0-9]{4})/([^/]+)/test(/(.*))?/?$] => index.php?category_name=$1&year=$2&name=$3&test=$5 [.+?/[0-9]{4}/[^/]+/([^/]+)/test(/(.*))?/?$] => index.php?attachment=$1?&test=$2 [.+?/[0-9]{4}/[^/]+/attachment/([^/]+)/test(/(.*))?/?$] => index.php?attachment=$1?&test=$2 [(.+?)/([0-9]{4})/([^/]+)(/[0-9]+)?/?$] => index.php?category_name=$1&year=$2&name=$3&page=$4 [.+?/[0-9]{4}/[^/]+/([^/]+)/?$] => index.php?attachment=$1 [.+?/[0-9]{4}/[^/]+/([^/]+)/trackback/?$] => index.php?attachment=$1&tb=1 [.+?/[0-9]{4}/[^/]+/([^/]+)/feed/(feed|rdf|rss|rss2|atom)/?$] => index.php?attachment=$1&feed=$2 [.+?/[0-9]{4}/[^/]+/([^/]+)/(feed|rdf|rss|rss2|atom)/?$] => index.php?attachment=$1&feed=$2 [.+?/[0-9]{4}/[^/]+/attachment/([^/]+)/?$] => index.php?attachment=$1 [.+?/[0-9]{4}/[^/]+/attachment/([^/]+)/trackback/?$] => index.php?attachment=$1&tb=1 [.+?/[0-9]{4}/[^/]+/attachment/([^/]+)/feed/(feed|rdf|rss|rss2|atom)/?$] => index.php?attachment=$1&feed=$2 [.+?/[0-9]{4}/[^/]+/attachment/([^/]+)/(feed|rdf|rss|rss2|atom)/?$] => index.php?attachment=$1&feed=$2 [(.+?)/([0-9]{4})/feed/(feed|rdf|rss|rss2|atom)/?$] => index.php?category_name=$1&year=$2&feed=$3 [(.+?)/([0-9]{4})/(feed|rdf|rss|rss2|atom)/?$] => index.php?category_name=$1&year=$2&feed=$3 [(.+?)/([0-9]{4})/page/?([0-9]{1,})/?$] => index.php?category_name=$1&year=$2&paged=$3 [(.+?)/([0-9]{4})/test(/(.*))?/?$] => index.php?category_name=$1&year=$2&test=$4 [(.+?)/([0-9]{4})/?$] => index.php?category_name=$1&year=$2 [(.+?)/feed/(feed|rdf|rss|rss2|atom)/?$] => index.php?category_name=$1&feed=$2 [(.+?)/(feed|rdf|rss|rss2|atom)/?$] => index.php?category_name=$1&feed=$2 [(.+?)/page/?([0-9]{1,})/?$] => index.php?category_name=$1&paged=$2 [(.+?)/test(/(.*))?/?$] => index.php?category_name=$1&test=$3 [(.+?)/?$] => index.php?category_name=$1 )
#15
@
18 years ago
I applied the patch, and it does add the endpoint for the main blog, so www.example.com/foo/param and www.example.com/foo/param work correctly.
It still doesn't add an endpoint to the feed, as noted above. I guess that is not going to be possible, since the feed is implemented as an endpoint as well. Same with pages. But it seems to me that if I ask for an endpoint to be added to everything, that WordPress should add it to everything it generates.
And no, I didn't want to add language to the permalink structure, I really do want endpoints, so that the suffix is added to categories, month archives, etc. As far as I can tell, the permalink structure is used only for blog posts, right?
#18
follow-up:
↓ 19
@
18 years ago
Since the patch (which I had tested, see previous comment) has been applied, I think we ought to close this bug. I don't think it's practical to add endpoints to endpoints, unfortunately, so I think the feed additions will just have to stay broken. Thoughts?
#19
in reply to:
↑ 18
@
18 years ago
Replying to jhodgdon:
Since the patch (which I had tested, see previous comment) has been applied, I think we ought to close this bug. I don't think it's practical to add endpoints to endpoints, unfortunately, so I think the feed additions will just have to stay broken. Thoughts?
This sounds sane to me.
If anything we need a new ticket to say that WP_Rewrite needs a rewrite ;-)
Forgot to mention, tested in 2.1.1 release, as well as [5012].