wp-cli / config-command

Generates and reads the wp-config.php file.
MIT License
38 stars 36 forks source link

"wp config create" generates wrong DB_PASSWORD in wp-config.php when db password has " #180

Open schplurtz opened 7 months ago

schplurtz commented 7 months ago

Bug Report

Describe the current, buggy behavior

If db password has ", then wp config create --dbpass='abcd"efgh' --other... will create a wrong DB_PASSWORDline in wp-config.php. The resulting line is:

define( 'DB_PASSWORD', 'abcd\"efgh' );

This is wrong. There should not be the \

Describe how other contributors can replicate this bug

  1. create a db and a user that has " in the password. eg these sql commands:
    create database wp ;
    grant all privileges on wp.* to uwp@'localhost' identified by 'abcd"efgh';
    flush privileges;
  2. run wp config create. This is where wp generates the wrong line
    mkdir -p foo
    cd foo
    wp core download
    wp config create --dbname=wp --dbuser=uwp --dbpass='abcd"efgh' --dbhost=localhost --dbprefix=wp_

Describe what you would expect as the correct outcome

The " on the DB_PASSWORD line should not be escaped. Given the above commands, the DB_PASSWORD line in wp-config.php should read:

define( 'DB_PASSWORD', 'abcd"efgh' );

Let us know what environment you are running this on

OS: Linux 6.1.21-v8+ #1642 SMP PREEMPT Mon Apr  3 17:24:16 BST 2023 aarch64
Shell:  /usr/sbin/nologin
PHP binary: /usr/bin/php8.3
PHP version:    8.3.6
php.ini used:   /etc/php/8.3/cli/php.ini
MySQL binary:   /usr/bin/mysql
MySQL version:  mysql  Ver 15.1 Distrib 10.11.3-MariaDB, for debian-linux-gnueabihf (armv7l) using  EditLine wrapper
SQL modes:
WP-CLI root dir:    phar://wp-cli.phar/vendor/wp-cli/wp-cli
WP-CLI vendor dir:  phar://wp-cli.phar/vendor
WP_CLI phar path:   /var/www/example.com
WP-CLI packages dir:
WP-CLI cache dir:   /var/www/.wp-cli/cache
WP-CLI global config:
WP-CLI project config:
WP-CLI version: 2.10.0

Provide a possible solution

Not a solution, but I found 2 workarounds:

  1. don't use " in db password, obviously
  2. use wp config set afterwards. Strangely, it generates a correct line in wp-config.php.
    wp config set DB_PASSWORD 'abcd"efgh'
danielbachhuber commented 6 months ago

Thanks for the report, @schplurtz !

Feel free to submit a pull request, if you'd like. Here is some guidance on our pull request best practices.

ernilambar commented 6 months ago

Looks like we have used addslashes() function for all values. Is their better function to avoid this issue? Or should we implement special check for double apostrophe?

Ref: https://github.com/wp-cli/config-command/blob/main/src/Config_Command.php#L1204-L1206

    if ( is_string( $value ) ) {
        return addslashes( $value );
    }
UmeshSingla commented 6 months ago

Looks like the escaping is on purpose. Ref: https://github.com/wp-cli/config-command/issues/93 https://github.com/wp-cli/config-command/pull/95

schplurtz commented 6 months ago

I'm glad to se that there is activity on this bug report, although I fail to see how a problem with single quote (#93) is in any way related to this problem with double quote that are escaped but should not.

PHP rules for escaping inside single quotes are so simple : only escape ' and \, that I expected the fix to be quick and easy. I'm really sorry to give you so much work and wish you good luck with all that.

UmeshSingla commented 5 months ago

PR https://github.com/wp-cli/config-command/pull/181 adds a fix for the issue. Also takes care of https://github.com/wp-cli/config-command/issues/94

Related: https://github.com/wp-cli/wp-cli/issues/5955

Kerfred commented 5 months ago

Tested with success. PR #181 fixes the issue.

swissspidy commented 3 months ago

Reopening because #181 was reverted for the time being