Opened 5 weeks ago
Last modified 4 weeks ago
#65820 new enhancement
Default first check for is_login()
| Reported by: | josephscott | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Awaiting Review |
| Component: | Login and Registration | Version: | |
| Severity: | normal | Keywords: | has-patch has-unit-tests close |
| Cc: | Focuses: |
Description
The is_login() function is an easy way to tell if the request is for the login page, see https://core.trac.wordpress.org/ticket/19898. The trade off is that wp_login_url() triggers a series of other activities that are more work than is needed for the default case where the login page has not moved.
Before going through that work we could check quickly check for the default condition as a fast path alternative.
Change History (5)
This ticket was mentioned in PR #12864 on WordPress/wordpress-develop by @josephscott.
5 weeks ago
#1
- Keywords has-patch has-unit-tests added
#2
in reply to: ↑ description
@
5 weeks ago
- Keywords close added
Replying to josephscott:
The trade off is that
wp_login_url()triggers a series of other activities that are more work than is needed
What activities are these? It seems to be just constructing the URL. Given that the end result is a login_url filter, it seems this function needs to always be called to determine whether it is the login page.
Maybe I replaced wp-login.php with something else entirely and I require login via my-super-secret-login.php. As it stands right now, your PR would cause is_login() check to return true when in actuality it isn't the login page.
So I think the existing is_login() logic is sound and it isn't doing unnecessary work.
#3
@
4 weeks ago
It isn't a ton of work, but if it isn't needed, then it shouldn't be done.
The abbreviated chain from is_login() for multi-site:
wp_login_url()site_url()get_site_url()switch_to_blog()get_current_blog_id()absint()
get_option()set_url_scheme()is_ssl()preg_replace()
Depending on the setup and conditions that will make database and/or cache storage calls.
If you moved the login page to my-super-secret-login.php then the fast path default detection fails and the code executes the as it does today. My PR in that case would change nothing, since the file isn't /wp-login.php.
Please take another look at this.
#4
@
4 weeks ago
But let's say you want to actually serve requests to /wp-login.php from WordPress, let's say as a 404 template. With this change, is_login() would erroneously return true.
#5
@
4 weeks ago
Funny enough, that actually wouldn't be a change - at least for some cases.
Here is how a I setup a WP 7.0.4 site to have WP process /wp-login.php via the theme 404 template, via wp-content/mu-plugins/login-404.php:
<?php
add_action( 'login_init', function () {
global $wp_query;
if ( ! ( $wp_query instanceof WP_Query ) ) {
$wp_query = new WP_Query();
}
$wp_query->set_404();
status_header( 404 );
nocache_headers();
$template = get_404_template();
if ( ! $template ) {
$template = get_index_template();
}
include $template;
error_log( var_export( is_login(), true ) );
exit;
} );
Requesting /wp-login.php on the site showed the theme 404 page and is_login() still returns true.
Then I tried this at the Nginx level, changing how /wp-login.php requests are handled. In order to avoid redirect loops from WP ( it tries to be overly helpful ) I had to point it directly at index.php:
location = /wp-login.php {
fastcgi_pass 127.0.0.1:9000;
include fastcgi_params;
fastcgi_param SCRIPT_FILENAME $document_root/index.php;
fastcgi_param REQUEST_URI /wp-login-gone;
}
Then added logging to wp-content/themes/twentytwentyfive/patterns/hidden-404.php -
if ( is_404() ) {
error_log( 'hidden-404 pattern: is_login() = ' . var_export( is_login(), true ) );
}
Which also reports true for requests to /wp-login.php that show the theme 404 page.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
https://core.trac.wordpress.org/ticket/65820
AI assistance: Yes
Tool(s): Claude
Model(s): Opus 4.8
Used for: Claude wrote the tests in this change