Make WordPress Core

Opened 6 years ago

Last modified 6 years ago

#43752 new defect (bug)

ID, post_parent, menu_order on global $post object is a string in edit context; expecting int

Reported by: javorszky's profile javorszky Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.9.5
Component: Posts, Post Types Keywords: needs-patch needs-unit-tests
Focuses: Cc:

Description

When I'm on an edit post screen, the ID, post_parent, menu_order attributes on the global $post object are strings. I expect them to be integers.

To quickly check, put this in a plugin:

<?php
add_action( 'add_meta_boxes', function(){
        add_meta_box( 'foo', 'bar', function( $post ) {
                var_dump($post->ID);
        });
});

Here's what's happening:

  1. in wp-admin/post.php the edit case happens, and within that the post gets reloaded here: https://github.com/WordPress/WordPress/blob/4.9.5/wp-admin/post.php#L167
  2. that function will run the post object through its own filter with filter edit here: https://github.com/WordPress/WordPress/blob/4.9.5/wp-includes/post.php#L552
  3. because at the time $this->filter = "raw", and the $filter is edit, that will run the object through sanitize_post here https://github.com/WordPress/WordPress/blob/4.9.5/wp-includes/class-wp-post.php#L354
  4. sanitize_post will, in turn, run all the fields through sanitize_post_field here: https://github.com/WordPress/WordPress/blob/4.9.5/wp-includes/post.php#L1940
  5. and even though we have 3 fields set as int (https://github.com/WordPress/WordPress/blob/4.9.5/wp-includes/post.php#L1973), by the time we get to this part (https://github.com/WordPress/WordPress/blob/4.9.5/wp-includes/post.php#L2027-L2034), those three will be ran through esc_attr
  6. esc_attr will feed it through _wp_specialchars here https://github.com/WordPress/WordPress/blob/4.9.5/wp-includes/formatting.php#L3978
  7. which begins with $string = (string) $string; here https://github.com/WordPress/WordPress/blob/4.9.5/wp-includes/formatting.php#L912

The part that throws me off is that sanitize_post_field declares these three properties to be integers at the beginning of the function, so I sort of expected them to come out as integers on the other end.

Change History (5)

#1 @javorszky
6 years ago

Related WooCommerce issue on https://github.com/woocommerce/woocommerce/issues/19704 (tldr: it being a string causes it to slow down due to WC expecting the prop to be an int)

#2 follow-up: @javorszky
6 years ago

Original props goes to @alexrsoap for bumping into this and doing debugging that uncovered this.

#3 in reply to: ↑ 2 @alexrsoap
6 years ago

Replying to javorszky:

Original props goes to @alexrsoap for bumping into this and doing debugging that uncovered this.

Thanks for the props. Thanks to you for the legwork of verification and reporting.

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


6 years ago

#5 @SergeyBiryukov
6 years ago

  • Keywords needs-patch needs-unit-tests added
Note: See TracTickets for help on using tickets.