Skip to content

Conversation

lkraav
Copy link
Contributor

@lkraav lkraav commented Mar 9, 2022

This reverts commit fc606db.

We found a more correct location to improve a previously existing sanity check.

Fixes #1080, but better. Sorry @kasparsd 😄

You were right, there was a connection between #1038 and #1080 all along. I now traced down how all meta is added to a record, and found we're already trying to prevent empty values. Unfortunately

  • existing $stream_meta is_null() check alone is weak
  • $user_meta being a nested array, its actual contents isn't checked at all

strlen trick seems to be a solid one, considering hundreds of upvotes on PHP docs. I always disliked having to do a separate empty string check for is_scalar() result.

Now that previous version is in, this PR doesn't have urgency for its own release.

This reverts commit fc606db.

We found a more correct location to improve a previously existing sanity check.
//
// @see https://www.php.net/manual/en/function.array-filter.php#111091
$stream_meta = array_filter( $args, 'strlen' );
$user_meta = array_filter( $user_meta, 'strlen' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lkraav Were you able to confirm that this works with the nested $user_meta array?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3.4.3: prevent DB_Driver_WPDB::insert_record() from inserting empty meta values
2 participants