#4873 closed defect (bug) (fixed)
visitor redirected to 404 after logging in for comment
| Reported by: | eightize | Owned by: | markjaquith |
|---|---|---|---|
| Priority: | normal | Milestone: | 2.3 |
| Component: | General | Version: | 2.3 |
| Severity: | major | Keywords: | has-patch |
| Cc: | Focuses: |
Description
When a directory in the path to wordpress contains an unusual character, such as +, the redirect after a visitor logs in to post a comment strips that character out of the path name, so user ends up at the 404 not found page.
to reproduce the error:
- make the directory for wordpress contain a + character, and set wordpress to require visitors to register in order to post comments.:
www.site.com/test+folder/
- go to post a comment (you must be logged out first).
- after entering username and password, the page is redirected to www.site.com/testfolder/?p=xx (the + is stripped).
i believe i resolved the error on my site by correcting the regular expression in wp_redirect() in the file wp-includes/pluggable.php.
a diff is attached to this post.
Attachments (5)
Change History (32)
#2
@
19 years ago
- Keywords has-patch needs-testing 2nd-opinion added; comment login redirect error removed
- Priority normal → high
- Severity normal → major
- Version → 2.3
His problem applies to all current versions of WordPress, I found the code he posted in both trunk and 2.2.2.
I believe that his fix is the correct one, that regular expression did not escape some special characters. Could use verification by somebody more skilled in regexp knowledge than myself, though.
Anyway, attached patch for trunk based on eightize's incomplete diff.
#4
@
19 years ago
- Milestone 2.3 → 2.2.3
- Resolution fixed
- Status closed → reopened
We should probably backport this to 2.2.x and 2.0.x
#11
@
19 years ago
- Milestone 2.2.3 → 2.4 (next)
- Priority high → normal
- Resolution fixed
- Status closed → reopened
So... that fix did nothing. + doesn't need to be escaped in that context.
If someone finds a fix and it's really obvious, maybe we can slip this into 2.3 -- but it's very much an edge case, so not a huge priority.
#12
@
19 years ago
I have tested current trunk (r5996) with a + in the dir name and cannot reproduce this issue.
PHP 5.2.4_pre200708051230-pl2-gentoo with Suhosin-Patch 0.9.6.2
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6
IE 6.0
#14
follow-ups:
↓ 15
↓ 16
@
19 years ago
i appologize for my slow response. i am testing with development version (2.3-beta1). The same problem also occurs under 2.2.2. a sample url for a page that would not work is:
http://localhost/wp/tr+unk/wp-login.php?redirect_to=http://localhost/wp/tr+unk/?p=1
which will redirect to
http://localhost/wp/trunk/?p=1
#15
in reply to: ↑ 14
;
follow-up:
↓ 17
@
19 years ago
- Keywords reporter-feedback added; has-patch needs-testing 2nd-opinion removed
Replying to eightize:
i appologize for my slow response. i am testing with development version (2.3-beta1). The same problem also occurs under 2.2.2. a sample url for a page that would not work is:
http://localhost/wp/tr+unk/wp-login.php?redirect_to=http://localhost/wp/tr+unk/?p=1which will redirect to
http://localhost/wp/trunk/?p=1
Ok.
Can you detail what Browser / WebServer / WebServer Platform you are testing on?
i.e. IE/FF/Opera Apache2/whatever Linux/Windows/OS X
#16
in reply to: ↑ 14
@
19 years ago
btw: i'm using Firefox 2.0.0.6 browser on the 2.3-beta1, with Apache/1.3.33 (Darwin) PHP/4.4.7
#17
in reply to: ↑ 15
;
follow-up:
↓ 19
@
19 years ago
Replying to westi:
i have removed my patch from this live version, so you can see it also here (version 2.2.2):
#18
@
19 years ago
- Owner changed from to
- Status reopened → new
Ah ha.
Can reproduce on your site.
It seems to be something to do with the parsing of the redirect info out of the url rather than wp_redirect itself.
Note to self... read the description very carefully.
I will look into this further.
#19
in reply to: ↑ 17
@
19 years ago
Replying to eightize:
the live version is on Apache/1.3.37 Server php 4.4.4
#20
@
19 years ago
i have removed my patch from this live version
Your patch, in my testing, does nothing. "+" gets through either way.
Try this:
<?php
$test = $test2 = '+';
$test = preg_replace('|[^+]|', '', $test);
$test2 = preg_replace('|[^\+]|', '', $test);
var_dump($test);
var_dump($test2);
?>
You should get:
string(1) "+" string(1) "+"
#21
@
19 years ago
Sorry, make that:
<?php
$test = $test2 = '+';
$test = preg_replace('|[^+]|', '', $test);
$test2 = preg_replace('|[^\+]|', '', $test2);
var_dump($test);
var_dump($test2);
?>
#22
follow-up:
↓ 24
@
19 years ago
- Owner changed from to
- Status new → assigned
Okay, found the issue. It's an issue with the default themes, of all things.
#24
in reply to: ↑ 22
@
19 years ago
Replying to markjaquith:
Okay, found the issue. It's an issue with the default themes, of all things.
That sounds like it.
I go nearly that far earlier before I had to go out
+1 to marks patch.
Not tested yet.
#26
in reply to: ↑ 25
@
19 years ago
Replying to markjaquith:
Thank you all. I've applied markjaquith's patch to my live version and it works (and removed my - ahem - 'patch'.). Sorry about my ignorant red herring. The instructions for posting specifically warn about that too.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
eightize, you didn't tell us what version of WordPress you are using.