pluggable.php: auth_redirect() contains invalid test for SSL request

[wet]

​auth_redirect() tries to locate 'http' in $_SERVER['REQUEST_URI'] to determine whether SSL is used:

if ( 0 === strpos($_SERVER['REQUEST_URI'], 'http') ) {
	wp_redirect(preg_replace('|^http://|', 'https://', $_SERVER['REQUEST_URI']));

$_SERVER['REQUEST_URI'] won't contain any protocol part, so this test will not work.

The ​alternate code path apparently builds yet another SSL admin URI.

[wet]

Wouldn't this need to be fixed for 2.6?

[ryan]

Some setups do have the protocol in REQUEST_URI. This is just a cover your ass check. It doesn't hurt anything.

[grotfl1]

This is still a bug, but for a different reason. Instead of this line:

if ( 0 === strpos($_SERVER['REQUEST_URI'], 'http') ) {

There should be this one:

if ( 0 === strpos($_SERVER['REQUEST_URI'], 'http:') ) {

Otherwise there would be a redirect loop (because 'https:/...' also starts with 'http'!).

I stumbled across this because I did have a redirect loop (though this was not the reason for it). I guess there aren't many setups that would have protocol in REQUEST_URI or this bug would have surfaced a long time ago. ;)

Btw, there are two such checks in auth_redirect(). IMHO they should both be removed.

[ryan]

Replying to grotfl1:

This is still a bug, but for a different reason. Instead of this line:

if ( 0 === strpos($_SERVER['REQUEST_URI'], 'http') ) {

There should be this one:

if ( 0 === strpos($_SERVER['REQUEST_URI'], 'http:') ) {

Otherwise there would be a redirect loop (because 'https:/...' also starts with 'http'!).

I stumbled across this because I did have a redirect loop (though this was not the reason for it). I guess there aren't many setups that would have protocol in REQUEST_URI or this bug would have surfaced a long time ago. ;)

Btw, there are two such checks in auth_redirect(). IMHO they should both be removed.

'http' instead of 'http:' is done on purpose. Since this is inside an !is_ssl() check, I don't see a redirect loop happening.