From 7283b11d147c7fab4915623c627a75a612534a6b Mon Sep 17 00:00:00 2001 From: Dick Tang Date: Fri, 10 Jun 2016 21:37:55 +0800 Subject: [PATCH 1/2] test: compression edge case verification (#256) test compressed SET/GET under various settings of - compression_factor - compression_threshold - data length --- package.xml | 1 + tests/compression_conditions.phpt | 125 ++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+) create mode 100644 tests/compression_conditions.phpt diff --git a/package.xml b/package.xml index 2091f2e2..27491332 100644 --- a/package.xml +++ b/package.xml @@ -94,6 +94,7 @@ Tests + diff --git a/tests/compression_conditions.phpt b/tests/compression_conditions.phpt new file mode 100644 index 00000000..ca3c2d77 --- /dev/null +++ b/tests/compression_conditions.phpt @@ -0,0 +1,125 @@ +--TEST-- +Memcached compression test +--SKIPIF-- + +--FILE-- +setOption(Memcached::OPT_COMPRESSION, false); + } else { + $m->setOption(Memcached::OPT_COMPRESSION, true); + $m->setOption(Memcached::OPT_COMPRESSION_TYPE, get_compression($set_compression)); + } + + $m->set($key, $value, 1800); + + $value_back = $m->get($key); + var_dump($value === $value_back); +} + +fetch_with_compression($m, 'hello01', $data, 'zlib', 1.3, 4); +fetch_with_compression($m, 'hello02', $data, 'fastlz', 1.3, 4); +fetch_with_compression($m, 'hello03', $data, '', 1.3, 4); +fetch_with_compression($m, 'hello04', $short_data, 'zlib', 1.3, 4); +fetch_with_compression($m, 'hello05', $short_data, 'fastlz', 1.3, 4); +fetch_with_compression($m, 'hello06', $short_data, '', 1.3, 4); +fetch_with_compression($m, 'hello11', $data, 'zlib', 0.3, 4); +fetch_with_compression($m, 'hello12', $data, 'fastlz', 0.3, 4); +fetch_with_compression($m, 'hello13', $data, '', 0.3, 4); +fetch_with_compression($m, 'hello14', $short_data, 'zlib', 0.3, 4); +fetch_with_compression($m, 'hello15', $short_data, 'fastlz', 0.3, 4); +fetch_with_compression($m, 'hello16', $short_data, '', 0.3, 4); +fetch_with_compression($m, 'hello21', $data, 'zlib', 1.3, 2000); +fetch_with_compression($m, 'hello22', $data, 'fastlz', 1.3, 2000); +fetch_with_compression($m, 'hello23', $data, '', 1.3, 2000); +fetch_with_compression($m, 'hello24', $short_data, 'zlib', 1.3, 2000); +fetch_with_compression($m, 'hello25', $short_data, 'fastlz', 1.3, 2000); +fetch_with_compression($m, 'hello26', $short_data, '', 1.3, 2000); +fetch_with_compression($m, 'hello31', $data, 'zlib', 0.3, 2000); +fetch_with_compression($m, 'hello32', $data, 'fastlz', 0.3, 2000); +fetch_with_compression($m, 'hello33', $data, '', 0.3, 2000); +fetch_with_compression($m, 'hello34', $short_data, 'zlib', 0.3, 2000); +fetch_with_compression($m, 'hello35', $short_data, 'fastlz', 0.3, 2000); +fetch_with_compression($m, 'hello36', $short_data, '', 0.3, 2000); +?> +--EXPECT-- +len=[4877] set=[zlib] factor=[1.3] threshold=[4] +bool(true) +len=[4877] set=[fastlz] factor=[1.3] threshold=[4] +bool(true) +len=[4877] set=[] factor=[1.3] threshold=[4] +bool(true) +len=[7] set=[zlib] factor=[1.3] threshold=[4] +Memcached::set(): could not compress value +bool(true) +len=[7] set=[fastlz] factor=[1.3] threshold=[4] +bool(true) +len=[7] set=[] factor=[1.3] threshold=[4] +bool(true) +len=[4877] set=[zlib] factor=[0.3] threshold=[4] +bool(true) +len=[4877] set=[fastlz] factor=[0.3] threshold=[4] +bool(true) +len=[4877] set=[] factor=[0.3] threshold=[4] +bool(true) +len=[7] set=[zlib] factor=[0.3] threshold=[4] +Memcached::set(): could not compress value +bool(true) +len=[7] set=[fastlz] factor=[0.3] threshold=[4] +bool(true) +len=[7] set=[] factor=[0.3] threshold=[4] +bool(true) +len=[4877] set=[zlib] factor=[1.3] threshold=[2000] +bool(true) +len=[4877] set=[fastlz] factor=[1.3] threshold=[2000] +bool(true) +len=[4877] set=[] factor=[1.3] threshold=[2000] +bool(true) +len=[7] set=[zlib] factor=[1.3] threshold=[2000] +bool(true) +len=[7] set=[fastlz] factor=[1.3] threshold=[2000] +bool(true) +len=[7] set=[] factor=[1.3] threshold=[2000] +bool(true) +len=[4877] set=[zlib] factor=[0.3] threshold=[2000] +bool(true) +len=[4877] set=[fastlz] factor=[0.3] threshold=[2000] +bool(true) +len=[4877] set=[] factor=[0.3] threshold=[2000] +bool(true) +len=[7] set=[zlib] factor=[0.3] threshold=[2000] +bool(true) +len=[7] set=[fastlz] factor=[0.3] threshold=[2000] +bool(true) +len=[7] set=[] factor=[0.3] threshold=[2000] +bool(true) From c85f34c01ba884987fa3ed4daf7846e503a89cdf Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Wed, 25 Jan 2017 00:12:44 -0800 Subject: [PATCH 2/2] Make sure that s_compress_value() will always leave a valid payload, even if it did not get compressed --- php_memcached.c | 53 ++++++++++++++++--------------- tests/compression_conditions.phpt | 2 -- 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/php_memcached.c b/php_memcached.c index b014ecc6..1e73a4f9 100644 --- a/php_memcached.c +++ b/php_memcached.c @@ -830,6 +830,7 @@ zend_bool s_compress_value (php_memc_compression_type compression_type, zend_str /* status */ zend_bool compress_status = 0; zend_string *payload = *payload_in; + uint32_t compression_type_flag = 0; /* Additional 5% for the data */ size_t buffer_size = (size_t) (((double) ZSTR_LEN(payload) * 1.05) + 1.0); @@ -847,7 +848,7 @@ zend_bool s_compress_value (php_memc_compression_type compression_type, zend_str if (compressed_size > 0) { compress_status = 1; - MEMC_VAL_SET_FLAG(*flags, MEMC_VAL_COMPRESSION_FASTLZ); + compression_type_flag = MEMC_VAL_COMPRESSION_FASTLZ; } } break; @@ -859,7 +860,7 @@ zend_bool s_compress_value (php_memc_compression_type compression_type, zend_str if (status == Z_OK) { compress_status = 1; - MEMC_VAL_SET_FLAG(*flags, MEMC_VAL_COMPRESSION_ZLIB); + compression_type_flag = MEMC_VAL_COMPRESSION_ZLIB; } } break; @@ -869,31 +870,29 @@ zend_bool s_compress_value (php_memc_compression_type compression_type, zend_str break; } - if (!compress_status) { - php_error_docref(NULL, E_WARNING, "could not compress value"); - efree (buffer); - return 0; - } - - /* This means the value was too small to be compressed, still a success */ + /* This means the value was too small to be compressed and ended up larger */ if (ZSTR_LEN(payload) <= (compressed_size * MEMC_G(compression_factor))) { - MEMC_VAL_DEL_FLAG(*flags, MEMC_VAL_COMPRESSION_FASTLZ | MEMC_VAL_COMPRESSION_ZLIB); - efree (buffer); - return 1; + compress_status = 0; } - MEMC_VAL_SET_FLAG(*flags, MEMC_VAL_COMPRESSED); + /* Replace the payload with the compressed copy */ + if (compress_status) { + MEMC_VAL_SET_FLAG(*flags, MEMC_VAL_COMPRESSED | compression_type_flag); + payload = zend_string_realloc(payload, compressed_size + sizeof(uint32_t), 0); - payload = zend_string_realloc(payload, compressed_size + sizeof(uint32_t), 0); + /* Copy the uin32_t at the beginning */ + memcpy(ZSTR_VAL(payload), &original_size, sizeof(uint32_t)); + memcpy(ZSTR_VAL(payload) + sizeof (uint32_t), buffer, compressed_size); + efree(buffer); - /* Copy the uin32_t at the beginning */ - memcpy(ZSTR_VAL(payload), &original_size, sizeof(uint32_t)); - memcpy(ZSTR_VAL(payload) + sizeof (uint32_t), buffer, compressed_size); - efree(buffer); + zend_string_forget_hash_val(payload); + *payload_in = payload; - zend_string_forget_hash_val(payload); - *payload_in = payload; - return 1; + return 1; + } + + /* Original payload was not modified */ + return 0; } static @@ -1043,11 +1042,13 @@ zend_string *s_zval_to_payload(php_memc_object_t *intern, zval *value, uint32_t /* If we have compression flag, compress the value */ if (should_compress) { - /* status */ - if (!s_compress_value (memc_user_data->compression_type, &payload, flags)) { - zend_string_release(payload); - return NULL; - } + /* s_compress_value() will always leave a valid payload, even if that payload + * did not actually get compressed. The flags will be set according to the + * to the compression type or no compression. + * + * No need to check the return value because the payload is always valid. + */ + (void)s_compress_value (memc_user_data->compression_type, &payload, flags); } if (memc_user_data->set_udf_flags >= 0) { diff --git a/tests/compression_conditions.phpt b/tests/compression_conditions.phpt index ca3c2d77..749ebe8a 100644 --- a/tests/compression_conditions.phpt +++ b/tests/compression_conditions.phpt @@ -80,7 +80,6 @@ bool(true) len=[4877] set=[] factor=[1.3] threshold=[4] bool(true) len=[7] set=[zlib] factor=[1.3] threshold=[4] -Memcached::set(): could not compress value bool(true) len=[7] set=[fastlz] factor=[1.3] threshold=[4] bool(true) @@ -93,7 +92,6 @@ bool(true) len=[4877] set=[] factor=[0.3] threshold=[4] bool(true) len=[7] set=[zlib] factor=[0.3] threshold=[4] -Memcached::set(): could not compress value bool(true) len=[7] set=[fastlz] factor=[0.3] threshold=[4] bool(true)