wp_update_comment can cause database error with new filter

[dshanske]

In 4.6, the comment data array was passed into the edit_comment action. In 4.7, the wp_update_comment_data filter was added to wp_update_comment.

However, wp_array_slice_assoc is called to filter keys out of the array before it is passed to both.

It should be called after and only to filter the data going into the database. By adding a key to the array at the filter you can cause a database error.

By removing extra keys from the data before it hits edit_comment, it also makes it impossible to pass data to be saved as metadata.

[dshanske]

The patch submitted not only makes it so the database error cannot occur which is the more important issue, but preserves any data added to the comment array for the filter and hook.

[rachelbaker]

@dshanske Prior to [38674] the sliced data was passed to the database update statement and then the edit_comment action, this doesn't look like a regression to me. Am I missing something?

I understand that you are approaching this with a specific use-case in mind (updating meta). However, I am not convinced other folks would expect the $data parameter in the edit_comment action to differ from the specific keys/values used to update the comment table.

[dshanske]

@rachelbaker the fact that the filter could add keys to the array and cause a database error when the update is run is still an issue. I tagged it for 4.7.1 for that reason. The slice still should come after the filter for that reason.

Should I split the ticket because I consider that a bug in the filter? I just figured I would address both.

[rachelbaker]

@dshanske Yeah, can you create a new ticket for the enhancement? And I will get the bugfix in 4.7.1

[rachelbaker]

Slice the $data array after the wp_update_comment_data filter

[dshanske]

@rachelbaker Will hold off on a new ticket as #36784 is tagged for 4.8 early and addresses the same use case.

[rachelbaker]

In 39640:

Comments: Fix placement of the wp_update_comment_data filter to safeguard filtered data from triggering a database error.

Introduced in [38674], the wp_update_comment_data filter took place after the $data was sliced and prepared for the database update statement. The location of the filter assumed the result of anyone applying it would not change the data type or make structural modifications or additions to the $data array. 😅

This moves the wp_update_comment_data filter to take place before the $data is sliced and prepared for the database update statement.

Props dshanske for initial patch.

Fixes #39380.

[rachelbaker]

In 39641:

Comments: Fix placement of the wp_update_comment_data filter to safeguard filtered data from triggering a database error.

Introduced in [38674], the wp_update_comment_data filter took place after the $data was sliced and prepared for the database update statement. The location of the filter assumed the result of anyone applying it would not change the data type or make structural modifications or additions to the $data array. 😅
This moves the wp_update_comment_data filter to take place before the $data is sliced and prepared for the database update statement.

Merges [39640] to the 4.7 branch.

Props dshanske for initial patch.
Fixes #39380.