WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 4 years ago

Last modified 4 years ago

#25477 closed defect (bug) (fixed)

Notice: Trying to get property of non-object in .../wp-includes/post.php on line 2847

Reported by: jpswade Owned by: boonebgorges
Milestone: 4.4 Priority: normal
Severity: normal Version: 3.6.1
Component: Posts, Post Types Keywords: has-patch
Focuses: Cc:
PR Number:

Description

When I use wp_insert_post() to add a new post I get the following error:

Notice:  Trying to get property of non-object in .../wp-includes/post.php on line 2847

If it is something caused by me, I can only assume that there aren't sufficient checks in place to prevent this error from happening.

Attachments (2)

25477.diff (554 bytes) - added by jdgrimes 6 years ago.
Check that the taxonomy was valid before attempting to access the taxonomy object properties.
25477.2.diff (634 bytes) - added by jdgrimes 6 years ago.
Tell devs they are _doing_it_wrong() when an invalid taxonomy is detected

Download all attachments as: .zip

Change History (18)

#1 @ocean90
6 years ago

This belongs to taxonomy stuff, see https://core.trac.wordpress.org/browser/tags/3.6.1/wp-includes/post.php#L2847.

How you are using wp_insert_post()?

#2 @jdgrimes
6 years ago

It looks like this is caused by passing an invalid taxonomy name, which causes get_taxonomy() to return false rather than the expected taxonomy object. The code tries to access properties of the taxonomy object without ever checking if it is valid.

#3 @jpswade
6 years ago

I suspect you're correct and it's the way I've formatted my taxonomy.

My $post is as follows:

Array
(
    [post_content] => Full content
    [post_excerpt] => Short excerpt
    [post_name] => Post name
    [post_title] => Post title
    [post_status] => publish
    [post_type] => custom-post-type
    [tags_input] => this, that, other
    [tax_input] => Array
        (
            [custom_tax] => String
        )
)

I suspect I need to do it as:

    [tax_input] => Array
        (
            [custom_tax] => Array
            (
                        [0] => String
            )
        )

It seems as through the taxonomy isn't being validated before trying to access the object.

#4 @jpswade
6 years ago

The code below is based on the code provided on the wp_insert_post page here:
http://codex.wordpress.org/Function_Reference/wp_insert_post

To replicate this error, simply use the following:

<?php

$post_type = 'custom-post-type';

error_reporting(E_ALL);
ini_set('display_errors', 'on');

$p = array();
$p['post_content'] = "Full content";
$p['post_excerpt'] = "Short description";
$p['post_name'] = 'test-name';
$p['post_title'] = 'Test Title';
$p['post_status'] = 'publish';
$p['post_type'] = isset($post_type) ? $post_type : NULL;
$p['tags_input'] = 'test, testing';
$p['tax_input'] = array('taxonomy_name' => array('term', 'term2', 'term3'));
$post_id = wp_insert_post($p);
echo $post_id;

//eof

Errors:

Notice: Trying to get property of non-object in .../wp-includes/post.php on line 2847

Notice: Trying to get property of non-object in .../wp-includes/post.php on line 2847

Notice: Trying to get property of non-object in .../wp-includes/post-template.php on line 29

Are you able to replicate this?

#5 @jdgrimes
6 years ago

The errors are because 'taxonomy_name' isn't a registered taxonomy name. If you change that to a name of a registered taxonomy, the errors should go away.

@jdgrimes
6 years ago

Check that the taxonomy was valid before attempting to access the taxonomy object properties.

#6 @jdgrimes
6 years ago

  • Keywords has-patch added

That patch checks that the taxonomy is valid, and skips over it if it isn't.

#7 follow-ups: @nacin
6 years ago

If you try to pass a taxonomy name that isn't valid, we should probably error. Or in this case, even just the notice is pretty good for alerting a developer that something is wrong. We shouldn't suppress developer errors.

#8 in reply to: ↑ 7 @jdgrimes
6 years ago

Replying to nacin:

If you try to pass a taxonomy name that isn't valid, we should probably error. Or in this case, even just the notice is pretty good for alerting a developer that something is wrong. We shouldn't suppress developer errors.

Yeah, I was thinking along the same lines, I was just thinking that there are other places in core where invalid stuff is just ignored - but maybe most of those places are dealing with user input, rather than here where it is probably developer input (as in this case).

#9 in reply to: ↑ 7 @helen
6 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Replying to nacin:

We shouldn't suppress developer errors.

Strongly agree.

#10 follow-up: @jpswade
6 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

I've managed to reproduce this error again today.

Notice: Trying to get property of non-object in /home/.../public_html/wp-includes/post.php on line 2925

Although I agree that Wordpress *should NOT* suppress errors, I firmly believe that better error handling should be introduced.

The problem is that the error is occurring in post.php in the wp_insert_attachment() function. On line 2925, the error is because get_taxonomy() in taxonomy.php is returning false rather than an object.

Either, get_taxonomy() should give an error (rather than just returning false) or wp_insert_attachment() should check that get_taxonomy() is actually returning an object or return an error.

Again I am firmly saying that errors should *NOT* be suppressed and in fact I'm saying they should be handled.

#11 in reply to: ↑ 10 @SergeyBiryukov
6 years ago

  • Component changed from General to Posts, Post Types
  • Milestone set to Awaiting Review

Replying to jpswade:

Either, get_taxonomy() should give an error (rather than just returning false) or wp_insert_attachment() should check that get_taxonomy() is actually returning an object or return an error.

Either option would break backwards compatibility, as get_taxonomy() can only return an object or a boolean value, and wp_insert_attachment() can only return an integer:
tags/3.8.1/src/wp-includes/taxonomy.php#L197
tags/3.8.1/src/wp-includes/post.php#L3966

We could probably add a _doing_it_wrong() message in there.

#12 @wonderboymusic
6 years ago

  • Keywords needs-refresh added
  • Milestone changed from Awaiting Review to Future Release

@jdgrimes
6 years ago

Tell devs they are _doing_it_wrong() when an invalid taxonomy is detected

#13 @jdgrimes
6 years ago

  • Keywords needs-refresh removed

25477.2.diff added. Could probably use some tweaking of the error message. And I didn't attempt to add unit tests.

#14 @wonderboymusic
4 years ago

  • Milestone changed from Future Release to 4.4
  • Owner set to boonebgorges
  • Status changed from reopened to assigned

We should do something here.

#15 @boonebgorges
4 years ago

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

In 34248:

Throw a notice when an invalid tax is passed to wp_insert_post().

Props jdgrimes.
Fixes #25477.

#16 @selnomeria
4 years ago

I think, you are making a custom call before 'init'....
you should execute those commands in

function your_function(){
  YOUR CODES HEREEEEEEEEEEE............
}
ADD_ACTION('init','your_function');
Last edited 4 years ago by selnomeria (previous) (diff)
Note: See TracTickets for help on using tickets.