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:
wp_insert_term()should return the existingempty_term_nameWP_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.- The terms_names loop in
_insert_post()should reject anything that is not a string with anIXR_Error, matching theInvalid 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
#2
@
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:
- Before Patch (trunk):
wp_insert_term( array( 'foo' ), 'post_tag' )throws a fatalTypeError: trim(): Argument #1 ($string) must be of type string, array given.- XML-RPC
wp.newPostwith array interms_namestriggers HTTP 500 instead of a structured XML-RPC fault.
- After PR #13521:
wp_insert_term()safely returnsWP_Error( 'empty_term_name' )for non-scalar$termvalues without fatals.- Numeric term names (
2024,1.5) continue to work properly as strings. - XML-RPC
_insert_post()cleanly returnsIXR_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
@
2 weeks ago
### Test Report
Tested: PR 13521
Environment: WordPress 7.2-alpha, PHP 8.3
#### Findings:
- On
trunk, passing non-scalar inputs towp_insert_term()results in an unhandled fatalTypeError: trim(): Argument #1 ($string) must be of type string, array giveninwp-includes/taxonomy.php:2561. - Applied PR 13521:
wp_insert_term()safely returnsWP_Error( 'empty_term_name' )when$termis non-scalar.- Numeric term names continue to function normally.
- XML-RPC
_insert_post()returns a 403IXR_Error( 'Invalid term name.' )instead of an HTTP 500 fatal. - All tests in
Tests_Term_wpInsertTerm(43 tests) and XML-RPCnewPost(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.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
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.