visitor redirected to 404 after logging in for comment

[eightize]

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:

1. 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/

2. go to post a comment (you must be logged out first).
3. 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.

[foolswisdom]

eightize, you didn't tell us what version of WordPress you are using.

[Otto42]

Recreated diff for trunk

[Otto42]

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.

[ryan]

(In [5986]) Escape special chars in regex. Props eightize and Otto42. fixes #4873

[markjaquith]

We should probably backport this to 2.2.x and 2.0.x

[westi]

Backporting sounds very sensible in this case.

[Otto42]

Backported patch for 2.2.2

[Otto42]

Added patch for 2.2.2.

[westi]

Backport for 2.2

[westi]

Snap two matching patches ;-)

[markjaquith]

(In [5988]) Escape special chars in regex. Props eightize and Otto42. fixes #4873 for 2.2.3

[markjaquith]

(In [5989]) Escape special chars in regex. Props eightize and Otto42. fixes #4873 for 2.0.12

[markjaquith]

(In [5993]) Roll back [5986], [5988], [5989]. We are in a char class, so no escaping needed. Props mdawaffe. see #4873

[markjaquith]

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.

[westi]

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

[westi]

That should read that I have tested with

FF 2.0.0.6 and IE 6.0

[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=1

which will redirect to

http://localhost/wp/trunk/?p=1

[westi]

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=1

which 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

[eightize]

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

[eightize]

Replying to westi:

i have removed my patch from this live version, so you can see it also here (version 2.2.2):

​http://www.eightize.com/turtle+mole/

[westi]

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.

[eightize]

Replying to eightize:

the live version is on Apache/1.3.37 Server php 4.4.4

[markjaquith]

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) "+"

[markjaquith]

Sorry, make that:

<?php

$test = $test2 = '+';

$test  = preg_replace('|[^+]|', '', $test);
$test2 = preg_replace('|[^\+]|', '', $test2);

var_dump($test);
var_dump($test2);

?>

[markjaquith]

Okay, found the issue. It's an issue with the default themes, of all things.

[markjaquith]

urlencode() in the themes for login links.

[westi]

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.

[markjaquith]

(In [6006]) urlencode() redirect_to param in login links for both themes. fixes #4873 for trunk

[eightize]

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.