Repository navigation
Improve wp-settings.php detection in runner #6038
Description
Bug Report
- Yes, I reviewed the contribution guidelines.
- Yes, more specifically, I reviewed the guidelines on how to write clear bug reports.
Describe the current, buggy behavior
Currently, you must have a require AND wp-settings.php on the same line.
- They can use
requireorrequire_once - They can use parenthesis
require_once('....');or notrequire_once '....'; - They can use ABSPATH
require_once ABSPATH . 'wp-settings.php';or notrequire_once 'wp-settings.php'; - They can have any other whitespace or use single / double / combination of quotation
The logic for this happens in a very basic foreach loop to identify the require call in a single line:
Lines 728 to 743 in 597ae1d
What breaks is when you use other valid forms of PHP to require the file. This may be due to typos or preference of a developer but they all still result in a working WordPress installation and admin area but broken WP-CLI commands.
Describe how other contributors can replicate this bug
- Edit your wp-config.php file and in the wp-settings.php line at the bottom, change it to the below example
- Run any WP-CLI command like
wp db prefixorwp post list - See the error:
Error: Strange wp-config.php file: wp-settings.php is not loaded directly.
wp-config.php change:
require_once
ABSPATH . 'wp-settings.php';Also valid:
require_once
(ABSPATH . 'wp-settings.php');Describe what you expect as the correct outcome
I would expect WP-CLI to work if the WP admin area is working.
Let us know what environment you are running this on
Encountered on actual customer sites on a large host
Provide a possible solution
Maybe just do one regex check across all of the wp-config.php code instead of requiring it to have one line. This is a bit complicated since the method WP_CLI\Runner::get_wp_config_code() itself intends to get the config code to run excluding that require.
But I'd assume something like this would be better:
public function get_wp_config_code( $wp_config_path = '' ) {
if ( empty( $wp_config_path ) ) {
$wp_config_path = Utils\locate_wp_config();
}
$wp_config_code = explode( "\n", file_get_contents( $wp_config_path ) );
// Detect and strip byte-order marks (BOMs).
// This code assumes they can only be found on the first line.
foreach ( self::BYTE_ORDER_MARKS as $bom_name => $bom_sequence ) {
WP_CLI::debug( "Looking for {$bom_name} BOM", 'bootstrap' );
$length = strlen( $bom_sequence );
while ( substr( $wp_config_code[0], 0, $length ) === $bom_sequence ) {
WP_CLI::warning(
"{$bom_name} byte-order mark (BOM) detected in wp-config.php file, stripping it for parsing."
);
$wp_config_code[0] = substr( $wp_config_code[0], $length );
}
}
$wp_config_code = implode( "\n", $wp_config_code );
preg_match_all( '/^\s*(require|require_once)\s*.+wp-settings\.php.+;[ \t]*$/m', $wp_config_code, $matches );
if ( empty( $matches[0] ) ) {
WP_CLI::error( 'Strange wp-config.php file: wp-settings.php is not loaded directly.' );
}
$source = str_replace( $matches[0], '', $wp_config_code );
$source = Utils\replace_path_consts( $source, $wp_config_path );
return preg_replace( '|^\s*\<\?php\s*|', '', $source );
}
Activity
sclark3-godaddy commentedon Jan 28, 2025
Probably better regex that you could do, I was just being quick and dirty!
[-]Valid PHP not recognized when requiring wp-settings.php with line breaks[/-][+]Improve `wp-settings.php` detection in runner[/+]swissspidy commentedon Jan 28, 2025
Agreed that there is room for improvement, but we need to draw the line at some point as well. Maybe we can find some middle ground.
sclark3-godaddy commentedon Jan 29, 2025
Could that make that function filterable by mu-plugins or something that could allow more flexibility on certain environments if this kind of support is not suitable for most sites.
swissspidy commentedon Jan 29, 2025
If you would go such lenghts, why not just fix the config file?
sclark3-godaddy commentedon Jan 29, 2025
It's valid PHP, maybe not the code standards we'd prefer but the regex logic is rigid and does not cover all variances of valid PHP.
If there's no filter, then we'll have to add additional tooling that has to change a customer file from their own intended (or unintended) formatting. That's fine with me myself but we try to be adverse with changing customer files across over a million sites. It seems like wasted resources at that scale IMO.
sclark3-godaddy commentedon Jan 29, 2025
The tooling could look like this for example:
swissspidy commentedon Jan 29, 2025
Have people suddenly become super creative modifying the default config files?
As I said, there is room for improvement (spaces, newlines, parentheses, etc.), and your suggested regex looks like a good start.
But I don't think we should be supporting more extreme cases, e.g.
$settings = 'wp-settings.php; require_once( ABSPATH . $settings );Otherwise we'd suddenly need a PHP parser for this simple check, which is way overkill. Especially since this check barely causes any issues (the code hasn't changed in 8 years!)
sclark3-godaddy commentedon Jan 29, 2025
I think it's fair to leave this at formatting only and variables would be out of scope.
sclark3-godaddy commentedon Jan 29, 2025
I honestly wish the setup here was different because the way WP-CLI has to do this to avoid loading WP is pretty hacky. Since WP itself should allow for a constant check to bail loading WP so that WP-CLI itself can do what it needs with wp-config.php
Maybe that's something for the future in core.