-
Notifications
You must be signed in to change notification settings - Fork 57
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.
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
bf6a7ac
a17fa51
2d5ac1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,17 +71,55 @@ 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'; | ||
| /* | ||
| * 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 = null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * 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 ) { | ||
| // 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 = null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -134,14 +172,65 @@ 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. | ||
| */ | ||
| public function close() { | ||
| if ( ! $this->dbh ) { | ||
| return false; | ||
| } | ||
|
|
||
| $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 ( '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. | ||
| * | ||
|
|
@@ -444,15 +533,35 @@ 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. | ||
| * Strip invalid UTF-8 characters from non-ASCII queries. | ||
| * | ||
| * 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 | ||
| * SQLite stores all text as UTF-8, so we simply ensure the query | ||
| * contains only valid UTF-8 sequences rather than using the parent's | ||
| * MySQL-specific charset detection pipeline. | ||
| */ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| if ( $this->check_current_query && ! $this->check_ascii( $query ) ) { | ||
| if ( function_exists( 'mb_convert_encoding' ) ) { | ||
| $stripped_query = mb_convert_encoding( $query, 'UTF-8', 'UTF-8' ); | ||
| } else { | ||
| $stripped_query = htmlspecialchars_decode( | ||
| htmlspecialchars( $query, ENT_NOQUOTES | ENT_SUBSTITUTE, 'UTF-8' ), | ||
| ENT_NOQUOTES | ||
| ); | ||
| } | ||
|
|
||
| if ( $stripped_query !== $query ) { | ||
| $this->insert_id = 0; | ||
| $this->last_query = $query; | ||
|
|
||
| wp_load_translations_early(); | ||
|
|
||
| $this->last_error = __( 'WordPress database error: Could not perform query because it contains invalid data.' ); | ||
|
|
||
| return false; | ||
| } | ||
| } | ||
| $this->check_current_query = true; | ||
|
|
||
| $this->_do_query( $query ); | ||
|
|
||
| if ( $this->last_error ) { | ||
|
|
@@ -599,21 +708,35 @@ 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. | ||
| * This overrides wpdb::has_cap() while closely mirroring its implementation. | ||
| * The override is needed because the parent's 'utf8mb4' capability check calls | ||
| * mysqli_get_client_info(), which is environment-dependent and not applicable | ||
| * for SQLite. | ||
| * | ||
| * @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. | ||
| */ | ||
| public function has_cap( $db_cap ) { | ||
| return 'subqueries' === strtolower( $db_cap ); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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_cap = strtolower( $db_cap ); | ||
|
|
||
| switch ( $db_cap ) { | ||
| case 'collation': | ||
| case 'group_concat': | ||
| case 'subqueries': | ||
| case 'set_charset': | ||
| case 'utf8mb4': | ||
| return true; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a version_compare for this one |
||
| case 'utf8mb4_520': | ||
| return version_compare( $GLOBALS['wp_version'], '4.6', '>=' ); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this