Skip to content

Commit a302564

Browse files
michael-grundermichael-grunder
committed
Fix parsing of 'zipped' replies for various uses
As discovered in issue phpredis#523, phpredis was attempting to unserialize both the keys *and* scores for commands like zRangeByScore. This had to do with the logic around deserialization in the response. In addition, this same bug would have caused issues when running commands like $r->config('get'), because that too, would have tried to unserialize the values, which we don't want to do. This commit reworks parsing and zipping up replies by allowing the call to be configured to unseraialize any combination of keys or values (or none or both).
1 parent c1f862c commit a302564

File tree

5 files changed

+307
-225
lines changed

5 files changed

+307
-225
lines changed

‎library.c‎

Lines changed: 196 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,14 @@ PHPAPI int usleep(unsigned int useconds);
3232
#define usleep Sleep
3333
#endif
3434

35-
#define UNSERIALIZE_ONLY_VALUES 0
36-
#define UNSERIALIZE_ALL 1
35+
#define UNSERIALIZE_NONE 0
36+
#define UNSERIALIZE_KEYS 1
37+
#define UNSERIALIZE_VALS 2
38+
#define UNSERIALIZE_ALL 3
39+
40+
#define SCORE_DECODE_NONE 0
41+
#define SCORE_DECODE_INT 1
42+
#define SCORE_DECODE_DOUBLE 2
3743

3844
extern zend_class_entry *redis_ce;
3945
extern zend_class_entry *redis_exception_ce;
@@ -115,7 +121,7 @@ PHP_REDIS_API int redis_check_eof(RedisSock *redis_sock TSRMLS_DC)
115121
}
116122

117123

118-
PHP_REDIS_API int
124+
PHP_REDIS_API int
119125
redis_sock_read_scan_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
120126
REDIS_SCAN_TYPE type, long *iter)
121127
{
@@ -150,13 +156,13 @@ redis_sock_read_scan_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
150156
scan command this is. They all come back in slightly different ways */
151157
switch(type) {
152158
case TYPE_SCAN:
153-
return redis_sock_read_multibulk_reply_raw(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL, NULL);
159+
return redis_mbulk_reply_raw(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL, NULL);
154160
case TYPE_SSCAN:
155161
return redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL, NULL);
156162
case TYPE_ZSCAN:
157-
return redis_sock_read_multibulk_reply_zipped(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL, NULL);
163+
return redis_mbulk_reply_zipped_keys_dbl(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL, NULL);
158164
case TYPE_HSCAN:
159-
return redis_sock_read_multibulk_reply_zipped_strings(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL, NULL);
165+
return redis_mbulk_reply_zipped_vals(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL, NULL);
160166
default:
161167
return -1;
162168
}
@@ -189,9 +195,10 @@ PHP_REDIS_API zval *redis_sock_read_multibulk_reply_zval(INTERNAL_FUNCTION_PARAM
189195
MAKE_STD_ZVAL(z_tab);
190196
array_init(z_tab);
191197

192-
redis_sock_read_multibulk_reply_loop(INTERNAL_FUNCTION_PARAM_PASSTHRU,
193-
redis_sock, z_tab, numElems, 1, UNSERIALIZE_ALL);
194-
return z_tab;
198+
redis_mbulk_reply_loop(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, z_tab,
199+
numElems, UNSERIALIZE_ALL);
200+
201+
return z_tab;
195202
}
196203

197204
/**
@@ -944,15 +951,90 @@ PHP_REDIS_API void redis_long_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *
944951
}
945952
}
946953

954+
/* Helper method to convert [key, value, key, value] into [key => value,
955+
* key => value] when returning data to the caller. Depending on our decode
956+
* flag we'll convert the value data types */
957+
static void array_zip_values_and_scores(RedisSock *redis_sock, zval *z_tab,
958+
int decode TSRMLS_DC)
959+
{
960+
961+
zval *z_ret;
962+
HashTable *keytable;
963+
964+
MAKE_STD_ZVAL(z_ret);
965+
array_init(z_ret);
966+
keytable = Z_ARRVAL_P(z_tab);
967+
968+
for(zend_hash_internal_pointer_reset(keytable);
969+
zend_hash_has_more_elements(keytable) == SUCCESS;
970+
zend_hash_move_forward(keytable)) {
971+
972+
char *tablekey, *hkey, *hval;
973+
unsigned int tablekey_len;
974+
int hkey_len;
975+
unsigned long idx;
976+
zval **z_key_pp, **z_value_pp;
977+
978+
zend_hash_get_current_key_ex(keytable, &tablekey, &tablekey_len, &idx, 0, NULL);
979+
if(zend_hash_get_current_data(keytable, (void**)&z_key_pp) == FAILURE) {
980+
continue; /* this should never happen, according to the PHP people. */
981+
}
982+
983+
/* get current value, a key */
984+
convert_to_string(*z_key_pp);
985+
hkey = Z_STRVAL_PP(z_key_pp);
986+
hkey_len = Z_STRLEN_PP(z_key_pp);
987+
988+
/* move forward */
989+
zend_hash_move_forward(keytable);
990+
991+
/* fetch again */
992+
zend_hash_get_current_key_ex(keytable, &tablekey, &tablekey_len, &idx, 0, NULL);
993+
if(zend_hash_get_current_data(keytable, (void**)&z_value_pp) == FAILURE) {
994+
continue; /* this should never happen, according to the PHP people. */
995+
}
996+
997+
/* get current value, a hash value now. */
998+
hval = Z_STRVAL_PP(z_value_pp);
947999

1000+
/* Decode the score depending on flag */
1001+
if (decode == SCORE_DECODE_INT && Z_STRLEN_PP(z_value_pp) > 0) {
1002+
add_assoc_long_ex(z_ret, hkey, 1+hkey_len, atoi(hval+1));
1003+
} else if (decode == SCORE_DECODE_DOUBLE) {
1004+
add_assoc_double_ex(z_ret, hkey, 1+hkey_len, atof(hval));
1005+
} else {
1006+
zval *z = NULL;
1007+
MAKE_STD_ZVAL(z);
1008+
*z = **z_value_pp;
1009+
zval_copy_ctor(z);
1010+
add_assoc_zval_ex(z_ret, hkey, 1+hkey_len, z);
1011+
}
1012+
1013+
/*
1014+
if(use_atof) {
1015+
add_assoc_double_ex(z_ret, hkey, 1+hkey_len, atof(hval));
1016+
} else {
1017+
zval *z = NULL;
1018+
MAKE_STD_ZVAL(z);
1019+
*z = **z_value_pp;
1020+
zval_copy_ctor(z);
1021+
add_assoc_zval_ex(z_ret, hkey, 1+hkey_len, z);
1022+
}*/
9481023

949-
PHP_REDIS_API int redis_sock_read_multibulk_reply_zipped_with_flag(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, int flag) {
1024+
}
1025+
/* replace */
1026+
zval_dtor(z_tab);
1027+
*z_tab = *z_ret;
1028+
zval_copy_ctor(z_tab);
1029+
zval_dtor(z_ret);
9501030

951-
/*
952-
int ret = redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, z_tab TSRMLS_CC);
953-
array_zip_values_and_scores(return_value, 0);
954-
*/
1031+
efree(z_ret);
1032+
}
9551033

1034+
static int
1035+
redis_mbulk_reply_zipped(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
1036+
zval *z_tab, int unserialize, int decode)
1037+
{
9561038
char inbuf[1024];
9571039
int numElems;
9581040
zval *z_multi_result;
@@ -982,10 +1064,12 @@ PHP_REDIS_API int redis_sock_read_multibulk_reply_zipped_with_flag(INTERNAL_FUNC
9821064
MAKE_STD_ZVAL(z_multi_result);
9831065
array_init(z_multi_result); /* pre-allocate array for multi's results. */
9841066

985-
redis_sock_read_multibulk_reply_loop(INTERNAL_FUNCTION_PARAM_PASSTHRU,
986-
redis_sock, z_multi_result, numElems, 1, flag ? UNSERIALIZE_ALL : UNSERIALIZE_ONLY_VALUES);
1067+
/* Grab our key, value, key, value array */
1068+
redis_mbulk_reply_loop(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock,
1069+
z_multi_result, numElems, unserialize);
9871070

988-
array_zip_values_and_scores(redis_sock, z_multi_result, 0 TSRMLS_CC);
1071+
/* Zip keys and values */
1072+
array_zip_values_and_scores(redis_sock, z_multi_result, decode TSRMLS_CC);
9891073

9901074
IF_MULTI_OR_PIPELINE() {
9911075
add_next_index_zval(z_tab, z_multi_result);
@@ -999,13 +1083,35 @@ PHP_REDIS_API int redis_sock_read_multibulk_reply_zipped_with_flag(INTERNAL_FUNC
9991083
return 0;
10001084
}
10011085

1002-
PHP_REDIS_API int redis_sock_read_multibulk_reply_zipped(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx) {
1086+
/* Zipped key => value reply but we don't touch anything (e.g. CONFIG GET) */
1087+
PHP_REDIS_API int redis_mbulk_reply_zipped_raw(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx)
1088+
{
1089+
return redis_mbulk_reply_zipped(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock,
1090+
z_tab, UNSERIALIZE_NONE, SCORE_DECODE_NONE);
1091+
}
10031092

1004-
return redis_sock_read_multibulk_reply_zipped_with_flag(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, z_tab, 1);
1093+
/* Zipped key => value reply unserializing keys and decoding the score as an integer (PUBSUB) */
1094+
PHP_REDIS_API int redis_mbulk_reply_zipped_keys_int(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
1095+
zval *z_tab, void *ctx)
1096+
{
1097+
return redis_mbulk_reply_zipped(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock,
1098+
z_tab, UNSERIALIZE_KEYS, SCORE_DECODE_INT);
10051099
}
10061100

1007-
PHP_REDIS_API int redis_sock_read_multibulk_reply_zipped_strings(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx) {
1008-
return redis_sock_read_multibulk_reply_zipped_with_flag(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, z_tab, 0);
1101+
/* Zipped key => value reply unserializing keys and decoding the score as a double (ZSET commands) */
1102+
PHP_REDIS_API int redis_mbulk_reply_zipped_keys_dbl(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
1103+
zval *z_tab, void *ctx)
1104+
{
1105+
return redis_mbulk_reply_zipped(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock,
1106+
z_tab, UNSERIALIZE_KEYS, SCORE_DECODE_DOUBLE);
1107+
}
1108+
1109+
/* Zipped key => value reply where only the values are unserialized (e.g. HMGET) */
1110+
PHP_REDIS_API int redis_mbulk_reply_zipped_vals(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
1111+
zval *z_tab, void *ctx)
1112+
{
1113+
return redis_mbulk_reply_zipped(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock,
1114+
z_tab, UNSERIALIZE_VALS, SCORE_DECODE_NONE);
10091115
}
10101116

10111117
PHP_REDIS_API void redis_1_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx) {
@@ -1090,7 +1196,7 @@ PHP_REDIS_API void redis_ping_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *
10901196
}
10911197

10921198
/* Response for DEBUG object which is a formatted single line reply */
1093-
PHP_REDIS_API void redis_debug_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
1199+
PHP_REDIS_API void redis_debug_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
10941200
zval *z_tab, void *ctx)
10951201
{
10961202
char *resp, *p, *p2, *p3, *p4;
@@ -1116,7 +1222,7 @@ PHP_REDIS_API void redis_debug_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock
11161222
while((p2 = strchr(p, ':'))!=NULL) {
11171223
/* Null terminate at the ':' */
11181224
*p2++ = '\0';
1119-
1225+
11201226
/* Null terminate at the space if we have one */
11211227
if((p3 = strchr(p2, ' '))!=NULL) {
11221228
*p3++ = '\0';
@@ -1138,7 +1244,7 @@ PHP_REDIS_API void redis_debug_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock
11381244
} else {
11391245
add_assoc_string(z_result, p, p2, 1);
11401246
}
1141-
1247+
11421248
p = p3;
11431249
}
11441250

@@ -1421,8 +1527,8 @@ PHP_REDIS_API int redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAMETERS,
14211527
MAKE_STD_ZVAL(z_multi_result);
14221528
array_init(z_multi_result); /* pre-allocate array for multi's results. */
14231529

1424-
redis_sock_read_multibulk_reply_loop(INTERNAL_FUNCTION_PARAM_PASSTHRU,
1425-
redis_sock, z_multi_result, numElems, 1, UNSERIALIZE_ALL);
1530+
redis_mbulk_reply_loop(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock,
1531+
z_multi_result, numElems, UNSERIALIZE_ALL);
14261532

14271533
IF_MULTI_OR_PIPELINE() {
14281534
add_next_index_zval(z_tab, z_multi_result);
@@ -1437,7 +1543,7 @@ PHP_REDIS_API int redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAMETERS,
14371543
/**
14381544
* Like multibulk reply, but don't touch the values, they won't be compressed. (this is used by HKEYS).
14391545
*/
1440-
PHP_REDIS_API int redis_sock_read_multibulk_reply_raw(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx)
1546+
PHP_REDIS_API int redis_mbulk_reply_raw(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx)
14411547
{
14421548
char inbuf[1024];
14431549
int numElems;
@@ -1468,8 +1574,8 @@ PHP_REDIS_API int redis_sock_read_multibulk_reply_raw(INTERNAL_FUNCTION_PARAMETE
14681574
MAKE_STD_ZVAL(z_multi_result);
14691575
array_init(z_multi_result); /* pre-allocate array for multi's results. */
14701576

1471-
redis_sock_read_multibulk_reply_loop(INTERNAL_FUNCTION_PARAM_PASSTHRU,
1472-
redis_sock, z_multi_result, numElems, 0, UNSERIALIZE_ALL);
1577+
redis_mbulk_reply_loop(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock,
1578+
z_multi_result, numElems, UNSERIALIZE_NONE);
14731579

14741580
IF_MULTI_OR_PIPELINE() {
14751581
add_next_index_zval(z_tab, z_multi_result);
@@ -1481,6 +1587,42 @@ PHP_REDIS_API int redis_sock_read_multibulk_reply_raw(INTERNAL_FUNCTION_PARAMETE
14811587
return 0;
14821588
}
14831589

1590+
PHP_REDIS_API void
1591+
redis_mbulk_reply_loop(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
1592+
zval *z_tab, int count, int unserialize)
1593+
{
1594+
char *line;
1595+
int len;
1596+
1597+
while(count > 0) {
1598+
line = redis_sock_read(redis_sock, &len TSRMLS_CC);
1599+
if (line != NULL) {
1600+
zval *z = NULL;
1601+
int unwrap;
1602+
1603+
/* We will attempt unserialization, if we're unserializing everything,
1604+
* or if we're unserializing keys and we're on a key, or we're
1605+
* unserializing values and we're on a value! */
1606+
unwrap = unserialize == UNSERIALIZE_ALL ||
1607+
(unserialize == UNSERIALIZE_KEYS && count % 2 == 0) ||
1608+
(unserialize == UNSERIALIZE_VALS && count % 2 != 0);
1609+
1610+
if (unwrap && redis_unserialize(redis_sock, line, len, &z TSRMLS_CC)) {
1611+
efree(line);
1612+
add_next_index_zval(z_tab, z);
1613+
} else {
1614+
add_next_index_stringl(z_tab, line, len, 0);
1615+
}
1616+
} else {
1617+
add_next_index_bool(z_tab, 0);
1618+
}
1619+
1620+
count--;
1621+
}
1622+
}
1623+
1624+
1625+
/*
14841626
PHP_REDIS_API int
14851627
redis_sock_read_multibulk_reply_loop(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
14861628
zval *z_tab, int numElems, int unwrap_key, int unserialize_even_only)
@@ -1509,24 +1651,24 @@ redis_sock_read_multibulk_reply_loop(INTERNAL_FUNCTION_PARAMETERS, RedisSock *re
15091651
}
15101652
return 0;
15111653
}
1654+
*/
15121655

1513-
/**
1514-
* redis_sock_read_multibulk_reply_assoc
1515-
*/
1516-
PHP_REDIS_API int redis_sock_read_multibulk_reply_assoc(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx)
1656+
/* Specialized multibulk processing for HMGET where we need to pair requested
1657+
* keys with their returned values */
1658+
PHP_REDIS_API int redis_mbulk_reply_assoc(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx)
15171659
{
15181660
char inbuf[1024], *response;
15191661
int response_len;
1520-
int i, numElems;
1521-
zval *z_multi_result;
1662+
int i, numElems;
1663+
zval *z_multi_result;
15221664

15231665
zval **z_keys = ctx;
15241666

15251667
if(-1 == redis_check_eof(redis_sock TSRMLS_CC)) {
15261668
return -1;
15271669
}
15281670
if(php_stream_gets(redis_sock->stream, inbuf, 1024) == NULL) {
1529-
redis_stream_close(redis_sock TSRMLS_CC);
1671+
redis_stream_close(redis_sock TSRMLS_CC);
15301672
redis_sock->stream = NULL;
15311673
redis_sock->status = REDIS_SOCK_STATUS_FAILED;
15321674
redis_sock->mode = ATOMIC;
@@ -1550,30 +1692,30 @@ PHP_REDIS_API int redis_sock_read_multibulk_reply_assoc(INTERNAL_FUNCTION_PARAME
15501692
for(i = 0; i < numElems; ++i) {
15511693
response = redis_sock_read(redis_sock, &response_len TSRMLS_CC);
15521694
if(response != NULL) {
1553-
zval *z = NULL;
1554-
if(redis_unserialize(redis_sock, response, response_len, &z TSRMLS_CC) == 1) {
1555-
efree(response);
1556-
add_assoc_zval_ex(z_multi_result, Z_STRVAL_P(z_keys[i]), 1+Z_STRLEN_P(z_keys[i]), z);
1557-
} else {
1558-
add_assoc_stringl_ex(z_multi_result, Z_STRVAL_P(z_keys[i]), 1+Z_STRLEN_P(z_keys[i]), response, response_len, 0);
1559-
}
1560-
} else {
1561-
add_assoc_bool_ex(z_multi_result, Z_STRVAL_P(z_keys[i]), 1+Z_STRLEN_P(z_keys[i]), 0);
1562-
}
1563-
zval_dtor(z_keys[i]);
1564-
efree(z_keys[i]);
1695+
zval *z = NULL;
1696+
if(redis_unserialize(redis_sock, response, response_len, &z TSRMLS_CC) == 1) {
1697+
efree(response);
1698+
add_assoc_zval_ex(z_multi_result, Z_STRVAL_P(z_keys[i]), 1+Z_STRLEN_P(z_keys[i]), z);
1699+
} else {
1700+
add_assoc_stringl_ex(z_multi_result, Z_STRVAL_P(z_keys[i]), 1+Z_STRLEN_P(z_keys[i]), response, response_len, 0);
1701+
}
1702+
} else {
1703+
add_assoc_bool_ex(z_multi_result, Z_STRVAL_P(z_keys[i]), 1+Z_STRLEN_P(z_keys[i]), 0);
1704+
}
1705+
zval_dtor(z_keys[i]);
1706+
efree(z_keys[i]);
15651707
}
15661708
efree(z_keys);
15671709

15681710
IF_MULTI_OR_PIPELINE() {
15691711
add_next_index_zval(z_tab, z_multi_result);
15701712
} else {
1571-
*return_value = *z_multi_result;
1572-
zval_copy_ctor(return_value);
1573-
INIT_PZVAL(return_value);
1574-
zval_dtor(z_multi_result);
1575-
efree(z_multi_result);
1576-
}
1713+
*return_value = *z_multi_result;
1714+
zval_copy_ctor(return_value);
1715+
INIT_PZVAL(return_value);
1716+
zval_dtor(z_multi_result);
1717+
efree(z_multi_result);
1718+
}
15771719
return 0;
15781720
}
15791721

0 commit comments

Comments
 (0)