parse_wp_config fails to update string values due to empty line comments

[johnrom]

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

In PHP 8.0, 8.1, 8.2 parse_wp_config fails to update string constant values that contain double-slashes when there are empty line comments in wp-config.

Describe how other contributors can replicate this bug

• Set up WP CLI on PHP 8.0+
• Create a wp-config.php and add the following custom code:

  /* Add any custom values between this line and the "stop editing" line. */
  
  // Note the offending line comment below, without a trailing space.
  //
  define( 'WP_HOME', 'https://wordpress.org' );
  
  /* That's all, stop editing! Happy publishing. */

• Run wp config set 'WP_HOME' 'https://wordpress.com'
  • The update will not work, even though the message says it replaced the value correctly.
• Remove the empty line comment

  // Now there is no empty line comment in this file.
  define( 'WP_HOME', 'https://wordpress.org' );

• Run wp config set 'WP_HOME' 'https://wordpress.com'
  • The update will work.

Describe what you would expect as the correct outcome

wp config set WP_HOME 'https://wordpress.com' should work even if there is an empty line comment in the file.

Let us know what environment you are running this on

I tried in various environments, including a linux server, Windows (Git Bash), Windows Subsystem for Linux. wp-config.php uses LF line endings.

OS:     Linux 5.15.90.1-microsoft-standard-WSL2 #1 SMP Fri Jan 27 02:56:13 UTC 2023 x86_64
Shell:  /bin/bash
PHP binary:     /usr/bin/php
PHP version:    8.2.12
php.ini used:   /etc/php/8.2/cli/php.ini
MySQL binary:   /usr/bin/mysql
MySQL version:  mysql  Ver 15.1 Distrib 10.3.38-MariaDB, for debian-linux-gnu (x86_64) using readline 5.2
SQL modes:
WP-CLI root dir:        /home/redacted/projects/wp-cli-config-command/vendor/wp-cli/wp-cli
WP-CLI vendor dir:      /home/redacted/projects/wp-cli-config-command/vendor
WP_CLI phar path:
WP-CLI packages dir:
WP-CLI cache dir:       /home/redacted/.wp-cli/cache
WP-CLI global config:
WP-CLI project config:  /home/redacted/projects/wp-cli-config-command/wp-cli.yml
WP-CLI version: 2.9.0

Provide a possible solution

Have a look at:

wp-config-transformer/src/WPConfigTransformer.php

Lines 289 to 293 in b1a6a01

	foreach ( token_get_all( $src ) as $token ) {
	if ( in_array( $token[0], array( T_COMMENT, T_DOC_COMMENT ), true ) ) {
	$src = str_replace( $token[1], '', $src );
	}
	}

In PHP 7, token_get_all returns empty line comments as //\n, but PHP 8 returns empty line comments as //.

In the loop, it will check each token, and when it finds one that matches the T_COMMENT or T_DOC_COMMENT token, it searches the full text of the wp-config.php and replaces all instances of the full matched string. In the case of a regular comment, that means it searches the full file for, // this is a comment. However, in the case of an empty comment, it searches the full file for //\n or // depending on PHP version, including inside strings like https://wordpress.org.

After the empty comments are replaced, the constant values cached in WPConfigTransformer->wp_configs['constant'][ $name ]['src'] no longer have the double-slash in them. Thus, when it comes time to update the contents of WP Config, it searches for the incorrectly parsed original value, https:wordpress.org, finds nothing to replace, and then saves wp-config with no changes.

Three solutions come to mind:

1. Cheat and substitute the exact match // for a regex on something like //$ (newlines and EOF with multiline mode) during the replacement.

• Easiest and most backwards compatible way to solve this specific issue.
• Doesn't handle other edge cases, e.g. where text like // abc might actually be in a text string such as one generated by the Secret Key Generator.

2. Use a more comprehensive approach to removing tokens that supports removing only the currently detected token.

• This would be complex or require a library because token_get_all doesn't support string offsets and the offsets would change as tokens were removed.
• However, this would be the most logical way to do it.

Provide additional context/Screenshots

I added some WP_CLI::log code locally in WPConfigTransformer to see what was going on: