WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 16 months ago

#22342 closed enhancement (invalid)

WP_DEBUG "undefined index" notices in get_metadata() for empty arrays

Reported by: doublesharp Owned by:
Milestone: Priority: normal
Severity: minor Version: 3.1
Component: Options, Meta APIs Keywords: has-patch needs-testing dev-feedback 2nd-opinion
Focuses: Cc:

Description

When the WP_DEBUG constant is set, calling get_metadata() in /wp-includes/meta.php with $single==true can cause "undefined index" notices when the meta data value is an empty array. As null is an acceptable return value for this function, silencing the notice with @ will remove noise when debugging without the overhead of checking the array length or that the 0 index isset().

Attachments (3)

meta.php.patch (307 bytes) - added by doublesharp 3 years ago.
silences undefined index notice with @$check[0]
meta.php.2.patch (436 bytes) - added by doublesharp 3 years ago.
silences undefined index notice with @$check[0]
test-22342.php (604 bytes) - added by JD55 23 months ago.

Download all attachments as: .zip

Change History (10)

@doublesharp3 years ago

silences undefined index notice with @$check[0]

comment:1 @doublesharp3 years ago

  • Keywords dev-feedback 2nd-opinion added

@doublesharp3 years ago

silences undefined index notice with @$check[0]

comment:2 @SergeyBiryukov3 years ago

  • Version changed from trunk to 3.1

@JD5523 months ago

comment:3 @JD5523 months ago

I think there is really a larger issue here. I don't think that the logic behind this is correct:

if ( $single && is_array( $check ) )
       return $check[0];

This makes it impossible to return an array for the single meta value, which is a real problem if your meta value is stored as an array. I've attached a some code that demonstrates this. I think that this check should be removed completely, and $check should be returned 'raw'. As nacin said in a related ticket:

In many locations throughout core, we simply trust the plugin to return the right data, and if they don't, then the sky may fall on their head.

So it should just be this:

if ( null !== $check )
      return $check;

It should be up to the plugin to return an appropriate value based on $single. It has to be, because we have no way of knowing whether a value is appropriate.

comment:4 follow-up: @nacin23 months ago

While I am not a huge fan of how this is implemented, I don't think there is a bug here, and it's not impossible to return an array.

It seems the onus is on the plugin author to look at the $single argument passed to the get_{$meta_type}_metadata filter, and obey it. As in, if you want to return an array, then do so if $single, but wrap it in an array (with key 0 being your array) if not $single.

comment:5 in reply to: ↑ 4 @JD5523 months ago

Replying to nacin:

if you want to return an array, then do so if $single, but wrap it in an array (with key 0 being your array) if not $single.

I hadn't thought of that hack. But it still seems like, well, a hack, to me. If its going to stay this way it should be mentioned in the codex so people know this is a 'feature' and not a bug.

comment:6 @jdgrimes22 months ago

I've made a note of this in the codex, so if it changes that needs to be updated.

comment:7 @nacin16 months ago

  • Component changed from Warnings/Notices to Options and Meta
  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

Thanks for adding a note to the Codex, jdgrimes.

Note: See TracTickets for help on using tickets.