Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions src/wp-includes/class-wp-xmlrpc-server.php
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,10 @@ public function login_pass_ok(
* @param string|array $data Escape single string or array of strings.
* @return string|null Returns with string if passed, alters by-reference
* when array is passed.
*
* @phpstan-template T of string|array
* @phpstan-param T $data
* @phpstan-param-out T $data
*/
public function escape( &$data ) {
if ( ! is_array( $data ) ) {
Expand All @@ -387,6 +391,8 @@ public function escape( &$data ) {
$v = wp_slash( $v );
}
}

// @phpstan-ignore paramOut.type (The loop only slashes string values in place, so the shape of T is preserved.)
return null;
}

Expand Down Expand Up @@ -1658,7 +1664,11 @@ protected function _insert_post( $user, $content_struct ) {
return new IXR_Error( 401, __( 'Sorry, you are not allowed to assign a term to one of the given taxonomies.' ) );
}

$term_ids = $post_data['terms'][ $taxonomy ];
$term_ids = $post_data['terms'][ $taxonomy ];
if ( ! is_array( $term_ids ) ) {
return new IXR_Error( 403, __( 'Invalid term ID.' ) );
}

$terms[ $taxonomy ] = array();
foreach ( $term_ids as $term_id ) {
$term = get_term_by( 'id', $term_id, $taxonomy );
Expand Down Expand Up @@ -1705,11 +1715,23 @@ protected function _insert_post( $user, $content_struct ) {
// Filter out non-ambiguous term names.
$ambiguous_tax_term_counts = array_filter( $tax_term_names_count, array( $this, '_is_greater_than_one' ) );

$ambiguous_terms = array_keys( $ambiguous_tax_term_counts );
// Cast back to strings, since array keys turn numeric names into integers.
$ambiguous_terms = array_map( 'strval', array_keys( $ambiguous_tax_term_counts ) );
}

$term_names = $post_data['terms_names'][ $taxonomy ];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment by Claude Opus 5.5

$term_names itself is not checked to be an array. Sending terms_names => array( 'post_tag' => 'foo' ) makes the foreach emit a "foreach() argument must be of type array|object" warning, and the terms are silently ignored instead of returning an error. The $term_ids branch has the same gap. Possibly out of scope, but it sits right next to the new per-item guard.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment by Claude Opus 5.5

Addressed in cb770c2. Both the terms and terms_names branches now check that the per-taxonomy value is an array, returning the existing "Invalid term ID." (403) or "Invalid term name." (401) error respectively, instead of emitting a foreach warning and silently dropping the terms. test_terms_not_array covers a string and an int for terms and a string for terms_names; all three cases hit the warning before the fix. Since wp.editPost shares _insert_post(), it is covered too.

if ( ! is_array( $term_names ) ) {
return new IXR_Error( 401, __( 'Invalid term name.' ) );
}

foreach ( $term_names as $term_name ) {
// Numeric names may arrive as <int> or <double> values.
if ( is_int( $term_name ) || is_float( $term_name ) ) {
$term_name = (string) $term_name;
} elseif ( ! is_string( $term_name ) ) {
return new IXR_Error( 401, __( 'Invalid term name.' ) );
}

if ( in_array( $term_name, $ambiguous_terms, true ) ) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment by Claude Opus 5.5

Ambiguous numeric term names cannot be detected here. $ambiguous_terms comes from array_keys( array_count_values( ... ) ), which converts numeric-string names like "2024" into int keys. Since $term_name is now guaranteed to be a string, the strict in_array( '2024', array( 2024 ), true ) is always false, so a hierarchical taxonomy with two terms named "2024" silently resolves to whichever get_term_by() returns first. (The string case predates this PR, but an int input used to match, and now no input can trigger the check.) Mapping $ambiguous_terms through strval would fix it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment by Claude Opus 5.5

Addressed in 5f65187. $ambiguous_terms is now mapped through strval, so numeric names match the (string) term name in the strict in_array() check. test_terms_names_ambiguous_numeric_term_name covers a hierarchy with two categories named "2024", sent both as '2024' and 2024; both cases failed before the fix.

return new IXR_Error( 401, __( 'Ambiguous term name used in a hierarchical taxonomy. Please use term ID instead.' ) );
}
Expand Down Expand Up @@ -2198,6 +2220,10 @@ public function wp_newTerm( $args ) {
// Hold the data of the term.
$term_data = array();

if ( isset( $content_struct['name'] ) && ! ( is_string( $content_struct['name'] ) || is_int( $content_struct['name'] ) || is_float( $content_struct['name'] ) ) ) {
return new IXR_Error( 403, __( 'Invalid term name.' ) );
}

$term_data['name'] = trim( $content_struct['name'] );
if ( empty( $term_data['name'] ) ) {
return new IXR_Error( 403, __( 'The term name cannot be empty.' ) );
Expand Down Expand Up @@ -2316,6 +2342,10 @@ public function wp_editTerm( $args ) {
}

if ( isset( $content_struct['name'] ) ) {
if ( ! ( is_string( $content_struct['name'] ) || is_int( $content_struct['name'] ) || is_float( $content_struct['name'] ) ) ) {
return new IXR_Error( 403, __( 'Invalid term name.' ) );
}

$term_data['name'] = trim( $content_struct['name'] );

if ( empty( $term_data['name'] ) ) {
Expand Down Expand Up @@ -3541,6 +3571,10 @@ public function wp_newCategory( $args ) {
return new IXR_Error( 401, __( 'Sorry, you are not allowed to add a category.' ) );
}

if ( isset( $category['name'] ) && ! ( is_string( $category['name'] ) || is_int( $category['name'] ) || is_float( $category['name'] ) ) ) {
return new IXR_Error( 403, __( 'Invalid term name.' ) );
}

/*
* If no slug was provided, make it empty
* so that WordPress will generate one.
Expand Down
26 changes: 21 additions & 5 deletions src/wp-includes/taxonomy.php
Original file line number Diff line number Diff line change
Expand Up @@ -2545,10 +2545,11 @@ function wp_get_object_terms( $object_ids, $taxonomies, $args = array() ) {
* @global wpdb $wpdb WordPress database abstraction object.
*
* @since 2.3.0
* @since 7.2.0 A `$term` that is not a string or number now returns a WP_Error instead of causing a fatal error.
*
* @param string $term The term name to add.
* @param string $taxonomy The taxonomy to which to add the term.
* @param array|string $args {
* @param string|int|float $term The term name to add. A number is stored as its string form.
* @param string $taxonomy The taxonomy to which to add the term.
* @param array|string $args {
* Optional. Array or query string of arguments for inserting a term.
*
* @type string $alias_of Slug of the term to make this term an alias of.
Expand Down Expand Up @@ -2602,7 +2603,14 @@ function wp_insert_term( $term, $taxonomy, $args = array() ) {
return new WP_Error( 'invalid_term_id', __( 'Invalid term ID.' ) );
}

if ( '' === trim( $term ) ) {
// Null and false are treated as a missing name rather than an invalid one, as they were before.
if ( null === $term || false === $term ) {
$term = '';
} elseif ( ! ( is_string( $term ) || is_int( $term ) || is_float( $term ) ) ) {
return new WP_Error( 'invalid_term_name', __( 'Invalid term name.' ) );
}

if ( '' === trim( (string) $term ) ) {
return new WP_Error( 'empty_term_name', __( 'A name is required for this term.' ) );
}

Expand Down Expand Up @@ -2957,6 +2965,7 @@ function wp_insert_term( $term, $taxonomy, $args = array() ) {
* exists under.
*
* @since 2.3.0
* @since 7.2.0 A term that is not a string or number now returns a WP_Error instead of causing a fatal error.
*
* @global wpdb $wpdb WordPress database abstraction object.
*
Expand All @@ -2983,6 +2992,13 @@ function wp_set_object_terms( $object_id, $terms, $taxonomy, $append = false ) {
$terms = array( $terms );
}

// Validate all terms before making any changes. Null and false are skipped below, as they were before.
foreach ( $terms as $term ) {
if ( ! ( is_string( $term ) || is_int( $term ) || is_float( $term ) || null === $term || false === $term ) ) {
return new WP_Error( 'invalid_term_name', __( 'Invalid term name.' ) );
}
}

if ( ! $append ) {
$old_tt_ids = wp_get_object_terms(
$object_id,
Expand All @@ -3001,7 +3017,7 @@ function wp_set_object_terms( $object_id, $terms, $taxonomy, $append = false ) {
$new_tt_ids = array();

foreach ( (array) $terms as $term ) {
if ( '' === trim( $term ) ) {
if ( '' === trim( (string) $term ) ) {
continue;
}

Expand Down
62 changes: 62 additions & 0 deletions tests/phpunit/tests/term/wpInsertTerm.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,68 @@ public function test_wp_insert_term_term_trims_to_empty_string() {
$this->assertSame( 'empty_term_name', $found->get_error_code() );
}

/**
* Tests that a term name that is not a string or number returns a WP_Error instead of causing a fatal error.
*
* @ticket 66108
*
* @dataProvider data_wp_insert_term_invalid_term_name_type
*
* @param mixed $name The term name to insert.
* @param non-empty-string $expected_error_code The expected error code.
*/
public function test_wp_insert_term_invalid_term_name_type( $name, string $expected_error_code ): void {
$found = wp_insert_term( $name, 'post_tag' ); // @phpstan-ignore argument.type (Intentionally passing invalid types.)

$this->assertWPError( $found );
$this->assertSame( $expected_error_code, $found->get_error_code() );
}

/**
* Data provider for {@see self::test_wp_insert_term_invalid_term_name_type()}.
*
* @return array<truthy-string, array{ mixed, non-empty-string }>
*/
public function data_wp_insert_term_invalid_term_name_type(): array {
return array(
'array' => array( array( 'foo', 'bar' ), 'invalid_term_name' ),
'object' => array( new stdClass(), 'invalid_term_name' ),
'true' => array( true, 'invalid_term_name' ),
'false' => array( false, 'empty_term_name' ),
'null' => array( null, 'empty_term_name' ),
);
}

/**
* Tests that a numeric term name is inserted and stored as its string form.
*
* @ticket 66108
*
* @dataProvider data_wp_insert_term_numeric_term_name
*
* @param int|float $name The term name to insert.
*/
public function test_wp_insert_term_numeric_term_name( $name ): void {
$found = wp_insert_term( $name, 'post_tag' );

$this->assertNotWPError( $found );
Comment thread
westonruter marked this conversation as resolved.
$term = get_term( $found['term_id'], 'post_tag' );
$this->assertInstanceOf( WP_Term::class, $term );
$this->assertSame( (string) $name, $term->name );
}

/**
* Data provider for {@see self::test_wp_insert_term_numeric_term_name()}.
*
* @return array<truthy-string, array{ int|float }>
*/
public function data_wp_insert_term_numeric_term_name(): array {
return array(
'integer' => array( 2024 ),
'float' => array( 1.5 ),
);
}

public function test_wp_insert_term_parent_does_not_exist() {
$found = wp_insert_term(
'foo',
Expand Down
52 changes: 52 additions & 0 deletions tests/phpunit/tests/term/wpSetObjectTerms.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
*/
class Tests_Term_WpSetObjectTerms extends WP_UnitTestCase {
protected static $taxonomy = 'category';

/**
* @var int[]
*/
protected static $post_ids = array();
protected static $term_ids = array();

Expand All @@ -15,6 +19,54 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) {
self::$term_ids = $factory->term->create_many( 5, array( 'taxonomy' => self::$taxonomy ) );
}

/**
* Tests that a term that is not a string or number returns a WP_Error without changing existing terms.
*
* @ticket 66108
*
* @dataProvider data_invalid_term_type
*
* @param mixed $term The invalid term to set alongside a valid one.
*/
public function test_invalid_term_type( $term ): void {
$post_id = self::$post_ids[0];
wp_set_object_terms( $post_id, array( 'existing' ), 'post_tag' );

$result = wp_set_object_terms( $post_id, array( 'new', $term ), 'post_tag' );

$this->assertWPError( $result );
$this->assertSame( 'invalid_term_name', $result->get_error_code() );
$this->assertSame( array( 'existing' ), wp_get_object_terms( $post_id, 'post_tag', array( 'fields' => 'names' ) ) );
$this->assertNull( term_exists( 'new', 'post_tag' ) );
}

/**
* Data provider for {@see self::test_invalid_term_type()}.
*
* @return array<truthy-string, array{ mixed }>
*/
public function data_invalid_term_type(): array {
return array(
'array' => array( array( 'foo' ) ),
'object' => array( new stdClass() ),
'true' => array( true ),
);
}

/**
* Tests that null and false terms are skipped rather than rejected.
*
* @ticket 66108
*/
public function test_null_and_false_terms_are_skipped(): void {
$post_id = self::$post_ids[0];

$result = wp_set_object_terms( $post_id, array( 'foo', null, false ), 'post_tag' );

$this->assertNotWPError( $result );
$this->assertSame( array( 'foo' ), wp_get_object_terms( $post_id, 'post_tag', array( 'fields' => 'names' ) ) );
}

/**
* @ticket 26570
*/
Expand Down
20 changes: 20 additions & 0 deletions tests/phpunit/tests/xmlrpc/mw/editPost.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,26 @@ public function test_edit_own_post() {
$this->assertSame( $new_title, $out->post_title );
}

/**
* Tests that a keyword that is not a string or number does not cause a fatal error.
*
* @ticket 66108
*/
public function test_invalid_keyword(): void {
$author_id = $this->make_user_by_role( 'author' );
$post_id = self::factory()->post->create( array( 'post_author' => $author_id ) );
wp_set_post_tags( $post_id, array( 'existing' ) );

$post = array(
'title' => 'Test',
'mt_keywords' => array( 'foo', array( 'bar' ) ),
);
$result = $this->myxmlrpcserver->mw_editPost( array( $post_id, 'author', 'author', $post ) );

$this->assertNotIXRError( $result );
$this->assertSame( array( 'existing' ), wp_get_post_tags( $post_id, array( 'fields' => 'names' ) ) );
}

public function test_capable_edit_others_post() {
$this->make_user_by_role( 'editor' );
$contributor_id = $this->make_user_by_role( 'contributor' );
Expand Down
18 changes: 18 additions & 0 deletions tests/phpunit/tests/xmlrpc/mw/newPost.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,24 @@ public function test_incapable_user() {
$this->assertSame( 401, $result->code );
}

/**
* Tests that a keyword that is not a string or number does not cause a fatal error.
*
* @ticket 66108
*/
public function test_invalid_keyword(): void {
$this->make_user_by_role( 'author' );

$post = array(
'title' => 'Test',
'mt_keywords' => array( 'foo', array( 'bar' ) ),
);
$result = $this->myxmlrpcserver->mw_newPost( array( 1, 'author', 'author', $post ) );

$this->assertNotIXRError( $result );
$this->assertSame( array(), wp_get_post_tags( (int) $result, array( 'fields' => 'names' ) ) );
}

public function test_no_content() {
$this->make_user_by_role( 'author' );

Expand Down
44 changes: 44 additions & 0 deletions tests/phpunit/tests/xmlrpc/wp/editTerm.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
* @group xmlrpc
*/
class Tests_XMLRPC_wp_editTerm extends WP_XMLRPC_UnitTestCase {
/**
* @var int
*/
protected static $parent_term;
protected static $child_term;
protected static $post_tag;
Expand Down Expand Up @@ -114,6 +117,47 @@ public function test_empty_term() {
$this->assertSame( __( 'Empty Term.' ), $result->message );
}

/**
* Tests that a term name that is not a string or number returns an error instead of causing a fatal error.
*
* @ticket 66108
*
* @dataProvider data_invalid_term_name
*
* @param mixed $name The term name to send.
*/
public function test_invalid_term_name( $name ): void {
$this->make_user_by_role( 'editor' );

$result = $this->myxmlrpcserver->wp_editTerm(
array(
1,
'editor',
'editor',
self::$parent_term,
array(
'taxonomy' => 'category',
'name' => $name,
),
)
);
$this->assertIXRError( $result );
$this->assertSame( 403, $result->code );
$this->assertSame( 'Invalid term name.', $result->message );
}

/**
* Data provider for {@see self::test_invalid_term_name()}.
*
* @return array<truthy-string, array{ mixed }>
*/
public function data_invalid_term_name(): array {
return array(
'array' => array( array( 'foo' ) ),
'boolean' => array( true ),
);
}

public function test_empty_term_name() {
$this->make_user_by_role( 'editor' );

Expand Down
Loading
Loading