Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
139 changes: 127 additions & 12 deletions src/wp-includes/class-wp-xmlrpc-server.php
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,25 @@ protected function _is_fields_array( $fields ): bool {
return true;
}

/**
* Checks that the content struct argument received from a client is an array.
*
* @since 7.2.0
*
* @param mixed $content_struct The content struct argument to check.
* @return bool True if `$content_struct` is an array, false otherwise.
*
* @phpstan-assert-if-true array $content_struct
*/
protected function _is_content_struct_array( $content_struct ): bool {
if ( ! is_array( $content_struct ) ) {
$this->error = new IXR_Error( 400, __( 'The content struct argument must be an array.' ) );
return false;
}

return true;
}
Comment on lines +838 to +847

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.

Question: Is it the case that the struct should alwatys be an array<string, string>? If so, this could be hardened to eliminate some is_string() checks below:

Suggested change
* @phpstan-assert-if-true array $content_struct
*/
protected function _is_content_struct_array( $content_struct ): bool {
if ( ! is_array( $content_struct ) ) {
$this->error = new IXR_Error( 400, __( 'The content struct argument must be an array.' ) );
return false;
}
return true;
}
* @phpstan-assert-if-true array<string, string> $content_struct
*/
protected function _is_content_struct_array( $content_struct ): bool {
if ( ! is_array( $content_struct ) ) {
$this->error = new IXR_Error( 400, __( 'The content struct argument must be an array.' ) );
return false;
}
foreach ( $content_struct as $key => $value ) {
if ( ! is_string( $key ) ) {
$this->error = new IXR_Error( 400, __( 'The content struct argument must be an array with string keys.' ) );
return false;
}
if ( ! is_string( $value ) ) {
$this->error = new IXR_Error( 400, __( 'The content struct argument must be an array with string values.' ) );
return false;
}
}
return true;
}


/**
* Prepares taxonomy data for return in an XML-RPC object.
*
Expand Down Expand Up @@ -910,11 +929,13 @@ protected function _prepare_term( $term ) {
/**
* Converts a WordPress date string to an IXR_Date object.
*
* @since 7.2.0 A value that is not a string is treated as an empty date.
*
* @param string $date Date string to convert.
* @return IXR_Date IXR_Date object.
*/
protected function _convert_date( $date ) {
if ( '0000-00-00 00:00:00' === $date ) {
if ( ! is_string( $date ) || '0000-00-00 00:00:00' === $date ) {
return new IXR_Date( '00000000T00:00:00Z' );
}
return new IXR_Date( mysql2date( 'Ymd\TH:i:s', $date, false ) );
Expand All @@ -928,12 +949,36 @@ protected function _convert_date( $date ) {
* @return IXR_Date IXR_Date object.
*/
protected function _convert_date_gmt( $date_gmt, $date ) {
if ( '0000-00-00 00:00:00' !== $date && '0000-00-00 00:00:00' === $date_gmt ) {
if ( is_string( $date ) && '0000-00-00 00:00:00' !== $date && '0000-00-00 00:00:00' === $date_gmt ) {
return new IXR_Date( get_gmt_from_date( mysql2date( 'Y-m-d H:i:s', $date, false ), 'Ymd\TH:i:s' ) );
}
return $this->_convert_date( $date_gmt );
}

/**
* Converts a client-supplied date value to an IXR_Date object.
*
* XML-RPC clients may send a date either as a dateTime.iso8601 value, which
* arrives as an IXR_Date object, or as a plain string. Any other type cannot
* be a date and results in an error.
*
* @since 7.2.0
*
* @param mixed $date Client-supplied date value.
* @return IXR_Date|IXR_Error IXR_Date object on success, IXR_Error if the value is not a date.

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.

Looks like there's an opportunity to make this a bit more deterministic as to what the return value will be:

Suggested change
* @return IXR_Date|IXR_Error IXR_Date object on success, IXR_Error if the value is not a date.
* @return IXR_Date|IXR_Error IXR_Date object on success, IXR_Error if the value is not a date.
*
* @phpstan-return ( $date is IXR_Date|string ? IXR_Date : IXR_Error )

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.

But see other comments.

*/
protected function _convert_client_date( $date ) {
if ( $date instanceof IXR_Date ) {
return $date;
}
Comment on lines +971 to +973

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.

Is an IXR_Date actually ever passed into this method?


if ( is_string( $date ) ) {
return $this->_convert_date( $date );
}

return new IXR_Error( 400, __( 'Dates must be a dateTime.iso8601 value or a string.' ) );
}
Comment on lines +970 to +980

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.

With my suggestion for _is_content_struct_array(), this method may not be needed and _convert_date() could be used alone.


/**
* Prepares post data for return in an XML-RPC object.
*
Expand Down Expand Up @@ -1305,6 +1350,7 @@ protected function _prepare_user( $user, $fields ) {
* Creates a new post for any registered post type.
*
* @since 3.4.0
* @since 7.2.0 Returns an error if the content struct argument is not an array.
*
* @link https://en.wikipedia.org/wiki/RSS_enclosure for information on RSS enclosures.
*
Expand Down Expand Up @@ -1359,21 +1405,25 @@ public function wp_newPost( $args ) {
$password = $args[2];
$content_struct = $args[3];

if ( ! $this->_is_content_struct_array( $content_struct ) ) {
return $this->error;
}

$user = $this->login( $username, $password );
if ( ! $user ) {
return $this->error;
}

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.

I think this warrants adding a type check for the $content_struct. Something like this:

Suggested change
}
}
if ( ! is_array( $content_struct ) ) {
return new IXR_Error( 400, __( 'Invalid parameters.', 'xmlrpc-server' ) );
}

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.

Same goes for wp_editPost below.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've added a _is_content_struct_array() helper method to check for this. It is now applied in the places where we expect that struct, and tests against those.


// Convert the date field back to IXR form.
if ( isset( $content_struct['post_date'] ) && ! ( $content_struct['post_date'] instanceof IXR_Date ) ) {
if ( isset( $content_struct['post_date'] ) && is_string( $content_struct['post_date'] ) ) {

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.

Per my above suggestion, this is_string() check could be removed.

Suggested change
if ( isset( $content_struct['post_date'] ) && is_string( $content_struct['post_date'] ) ) {
if ( isset( $content_struct['post_date'] ) ) {

$content_struct['post_date'] = $this->_convert_date( $content_struct['post_date'] );
}

/*
* Ignore the existing GMT date if it is empty or a non-GMT date was supplied in $content_struct,
* since _insert_post() will ignore the non-GMT date if the GMT date is set.
*/
if ( isset( $content_struct['post_date_gmt'] ) && ! ( $content_struct['post_date_gmt'] instanceof IXR_Date ) ) {
if ( isset( $content_struct['post_date_gmt'] ) && is_string( $content_struct['post_date_gmt'] ) ) {

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.

Ditto.

Suggested change
if ( isset( $content_struct['post_date_gmt'] ) && is_string( $content_struct['post_date_gmt'] ) ) {
if ( isset( $content_struct['post_date_gmt'] ) ) {

if ( '0000-00-00 00:00:00' === $content_struct['post_date_gmt'] || isset( $content_struct['post_date'] ) ) {
unset( $content_struct['post_date_gmt'] );
} else {
Expand Down Expand Up @@ -1550,10 +1600,20 @@ protected function _insert_post( $user, $content_struct ) {

// Do some timestamp voodoo.
if ( ! empty( $post_data['post_date_gmt'] ) ) {
$post_date_gmt = $this->_convert_client_date( $post_data['post_date_gmt'] );
if ( $post_date_gmt instanceof IXR_Error ) {
return $post_date_gmt;
}
Comment on lines +1604 to +1606

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.

This seems like it could be removed as well, but it might need some PHPStan typing support.

Suggested change
if ( $post_date_gmt instanceof IXR_Error ) {
return $post_date_gmt;
}


// We know this is supposed to be GMT, so we're going to slap that Z on there by force.
$date_created = rtrim( $post_data['post_date_gmt']->getIso(), 'Z' ) . 'Z';
$date_created = rtrim( $post_date_gmt->getIso(), 'Z' ) . 'Z';
} elseif ( ! empty( $post_data['post_date'] ) ) {
$date_created = $post_data['post_date']->getIso();
$post_date = $this->_convert_client_date( $post_data['post_date'] );
if ( $post_date instanceof IXR_Error ) {
return $post_date;
}
Comment on lines +1612 to +1614

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.

Suggested change
if ( $post_date instanceof IXR_Error ) {
return $post_date;
}


$date_created = $post_date->getIso();
}

// Default to not flagging the post date to be edited unless it's intentional.
Expand Down Expand Up @@ -1753,6 +1813,7 @@ static function ( $value ) {
* should be changed. All other fields will retain their existing values.
*
* @since 3.4.0
* @since 7.2.0 Returns an error if the content struct argument is not an array.
*
* @param array $args {
* Method arguments. Note: arguments must be ordered as documented.
Expand All @@ -1777,6 +1838,10 @@ public function wp_editPost( $args ) {
$post_id = (int) $args[3];
$content_struct = $args[4];

if ( ! $this->_is_content_struct_array( $content_struct ) ) {
return $this->error;
}

$user = $this->login( $username, $password );
if ( ! $user ) {
return $this->error;
Expand All @@ -1792,8 +1857,18 @@ public function wp_editPost( $args ) {
}

if ( isset( $content_struct['if_not_modified_since'] ) ) {
$if_not_modified_since = $this->_convert_client_date( $content_struct['if_not_modified_since'] );
if ( $if_not_modified_since instanceof IXR_Error ) {
return $if_not_modified_since;
}
Comment on lines +1861 to +1863

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.

Per above, this could never happen because $content_struct['if_not_modified_since'] would always be a string.

Suggested change
if ( $if_not_modified_since instanceof IXR_Error ) {
return $if_not_modified_since;
}


$post_modified_timestamp = false;
if ( is_string( $post['post_modified_gmt'] ) ) {
$post_modified_timestamp = mysql2date( 'U', $post['post_modified_gmt'] );
}

// If the post has been modified since the date provided, return an error.
if ( mysql2date( 'U', $post['post_modified_gmt'] ) > $content_struct['if_not_modified_since']->getTimestamp() ) {
if ( false !== $post_modified_timestamp && $post_modified_timestamp > $if_not_modified_since->getTimestamp() ) {
return new IXR_Error( 409, __( 'There is a revision of this post that is more recent.' ) );
}
}
Expand Down Expand Up @@ -3861,6 +3936,7 @@ public function wp_deleteComment( $args ) {
* - 'status'. Common statuses are 'approve', 'hold', 'spam'. See get_comment_statuses() for more details.
*
* @since 2.7.0
* @since 7.2.0 Returns an error if the content struct argument is not an array.
*
* @param array $args {
* Method arguments. Note: arguments must be ordered as documented.
Expand All @@ -3881,6 +3957,10 @@ public function wp_editComment( $args ) {
$comment_id = (int) $args[3];
$content_struct = $args[4];

if ( ! $this->_is_content_struct_array( $content_struct ) ) {
return $this->error;
}

$user = $this->login( $username, $password );
if ( ! $user ) {
return $this->error;
Expand Down Expand Up @@ -3913,8 +3993,13 @@ public function wp_editComment( $args ) {

// Do some timestamp voodoo.
if ( ! empty( $content_struct['date_created_gmt'] ) ) {
$date_created_gmt = $this->_convert_client_date( $content_struct['date_created_gmt'] );
if ( $date_created_gmt instanceof IXR_Error ) {
return $date_created_gmt;
}
Comment on lines +3997 to +3999

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.

Suggested change
if ( $date_created_gmt instanceof IXR_Error ) {
return $date_created_gmt;
}


// We know this is supposed to be GMT, so we're going to slap that Z on there by force.
$date_created = rtrim( $content_struct['date_created_gmt']->getIso(), 'Z' ) . 'Z';
$date_created = rtrim( $date_created_gmt->getIso(), 'Z' ) . 'Z';

$comment['comment_date'] = get_date_from_gmt( $date_created );
$comment['comment_date_gmt'] = iso8601_to_datetime( $date_created, 'gmt' );
Expand Down Expand Up @@ -5438,6 +5523,7 @@ public function blogger_deletePost( $args ) {
* - wp_post_thumbnail
*
* @since 1.5.0
* @since 7.2.0 Returns an error if the content struct argument is not an array.
*
* @param array $args {
* Method arguments. Note: arguments must be ordered as documented.
Expand All @@ -5458,6 +5544,10 @@ public function mw_newPost( $args ) {
$content_struct = $args[3];
$publish = $args[4] ?? 0;

if ( ! $this->_is_content_struct_array( $content_struct ) ) {
return $this->error;
}

$user = $this->login( $username, $password );
if ( ! $user ) {
return $this->error;
Expand Down Expand Up @@ -5671,10 +5761,20 @@ public function mw_newPost( $args ) {

// Do some timestamp voodoo.
if ( ! empty( $content_struct['date_created_gmt'] ) ) {
$date_created_gmt = $this->_convert_client_date( $content_struct['date_created_gmt'] );
if ( $date_created_gmt instanceof IXR_Error ) {
return $date_created_gmt;
}

// We know this is supposed to be GMT, so we're going to slap that Z on there by force.
$date_created = rtrim( $content_struct['date_created_gmt']->getIso(), 'Z' ) . 'Z';
$date_created = rtrim( $date_created_gmt->getIso(), 'Z' ) . 'Z';
} elseif ( ! empty( $content_struct['dateCreated'] ) ) {
$date_created = $content_struct['dateCreated']->getIso();
$date_created_object = $this->_convert_client_date( $content_struct['dateCreated'] );
if ( $date_created_object instanceof IXR_Error ) {
return $date_created_object;
}

Comment on lines +5773 to +5776

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.

Suggested change
if ( $date_created_object instanceof IXR_Error ) {
return $date_created_object;
}

$date_created = $date_created_object->getIso();
}

$post_date = '';
Expand Down Expand Up @@ -5833,6 +5933,7 @@ public function attach_uploads( $post_id, $post_content ) {
* Edits a post.
*
* @since 1.5.0
* @since 7.2.0 Returns an error if the content struct argument is not an array.
*
* @param array $args {
* Method arguments. Note: arguments must be ordered as documented.
Expand All @@ -5854,6 +5955,10 @@ public function mw_editPost( $args ) {
$content_struct = $args[3];
$publish = $args[4] ?? 0;

if ( ! $this->_is_content_struct_array( $content_struct ) ) {
return $this->error;
}

$user = $this->login( $username, $password );
if ( ! $user ) {
return $this->error;
Expand Down Expand Up @@ -6077,10 +6182,20 @@ public function mw_editPost( $args ) {

// Do some timestamp voodoo.
if ( ! empty( $content_struct['date_created_gmt'] ) ) {
$date_created_gmt = $this->_convert_client_date( $content_struct['date_created_gmt'] );
if ( $date_created_gmt instanceof IXR_Error ) {
return $date_created_gmt;
}
Comment on lines +6186 to +6188

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.

Suggested change
if ( $date_created_gmt instanceof IXR_Error ) {
return $date_created_gmt;
}


// We know this is supposed to be GMT, so we're going to slap that Z on there by force.
$date_created = rtrim( $content_struct['date_created_gmt']->getIso(), 'Z' ) . 'Z';
$date_created = rtrim( $date_created_gmt->getIso(), 'Z' ) . 'Z';
} elseif ( ! empty( $content_struct['dateCreated'] ) ) {
$date_created = $content_struct['dateCreated']->getIso();
$date_created_object = $this->_convert_client_date( $content_struct['dateCreated'] );
if ( $date_created_object instanceof IXR_Error ) {
return $date_created_object;
}
Comment on lines +6194 to +6196

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.

Suggested change
if ( $date_created_object instanceof IXR_Error ) {
return $date_created_object;
}


$date_created = $date_created_object->getIso();
}

// Default to not flagging the post date to be edited unless it's intentional.
Expand Down
26 changes: 26 additions & 0 deletions tests/phpunit/tests/xmlrpc/mw/editPost.php
Original file line number Diff line number Diff line change
Expand Up @@ -335,4 +335,30 @@ public function test_draft_not_prematurely_published() {
$future_date_string = date_format( date_create( "@{$future_time}" ), 'Y-m-d H:i:s' );
$this->assertSame( $future_date_string, $after->post_date );
}

/**
* @ticket 66107
*/
public function test_string_date_created_is_accepted(): void {
$editor_id = $this->make_user_by_role( 'editor' );
$post_id = self::factory()->post->create( array( 'post_author' => $editor_id ) );

$date_string = '1984-01-11 05:00:00';
$result = $this->myxmlrpcserver->mw_editPost( array( $post_id, 'editor', 'editor', array( 'dateCreated' => $date_string ) ) );
$this->assertNotIXRError( $result );
$this->assertTrue( $result );
$this->assertSame( $date_string, get_post( $post_id )->post_date );
}

/**
* @ticket 66107
*/
public function test_non_array_content_struct_returns_error(): void {
$editor_id = $this->make_user_by_role( 'editor' );
$post_id = self::factory()->post->create( array( 'post_author' => $editor_id ) );

$result = $this->myxmlrpcserver->mw_editPost( array( $post_id, 'editor', 'editor', 'not a struct' ) );
$this->assertIXRError( $result );
$this->assertSame( 400, $result->code );
}
}
27 changes: 27 additions & 0 deletions tests/phpunit/tests/xmlrpc/mw/newPost.php
Original file line number Diff line number Diff line change
Expand Up @@ -201,4 +201,31 @@ public function test_draft_post_date() {
$this->assertSame( 'draft', $out->post_status );
$this->assertSame( '0000-00-00 00:00:00', $out->post_date_gmt );
}

/**
* @ticket 66107
*/
public function test_string_date_created_is_accepted(): void {
$this->make_user_by_role( 'author' );

$date_string = '1984-01-11 05:00:00';
$post = array(
'title' => 'Test',
'dateCreated' => $date_string,
);
$result = $this->myxmlrpcserver->mw_newPost( array( 1, 'author', 'author', $post ) );
$this->assertNotIXRError( $result );
$this->assertSame( $date_string, get_post( $result )->post_date );
}

/**
* @ticket 66107
*/
public function test_non_array_content_struct_returns_error(): void {
$this->make_user_by_role( 'author' );

$result = $this->myxmlrpcserver->mw_newPost( array( 1, 'author', 'author', 'not a struct' ) );
$this->assertIXRError( $result );
$this->assertSame( 400, $result->code );
}
}
29 changes: 29 additions & 0 deletions tests/phpunit/tests/xmlrpc/wp/editComment.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,33 @@ public function test_trash_comment() {

$this->assertSame( 'trash', get_comment( $comment_id )->comment_approved );
}

/**
* @ticket 66107
* @ticket 42995
*/
public function test_string_date_created_gmt_is_accepted(): void {
$this->make_user_by_role( 'administrator' );
$comment_id = self::factory()->comment->create();

$date_string = '1984-01-11 05:00:00';
$result = $this->myxmlrpcserver->wp_editComment(
array( 1, 'administrator', 'administrator', $comment_id, array( 'date_created_gmt' => $date_string ) )
);
$this->assertNotIXRError( $result );
$this->assertTrue( $result );
$this->assertSame( $date_string, get_comment( $comment_id )->comment_date_gmt );
}

/**
* @ticket 66107
*/
public function test_non_array_content_struct_returns_error(): void {
$this->make_user_by_role( 'administrator' );
$comment_id = self::factory()->comment->create();

$result = $this->myxmlrpcserver->wp_editComment( array( 1, 'administrator', 'administrator', $comment_id, 'not a struct' ) );
$this->assertIXRError( $result );
$this->assertSame( 400, $result->code );
}
}
Loading
Loading