wp-cli / wp-config-transformer

Programmatically edit a wp-config.php file
MIT License
81 stars 25 forks source link

`parse_wp_config` fails to update string values due to empty line comments #47

Closed johnrom closed 10 months ago

johnrom commented 10 months ago

Bug Report

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

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: https://github.com/wp-cli/wp-config-transformer/blob/b1a6a013e4a8c74b29ba185368b78a140b3268da/src/WPConfigTransformer.php#L289-L293

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:

image