Redirect wp-signup.php to wp_signup_location

[dwieeb]

I noticed that if wp-signup.php is accessed directly and wp_signup_location is not the default "site_url( 'wp-signup.php' )", it does not redirect to the signup location.

I've attached a potential patch. It works for me.

[jeremyfelt]

Does it make sense to handle it this way? The patch should probably be refreshed if so, but it would be a quick add. I could also see using an earlier, already available action to redirect the request.

[jeremyfelt]

17630.diff​ uses network_site_url() instead of site_url() to align with all other uses of the wp_signup_location filter.

[nacin]

I sense this could trigger an infinite redirect?

[jeremyfelt]

Replying to nacin:

I sense this could trigger an infinite redirect?

Because of the strict comparison? I thought that at first, but then figured these should always be full URLs/strings.

Though I guess it could be filtered to wp-signup.php?custom=thing and throw everything off.

[jeremyfelt]

17630.1.diff​ strips any query string from the filtered URL before comparing to the network signup URL and only redirects if the combination of domain/path is different.

[jeremyfelt]

17630.2.diff​ adds docs to the new wp_signup_location filter added in the patch.

[SergeyBiryukov]

die() in line 49 should probably be exit(), for consistency with [16847].

[jeremyfelt]

17630.3.diff​ uses exit; rather than die();, see [16847].

Interesting enough, there are a few other uses of wp_redirect() in wp-signup.php that still use die();.

[nacin]

Replying to jeremyfelt:

Interesting enough, there are a few other uses of wp_redirect() in wp-signup.php that still use die();.

Doesn't matter. [16847] was to enforce we were killing the script after a redirect, not that we were using exit over die. Though exit does seem to be our preferred statement.

This just hasn't been tested. I fear infinite redirects and other side effects. Someone filtering wp_signup_location can *also* choose to redirect wp-signup.php if they wanted to.

[chriscct7]

Refreshed patch

[jeremyfelt]

I think we'd be better off with an action here rather than correctly trying to guess and match a default location to avoid redirects.

wp-activate.php has an activate_header hook that can be used for this. The signup_header hook in wp-signup.php only runs inside wp_head, which is too late. We could do something like pre_signup_header.

Let's come up with a good action name or close this ticket. :)

[DrewAPicture]

@jeremyfelt If you'd rather go the action route, why not just rely on the existing signup_header hook and call this good?

[jeremyfelt]

signup_header only fires inside wp_head. At that point, headers are already sent and its too late to redirect. The corresponding action in wp-activate.php is activate_wp_head.

Naming!

[DrewAPicture]

How about before_signup_header to match before_signup_form?

[jeremyfelt]

Replying to DrewAPicture:

How about before_signup_header to match before_signup_form?

I like that. It should probably go just above get_header() for sanity.

This would be a good first bug with the action and associated docs. :)

[sujin2f]

@jeremyfelt I don't understand why did you want to set an action because there is wp_signup_location filter already. If we make a new action, people will have to set one filter and one action in order to change the signup location. Just for preventing redirection loop?

I think we'd be better off with an action here rather than correctly trying to guess and match a default location to avoid redirects.

[pbearne]

Patch for extra action

[pbearne]

Added the action just before the head

[pbearne]

code tidy with patch

[pbearne]

For my entertainment, I cleaned the code in this file while I had it open :-)

Set the ifs' to yodia
Made the comments and default match
WhiteSpace etc.

I know this should be its own ticket, but I don't it here as it does include the current patches

Enjoy

[DrewAPicture]

@pbearne: As a matter of course, we don't update files to follow coding standards unless the code is already being touched for a legitimate reason. This helps keep the blame records clean. See the ​refactoring article in the Core Contributor Handbook for more on that.

17630.2.patch​ looks pretty good. Some notes:

• The @since version on the wp_signup_location hook doc should be updated to 4.4.0
• Looks like there's an extra space before the closing quote for the before_signup_header action tag
• Let's remove the extra whitespacing

[jeremyfelt]

Per comment 13, I think we're okay to skip the re-use of the wp_signup_location filter here (which was added in 4.3 for wp-login) and instead add a header action that can be used to redirect if this file is ever hit. Sticking with before_signup_header should be good in this case.

[jeremyfelt]

Replying to sujin2f:

@jeremyfelt I don't understand why did you want to set an action because there is wp_signup_location filter already. If we make a new action, people will have to set one filter and one action in order to change the signup location. Just for preventing redirection loop?

Correct. Someone would probably have the easiest time redirecting wp-signup.php manually at the server level, but for those needing to force it via plugin, a second action to short-circuit the file should be fine. Reliance on the current wp_signup_location filter is a bit iffy, mostly because we would need to compare it with network_site_url( 'wp-signup.php' ) which could also be filtered. We could make an attempt at comparing it with $_SERVER['SCRIPT_NAME'], but I'm not sure how reliable that would be.

The use of a new action allows for a general short-circuit of the wp-signup.php load and would also help anyone trying to replace the signup experience without changing the location. It also aligns us a bit more with what actions are available in wp-activate.php.

[jeremyfelt]

@pbearne I'm going to assign you as the owner to refresh the patch per Drew's comment. Note that it's not applying to trunk cleanly at the moment. Let's also remove the use of wp_signup_location filter entirely and stick to the new action. Thanks!

[pbearne]

@jeremyfelt here you go I have tested the patch applies against trunk so we should be good

[pbearne]

removed extra space from action name and removed not needed filter

[jeremyfelt]

In 35159:

MS: Introduce action before_signup_header.

This aligns wp-signup.php a bit more with wp-activate.php` and, among other things, allows a plugin to redirect signup requests.

Props pbearne.
Fixes #17630.