WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

#37738 closed defect (bug) (maybelater)

WP_Post::get_instance returns erroneous data for invalid input such as an array or object instead of false.

Reported by: deeptiboddapati Owned by: boonebgorges
Milestone: Priority: normal
Severity: normal Version: 3.5
Component: Posts, Post Types Keywords: has-patch has-unit-tests
Focuses: Cc:
PR Number:

Description

Steps to reproduce the problem:

  1. On any wp site which has a post with id =1 run:
$post =  WP_Post::get_instance(array("a","b"));

print_r($post); //will print the post with id = 1 (most probably hello world)
  1. It does the same thing with objects
$obj = (object) array("a","b");
$post =  WP_Post::get_instance($obj);
print_r($post);
  1. floats it rounds down to the nearest int and tries to get that post
$post =  WP_Post::get_instance(3.5);
returns post with id = 3
  1. If you pass in a boolean it returns the first post for true and false for false.
$post =  WP_Post::get_instance(true); //returns post id =1
$post =  WP_Post::get_instance(false); //returns false

Attachments (4)

class-wp-post.php (1.3 KB) - added by deeptiboddapati 3 years ago.
Test class for WP_Post:get_instance showing where it fails.
class-wp-post.php.patch (434 bytes) - added by deeptiboddapati 3 years ago.
Adds a line to check if the input param is an int,bool or float and returns false if it isn't.
37738.php.patch (416 bytes) - added by deeptiboddapati 3 years ago.
changed so it accepts numeric strings like '1' or '1.3231'
37738.php (1.4 KB) - added by deeptiboddapati 3 years ago.
Added coverage for numeric strings in testclass

Download all attachments as: .zip

Change History (13)

@deeptiboddapati
3 years ago

Test class for WP_Post:get_instance showing where it fails.

@deeptiboddapati
3 years ago

Adds a line to check if the input param is an int,bool or float and returns false if it isn't.

#1 @swissspidy
3 years ago

  • Keywords has-patch has-unit-tests added
  • Version changed from trunk to 3.5

Similar: #33372

#2 @deeptiboddapati
3 years ago

That ticket was really good to read. It helped me notice that the patch missed numeric strings such as '1' or '1.4'. In that ticket they used is_scalar but that returns true for 'apple' as well as '1'. I used is_numeric in this redone patch since it returns true for '1.36546' or '3' but not for '3rd' or 'apple'.

Additionally negative integers don't seem to be a problem. They return false because the ID field in wp_posts is unsigned and as a result there are no posts with negative IDs. The code handles it the same as it would if there were no post with that number.

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

@deeptiboddapati
3 years ago

changed so it accepts numeric strings like '1' or '1.3231'

@deeptiboddapati
3 years ago

Added coverage for numeric strings in testclass

#3 @boonebgorges
3 years ago

  • Milestone changed from Awaiting Review to 4.7

@deeptiboddapati Thank you for the patch and for the tests!

It seems incorrect to me that 1.4 should be rounded down to 1. That may be how things currently work, but it definitely feels wrong. I assume that backward compatibility will be covered by ensuring that get_post( 1.0 ) does match post 1 (but not get_post( 1.5 ))

#4 @boonebgorges
3 years ago

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

In 38381:

Don't improperly cast IDs when fetching post, user, or term objects.

Blindly casting passed IDs to integers can generate false positives
when the ID is cast to 1.

Props deeptiboddapati.
Fixes #37738.

#5 @johnjamesjacoby
3 years ago

  • Milestone changed from 4.7 to 4.8
  • Resolution fixed deleted
  • Status changed from closed to reopened

There are reports coming in via Slack that this caused issues with plugins that weren't prepared to correctly handle the newer, quality-controlled return values. After a quick chat, I'm reopening this so we can: revert this to stop that plugin breakage, and look into a better solution in 4.8 or beyond.

#6 @boonebgorges
3 years ago

In 39992:

Revert to pre-4.7 behavior for fetching object instances by id.

This changeset reverts [38381], which caused inconsistencies in the way the
REST API fetches posts and other objects.

See #38792, #37738.

#7 @boonebgorges
3 years ago

In 39993:

Revert to pre-4.7 behavior for fetching object instances by id.

This changeset reverts [38381], which caused inconsistencies in the way the
REST API fetches posts and other objects.

Merge of [39992] to the 4.7 branch.

See #38792, #37738.

This ticket was mentioned in Slack in #core by flixos90. View the logs.


2 years ago

#9 @boonebgorges
2 years ago

  • Milestone 4.8 deleted
  • Resolution set to maybelater
  • Status changed from reopened to closed

Making these changes in a backward-compatible way has proved difficult. Let's return to this at some point in the future.

Note: See TracTickets for help on using tickets.