Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deleting a constant with concatenated function deletes much more #101

Open
2 tasks done
todeveni opened this issue Nov 13, 2019 · 2 comments
Open
2 tasks done

deleting a constant with concatenated function deletes much more #101

todeveni opened this issue Nov 13, 2019 · 2 comments

Comments

@todeveni
Copy link

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

  • A wp-config.php file with a constant definition concatenating a function, like in my example
  • Run wp config delete USER_PATH
<?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
Copy link

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:

  • To work around this, I sidestepped using the remove() function and implemented direct file manipulation based on regex matching for both constants and variables:
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.

  • I tried adjusting the regex to better match only the exact line of the constant definition, but capturing the end of the line precisely in cases of dynamic PHP functions proved challenging. Manually checking the file post-execution confirmed that the deletion was not confined to just the targeted line.

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
Copy link
Member

ernilambar commented Jun 20, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants