Skip to content

Filter empty database parameters to prevent MySQL command failures #308

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.

Sign up for GitHub

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

Draft
Copilot wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Filter empty database parameters to prevent MySQL command failures #308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b0f6bcc
Initial plan
Copilot Feb 1, 2026
a265c7d
Fix empty character set parameter issue in mysqlcheck commands
Copilot Feb 1, 2026
878e3e9
Address code review feedback: handle null values and improve test
Copilot Feb 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions features/db-check.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,37 @@ Feature: Check the database
When I try `wp db check --no-defaults --debug`
Then STDERR should match #Debug \(db\): Running shell command: /usr/bin/env (mysqlcheck|mariadb-check) --no-defaults %s#

Scenario: Empty DB credentials should not cause empty parameter errors
Given a WP install
And a wp-config.php file:
"""
<?php
define( 'DB_NAME', 'wp_cli_test' );
define( 'DB_USER', 'wp_cli_test' );
define( 'DB_PASSWORD', 'password1' );
define( 'DB_HOST', 'localhost' );
define( 'DB_CHARSET', '' );
define( 'DB_COLLATE', '' );
$table_prefix = 'wp_';
require_once ABSPATH . 'wp-settings.php';
"""

When I run `wp db check --debug`
Then STDERR should contain:
"""
Debug (db): Final MySQL command:
"""
And STDERR should not contain:
"""
--default-character-set=''
"""
And STDERR should not contain:
"""
--default-character-set=
"""
And the return code should be 0
And STDOUT should contain:
"""
Success: Database checked.
"""

Comment on lines +139 to +172
Copy link

@gemini-code-assist gemini-code-assist bot Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This test scenario is a great addition for verifying the behavior with empty credentials. However, it specifically tests an empty DB_CHARSET, which is already handled by existing logic in DB_Command::run() (&& constant( 'DB_CHARSET' )) and doesn't actually exercise the new array_filter logic.

To create a more effective test that validates the new filtering mechanism, consider testing a scenario where DB_HOST or DB_USER is an empty string. This would directly test whether the array_filter correctly removes the empty parameter before it's passed to the MySQL command.

For example, you could add a scenario like this:

  Scenario: Empty DB_HOST should not be passed as a command-line argument
    Given a WP install
    And a wp-config.php file:
      """
      <?php
      define( 'DB_NAME', 'wp_cli_test' );
      define( 'DB_USER', 'wp_cli_test' );
      define( 'DB_PASSWORD', 'password1' );
      define( 'DB_HOST', '' );
      define( 'DB_CHARSET', 'utf8' );
      $table_prefix = 'wp_';
      require_once ABSPATH . 'wp-settings.php';
      """

    When I run `wp db check --debug`
    Then STDERR should not contain "--host=''"
    And STDERR should not contain "--host="
    # Depending on environment, an empty host might default to localhost socket and succeed.
    And the return code should be 0

9 changes: 9 additions & 0 deletions src/DB_Command.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1827,6 +1827,15 @@ private static function run( $cmd, $assoc_args = [], $send_to_shell = true, $int
unset( $assoc_args['dbpass'], $assoc_args['password'] );
}

// Filter out empty string values to avoid passing empty parameters to MySQL commands
// which can cause errors like "Character set '' is not a compiled character set"
$required = array_filter(
$required,
static function ( $value ) {
return null !== $value && '' !== $value;
}
);

$final_args = array_merge( $required, $assoc_args );
Comment on lines +1830 to 1839
Copy link

@gemini-code-assist gemini-code-assist bot Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current implementation filters empty values from the $required array before merging with $assoc_args. However, this approach has two issues:

  1. It doesn't filter empty values that might come from command-line arguments (i.e., in $assoc_args). For example, wp db check --host='' would still pass an empty host parameter.
  2. Values in $assoc_args overwrite values from $required during the merge. If an empty value is passed via the command line for a parameter that has a non-empty value in wp-config.php, the filtering on $required becomes ineffective.

To ensure all empty parameters are correctly filtered, regardless of their origin, the filtering should be applied to the $final_args array after merging.

		$final_args = array_merge( $required, $assoc_args );

		// Filter out empty string values to avoid passing empty parameters to MySQL commands
		// which can cause errors like "Character set '' is not a compiled character set"
		$final_args = array_filter(
			$final_args,
			static function ( $value ) {
				return null !== $value && '' !== $value;
			}
		);


// Adapt ordering of arguments.
Expand Down
Loading