WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#9128 closed defect (bug) (fixed)

array_merge() Error when Adding Roles via Plugins

Reported by: johnkolbert Owned by:
Milestone: 3.0 Priority: normal
Severity: major Version: 2.7
Component: Role/Capability Keywords: has-patch commit
Focuses: Cc:

Description

Scenario:
Adding a new user role that has the same capabilities as the "Administrator" role.

Code Used:

global $wp_roles;
$admin_role = $wp_roles->get_role("administrator");
$wp_roles->add_role("superadmin", Super Admin, $admin_role->capabilities);


Error Encountered:

Warning: array_merge() [function.array-merge]: Argument #2 is not an array in /Applications/MAMP/htdocs/wp2pt7/wp-includes/capabilities.php on line 537

Warning: array_merge() [function.array-merge]: Argument #1 is not an array in /Applications/MAMP/htdocs/wp2pt7/wp-includes/capabilities.php on line 537

Warning: array_merge() [function.array-merge]: Argument #1 is not an array in /Applications/MAMP/htdocs/wp2pt7/wp-includes/capabilities.php on line 539

Suggested Fix:

PHP 5.0 is sensitive for array_merge only using arrays. By using typecasting you can merge other types.

Here's what capabilities.php should be for lines 534-539:

$this->allcaps = array();
		foreach ( (array) $this->roles as $role ) {
			$role =& $wp_roles->get_role( $role );
			$this->allcaps = array_merge( (array)$this->allcaps, (array)$role->capabilities );
		}
		$this->allcaps = array_merge( (array)$this->allcaps, (array)$this->caps );

Notice the addition of the (array) typecast in the array_merge functions.

Attachments (2)

9128.diff (663 bytes) - added by Denis-de-Bernardy 5 years ago.
9128.2.diff (2.2 KB) - added by dd32 4 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 johnkolbert5 years ago

Oops, forgot the quotes around "Super Admin" on the third line of code at the top.

comment:2 ryan5 years ago

  • Component changed from Administration to Role/Capability
  • Owner anonymous deleted

Denis-de-Bernardy5 years ago

comment:3 Denis-de-Bernardy5 years ago

  • Keywords has-patch tested added
  • Milestone changed from 2.7.2 to 2.8

comment:4 ryan5 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [11019]) Cast to array to fix warning. Props johnkolbert, Denis-de-Bernardy. fixes #9128

comment:5 andrew_l5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This bug is not fixed, and the above explanations miss the real problem.

In capabilities.php, in function get_role_caps, there is a foreach loop that is iterating on "$this->roles as $role". Then within the body of the loop, there is a line like:

$role =& $wp_roles->get_role( $role );

This line is making $role a pointer to an element in the $this->role_objects object (because that is what get_role has returned). Then on the next iteration through the loop, the foreach sets $role to a role string (like "contributor"). But $role is still a reference to an element of role_objects! So that element gets clobbered in place, corrupting the role_objects object and causing junk to be returned on subsequent accesses.

You can prove this by adding a couple var_dump() statements to monitor the state of role_objects as we iterate through the loop.

The fix for this is to use a different, unique variable name within the loop body, rather than reusing $role, which should only be the loop iterator.

comment:6 nacin4 years ago

  • Milestone changed from 2.8 to Future Release

dd324 years ago

comment:7 dd324 years ago

  • Keywords commit added; tested removed
  • Milestone changed from Future Release to 3.0

attachment 9128.2.diff added

  • Contains the Iterator naming conflict
  • Some Whitespace normalisation

comment:8 westi4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [12479]) Ensure we don't destory the $wp_roles->role_objects property when assigning a second role to a WP_User object. Fixes #9128 props dd32.

Closing Ticket.

Note: See TracTickets for help on using tickets.