Make WordPress Core

Opened 3 weeks ago

Last modified 3 weeks ago

#66007 reviewing defect (bug)

wp_update_term() reports success when the database write is rejected

Reported by: khokansardar Owned by: westonruter
Priority: normal Milestone: 7.2
Component: Taxonomy Version:
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses: rest-api

Description

wp_update_term() discards the return value of all three of its $wpdb->update() calls (wp-includes/taxonomy.php:3418, :3422, :3450). When a write is refused, the function still fires edited_terms / edited_term, clears the term cache, and returns array( 'term_id' => …, 'term_taxonomy_id' => … ) — indistinguishable from a real update.
wp_insert_term() has the same unchecked call at :2640.

Mechanism

wpdb::process_fields() (wp-includes/class-wpdb.php:2812) runs strip_invalid_text(), normalises each value to fit its column, compares the result against the input, and returns false if anything changed. $wpdb->update() then returns false without sending a query, so this is not MySQL truncation and strict mode is not involved.

The failure is also unrecoverable after the fact: wpdb::flush() resets last_error (wp-includes/class-wpdb.php:1927) and wp_update_term() runs several more queries before returning, so $wpdb->last_error is already empty when the caller receives the success array.

Reproducing

1. Ordinary input. terms.name is varchar(200) and neither wp_insert_term() nor
wp_update_term() checks its length. Reaching the limit does not require a 200-character name:
the default pre_term_name filters (_wp_specialchars, wp-includes/default-filters.php:33)
expand & to & after submission.

<?php
$name = str_repeat( 'a', 150 ) . str_repeat( ' & ', 12 ); // 186 characters as typed

$t = wp_insert_term( 'Example', 'category' );
$r = wp_update_term( $t['term_id'], 'category', array( 'name' => $name ) );

var_dump( $r );                              // array( 'term_id' => …, 'term_taxonomy_id' => … )
var_dump( get_term( $t['term_id'] )->name ); // "Example" — unchanged

2. Without any length involved. Any value process_fields() rejects behaves the same, so
a filter is enough:

<?php
add_filter( 'wp_update_term_data', function ( $data ) {
      $data['name'] = "Bad\xED\xA0\x80name";   // 19 characters, invalid UTF-8 seque
      return $data;
} );
// wp_update_term() returns the success array; the row is unchanged.

Both public write paths report success

  • RESTPOST /wp/v2/categories/<id> with the 186-character name returns 200 OK, the response body carries the old name, and the row is unchanged. The terms controller sets no maxLength on name.
  • Adminwp-admin/edit-tags.php:183-186 treats the truthy, non-WP_Error return as success and redirects with message=3, "Item updated."

Asymmetry with the insert path

wp_insert_term() checks both of its $wpdb->insert() calls (:2628, :2656) and returns WP_Error( 'db_insert_error' ) for the same input, so create and update disagree on identical data.

  • #36610 — over-length term names; the input-validation half. A length guard belongs there.
  • #46010, #31665 — the term slug side of the same 200-character column.

Change History (2)

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


3 weeks ago
#1

  • Keywords has-patch has-unit-tests added

wp_update_term() discards the return of all three of its $wpdb->update() calls. When wpdb::process_fields() refuses a value the write never reaches MySQL, yet the function still fires edited_terms and edited_term, clears the cache, and returns the success array — REST answers 200 with stale data. wp_insert_term() has the same gap on its empty-slug write.

Each write is now checked and returns WP_Error( 'db_update_error', …, $wpdb->last_error ) before the hooks fire. The comparison is strictly false ===, since $wpdb->update() returns 0 when the row matches but nothing changes, which is a success.

Five tests cover the rejected terms and term_taxonomy writes, a value rejected through wp_update_term_data, the hooks not firing, and a no-op update still succeeding.

Trac ticket: https://core.trac.wordpress.org/ticket/66007

## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Investigating the defect against trunk, drafting the patch, and writing the five unit tests. All changes were reviewed and validated by me.

#2 @westonruter
3 weeks ago

  • Milestone Awaiting Review7.2
  • Owner set to westonruter
  • Status newreviewing
Note: See TracTickets for help on using tickets.