Make WordPress Core

Opened 4 weeks ago

Last modified 2 weeks ago

#66108 reviewing defect (bug)

Fatal error in wp_insert_term() when the term name is not a string

Reported by: josephscott Owned by: westonruter
Priority: normal Milestone: 7.2
Component: XML-RPC Version:
Severity: normal Keywords: has-patch has-unit-tests changes-requested
Cc: Focuses:

Description

An XML-RPC wp.newPost request with a terms_names entry that is an array instead of a string ends in a fatal error:

curl -i -H 'Content-Type: text/xml' http://localhost:8889/xmlrpc.php \
--data '<?xml version="1.0"?><methodCall><methodName>wp.newPost</methodName><params><param><value><int>1</int></value></param><param><value><string>admin</string></value></param><param><value><string>password</string></value></param><param><value><struct><member><name>post_title</name><value><string>Nested term name</string></value></member><member><name>terms_names</name><value><struct><member><name>post_tag</name><value><array><data><value><array><data><value><string>foo</string></value></data></array></value></data></array></value></member></struct></value></member></struct></value></param></params></methodCall>'

The terms_names loop in _insert_post() does not check the type of each term name. get_term_by( 'name', $array, $taxonomy ) casts the value to a string and returns false, so the code goes on to create the term:

<?php
$term_info = wp_insert_term( $term_name, $taxonomy );

wp_insert_term() then runs '' === trim( $term ). That causes PHP to throw a TypeError for an array, so instead of an XML-RPC fault the client gets an HTTP 500.

Two things need to change:

  1. wp_insert_term() should return the existing empty_term_name WP_Error for a non-scalar $term. This covers every caller, including wp.newTerm, the REST terms controller, and plugins. Numeric term names keep working as they do today.
  2. The terms_names loop in _insert_post() should reject anything that is not a string with an IXR_Error, matching the Invalid term ID. check for terms by ID right above it.

I will have a PR to address both of those items.

Change History (5)

This ticket was mentioned in ​PR #13521 on ​WordPress/wordpress-develop by ​@josephscott.


4 weeks ago
#1

  • Keywords has-patch has-unit-tests added

https://core.trac.wordpress.org/ticket/66108

AI assistance: Yes
Tool(s): Claude Code
Model(s): Fable 5.1
Used for: Research and experimentation. Guided directions on writing the code changes. It wrote the tests.

#2 @westonruter
4 weeks ago

  • Keywords changes-requested added
  • Milestone Awaiting Review → 7.2

As I just ​commented on your PR for #66107, I think we should start adopting JSON Schema for the XML-RPC methods, and use REST API functions for sanitization and validation. This will greatly simplify the logic in the methods, as there won't need to be many isset and type check guards all over the place.

​@therssoftware commented on ​PR #13521:


2 weeks ago
#3

Tested and Verified ✅

Environment:

  • WordPress: 7.2-alpha (trunk)
  • PHP: 8.3

Verification:

  1. Before Patch (trunk):
    • wp_insert_term( array( 'foo' ), 'post_tag' ) throws a fatal TypeError: trim(): Argument #1 ($string) must be of type string, array given.
    • XML-RPC wp.newPost with array in terms_names triggers HTTP 500 instead of a structured XML-RPC fault.
  1. After PR #13521:
    • wp_insert_term() safely returns WP_Error( 'empty_term_name' ) for non-scalar $term values without fatals.
    • Numeric term names (2024, 1.5) continue to work properly as strings.
    • XML-RPC _insert_post() cleanly returns IXR_Error( 403, 'Invalid term name.' ).
    • PHPUnit:
      • tests/phpunit/tests/term/wpInsertTerm.php: Pass (43 tests, 166 assertions)
      • tests/phpunit/tests/xmlrpc/wp/newPost.php: Pass (32 tests, 78 assertions)
      • Full --group taxonomy: Pass (913 tests, 2383 assertions)
      • Full --group xmlrpc: Pass (356 tests, 1339 assertions)
    • PHPCS: 0 errors, 0 warnings on changed lines.

Regarding the discussion on JSON Schema adoption for XML-RPC: while a future schema-driven sanitization layer for XML-RPC is a worthy initiative, this PR addresses a real PHP 8+ fatal error in wp_insert_term() and _insert_post() cleanly and without breaking changes. Recommending approval.

#4 @therssoftware
2 weeks ago

### Test Report

Tested: ​PR 13521
Environment: WordPress 7.2-alpha, PHP 8.3

#### Findings:

  1. On trunk, passing non-scalar inputs to wp_insert_term() results in an unhandled fatal TypeError: trim(): Argument #1 ($string) must be of type string, array given in wp-includes/taxonomy.php:2561.
  2. Applied PR 13521:
    • wp_insert_term() safely returns WP_Error( 'empty_term_name' ) when $term is non-scalar.
    • Numeric term names continue to function normally.
    • XML-RPC _insert_post() returns a 403 IXR_Error( 'Invalid term name.' ) instead of an HTTP 500 fatal.
    • All tests in Tests_Term_wpInsertTerm (43 tests) and XML-RPC newPost (32 tests) pass.
    • Ran entire --group taxonomy (913 tests) and --group xmlrpc (356 tests) suites with 0 regressions.
    • Code conforms to WordPress Coding Standards.

PR 13521 resolves the fatal error cleanly and is ready for review.

#5 @westonruter
2 weeks ago

  • Owner set to westonruter
  • Status new → reviewing
Note: See TracTickets for help on using tickets.