Skip to content

Address WordPress PHPUnit test fails: charset detection, length validation etc #331

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

Open
chloe-pomegranate wants to merge 8 commits into
base: trunk
Choose a base branch
from
Open

Address WordPress PHPUnit test fails: charset detection, length validation etc #331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9013237
Address WordPress PHPUnit test fails: charset detection, length valid…
chloe-pomegranate Mar 20, 2026
3942a86
WordPress PHPUnit test fails: charset detection, length validation, c…
chloe-pomegranate Mar 31, 2026
2f1b435
Reconnect wpdb after closing the SQLite driver
JanJakes Jul 17, 2026
692a784
Preserve charset while disconnected
JanJakes Jul 17, 2026
7a69387
Delegate wpdb capability checks
JanJakes Jul 17, 2026
5359f88
Preserve wpdb type during metadata lookup
JanJakes Jul 17, 2026
09b9fc5
Update WordPress test expectations
JanJakes Jul 17, 2026
00c2ff3
Defer SQLite query UTF-8 validation
JanJakes Jul 17, 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
25 changes: 0 additions & 25 deletions .github/workflows/wp-tests-phpunit-run.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,36 +18,24 @@ const expectedErrors = [

const expectedFailures = [
'Tests_Admin_wpSiteHealth::test_object_cache_thresholds with data set #3',
'Tests_Comment::test_wp_new_comment_respects_comment_field_lengths',
'Tests_Comment::test_wp_update_comment',
'Tests_Comment_CheckComment::test_should_return_false_when_comment_previously_approved_is_enabled_and_author_does_not_have_approved_comment',
'Tests_Comment_CheckComment::test_should_return_true_when_comment_previously_approved_is_enabled_and_author_has_approved_comment',
'Tests_Comment_CheckComment::test_should_return_false_when_comment_previously_approved_is_enabled_and_user_does_not_have_a_previously_approved_comment_with_any_email',
'Tests_DB_Charset::test_get_column_charset with data set #0',
'Tests_DB_Charset::test_get_column_charset with data set #1',
'Tests_DB_Charset::test_get_column_charset with data set #2',
'Tests_DB_Charset::test_get_column_charset with data set #3',
'Tests_DB_Charset::test_get_column_charset with data set #4',
'Tests_DB_Charset::test_get_column_charset with data set #5',
'Tests_DB_Charset::test_get_column_charset with data set #6',
'Tests_DB_Charset::test_get_column_charset with data set #7',
'Tests_DB_Charset::test_get_column_charset_is_mysql_undefined with data set #0',
'Tests_DB_Charset::test_get_column_charset_is_mysql_undefined with data set #1',
'Tests_DB_Charset::test_get_column_charset_is_mysql_undefined with data set #2',
'Tests_DB_Charset::test_get_column_charset_is_mysql_undefined with data set #3',
'Tests_DB_Charset::test_get_column_charset_is_mysql_undefined with data set #4',
'Tests_DB_Charset::test_get_column_charset_is_mysql_undefined with data set #5',
'Tests_DB_Charset::test_get_column_charset_is_mysql_undefined with data set #6',
'Tests_DB_Charset::test_get_column_charset_is_mysql_undefined with data set #7',
'Tests_DB_Charset::test_get_column_charset_non_mysql with data set #0',
'Tests_DB_Charset::test_get_column_charset_non_mysql with data set #1',
'Tests_DB_Charset::test_get_column_charset_non_mysql with data set #2',
'Tests_DB_Charset::test_get_column_charset_non_mysql with data set #3',
'Tests_DB_Charset::test_get_column_charset_non_mysql with data set #4',
'Tests_DB_Charset::test_get_column_charset_non_mysql with data set #5',
'Tests_DB_Charset::test_get_column_charset_non_mysql with data set #6',
'Tests_DB_Charset::test_get_column_charset_non_mysql with data set #7',
'Tests_DB_Charset::test_process_field_charsets_on_nonexistent_table',
'Tests_DB_Charset::test_strip_invalid_text with data set #21',
'Tests_DB_Charset::test_strip_invalid_text with data set #22',
'Tests_DB_Charset::test_strip_invalid_text with data set #23',
Expand All @@ -64,23 +52,10 @@ const expectedFailures = [
'Tests_DB_Charset::test_strip_invalid_text with data set #39',
'Tests_DB_Charset::test_strip_invalid_text with data set #40',
'Tests_DB_Charset::test_strip_invalid_text with data set #41',
'Tests_DB_Charset::test_strip_invalid_text_for_column_bails_if_ascii_input_too_long',
'Tests_DB_dbDelta::test_spatial_indices',
'Tests_DB::test_charset_switched_to_utf8mb4',
'Tests_DB::test_close',
'Tests_DB::test_delete_value_too_long_for_field with data set "too long"',
'Tests_DB::test_has_cap',
'Tests_DB::test_insert_value_too_long_for_field with data set "too long"',
'Tests_DB::test_mysqli_flush_sync',
'Tests_DB::test_non_unicode_collations',
'Tests_DB::test_pre_get_col_charset_filter',
'Tests_DB::test_process_fields_on_nonexistent_table',
'Tests_DB::test_process_fields_value_too_long_for_field with data set "too long"',
'Tests_DB::test_query_value_contains_invalid_chars',
'Tests_DB::test_replace_value_too_long_for_field with data set "too long"',
'Tests_DB::test_replace',
'Tests_DB::test_supports_collation',
'Tests_DB::test_update_value_too_long_for_field with data set "too long"',
'Tests_Menu_Walker_Nav_Menu::test_start_el_with_empty_attributes with data set #1',
'Tests_Menu_Walker_Nav_Menu::test_start_el_with_empty_attributes with data set #2',
'Tests_Menu_Walker_Nav_Menu::test_start_el_with_empty_attributes with data set #3',
Expand Down
218 changes: 182 additions & 36 deletions packages/plugin-sqlite-database-integration/wp-includes/sqlite/class-wp-sqlite-db.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ class WP_SQLite_DB extends wpdb {
*/
protected $dbh;

/**
* Whether the PDO instance was provided externally through the global connection hook.
*
* @var bool
*/
private $is_pdo_external;

/**
* Backward compatibility, see wpdb::$allow_unsafe_unquoted_parameters.
*
Expand All @@ -38,6 +45,8 @@ class WP_SQLite_DB extends wpdb {
* @param string $dbname Database name.
*/
public function __construct( $dbname ) {
$this->is_pdo_external = isset( $GLOBALS['@pdo'] );

/**
* We need to initialize the "$wpdb" global early, so that the SQLite
* driver can configure the database. The call stack goes like this:
Expand Down Expand Up @@ -71,17 +80,59 @@ public function set_charset( $dbh, $charset = null, $collate = null ) {
}

/**
* Method to get the character set for the database.
* Hardcoded to utf8mb4 for now.
* Retrieves the character set for the given column.
*
* @param string $table The table name.
* @param string $column The column name.
* This overrides wpdb::get_col_charset() to enable the parent's implementation
* for SQLite by temporarily setting the is_mysql flag.
*
* @return string The character set.
* @see wpdb::get_col_charset()
*
* @param string $table Table name.
* @param string $column Column name.
* @return string|false|WP_Error Column character set as a string. False if the column has
* no character set. WP_Error object on failure.
*/
public function get_col_charset( $table, $column ) {
// Hardcoded for now.
return 'utf8mb4';
$original_is_mysql = $this->is_mysql ?? null;

/*
* The parent method returns early when `$this->is_mysql` is falsy.
* Since SQLite doesn't set this flag, we enable it temporarily so
* the parent can run its full logic — querying column metadata via
* SHOW FULL COLUMNS (which the SQLite driver translates) and
* populating the `$this->col_meta` cache.
*/
try {
$this->is_mysql = true;
return parent::get_col_charset( $table, $column );
} finally {
$this->is_mysql = $original_is_mysql;
}
}

/**
* Retrieves the maximum string length allowed in a given column.
*
* This overrides wpdb::get_col_length() to enable the parent's implementation
* for SQLite by temporarily setting the is_mysql flag.
*
* @see wpdb::get_col_length()
*
* @param string $table Table name.
* @param string $column Column name.
* @return array|false|WP_Error Column length information, false if the column has
* no length. WP_Error object on failure.
*/
public function get_col_length( $table, $column ) {
$original_is_mysql = $this->is_mysql ?? null;

// See get_col_charset() for an explanation of the is_mysql flag.
try {
$this->is_mysql = true;
return parent::get_col_length( $table, $column );
} finally {
$this->is_mysql = $original_is_mysql;
}
}

/**
Expand Down Expand Up @@ -130,14 +181,90 @@ public function set_sql_mode( $modes = array() ) {

/**
* Closes the current database connection.
* Noop in SQLite.
*
* @return bool True to indicate the connection was successfully closed.
* This overrides wpdb::close() while closely mirroring its implementation.
*
* @see wpdb::close()
*
* @return bool True if the connection was successfully closed,
* false if it wasn't, or if the connection doesn't exist.
*/

@JanJakes JanJakes Mar 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's just state that this closely mirrors the implementation of the parent method, like we do for other methods, keeping the parent method docs otherwise.

@chloe-pomegranate chloe-pomegranate Mar 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added this

public function close() {
if ( ! $this->dbh ) {
return false;
}

try {
if ( $this->dbh->inTransaction() ) {
$this->dbh->rollBack();
}
} catch ( Throwable $e ) {
return false;
}

/*
* @TODO: Replace and deprecate the $GLOBALS['@pdo'] injection mechanism.
* PDO has no close method and is released only when all references are unset.
* Until then, retain external PDOs so reconnects reuse the same database.
*/
if (
! $this->is_pdo_external
&& isset( $GLOBALS['@pdo'] )
&& $GLOBALS['@pdo'] === $this->dbh->get_connection()->get_pdo()
) {
unset( $GLOBALS['@pdo'] );
}

$this->dbh = null;
$this->ready = false;
$this->has_connected = false;

return true;
}

/**
* Determines the best charset and collation to use given a charset and collation.
*
* For example, when able, utf8mb4 should be used instead of utf8.
*
* This overrides wpdb::determine_charset() while closely mirroring its implementation.
* The override is needed because the parent checks for a mysqli connection object.
*
* @param string $charset The character set to check.
* @param string $collate The collation to check.
* @return array {
* The most appropriate character set and collation to use.
*
* @type string $charset Character set.
* @type string $collate Collation.
* }
*/
public function determine_charset( $charset, $collate ) {
if ( ! $this->dbh ) {
return compact( 'charset', 'collate' );
}

if ( 'utf8' === $charset ) {
$charset = 'utf8mb4';
}

if ( 'utf8mb4' === $charset ) {
// _general_ is outdated, so we can upgrade it to _unicode_, instead.
if ( ! $collate || 'utf8_general_ci' === $collate ) {
$collate = 'utf8mb4_unicode_ci';
} else {
$collate = str_replace( 'utf8_', 'utf8mb4_', $collate );
}
}

// _unicode_520_ is a better collation, we should use that when it's available.
if ( $this->has_cap( 'utf8mb4_520' ) && 'utf8mb4_unicode_ci' === $collate ) {
$collate = 'utf8mb4_unicode_520_ci';
}

return compact( 'charset', 'collate' );
}

/**
* Method to select the database connection.
*
Expand Down Expand Up @@ -268,13 +395,14 @@ public function flush() {
* @see wpdb::db_connect()
*
* @param bool $allow_bail Not used.
* @return void
* @return bool True on a successful connection, false on failure.
*/
public function db_connect( $allow_bail = true ) {
if ( $this->dbh ) {
return;
return $this->ready;
}
$this->init_charset();

$this->last_error = '';

$pdo = null;
if ( isset( $GLOBALS['@pdo'] ) ) {
Expand Down Expand Up @@ -317,7 +445,7 @@ public function db_connect( $allow_bail = true ) {
if ( null !== $pdo ) {
$options['pdo'] = $pdo;
}
$this->dbh = new WP_MySQL_On_SQLite(
$dbh = new WP_MySQL_On_SQLite(
sprintf(
'mysql-on-sqlite:path=%s;dbname=%s',
str_replace( ';', ';;', FQDB ),
Expand All @@ -327,27 +455,41 @@ public function db_connect( $allow_bail = true ) {
null,
$options
);
$this->dbh->setAttribute( PDO::ATTR_STRINGIFY_FETCHES, true ); // phpcs:ignore WordPress.DB.RestrictedClasses.mysql__PDO
$GLOBALS['@pdo'] = $this->dbh->get_connection()->get_pdo();
$dbh->setAttribute( PDO::ATTR_STRINGIFY_FETCHES, true ); // phpcs:ignore WordPress.DB.RestrictedClasses.mysql__PDO
$pdo = $dbh->get_connection()->get_pdo();
$this->dbh = $dbh;
$GLOBALS['@pdo'] = $pdo;
} catch ( Throwable $e ) {
$this->last_error = $this->format_error_message( $e );
}
if ( $this->last_error ) {
return false;
}
if ( ! $this->has_connected ) {
$this->init_charset();
}

$this->has_connected = true;
$this->set_charset( $this->dbh );

$this->ready = true;
$this->set_sql_mode();
return true;
}

/**
* Method to dummy out wpdb::check_connection()
* Checks that the database connection is available.
*
* @param bool $allow_bail Not used.
*
* @return bool
* @return bool True when the connection is available, false otherwise.
*/
public function check_connection( $allow_bail = true ) {
return true;
if ( $this->dbh ) {
return true;
}

return $this->db_connect( $allow_bail );
}

/**
Expand Down Expand Up @@ -397,7 +539,7 @@ public function query( $query ) {
$this->hide_errors();
}

if ( ! $this->ready ) {
if ( ! $this->ready && ! $this->check_connection() ) {
return false;
}

Expand All @@ -420,14 +562,13 @@ public function query( $query ) {
$last_query_count = count( $this->queries ?? array() );

/*
* @TODO: WPDB uses "$this->check_current_query" to check table/column
* charset and strip all invalid characters from the query.
* This is an involved process that we can bypass for SQLite,
* if we simply strip all invalid UTF-8 characters from the query.
* @TODO: wpdb uses "$this->check_current_query" and table metadata to
* reject queries containing invalid text. Implement equivalent handling
* for SQLite without relying on the MySQL-specific conversion pipeline.
*
* To do so, mb_convert_encoding can be used with an optional
* fallback to a htmlspecialchars method. E.g.:
* https://github.com/nette/utils/blob/be534713c227aeef57ce1883fc17bc9f9e29eca2/src/Utils/Strings.php#L42
* PCRE's "u" modifier can validate UTF-8 without constructing a converted
* query copy: 1 === preg_match( '//u', $query ). The implementation must
* preserve wpdb's exemptions for prevalidated and binary data.
*/

@JanJakes JanJakes Mar 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The TODO explains what should be done for SQLite. However, the implementation doesn't seem to follow it, and it removes all of that context. The TODO is there for a reason, and we should follow it.

$this->_do_query( $query );

Expand Down Expand Up @@ -559,21 +700,22 @@ protected function load_col_info() {
}

/**
* Method to return what the database can do.
* Determines whether the database supports a given feature.
*
* This overrides wpdb::has_cap() to avoid using MySQL functions.
* SQLite supports subqueries, but not support collation, group_concat and set_charset.
* The utf8mb4 check is handled here because older WordPress versions inspect
* the MySQL client library. All other capabilities use the parent logic.
*
* @see wpdb::has_cap()
*
* @param string $db_cap The feature to check for. Accepts 'collation',
* 'group_concat', 'subqueries', 'set_charset',
* 'utf8mb4', or 'utf8mb4_520'.
*
* @return bool Whether the database feature is supported, false otherwise.
* @param string $db_cap The feature to check for.
* @return bool True when the database feature is supported, false otherwise.
*/
public function has_cap( $db_cap ) {
return 'subqueries' === strtolower( $db_cap );

@JanJakes JanJakes Mar 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we still need this method override at all? The capabilities are derived from the MySQL version, so they depend on the db_version() implementation (currently hardcoded to 8.0).

if ( 'utf8mb4' === strtolower( $db_cap ) ) {
return true;
}

return parent::has_cap( $db_cap );
}

/**
Expand All @@ -592,9 +734,13 @@ public function db_version() {
/**
* Returns the version of the SQLite engine.
*
* @return string SQLite engine version as a string.
* @return string SQLite engine version, or an empty string while disconnected.
*/
public function db_server_info() {
if ( ! $this->dbh ) {
return '';
}

return $this->dbh->get_sqlite_version();
}

Expand Down
Loading