Skip to content

Conversation

@shreya0204
Copy link
Contributor

@shreya0204 shreya0204 commented Jun 20, 2024

Fixes #50
Fixes wp-cli/config-command#101

This pull request addresses the bug where using wp config delete to remove a constant from wp-config.php that includes concatenated functions or complex strings results in the incorrect deletion of more than just the targeted constant.

Current Behavior

Currently, when deleting a constant that includes concatenated functions or special characters, the regex used in the deletion function does not properly isolate the constant. This results in deleting additional lines or even other constants. For example:

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 );

Running wp config delete USER_PATH would improperly affect adjacent lines.

Steps to Replicate

  1. Have a wp-config.php file with a constant definition concatenating a function.
  2. Run wp config delete USER_PATH.

Expected Outcome

Only the line defining USER_PATH should be removed, leaving all other settings and definitions intact.

Issue Details

The issue stemmed from the regex pattern used in the removal function, which failed to correctly identify the boundaries of the constant definition when it involved concatenated functions or complex string values.

Proposed Solution

This PR introduces a revised regex pattern that more accurately matches and isolates define statements, even when they contain complex string values or are among multiple statements on a single line. The new regex handles variations in whitespace and formatting, ensuring that only the specific constant is removed.

@shreya0204 shreya0204 requested a review from a team as a code owner June 20, 2024 08:42
@ernilambar
Copy link
Member

@shreya0204 Thanks for the PR. Could you please add some tests also?

@shreya0204
Copy link
Contributor Author

shreya0204 commented Jun 20, 2024

@shreya0204 Thanks for the PR. Could you please add some tests also?

Can you guide me on how to add for this particular fix?

@ernilambar
Copy link
Member

@shreya0204 Thanks for the PR. Could you please add some tests also?

Can you guide me on how to add for this particular fix?

Tests regarding add/remove config is in this file - https://github.com/wp-cli/wp-config-transformer/blob/main/tests/AddRemoveTest.php This could provide idea for the test implementation. Here are the steps to run test https://github.com/wp-cli/wp-config-transformer?tab=readme-ov-file#testing

@shreya0204
Copy link
Contributor Author

@ernilambar I've implemented a new test to ensure the safe removal of constants that have concatenated strings as their values. This test specifically checks that only the targeted constant is removed, and that no additional lines or constants are inadvertently affected during the process.

@swissspidy swissspidy added this to the 1.4.2 milestone Mar 31, 2025
@swissspidy swissspidy added the bug label Mar 31, 2025
@codecov
Copy link

codecov bot commented Mar 31, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@swissspidy swissspidy merged commit b78cab1 into wp-cli:main Mar 31, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

preg_match_all fails with concatenated string deleting a constant with concatenated function deletes much more

3 participants