WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#16717 closed defect (bug) (fixed)

Wrong error handling in query.php::get_posts()

Reported by: fabifott Owned by:
Milestone: 3.1.1 Priority: normal
Severity: minor Version: 3.1
Component: Query Keywords: has-patch dev-feedback
Focuses: Cc:

Description

The bug seems to be in a back compat block, so not very important:
In lines 2187 and 2199 get_term_by() is called. In following line it is just checked if it returns are logical true value.:

$the_cat = get_term_by( $cat_query['field'], $cat_query['terms'][0], 'category' );
if ( $the_cat ) {
	$this->set( 'cat', $the_cat->term_id );
	$this->set( 'category_name', $the_cat->slug );
}

But get_term_by calls get_term(), and this function can return WP_Error. As a result, the error is not handled properly. This caused several warnings in Debug Mode, since the WP_Error object is treated as a Category object. The bug is actually in get_term_by() since it returns false in error case by definition.

Attachments (3)

16717.diff (557 bytes) - added by kawauso 3 years ago.
Thanks to rboren for confirming the method, check !is_wp_error()
16717.2.diff (899 bytes) - added by nacin 3 years ago.
Also handle the tag block.
16717.patch (1.6 KB) - added by hakre 3 years ago.
Port of http://core.trac.wordpress.org/attachment/ticket/16464/16464.2.patch to trunk.

Download all attachments as: .zip

Change History (14)

kawauso3 years ago

Thanks to rboren for confirming the method, check !is_wp_error()

comment:1 kawauso3 years ago

  • Keywords has-patch added

comment:2 nerx3 years ago

  • Cc nerx added
  • Keywords needs-patch added

This is new, I'm guessing you did play with the theme functions? Because $cat_query should not get field? if you handling in post.

comment:3 nacin3 years ago

  • Keywords needs-patch removed
  • Milestone changed from Awaiting Review to 3.1.1

This code is for back compat, but it's translating the new tax_query var to the cat/category_name vars. So this should go into 3.1.1.

nacin3 years ago

Also handle the tag block.

comment:4 ryan3 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [17506]) Check for WP_Error return from get_term_by(). Props fabifott, kawauso, nacin. fixes #16717 for trunk

comment:5 ryan3 years ago

(In [17507]) Check for WP_Error return from get_term_by(). Props fabifott, kawauso, nacin. fixes #16717 for 3.1

comment:6 hakre3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

hmm, #16464

Function should not return WP_Error. We've discussed that before.

comment:7 hakre3 years ago

The ticket here is basically a duplicate of #16464. In #16464 we came to the conclusion that get_term_by() should not return WP_Error on error but FALSE (as documented).

Previous to that conclusion there was an earlier suggestion to check for WP_Error (comparable to the changesets [17506] and [17507]) and the ticket had such a patch for weeks already (it was with a minor error which didn't get corrected because core developer feedback pointed out that the other patch is preferred).

That ticket had been put to Future Release ca. 4 weeks ago.

The new patch here is the solution of #16464 for current trunk (and it probably applies on to 3.1 branch as well.

Please review.

Last edited 3 years ago by hakre (previous) (diff)

comment:8 hakre3 years ago

  • Keywords dev-feedback added

comment:9 ryan3 years ago

id is inconsistent with slug and name so I think we should go forth with 16717.patch.

comment:10 ryan3 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [17526]) Return false instead of WP_Error from get_term_by() if the term does not exist. Makes fetching a term by id consistent with slug and name. Props hakre. fixes #16464 #16717 for trunk

comment:11 automattor3 years ago

(In [17527]) Return false instead of WP_Error from get_term_by() if the term does not exist. Makes fetching a term by id consistent with slug and name. Props hakre. fixes #16464 #16717 for 3.1

Note: See TracTickets for help on using tickets.