Harden WP eCommerce against APC cache deserialization errors #1835

Closed
wants to merge 21 commits into
from

Projects

None yet

3 participants

@JeffPyeBrook
Contributor

Harden countries class against bad cache based on issues noted on sites using GoDaddy APC caching reported here:

https://wordpress.org/support/topic/fatal-error-wpsc_countries/page/2?replies=38#post-6810134

JeffPyeBrook added some commits Apr 9, 2015
@JeffPyeBrook JeffPyeBrook move include of datamap class before country class in an attempt to a…
…void errors such as this:

Fatal error: WPSC_Countries::_dirty(): The script tried to execute a method or access a property of an incomplete object. Please ensure that the class definition "WPSC_Data_Map" of the object you are trying to operate on was loaded _before_ unserialize() gets called or provide a __autoload() function to load the class definition in wp-e-commerce/wpsc-includes/wpsc-countries.class.php on line 1142"
5194365
@JeffPyeBrook JeffPyeBrook some hardenning of the class against bad cache operations
d4af124
@JustinSainton JustinSainton and 1 other commented on an outdated diff Apr 12, 2015
wpsc-includes/wpsc-countries.class.php
$has_data = false;
- if ( is_array( $data ) ) {
- foreach ( $data as $variable => $value ) {
- if ( property_exists( $this, $variable ) ) {
+ $transient_is_valid = true;
+ foreach( self::$_maps_to_save_with_core_class as $map_name => $should_be_saved ) {
+ if ( $should_be_saved ) {
+ if ( ! is_a( $data[ $map_name ], 'WPSC_Data_Map' ) ) {
+ $transient_is_valid = false;
+ delete_transient( self::transient_name() );
+ error_log( __FUNCTION__ . ' transient data corrupted' );
@JustinSainton
JustinSainton Apr 12, 2015 Member

Let's remove this.

@JeffPyeBrook
JeffPyeBrook Apr 12, 2015 Contributor

remove the error log? or the whole check?

JeffPyeBrook added some commits Apr 12, 2015
@JeffPyeBrook JeffPyeBrook some potential fixes for the object cache weirdness we are seeing
a7e3254
@JeffPyeBrook JeffPyeBrook wrap get/set transient calls in utility function
serialize/deserialize transient data to stop APC from throwing PHP fatal error on deserialization
eef871b
@JeffPyeBrook JeffPyeBrook error_log calls removed
f0b53ac
@JeffPyeBrook JeffPyeBrook Wrap all calls to set/get/delete WordPress transients in a function t…
…o encode and serialize the

data.  Done to work around fatal error on some hosts using APC 3.1.12 or newer that happens when
APC tries to deserialize a transient that contains an array of objects, or objects that contain other objects.

Issue is discussed here:
https://wordpress.org/support/topic/fatal-error-wpsc_countries/page/2?replies=60#post-6819037
273acd1
@JeffPyeBrook JeffPyeBrook changed the title from Harden countries class against bad cache to Harden WP eCommerce against APC cache deserialization errors Apr 14, 2015
@JeffPyeBrook
Contributor

@JustinSainton can you give this PR a read and a test

@JustinSainton
Member

@JeffPyeBrook Would like to see 273acd1 mostly reverted, aside from the function definitions. Would like to keep this PR as tightly scope and low-touch as possible. We're dealing with an edge case here, and I'd like the PR to solve the reported issue. Trying to solve issues we're not yet having can only lead to pain and suffering.

JeffPyeBrook added some commits Apr 17, 2015
@JeffPyeBrook JeffPyeBrook always, unserialize, or the value could return as a improperly base64…
… decoded string, and even cause an infinite loop, crash or other bad stuff when a corrupted transient is returned
71c668c
@JeffPyeBrook JeffPyeBrook break up compound serialization satatements so that stack traces poin…
…t to to the line of an issue if there is a problem
67d2ee5
@JustinSainton
Member

@JeffPyeBrook Can you update the PR to reflect my last comment? I'd like to see this land in 4.0. Thanks!

@JustinSainton JustinSainton commented on the diff Apr 23, 2015
wpsc-includes/misc.functions.php
+ if ( ! $value ) {
+ xdebug_break();
+ }
+ } else {
+ $value = false;
+ }
+
+ // if there was a transient, but it could not be decoded, we delete the transient to get back
+ // to a working state
+ if ( false === $value ) {
+ _wpsc_delete_transient( $transient );
+ }
+ }
+ }
+
+ return $value;
@JustinSainton
JustinSainton Apr 23, 2015 Member

Value is not defined for all paths. $encoded_value, maybe?

@JustinSainton JustinSainton commented on an outdated diff Apr 23, 2015
wpsc-includes/misc.functions.php
+ * component objects. If the transient contents can not be decoded, the transient is deleted and the
+ * function will return false as if the tranient never existed.
+ *
+ * @since 3.9.3
+ * @param string $transient Transient name. Expected to not be SQL-escaped.
+ * @return mixed value of transient, false if transient did not exist
+ */
+function _wpsc_get_transient( $transient ) {
+ $encoded_value = get_transient( $transient );
+ if ( false !== $encoded_value ) {
+ if ( ! empty( $encoded_value ) && is_string( $encoded_value ) ) {
+ $serialized_value = @base64_decode( $encoded_value );
+ if ( is_string( $serialized_value ) ) {
+ $value = unserialize( $serialized_value );
+ if ( ! $value ) {
+ xdebug_break();
JeffPyeBrook added some commits Apr 23, 2015
@JeffPyeBrook JeffPyeBrook revert to unchanged
1b099dd
@JeffPyeBrook JeffPyeBrook revert to stock tranient calls 75fb0c0
@JeffPyeBrook JeffPyeBrook revert to stock transient calls aeedf3a
@JeffPyeBrook JeffPyeBrook revert to stock transient calls 232749b
@JeffPyeBrook JeffPyeBrook revert to stock transient calls d6ca79d
@JeffPyeBrook JeffPyeBrook revert to stock transient calls fd51d36
@JeffPyeBrook JeffPyeBrook revert to stock transient calls c181703
@JeffPyeBrook JeffPyeBrook revert to stock transient calls 15c6194
@JeffPyeBrook JeffPyeBrook revert to stock transient calls
329d50a
@JeffPyeBrook JeffPyeBrook revert to stock transient calls
1fcbb57
@JeffPyeBrook JeffPyeBrook revert to stock transient calls
fb1be09
@JeffPyeBrook JeffPyeBrook revert to stock transient calls
0a0dfb5
@JeffPyeBrook JeffPyeBrook revert to stock transient calls
428224c
@JustinSainton
Member

haha, what is going on here?

@JeffPyeBrook
Contributor

change localized in this branch JeffPyeBrook:harden-countries-class-against-bad-cache-redux., this PR is no longer

@coveralls
coveralls commented Dec 16, 2016 edited

Coverage Status

Changes Unknown when pulling 428224c on JeffPyeBrook:harden-countries-class-against-bad-cache into ** on wp-e-commerce:master**.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment