Opened 15 years ago
Closed 14 years ago
#12700 closed defect (bug) (invalid)
Malformed permalinks for feeds and paged / generate_rewrite_rules
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 3.0 |
Component: | Rewrite Rules | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
All rewrite rules(custom or standard options, non-default obviously) seem to suffer for a minor issue on rewrite generation, on iterations 5 and 6, $query is empty for those iterations (Line 1337 - the for loop).. resulting in malformed rules, ie. index.php?&feed ..
$feedquery $feedquery2 $pagequery
Are effected as a result, 6 rules then contain the malformed query string..
Line 1349 sets this to an empty string.
$query = ( isset($queries) && is_array($queries) ) ? $queries[$num_toks - 1] : '';
Lines 1368, 1382, and 1386 then set rules incorrectly.
A simple fix, just conditionalise the three lines with a string comparison for those lines...
if( '' == $query ) // etc..
Or just use a ternary comparison to do the same..
Could not find an existing report for this, so please close if is indeed duplicate.
First trac submission, go easy on me.. ;)
Attachments (1)
Change History (18)
#2
@
15 years ago
- Keywords reporter-feedback added
Also, could you provide some steps to reproduce this issue (and what to look at)?
#4
@
15 years ago
I have to be honest guys, i don't properly understand the procedure for creating a diff file, and whether i need to commit or just attach something here, i've reviewed Peter Westwoods guide on using Tortoise (on Windows here), but seeing as i've done very little with SVN, it's a little beyond my understanding (wasn't going to let that stop me reporting a "possible" bug though).
wp-includes/rewrite.php is the file in question (function - generate_rewrite_rules), replication should only require an apache driven server and choosing any permalink structure(non-default).
Print / Dump the rewrite array after the permalinks are generated and you should see the problem query strings produced, or at least i'm hoping you do, because i'm not doing any customization to the rewrite array on the local install i'm testing under.
Please let me know if there's anything else i can do to help, bearing in mind my noviceness using trac and SVN(happy to learn, just be prepared for the possibily i may make errors).
#5
@
15 years ago
- Keywords has-patch added; needs-patch reporter-feedback removed
12700.diff uses add_query_arg() to avoid malformed URLs.
#7
@
15 years ago
Bear with me, still figuring out the whole, apply/create patch routine..
Code looks sound to me though, i'd have never of thought to use add_query_arg, looks like a nice clean approach. Were you able to reproduce the problem?
#9
@
15 years ago
Spent a couple of hours trying to work the patch, tortoise seems to be giving me errors any which way i apply the patch, so i've had to give up for now (was becoming frustrated).
Managed to apply a self-created patch just fine, just couldn't get the patch downloaded here to work (wiped all files re-updated files via SVN, still no go).
Will reinstall tortoise soon and i'll try again, you'll have to excuse the noviceness for now, need to come back to the SVN lessons when i've got a fresh head on, else i'll drive myself insane getting nowhere.
Looks like you've verified the bug though, so i'll consider the report worthwhile all the same.. :)
#11
@
15 years ago
What is the actual bug here? Is it just the "invalid" url?
I only ask this because the URL patterns generated are only used internally, and from what i can see, does not cause error or bugs due to the way the PHP URL parsers are written.
I see no real objection to the attached patch in theory, however, It may be best to leave it as-is (at least for 3.0) for consistency sake, as thats whats used throughout the rest of the file (uses of $var .= '&' . $var . '='..etc..
So, Unless this is causing an actual error, this should probably be classed an enhancement, and left for 3.1's consideration.
#12
@
15 years ago
Enable permalinks(any structure), dump/print the rewrite array, and look at the rules..
print '<pre>'; print_r( get_option( 'rewrite_rules' ) ); print '<pre>';
.. or whichever method you prefer ..
You'll see six rules that look like so.
Example:
index.php?&feed
index.php?&paged
Are you saying these will still be mapped correctly by Apache even when they are malformed?
#13
@
15 years ago
Are you saying these will still be mapped correctly by Apache even when they are malformed?
They are never passed to Apache at all. They are never exposed outside of WordPress, They are never used directly by WordPress other than in parse_url() + parse_str() for internal querying purposes.
The result of index.php?&paged
would be parsed as:
array 'path' => string 'index.php' (length=9) 'query' => array 'paged' => string '' (length=0)
#14
@
15 years ago
Perhaps it was just an oversight in the original code then.
Are you saying we should leave it be and let parse_url/str deal with them later on in the processing.
This might cause confusion for anyone examining the rewrite array, or mislead people to think there's a bug, as it has done with me.
What is perhaps most important here is that it doesn't break anything, and for me it looks like the permalinks act appropriately in each case. Of course this does leave what appears to be malformed rules in the array, not that it matters a great deal, i'd personally just prefer the rules to look as they should.
#15
@
15 years ago
- Component changed from Permalinks to Rewrite Rules
- Milestone changed from 3.0 to 3.1
I'm not 100% against changing it, I'd just prefer the entire file to follow the same URI building scheme.
I'm setting this to 3.1 however, I'd prefer not to change it in the leadup to a release since its not causing any issues.
It would be a lot easier to follow your reasoning if you provided a diff file.
See http://core.trac.wordpress.org/wiki#HowtoSubmitPatches