Make WordPress Core

Opened 10 years ago

Last modified 8 days ago

#36610 new defect (bug)

Loss of multibyte category and tag names

Reported by: cfinke Owned by:
Priority: normal Milestone:
Component: Taxonomy Version:
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses:

Description

Some multibyte category and tag names can be lost during creation.

Example: create a category with the name テテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテAAA. It is 201 bytes long and will be truncated by $wpdb->strip_invalid_text_for_column() to 200 bytes (テテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテAA) before the category is created.

However, the category name AAAテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテテ is also 201 bytes, but when it is truncated to 200 bytes, it splits a multibyte character, so when wp_check_invalid_utf8() gets called, it will truncate the string to zero bytes out of an abundance of caution, since the string ends with something that is not valid utf8.

It's clear that the category creator was not submitting invalid utf8, and the true goal of $wpdb->strip_invalid_text_for_column() was to ensure that the text would fit in the DB column without auto-truncation by the DB engine, so the ideal behavior should be that the string is truncated to the longest possible length that remains valid and fits within the column.

One way to get around this data loss would be a wrapper around wp_check_invalid_utf8(). If wp_check_invalid_utf8() fails, chop a single byte off the end of the string and check it again, up to the point where you have checked the string without the last five bytes (as I believe that the longest a single character can be is six bytes, although I'm not positive about that and I think anything longer than four bytes is mostly theoretical). Or, fix $wpdb->strip_invalid_text_for_column() so that it doesn't truncate in the middle of a multibyte character.

There might be a solution lurking in mb_strlen(). If wp_check_invalid_utf8() returns an empty string, take bytes off of the original string (up to 5 bytes) until mb_strlen() returns a smaller number and then try wp_check_invalid_utf8().

Configuration details: Tested in WordPress trunk (4.5-RC1-37153) and PHP 5.2.17

Here's my wp_terms structure:

CREATE TABLE `wp_terms` (
  `term_id` bigint(20) unsigned NOT NULL AUTO_INCREMENT,
  `name` varchar(200) NOT NULL DEFAULT '',
  `slug` varchar(200) NOT NULL DEFAULT '',
  `term_group` bigint(10) NOT NULL DEFAULT '0',
  PRIMARY KEY (`term_id`),
  KEY `slug` (`slug`(191)),
  KEY `name` (`name`(191))
) ENGINE=MyISAM DEFAULT CHARSET=utf8;

See #36393 for discussion of a similar (but now-fixed) bug.

Change History (4)

#1 @ocean90
10 years ago

  • Keywords needs-patch added
  • Version trunk

#2 @jdeep
12 months ago

Picking this up.

#3 @khokansardar
9 days ago

Re-tested on trunk @ 4676feb1e2 (PHP 8.3, MySQL 8.0). The symptom in the description does not reproduce, but the underlying gap is still there and has a second, worse face.

The multibyte loss is gone

terms.name is char-typed, not byte-typed — wpdb::get_col_length() returns {"type":"char","length":200} for varchar(200) / utf8mb4_unicode_520_ci — so strip_invalid_text() truncates with mb_substr(), never mid-character, and wp_check_invalid_utf8() is never handed a split sequence. Both names from the description now store intact:

"AAA" + 99 x U+00E9   102 chars / 201 bytes  -> created, stored intact
99 x U+00E9 + "AAA"   102 chars / 201 bytes  -> created, stored intact
200 x U+00E9          200 chars / 400 bytes  -> created, stored intact

The boundary is 200 characters, regardless of byte length. (Tested on utf8mb4; a latin1 terms table takes the byte-truncation branch of strip_invalid_text() and may still differ.)

What remains

Crossing 200 characters is no longer silent truncation. wpdb::process_fields() (wp-includes/class-wpdb.php:2812) truncates, compares against the input, and returns false, so the query never reaches MySQL:

201 x "a" -> wp_insert_term() = WP_Error( 'db_insert_error', 'Could not insert term into the database.' )

No data loss, but a database-layer error for what is an input-validation problem — where wp_insert_user() (user.php:2321), wp_check_comment_data_max_lengths() (comment.php:1319), register_post_type() (post.php:1867) and register_taxonomy() (taxonomy.php:527) all return a descriptive WP_Error. Nothing in wp_insert_term() or wp_update_term() checks the length of name.

The limit is also easier to hit than it looks. The default pre_term_name filters (_wp_specialchars, wp-includes/default-filters.php:33) expand & to & after submission, so a name of 186 characters as typed, containing 12 ampersands, expands to 222 and is refused.

On the update path the same input reports success and saves nothing — filed separately as #66007, since it is a return-value bug rather than a validation one and reproduces without any length involved.

This ticket was mentioned in PR #13346 on WordPress/wordpress-develop by @irozum.


8 days ago
#4

  • Keywords has-patch has-unit-tests added; needs-patch removed

wp_insert_term() and wp_update_term() never validate the length of the term name before writing it, unlike the equivalent checks already in place for wp_insert_user(), wp_check_comment_data_max_lengths(), register_post_type(), and register_taxonomy(). When a sanitized name exceeds the wp_terms.name column's length, the create path fails with a generic db_insert_error ("Could not insert term into the database.") instead of a descriptive error, and the update path is worse: $wpdb->update()'s return value is never checked, so wp_update_term() reports success while silently discarding the change.

This adds a small private helper, _wp_check_term_name_length(), that reads the actual column length via $wpdb->get_col_length( $wpdb->terms, 'name' ) (falling back to the schema default of 200) and compares it against the name's character count with mb_strlen() — matching the DB column's char-typed length semantics rather than counting bytes. Both wp_insert_term() and wp_update_term() now call this right after their existing empty-name checks and return a new term_name_too_long WP_Error before attempting any database write.

The original ticket's reported symptom (multibyte category/tag names getting silently truncated to invalid UTF-8 and losing all their bytes) no longer reproduces on current trunk — strip_invalid_text() now truncates on character boundaries via mb_substr() for char-typed columns, so wp_check_invalid_utf8() never sees a split sequence. What remains, and what this PR fixes, is the missing length validation itself: crossing the limit no longer causes silent data loss, but it still surfaced as an unhelpful low-level DB error (or, on update, as a false "success").

The separate bug where wp_update_term() reports success without saving *any* change (unrelated to name length) is filed as its own ticket, #66007, and is intentionally out of scope here.

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Implementation, tests, and PR description. Reviewed by irozum.

Note: See TracTickets for help on using tickets.