New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memcached corrupting JSON encoded data #250

Closed
wetcoast opened this Issue May 20, 2016 · 6 comments

Comments

Projects
None yet
5 participants
@wetcoast

wetcoast commented May 20, 2016

Problem

Running the following sample code, json_decoding the memcached value results in the following JSON error messages:

  1. Syntax error
  2. Control character error, possibly incorrectly encoded
  3. Malformed UTF-8 characters, possibly incorrectly encoded

The error message generated cycles through the above list once the JSON encoded string is larger than 2000 characters.

No error messages are generated by just JSON encoding and decoding directly. The only time errors occur is when the value being decoded is retrieve from memcache.

Sample code

<?php
$memcache = new Memcached;
$memcache->addServer('localhost', 11211);
echo "<pre>\n";
$arr = array();
for ($i = 0; $i < 1000; $i++) {
    $arr[] = "abcdefghij";
    $json_str_in = json_encode($arr);
    if (FALSE === $memcache->set('test/jsondata', $json_str_in, 2)) {
        echo "getResultCode = " . Memcached::getResultCode() . PHP_EOL;
        exit;
    }
    $json_str_out = $memcache->get('test/jsondata');
    json_decode($json_str_in, TRUE);
    if (JSON_ERROR_NONE !== json_last_error()) {
        printf(
            'strlen of source value: %d, json_decode of source value: [%s]' . PHP_EOL,
            strlen($json_str_in),
            json_last_error_msg()
        );
    }
    json_decode($json_str_out, TRUE);
    if (JSON_ERROR_NONE !== json_last_error()) {
        printf(
            'strlen of memcached value: %d, json_decode of memcached value: [%s]' . PHP_EOL,
            strlen($json_str_out),
            json_last_error_msg()
        );
    }
}

PHP Version

PHP 7.0.5 (cli) (built: Apr 11 2016 15:53:52) ( NTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2016 Zend Technologies
    with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2016, by Zend Technologies
    with Xdebug v2.4.0, Copyright (c) 2002-2016, by Derick Rethans

Memcached Config

memcached support => enabled
Version => 3.0.0-dev
libmemcached version => 1.0.18
SASL support => yes
Session support => yes
igbinary support => yes
json support => yes
msgpack support => yes

Directive => Local Value => Master Value
memcached.compression_factor => 1.3 => 1.3
memcached.compression_threshold => 2000 => 2000
memcached.compression_type => fastlz => fastlz
memcached.default_binary_protocol => 0 => 0
memcached.default_connect_timeout => 0 => 0
memcached.default_consistent_hash => 0 => 0
memcached.serializer => igbinary => igbinary
memcached.sess_binary_protocol => 1 => 1
memcached.sess_connect_timeout => 1000 => 1000
memcached.sess_consistent_hash => no value => no value
memcached.sess_lock_expire => 0 => 0
memcached.sess_lock_max_wait => not set => not set
memcached.sess_lock_retries => 5 => 5
memcached.sess_lock_wait => not set => not set
memcached.sess_lock_wait_max => 0 => 0
memcached.sess_lock_wait_min => 0 => 0
memcached.sess_locking => 1 => 1
memcached.sess_number_of_replicas => 0 => 0
memcached.sess_persistent => 0 => 0
memcached.sess_prefix => memc.sess.key. => memc.sess.key.
memcached.sess_randomize_replica_read => no value => no value
memcached.sess_remove_failed_servers => 0 => 0
memcached.sess_sasl_password => no value => no value
memcached.sess_sasl_username => no value => no value
memcached.sess_server_failure_limit => 0 => 0
memcached.store_retry_count => 2 => 2

Memcache Server Version

1.4.13
@arisro

This comment has been minimized.

Show comment
Hide comment
@arisro

arisro May 30, 2016

Contributor

We noticed this very same problem.
It seems that when it does the decompression of compressed values (see the 2000B default threshold) it adds somewhere some non-utf8 character(s) that I couldn't find.

Contributor

arisro commented May 30, 2016

We noticed this very same problem.
It seems that when it does the decompression of compressed values (see the 2000B default threshold) it adds somewhere some non-utf8 character(s) that I couldn't find.

@ftzdomino

This comment has been minimized.

Show comment
Hide comment
@ftzdomino

ftzdomino Jun 1, 2016

i'm having the same issue with PHP serialized data. A workaround:
memcached.compression_threshold=9999999999

ftzdomino commented Jun 1, 2016

i'm having the same issue with PHP serialized data. A workaround:
memcached.compression_threshold=9999999999

@dictcp

This comment has been minimized.

Show comment
Hide comment
@dictcp

dictcp Jun 8, 2016

Contributor

we found a potential bug in decompress path. and the PR works for us so far. please try and see if the fix works for you
#252

Contributor

dictcp commented Jun 8, 2016

we found a potential bug in decompress path. and the PR works for us so far. please try and see if the fix works for you
#252

@dictcp

This comment has been minimized.

Show comment
Hide comment
@dictcp

dictcp Jun 8, 2016

Contributor

@wetcoast @arisro @ftzdomino could you try if the PR #252 works for you?

Contributor

dictcp commented Jun 8, 2016

@wetcoast @arisro @ftzdomino could you try if the PR #252 works for you?

@wetcoast

This comment has been minimized.

Show comment
Hide comment
@wetcoast

wetcoast Jun 8, 2016

we found a potential bug in decompress path. and the PR works for us so far. please try and see if the fix works for you
#252

works for us. thanks!

wetcoast commented Jun 8, 2016

we found a potential bug in decompress path. and the PR works for us so far. please try and see if the fix works for you
#252

works for us. thanks!

@arisro

This comment has been minimized.

Show comment
Hide comment
@arisro

arisro Jun 9, 2016

Contributor

Works fine for us too! Thank you so much for the fix @dictcp. One step closer to the PHP7 version release. :)

I want to also add that we upgraded our production to php7, using the packages from Ondrej's repo. We used as a workaround for this (we are json_encoding only one key) doing a utf8_encode() when fetching from memcached, before json_decoding.
We didn't notice yet (in 10 days) any other issues with the extension for our use cases, of course! We will have a bigger traffic spike this weekend, we'll monitor it and report if we have any performance issues with it.

Contributor

arisro commented Jun 9, 2016

Works fine for us too! Thank you so much for the fix @dictcp. One step closer to the PHP7 version release. :)

I want to also add that we upgraded our production to php7, using the packages from Ondrej's repo. We used as a workaround for this (we are json_encoding only one key) doing a utf8_encode() when fetching from memcached, before json_decoding.
We didn't notice yet (in 10 days) any other issues with the extension for our use cases, of course! We will have a bigger traffic spike this weekend, we'll monitor it and report if we have any performance issues with it.

krakjoe added a commit that referenced this issue Jun 9, 2016

Merge pull request #252 from dictcp/fix-null-string-decompress
fix #250: non-null-terminated zend_string in s_decompress_value

@krakjoe krakjoe closed this Jun 9, 2016

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