#33068 closed defect (bug) (fixed)
get_post() returning current object if 0
Reported by: | Rahe | Owned by: | audrasjb |
---|---|---|---|
Milestone: | 5.7 | Priority: | normal |
Severity: | normal | Version: | 3.5 |
Component: | Posts, Post Types | Keywords: | has-patch commit |
Focuses: | docs | 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 (5)
Change History (24)
#1
@
9 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!
#2
@
9 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.
#4
@
9 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; }
This ticket was mentioned in PR #277 on WordPress/wordpress-develop by Rahe.
4 years ago
#8
Trac ticket: https://core.trac.wordpress.org/ticket/33068
#11
@
4 years 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
@
4 years 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.
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
#14
@
4 years ago
I agree with @johnbillion that this is a wontfix due to backward-compatibility concerns.
The documentation could be improved to indicate:
- A falsey value returns the current global post inside the loop
- A numerically valid post ID that points to a non-existent post returns
null
.
@Rahe You're right be the behavior of an invalid ID is a little inconsistent (0 returns the global, a negative number or float returns null
). For valid IDs it's consistent but the return types differ.
#15
@
4 years ago
- Focuses docs added
- Keywords needs-patch added; needs-testing 2nd-opinion removed
Changing the focus of this ticket to docs
. The proposed code changes are a wontfix
. But we agree to improve the function's DocBlock to be more clear.
#16
@
4 years ago
- Keywords has-patch added; needs-patch removed
Thanks @peterwilsoncc, that makes sense to me. I tried to clarify the Documentation in 33068.3.diff
but I'm not so sure about the ideal wording.
#18
@
4 years ago
@audrasjb That reads well to me, in 33068.diff I've made a minor edit:
- Moved default value to end of parameter definition, as that seems to be standard
- Added a note about all falsey values returning the current global post,
get_post( [] )
for example.
hellofromtonya commented on PR #277:
4 years ago
#21
Was deemed too risky for backwards compatibility and marked as wontfix
. Closing the PR.
The original diff file