WordPress.org

Make WordPress Core

Opened 13 months ago

Last modified 4 weeks ago

#44723 assigned defect (bug)

The user ID in a `WP_User_Request` is not an integer as stated

Reported by: desrosj Owned by: desrosj
Milestone: 5.3 Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-patch has-unit-tests needs-refresh
Focuses: Cc:

Description

Discovered this while debugging something in #43985. The value of WP_User_Request->user_id is a string, not an integer as stated by the inline documentation. This was causing some unexpected behavior in get_user_locale(), which performs a strict comparison for 0 and was causing the condition to incorrectly evaluate as false.

Attachments (3)

44723.diff (565 bytes) - added by desrosj 13 months ago.
44723-unittests.diff (830 bytes) - added by andizer 11 months ago.
Unit test
44723-refresh.diff (565 bytes) - added by andizer 5 months ago.

Download all attachments as: .zip

Change History (20)

@desrosj
13 months ago

#1 @desrosj
13 months ago

  • Keywords needs-unit-tests added

44723.diff type casts the property as an integer. But, this could be a potentially breaking change. It may be better to change the documentation to indicate the value is a string.

#2 follow-up: @garrett-eclipse
13 months ago

Interesting enough but if you go down the rabbit hole although the codex states WP_Post's post_author is 'A numeric string, for compatibility reasons' it's default is the numeric 0 and in wp_insert_post it's expecting an (int) but if not provided uses the return of get_current_user_id which is an int.

$post_author in WP_Post - https://core.trac.wordpress.org/browser/tags/4.9.8/src/wp-includes/class-wp-post.php#L32
wp_insert_post Codex - https://developer.wordpress.org/reference/functions/wp_insert_post/
$user_id in wp_insert_post - https://core.trac.wordpress.org/browser/tags/4.9.8/src/wp-includes/post.php#L3139
get_current_user_id Codex - https://developer.wordpress.org/reference/functions/get_current_user_id/

So it's weird it seems it's always an int down the chain so not sure why it's saying it's a numeric string.

#3 in reply to: ↑ 2 @SergeyBiryukov
13 months ago

Replying to garrett-eclipse:

So it's weird it seems it's always an int down the chain so not sure why it's saying it's a numeric string.

See the discussion in #22324 and #25092.

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


13 months ago

#5 @garrett-eclipse
13 months ago

Thanks @SergeyBiryukov I appreciate the references, I think that should equip @desrosj to continue on here. Cheers

#6 @javorszky
13 months ago

I think this is related here: #43752

Last edited 12 months ago by SergeyBiryukov (previous) (diff)

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


12 months ago

#8 @TZ Media
12 months ago

Taking the other tickets mentioned into account, I believe changing the documentations is more in alighment with how such issues were handled in the past. And as long as WP does not use PHP7 strict type hinting, return value types can't be trusted anyway and must be checked/sanitized.

This ticket was mentioned in Slack in #core-privacy by desrosj. View the logs.


11 months ago

@andizer
11 months ago

Unit test

#10 @andizer
11 months ago

  • Keywords has-unit-tests added; needs-unit-tests removed

#11 @pento
11 months ago

  • Milestone changed from 4.9.9 to Future Release

This ticket was mentioned in Slack in #core-privacy by webdevlaw. View the logs.


11 months ago

#13 @garrett-eclipse
7 months ago

  • Keywords needs-refresh added
  • Owner set to desrosj
  • Status changed from new to assigned

#14 @andizer
5 months ago

I've added a new patch with the same fix. I suppose this is what meant with a refresh.

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


3 months ago

#16 @birgire
3 months ago

  • Milestone changed from Future Release to 5.3

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


4 weeks ago

Note: See TracTickets for help on using tickets.