Make WordPress Core

Opened 12 years ago

Closed 12 years ago

#19617 closed defect (bug) (fixed)

Use maybe_unserialize() for HTTP requests

Reported by: nacin's profile nacin Owned by:
Milestone: 3.4 Priority: normal
Severity: normal Version:
Component: Warnings/Notices Keywords: has-patch dev-feedback
Focuses: Cc:

Description

In a few cases, we use this convention: unserialize( wp_remote_retrieve_body( $response ) ). When the request fails, unserialize() gets an empty string, and that's no good.

I see this one every so often: Notice: unserialize(): Error at offset 0 of 11 bytes in /Users/nacin/Sites/beta/wp-includes/update.php on line 288

These are all of the unserialize() calls in core. Let's move all of them to maybe_unserialize() unless there is a good reason to keep them at unserialize() —

./wp-admin/includes/dashboard.php:1250:		$response = unserialize( wp_remote_retrieve_body( $response ) );
./wp-admin/includes/plugin-install.php:48:			$res = unserialize( wp_remote_retrieve_body( $request ) );
./wp-admin/includes/theme.php:413:			$res = unserialize( wp_remote_retrieve_body( $request ) );
./wp-admin/includes/upgrade.php:1090:				if ( !@unserialize( $value ) )
./wp-admin/includes/upgrade.php:1242:				if ( !@unserialize( $value ) )
./wp-admin/includes/upgrade.php:1406:	@ $kellogs = unserialize($option);
./wp-includes/ms-functions.php:848:	$meta = unserialize($signup->meta);
./wp-includes/update.php:188:	$response = unserialize( wp_remote_retrieve_body( $raw_response ) );
./wp-includes/update.php:288:	$response = unserialize( wp_remote_retrieve_body( $raw_response ) );
./wp-includes/user.php:886:			$b_roles = unserialize($caps_meta);

Attachments (2)

19617.patch (5.2 KB) - added by JustinSainton 12 years ago.
Changes serialize() to maybe_serialize() in listed files.
19617.2.patch (5.2 KB) - added by JustinSainton 12 years ago.
Refresh

Download all attachments as: .zip

Change History (6)

@JustinSainton
12 years ago

Changes serialize() to maybe_serialize() in listed files.

#1 @JustinSainton
12 years ago

  • Cc JustinSainton added
  • Keywords has-patch dev-feedback added

@JustinSainton
12 years ago

Refresh

#2 @dd32
12 years ago

In [19707]:

use maybe_unserialize() in update and API checks, Tighten up the checks on expected return data to avoid processing invalid responses after change. See #19617

#3 @dd32
12 years ago

I didn't touch the non-update cases, All of those cases will need to have extra validation applied, unserialize() returns false for invalid (non-serialised) data where as, maybe_unserialize() is going to pass it straight through causing the old false === unserialize() checks to fail.

#4 @nacin
12 years ago

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