Make WordPress Core

Opened 2 years ago

Last modified 2 years ago

#55796 new defect (bug)

SHORTINIT requires rest-api.php via rest_cookie_collect_status() via wp_get_current_user()

Reported by: johnjamesjacoby's profile johnjamesjacoby Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 2.0
Component: Application Passwords Keywords: 2nd-opinion
Focuses: rest-api Cc:

Description

Hello friends! 👋

I believe it may be unintended behavior that in order to use the SHORTINIT constant with standard cookie authentication, either wp-includes/rest-api.php must be included or its related default filters need to be removed.


wp_get_current_user() – jjj1.php

<?php

/**
 * Only initialize a "short" WordPress
 *
 */
define( 'SHORTINIT', true );

/**
 * Load WordPress
 */
require __DIR__ . '/wp-load.php';

/**
 * Create the main "wp" WordPress global
 */
$GLOBALS['wp'] = new WP();

/**
 * Set up the query vars
 */
wp( array(
	'jjj' => true
) );

Produces:

Fatal error: Uncaught Error: Call to undefined function wp_get_current_user() in wp-includes/class-wp.php:635
Stack trace:
#0 wp-includes/class-wp.php(768): WP->init()
#1 wp-includes/functions.php(1330): WP->main(Array)
#2 jjj1.php(31): wp(Array)

IMO, the inside of WP->init() has needed a function_exists() call around wp_get_current_user() since WordPress 2.0.0, and I'm only just now getting around to suggesting such 😅


rest_cookie_collect_status() - jjj2.php

<?php

/**
 * Only initialize a "short" WordPress
 */
define( 'SHORTINIT', true );

/**
 * Load WordPress
 */
require __DIR__ . '/wp-load.php';

// Require files used for cookie-based user authentication
require ABSPATH . WPINC . '/pluggable.php';
require ABSPATH . WPINC . '/kses.php';
require ABSPATH . WPINC . '/user.php';
require ABSPATH . WPINC . '/capabilities.php';
require ABSPATH . WPINC . '/class-wp-role.php';
require ABSPATH . WPINC . '/class-wp-roles.php';
require ABSPATH . WPINC . '/class-wp-user.php';
require ABSPATH . WPINC . '/class-wp-session-tokens.php';
require ABSPATH . WPINC . '/class-wp-user-meta-session-tokens.php';

/**
 * 'WP_PLUGIN_URL' and others are used by: wp_cookie_constants()
 */
wp_plugin_directory_constants();

/**
 * 'ADMIN_COOKIE_PATH' and others are used by: wp_set_auth_cookie()
 */
if ( is_multisite() ) {
	ms_cookie_constants();
}

/**
 * 'SECURE_AUTH_COOKIE' and others are used by: wp_parse_auth_cookie()
 */
wp_cookie_constants();

/**
 * Sets: 'FORCE_SSL_ADMIN' and 'FORCE_SSL_LOGIN'
 */
wp_ssl_constants();

/**
 * Create the main "wp" WordPress global
 */
$GLOBALS['wp'] = new WP();

/**
 * Set up the WordPress query
 */
wp( array(
	'jjj' => true
) );

Produces:

Fatal error: Uncaught TypeError: call_user_func_array(): Argument #1 ($callback) must be a valid callback, function "rest_cookie_collect_status" not found or invalid function name in wp-includes/class-wp-hook.php:309
Stack trace:
#0 wp-includes/class-wp-hook.php(331): WP_Hook->apply_filters('', Array
#1 wp-includes/plugin.php(476): WP_Hook->do_action(Array)
#2 wp-includes/pluggable.php(705): do_action('auth_cookie_mal...', false, '')
#3 wp-includes/class-wp-hook.php(307): wp_validate_auth_cookie(false)
#4 wp-includes/plugin.php(191): WP_Hook->apply_filters(false, Array)
#5 wp-includes/user.php(3583): apply_filters('determine_curre...', false)
#6 wp-includes/pluggable.php(70): _wp_get_current_user()
#7 wp-includes/class-wp.php(635): wp_get_current_user()
#8 wp-includes/class-wp.php(768): WP->init()
#9 wp-includes/functions.php(1330): WP->main(Array)
#10 jjj2.php(65): wp(Array)

This happens because wp-includes/default-filters.php assumes that the REST API will always be loaded, and the default pluggable.php versions of the cookie based authentication functions apply filters that the REST API also uses by default, including the Application Password feature.


If the REST API were a SHORTINIT auth requirement, my guess is that it would have been required earlier in wp-settings.php.

This is all somewhat of a catch-22 situation, because default-filters.php is loaded for SHORTINIT which is far ahead of when both cookie auth and the REST API are both included.

It is possible to work around this by removing the hooks, but obviously that only counts for today's hooks, and not future hooks if something new is introduced. I think a core code change will be required to decide when & how the REST API filters are applied. 😬

Change History (2)

#1 @TimothyBlynJacobs
2 years ago

This is interesting.

I would agree that the REST API shouldn't be a general SHORTINIT requirement. Having it end up being required if wp() is called is a bit more tricky though I think. What I mean by that, is that since the REST API is loaded through wp(), and we don't have any official way in WordPress Core of turning off the REST API, it doesn't seem too odd that we'd expect the REST API functions to be available when wp() is running.

That being said, while we don't allow turning off the REST API via a filter, it seems sensible to me that a developer using SHORTINIT should be able to control what bits are loaded including omitting the REST API.

Would it work to move all of the REST API related hooks that rely on functions defined in rest-api.php to a rest-api-filters.php file that would get included after wp-includes/rest-api.php in wp-settings.php?

#2 @johnjamesjacoby
2 years ago

Would it work to move all of the REST API related hooks (...)

Yes; that approach would circumvent this specific incongruity in a nice enough way.

Tangentially, because SHORTINIT entitles anyone to kinda just load whatever however, I think we both have the same hunch that other core features will go similarly haywire if their files aren't pulled in and their functions are missing but they are still hooked in via default-filters.php.

Some I found:

  • kses_init > set_current_user > wp->init()
  • _post_format_request > request > wp->parse_request()
  • _close_comments_for_old_posts > the_posts > wp->query_posts()

...and I guess I do not think it is a great pattern for KSES, Post Formats, or Comments to follow – having their own respective -filters.php files – but because it's all gotten a bit jumbled, maybe? 🤔

Note: See TracTickets for help on using tickets.