Make WordPress Core

Opened 6 weeks ago

Last modified 6 weeks ago

#61950 new defect (bug)

add_*_meta()/get_*_meta() undocumented type juggling for some meta values

Reported by: rodrigosprimo's profile rodrigosprimo Owned by:
Milestone: 6.7 Priority: normal
Severity: normal Version:
Component: Options, Meta APIs Keywords: needs-patch needs-unit-tests
Focuses: docs Cc:

Description

While working on a new WPCS sniff related to the get_*_meta() functions (https://github.com/WordPress/WordPress-Coding-Standards/pull/2465), I noticed there is some undocumented type juggling for some meta values.

For example, WP returns an empty string for an object meta created as false.

Here are all the cases that I found:

Value added via add_*_meta()Value returned by get_*_meta()
false ''
true '1'
0 '0'
1.5 '1.5'

I haven't checked, but I believe this behavior was not introduced recently, so changing it would be a breaking change and not really an option. That being said, I suggest that the DocBlock for those functions is updated to document this behavior explicitly. It is probably a good idea to add some tests as well to prevent unexpected changes in the future.

The description of the $meta_value parameter of those functions is "Metadata value. Must be serializable if non-scalar." An initial suggestion is to change it to mention the special cases listed above.

Change History (2)

#1 @jrf
6 weeks ago

  • Keywords needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to 6.7

#2 @azaozz
6 weeks ago

there is some undocumented type juggling for some meta values

Yep, this is pretty much the same behavior when saving and retrieving options. As the meta_value columns are set to LONGTEXT, all values seem to be converted to strings on saving to the DB. Then non-serialized values are returned as read from the DB.

I believe this behavior was not introduced recently

Right. This has been like that "forever" afaik.

I suggest that the DocBlock for those functions is updated to document this behavior explicitly

Yea, don't think anything else can be done at this point. I actually tried to make options saving and retrieving to respect data types, but at the end there were too many edge cases that it became risky and not practical.

A good docblock update for the different meta retrieval functions (post, comment, user, site, etc.) would include the full description about what's going on, and probably mention that the unexpected types can be avoided at the cost of a tiny bit more processing if the data is serialized. JSON encoding would also work in most cases, but may behave a bit unexpectedly when differentiating between objects and associative arrays.

Version 1, edited 6 weeks ago by azaozz (previous) (next) (diff)
Note: See TracTickets for help on using tickets.