WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 7 weeks ago

#33068 new defect (bug)

get_post() returning current object if 0

Reported by: Rahe Owned by:
Milestone: 5.7 Priority: normal
Severity: normal Version: 3.5
Component: Posts, Post Types Keywords: needs-testing 2nd-opinion
Focuses: Cc:

Description

When calling get_post(0) and the global object post is set, the current object is returned.
Like in this example :

global $post;
$post = get_post(384);
var_dump( get_post(0));

The get_post returns the 384 object, and not null as expected when needing the object 0, that doesn't exists.
I propose this patch for stricly checking this is not 0 for this case and not just the handy empty.

Attachments (3)

patch.diff (548 bytes) - added by Rahe 6 years ago.
The original diff file
33068.patch (466 bytes) - added by juliobox 6 years ago.
! is_null instead of empty()
33068-2.patch (1.1 KB) - added by Rahe 5 years ago.

Download all attachments as: .zip

Change History (13)

@Rahe
6 years ago

The original diff file

#1 @juliobox
6 years ago

  • Version changed from 4.2.2 to 3.5

Good catch,

A little of history:
WP 1.5.1, the get_post() function is added. The first parameter was (and is) $post. But at this time you can avoid it, it's required, no default value.
So if you try to use it with an empty string, a false, null or 0, the global var will be used if set.
WP 3.5, the first parameter gets its default value null. BUT we kept to test this with is_empty.

Since the $post parameter is set to null by default, i prefer to test this to be different.
Also, i already found that some plugin were creating a post with the ID 0, ok, it's not a good idea but now we know that this can break that. See what i mean.

Thanks anyway!

@juliobox
6 years ago

! is_null instead of empty()

#2 @Rahe
6 years ago

So if I call get_post() with a real WP_Post object on a singular page, my object is replaced by the current global.

#3 @juliobox
6 years ago

My bad!
Note: Don't try to patch at midnight again.

@Rahe
5 years ago

#4 @Rahe
5 years ago

I've added a patch file with the coding standards respected in the function.

Maybe we can replace the line :

	if ( ! $_post ) {
		return null;
	}

by :

	if ( ! $_post ) {
		return;
	}

#7 @Rahe
22 months ago

Happy to see my issue opened again, I hope this will be merged :)

#9 @Rahe
8 months ago

Updated the patch with PR from github.

#10 @audrasjb
2 months ago

  • Keywords needs-testing added
  • Milestone set to 5.7

Moving for 5.7 consideration

#11 @johnbillion
7 weeks ago

  • Keywords 2nd-opinion added

This is a risky change. There's definitely code out there that passes integer 0 and expects to get the current post. Changing this would break that code. WPGraphQL does it explicitly, there's bound to be many more instances of it implicitly or where the value comes from a variable.

If your code does not check for emptiness before passing a value to get_post() then it will not work as expected regardless of whether that value is integer 0 or anything else empty. Maybe the unexpectedly empty value is null instead of 0, but if it's not being checked then you can't be sure that get_post() will return what you expect.

Ideally get_post() would be strict about its accepted values but that ship sailed fifteen years ago.

Personally this seems like a wontfix for backwards compatibility concerns.

#12 @Rahe
7 weeks ago

Hello,

For me passing an integer is wanting to get this specific ID.
Post 0 doesn't exists and maybe post 56743 too.

Based on this behaviour, get_post on 56743 should return current post too.
The current behaviour for 0 is not logic for me.

Note: See TracTickets for help on using tickets.