WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#10354 closed defect (bug) (worksforme)

Argument not array in comment.php

Reported by: dawblog Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.8
Component: General Keywords:
Focuses: Cc:

Description (last modified by Denis-de-Bernardy)

Hi, I upgraded to 2.8 recently, and now cannot edit comments. The error is:

Warning: array_merge() [function.array-merge]: Argument #1 is not an array in wp-includes/comment.php on line 1097 Warning: Cannot modify header information - headers already sent by (output started at wp-includes/comment.php:1097) in wp-includes/classes.php on line 1586

thanks

Attachments (1)

wp-db-escape-patch.diff (1.5 KB) - added by sowsinsk 6 years ago.
Patch to wpdb->escape function to handle arrays gracefully

Download all attachments as: .zip

Change History (9)

comment:1 @Denis-de-Bernardy6 years ago

  • Description modified (diff)
  • Milestone set to 2.8.2

can't see anything returning anything but an array here, unless $GLOBALS[comment] is not an object. can you confirm this with all plugins disabled?

comment:2 @mrmist6 years ago

  • Keywords reporter-feedback added
  • Priority changed from high to normal
  • Summary changed from Error to Argument not array in comment.php

comment:3 @sowsinsk6 years ago

  • Keywords wp_update_comment added

I am having the same issue when calling wp_update_comment(). I believe the problem is line 1092 (in 2.8.4), which turns the $comment array that was retrieved via get_comment() into a string. That string is then passed to array_merge in line 1097, which throws the error.

The $wpdb->escape function is turning the $comment array into a string because it calls addslashes() on any non-object data and returns it (this includes arrays, which presumably get turned into a string because addslashes() expects a string). In previous versions (such as 2.7.1), there was code that recursed through the $comment array and called $wpdb->escape on each key/value pair, which worked correctly. However, as of 2.8 on, it just calls $wpdb->escape() on the array as a whole.

I'll also note that in previous versions (for instance 2.7.1) when escape() was called recursively via a foreach, it did not work 100% of the time... there would occasionally be an instance where the $comment array had a 'post' index with post data in it... this would also trip up the $wpdb->escape call, even though it was being called recursively on each index of the array. I believe the ideal solution would be for the escape() function to add recursive behavior based on the type of data passed to it. This way, if it encountered an array of arrays, it could recurse deeply through them and return the original array with deeply-escaped values.

@sowsinsk6 years ago

Patch to wpdb->escape function to handle arrays gracefully

comment:4 @sowsinsk6 years ago

  • Keywords has-patch escape added

I just uploaded a patch that makes the wpdb->escape() function handle arrays better by checking if the argument is an array and then recursively looping through the data instead of blindly calling addslashes() on it. This is the first patch I've ever tried to upload, so I'm a little hazy on what kind of tests need to be run, but I can tell you that it fixed all my problems.

An alternate patch would be to change the wp_update_comment() function to handle the recursion.

comment:5 @ryan5 years ago

  • Keywords has-patch removed
  • Milestone changed from 2.9 to Future Release

The patch is against an old version of WP. escape() handles arrays in 2.8, which is the version used by the original reporter.

comment:6 @johnbillion5 years ago

  • Keywords escape reporter-feedback array_merge editing wp_update_comment removed
  • Milestone Future Release deleted
  • Resolution set to fixed
  • Status changed from new to closed

This was fixed at some point. $wpdb->escape() successfully recursively escapes an array.

comment:7 @nacin5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:8 @nacin5 years ago

  • Resolution set to worksforme
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.