wp-cli / config-command

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

deleting a constant with concatenated function deletes much more #101

Open todeveni opened 4 years ago

todeveni commented 4 years ago

Bug Report

Describe the current, buggy behavior

Deleting a constant with concatenated function from wp-config.php with wp config delete deletes more than just that constant.

Describe how other contributors can replicate this bug

<?php

define( 'WP_DEBUG', false );

define( 'USER_PATH', '/var/www/' . get_current_user() );

define( 'THIS_AND', md5( 'that' ) );

if ( isset( $_SERVER['HTTP_X_FORWARDED_PROTO'] ) && 'https' === $_SERVER['HTTP_X_FORWARDED_PROTO'] ) {
    $_SERVER['HTTPS'] = 'on';
}

define( 'DISABLE_WP_CRON', true );

/* That's all, stop editing! Happy publishing. */

Describe what you would expect as the correct outcome

wp-config.php with just USER_PATH constant definition removed.

Let us know what environment you are running this on

OS: Linux 4.9.184-linuxkit #1 SMP Tue Jul 2 22:58:16 UTC 2019 x86_64
Shell:
PHP binary: /usr/local/bin/php
PHP version:    7.3.11
php.ini used:   /usr/local/etc/php/php.ini
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/html
WP-CLI packages dir:
WP-CLI global config:
WP-CLI project config:
WP-CLI version: 2.4.0
shreya0204 commented 2 months ago

I’ve been following this thread and experiencing the same issue with the wp config delete command removing more than just the targeted constant, especially when the definition includes concatenated functions. Here's a bit more about what I've tried and discovered:

Issue Recap: When using wp config delete to remove constants like:

define('USER_PATH', '/var/www/' . get_current_user());

the command ends up deleting additional lines, not just the USER_PATH line.

Deeper Dive: I noticed that the regex in our remove() function within the WPConfigTransformer class function was too broad, affecting more than the intended line. The regex seemed to fail in properly isolating the constant definition when it involved function concatenations.

Approach Tried:

if ('constant' === $type) {
    $pattern = "/define\s*\(\s*['\"]{$name}['\"]\s*,/";
} else {
    $pattern = '/^\\$' . $name . "\s*=\s*['\"][^'\"\;]*['\"]\s*;/";
}
// I iterated through each line to find and remove the exact match.

This approach allowed me to precisely target and delete only the intended lines. After making these adjustments, all my test cases ran perfectly, confirming that the specific constant or variable was successfully removed without impacting surrounding lines.

Suggested Next Steps: Given the effectiveness of directly manipulating file content with tailored regex patterns, it might be worthwhile for us to revisit how the remove() function handles pattern matching. Refining this function could ensure better accuracy and prevent unintended deletions in future releases.

I’m very open to feedback and would love to collaborate on fixing this issue.

ernilambar commented 2 months ago

I can also reproduce the issue. I tested with following feature test.

  @require-mysql
  Scenario: Delete constant with concatenated strings
    Given an empty directory
    And WP files
    And a wp-config-extra.php file:
      """
      define( 'USER_PATH', '/var/www/' . get_current_user() );
      """

    When I run `wp config create {CORE_CONFIG_SETTINGS} --skip-check --extra-php < wp-config-extra.php`
    Then the wp-config.php file should contain:
      """
      'USER_PATH',
      """

    When I run `wp config delete USER_PATH`
    Then the wp-config.php file should not contain:
      """
      'USER_PATH'
      """

But here in my case, rather than deleting unrelated changes, command is not deleting anything.