Make WordPress Core

Opened 9 years ago

Closed 8 years ago

#36529 closed defect (bug) (fixed)

Bug in get_permalink()

Reported by: dav4's profile dav4 Owned by: presskopp's profile Presskopp
Milestone: 4.6 Priority: normal
Severity: normal Version: 4.5
Component: Permalinks Keywords: good-first-bug has-patch
Focuses: Cc:

Description

In some cases, when default category (from get_option( 'default_category') was deleted (for example with another plugin), get_permalink() show:

PHP Notice: Trying to get property of non-object in /Applications/AMPPS/www/wordpress/wp-includes/link-template.php on line 193


Attachments (2)

36529.1.patch (590 bytes) - added by Presskopp 9 years ago.
36529.2.patch (583 bytes) - added by Presskopp 9 years ago.

Download all attachments as: .zip

Change History (11)

#1 @SergeyBiryukov
9 years ago

  • Keywords good-first-bug needs-patch added
  • Milestone changed from Awaiting Review to 4.6

Hi @dav4, welcome to Trac! Thanks for the ticket, I was able to confirm the issue.

get_term() can return a WP_Error object or null on error, but we only account for WP_Error.

We should account for null as well. Would you like to create a patch?

#2 @SergeyBiryukov
9 years ago

  • Component changed from General to Permalinks

@Presskopp
9 years ago

#3 @Presskopp
9 years ago

I couldn't hold back :)

I don't know if my patch is fitting well, but another question came up my mind. What about other occurences of get_term() in connection with is_wp_error, like in

wp-admin\includes\ajax-actions.php,
Line 491:

                        $parent = get_term( $parent->parent, $taxonomy->name );
                        if ( is_wp_error( $parent ) )
                                break;

wp-admin\includes\class-wp-links-list-table.php
Line 223:

                        $cat = get_term( $category, 'link_category', OBJECT, 'display' );
                        if ( is_wp_error( $cat ) ) {

and some more..

Last edited 9 years ago by Presskopp (previous) (diff)

@Presskopp
9 years ago

#4 follow-up: @Presskopp
9 years ago

I think the 36529.2 is a bit cleaner, using isset here

#5 in reply to: ↑ 4 @SergeyBiryukov
9 years ago

Replying to Presskopp:

I think the 36529.2 is a bit cleaner, using isset here

36529.1.patch is a good start.

  • We should check $default_category there instead of $category.
  • isset() is redundant there, it would always be set.
  • $category = '' is also redundant, we already have that assignment in line 167 earlier.

This should be enough:

if ( $default_category && ! is_wp_error( $default_category ) ) {
	$category = $default_category->slug;
}

What about other occurences of get_term() in connection with is_wp_error

Good catch, let's fix the other instances too.

#6 @Presskopp
9 years ago

I think it's not neccessary to change/fix all of these, but that's beyond my scope. May this list just be helpful.

\wp-includes\category-template.php
Line 45:

        $parent = get_term( $id, 'category' );
        if ( is_wp_error( $parent ) )

Line 151:

        $category = get_term( $cat_ID, 'category' );

        if ( is_wp_error( $category ) )

Line 1227:

                $link = get_term_link( $term, $taxonomy );
                if ( is_wp_error( $link ) ) {

\wp-includes\category.php
Line 91:

        $category = get_term( $category, 'category', $output, $filter );

        if ( is_wp_error( $category ) )

Line 204:

        $category = get_term( $cat_id, 'category' );
        if ( ! $category || is_wp_error( $category ) )

\wp-includes\class-wp-xmlrpc-server.php
Line 1894:

                        $parent_term = get_term( $parent_term_id , $taxonomy['name'] );

                        if ( is_wp_error( $parent_term ) )

Line 1972:

                $term = get_term( $term_id , $content_struct['taxonomy'] );

                if ( is_wp_error( $term ) )

Line 1992:

                        $parent_term = get_term( $parent_term_id , $taxonomy['name'] );

                        if ( is_wp_error( $parent_term ) )

Line 2063:

                $term = get_term( $term_id, $taxonomy->name );

                if ( is_wp_error( $term ) )

Line 2134:

                $term = get_term( $term_id , $taxonomy->name, ARRAY_A );

                if ( is_wp_error( $term ) )

\wp-includes\taxonomy.php
Line 948:

                $term = get_term( (int) $value, $taxonomy, $output, $filter );
                if ( is_wp_error( $term ) || is_null( $term ) ) {

Line 1025:

        $term = get_term( $term, $taxonomy );
        if ( is_wp_error($term) )

Line 1051:

        $term = get_term( $id, $taxonomy );

        if ( is_wp_error($term) )

Line 2308:

                $term_obj = get_term($term, $taxonomy);
                if ( is_wp_error( $term_obj ) )

Line 3928:

                        $term = get_term($term, $taxonomy);
                        if ( is_wp_error( $term ) )

Line 4854:

        $term = get_term( $term_id, $taxonomy );
        if ( ! $term || is_wp_error( $term ) ) {

Last edited 9 years ago by Presskopp (previous) (diff)

This ticket was mentioned in Slack in #slackhelp by joseffb. View the logs.


8 years ago

#8 @DrewAPicture
8 years ago

  • Keywords has-patch added; needs-patch removed
  • Owner set to Presskopp
  • Status changed from new to assigned

Assigning to mark the good-first-bug as "claimed".

#9 @SergeyBiryukov
8 years ago

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

In 37707:

Permalinks: Avoid a PHP notice in get_permalink() if default category is unavailable.

Props Presskopp.
Fixes #36529.

Note: See TracTickets for help on using tickets.