WordPress.org

Make WordPress Core

Opened 21 months ago

Closed 3 months ago

#44304 closed defect (bug) (duplicate)

API will create PHP warning and thus break code on empty category

Reported by: apermo Owned by:
Milestone: Priority: normal
Severity: minor Version: 4.7
Component: REST API Keywords: has-patch
Focuses: Cc:
PR Number:

Description

Example Request

array (size=3)
  'headers' => 
    array (size=)
      'Authorization' => string 'Basic randomstring==' (length=34)
  'body' => 
    array (size=15)
      'title' => string 'Test' (length=4)
      'status' => string 'publish' (length=7)
      'content' => string '' (length=0)
      'date' => string '2018-06-04 17:11:32' (length=19)
      'date_gmt' => string '2018-06-04 15:11:32' (length=19)
      'modified' => string '2018-06-05 10:24:01' (length=19)
      'modified_gmt' => string '2018-06-05 08:24:01' (length=19)
      'excerpt' => string '' (length=0)
      'slug' => string 'test' (length=4)
      'type' => string 'post' (length=4)
      'comment_status' => string 'closed' (length=6)
      'ping_status' => string 'closed' (length=6)
      'sticky' => boolean false
      'categories' => string '' (length=0)
      'tags' => string '' (length=0)
  'method' => string 'POST' (length=4)

This will lead to warnings when Debug is active on the remote node.

Warning: Invalid argument supplied for foreach() in /path/to/public_html/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php on line 1256

I suggest to replace (1251-1253):

<?php
if ( ! isset( $request[ $base ] ) ) {
        continue;
}

With:

<?php
if ( ! isset( $request[ $base ] ) || ! is_array( $request[ $base ] ) ) {
        continue;
}

This will avoid entering the loop and the warning

Attachments (1)

44304.diff (907 bytes) - added by skostadinov 20 months ago.
Added is_iterable() check for the handle_terms() and check_assign_terms_permission()

Download all attachments as: .zip

Change History (5)

#1 @swissspidy
21 months ago

  • Keywords needs-patch added

We could also use is_iterable() now, see #43619.

#2 @apermo
21 months ago

Fine for me too ;) As long as it avoids throwing a warning.

@skostadinov
20 months ago

Added is_iterable() check for the handle_terms() and check_assign_terms_permission()

#3 @skostadinov
20 months ago

  • Keywords has-patch added; needs-patch removed

#4 @TimothyBlynJacobs
3 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed
  • Version changed from 4.9.6 to 4.7

Hi @apermo, apologies for the delay in responding to your ticket.

I haven't been able to replicate your issue. The following unit test does not appear to generate any PHP warnings. I think this has been fixed by #43977. Feel free to reopen if you are still encountering an issue.

<?php
        /**
         * @ticket 44304
         */
        public function test_create_item_does_not_issue_warning_when_terms_is_empty_string() {
                wp_set_current_user( self::$editor_id );

                $request = new WP_REST_Request( 'POST', '/wp/v2/posts' );
                $request->add_header( 'content-type', 'application/x-www-form-urlencoded' );
                $params = $this->set_post_data( array( 'categories' => '' ) );
                $request->set_body_params( $params );
                $response = rest_get_server()->dispatch( $request );
                $this->assertCount( 0, $response->get_data()['categories'] );
        }
Note: See TracTickets for help on using tickets.