Added urlencode on wp_lostpassword_url()

[adhun]

I found that the function wp_lostpassword_url() was not encoding the redirect URL which was resulting in 404 error in some instances. To fix the issue, urlencode has been added to the function.

[adhun]

Patch File for urlencode on wp_lostpassword_url( )

[jnylen0]

which was resulting in 404 error in some instances

Can you be more specific here? How can we reproduce this breakage?

[adhun]

Replying to jnylen0:

which was resulting in 404 error in some instances

Can you be more specific here? How can we reproduce this breakage?

Yes, Steps to reproduce the issue

if you use wp_lostpassword_url(home_url()) on ​http://example.com it will generate a url like the following

http://example.com/wp-login.php?action=lostpassword&redirect_to=http://example.com

so the url parameter here contains slashes which not safe on all kind of servers.
so when we add urlencode for the redirect url will be

http://example.com/wp-login.php?action=lostpassword&redirect_to=http%3A%2F%2Fexample.com

and it is safe for all kind of servers.

[jnylen0]

not safe on all kind of servers.

On what server setups does this break?

[adhun]

Replying to jnylen0:

not safe on all kind of servers.

On what server setups does this break?

I was working on a shared hosting with Apache Version 2.2.32, PHP Version 5.6.30.
It was throwing 404 error when I tried to access a URL with a slash on GET parameter list.
When I applied urlencode() for the URL passing through GET parameter it fixed the issue.

For instance, in a query string, the ampersand (&) is used as a separator between key-value pairs. If you were to put an ampersand into one of those values, it would look like the separator between the end of a value and the beginning of the next key. So for special characters like this, we use percent encoding so that we can be sure that the data is unambiguously encoded.

[jnylen0]

Ok, thanks for the clarification. Which shared host is this? It would help to know how common this problem is.

In any case, I'm setting this ticket back to normal as this is a broken server configuration. / characters are specifically allowed in query strings ​per RFC 3986:

The query component is indicated by the first question mark ("?") character and terminated by a number sign ("#") character or by the end of the URI.

query = *( pchar / "/" / "?" )

The characters slash ("/") and question mark ("?") may represent data within the query component. Beware that some older, erroneous implementations may not handle such data correctly when it is used as the base URI for relative references (Section 5.1), apparently because they fail to distinguish query data from path data when looking for hierarchical separators.

Even though this should work everywhere, it obviously doesn't work sometimes, and this is noted in the RFC, so I don't see the harm in adding urlencode. Milestoning for 4.8.1 for discussion.

[adhun]

I found issue on the ​https://www.hostdime.com/ shared hosting
I think it's a good idea to keep the urlencode() since the users have less access to server configuration on shared hosting.

[jbpaul17]

Punting to 4.9 per today's bug scrub.

[johnbillion]

This actually has nothing to do with server configuration. The URL should be encoded as per @adhun's patch, otherwise any query parameters that are included in the URL that's passed to wp_lostpassword_url() will be treated as query parameters on the lost password URL.

@adhun's patch is correct, and mirrors behaviour in other functions such as wp_logout_url() and wp_login_url().

[johnbillion]

In 41121:

Login and Registration: Correctly encode the redirect location URL when it's passed as a query parameter to the lost password URL.

Props adhun

Fixes #40728