From 840ca085fcfe737554ede28088d4ae9adf457216 Mon Sep 17 00:00:00 2001 From: dweatherford Date: Sat, 8 Mar 2008 05:51:24 +0000 Subject: [PATCH] [thrift] php thrift_protocol crash fixes Summary: The convert_to_*_ex functions were being used improperly resulting in heap corruption in some cases; I just switched everything over to the non-ex versions since it shouldn't matter if I modify the value being serialized in place to coerce it to the proper type. Also fixed a potential crash for map, set, and list types when not passed an array, by first attempting an array conversion and then throwing a tprotocolexception if that doesn't succeed. (Actually, PHP might fatal there instead, it wasn't immediately clear from reading the code if that would be the case). Reviewed by: marcel Test plan: Ran under php-5.2.5, debug and release builds. No more heap corruption or memory leak complaints (the latter also a side effect of undesired zval reference separation). Revert: only if you love SIGSEGV git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665566 13f79535-47bb-0310-9956-ffa450edef68 --- .../thrift_protocol/php_thrift_protocol.cpp | 50 ++++++++++++------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp b/lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp index ffe081c8..7a402015 100644 --- a/lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp +++ b/lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp @@ -476,7 +476,7 @@ void binary_deserialize(int8_t thrift_typeID, PHPInputTransport& transport, zval zend_hash_index_update(return_value->value.ht, Z_LVAL_P(key), &value, sizeof(zval *), NULL); } else { - convert_to_string_ex(&key); + if (Z_TYPE_P(key) != IS_STRING) convert_to_string(key); zend_hash_update(return_value->value.ht, Z_STRVAL_P(key), Z_STRLEN_P(key) + 1, &value, sizeof(zval *), NULL); } zval_ptr_dtor(&key); @@ -522,7 +522,7 @@ void binary_deserialize(int8_t thrift_typeID, PHPInputTransport& transport, zval zend_hash_index_update(return_value->value.ht, Z_LVAL_P(key), &value, sizeof(zval *), NULL); } else { - convert_to_string_ex(&key); + if (Z_TYPE_P(key) != IS_STRING) convert_to_string(key); zend_hash_update(return_value->value.ht, Z_STRVAL_P(key), Z_STRLEN_P(key) + 1, &value, sizeof(zval *), NULL); } zval_ptr_dtor(&key); @@ -655,7 +655,7 @@ void binary_deserialize_spec(zval* zthis, PHPInputTransport& transport, HashTabl // and the type zend_hash_find(fieldspec, "type", 5, (void**)&val_ptr); - convert_to_long_ex(val_ptr); + if (Z_TYPE_PP(val_ptr) != IS_LONG) convert_to_long(*val_ptr); int8_t expected_ttype = Z_LVAL_PP(val_ptr); if (ttypes_are_compatible(ttype, expected_ttype)) { @@ -685,24 +685,24 @@ void binary_serialize(int8_t thrift_typeID, PHPOutputTransport& transport, zval* binary_serialize_spec(*value, transport, Z_ARRVAL_P(spec)); } return; case T_BOOL: - convert_to_boolean_ex(value); + if (Z_TYPE_PP(value) != IS_BOOL) convert_to_boolean(*value); transport.writeI8(Z_BVAL_PP(value) ? 1 : 0); return; case T_BYTE: - convert_to_long_ex(value); + if (Z_TYPE_PP(value) != IS_LONG) convert_to_long(*value); transport.writeI8(Z_LVAL_PP(value)); return; case T_I16: - convert_to_long_ex(value); + if (Z_TYPE_PP(value) != IS_LONG) convert_to_long(*value); transport.writeI16(Z_LVAL_PP(value)); return; case T_I32: - convert_to_long_ex(value); + if (Z_TYPE_PP(value) != IS_LONG) convert_to_long(*value); transport.writeI32(Z_LVAL_PP(value)); return; case T_I64: case T_U64: - convert_to_long_ex(value); + if (Z_TYPE_PP(value) != IS_LONG) convert_to_long(*value); transport.writeI64(Z_LVAL_PP(value)); return; case T_DOUBLE: { @@ -710,7 +710,7 @@ void binary_serialize(int8_t thrift_typeID, PHPOutputTransport& transport, zval* int64_t c; double d; } a; - convert_to_double_ex(value); + if (Z_TYPE_PP(value) != IS_DOUBLE) convert_to_double(*value); a.d = Z_DVAL_PP(value); transport.writeI64(a.c); } return; @@ -718,19 +718,23 @@ void binary_serialize(int8_t thrift_typeID, PHPOutputTransport& transport, zval* case T_UTF8: case T_UTF16: case T_STRING: - convert_to_string(*value); + if (Z_TYPE_PP(value) != IS_STRING) convert_to_string(*value); transport.writeString(Z_STRVAL_PP(value), Z_STRLEN_PP(value)); return; case T_MAP: { + if (Z_TYPE_PP(value) != IS_ARRAY) convert_to_array(*value); + if (Z_TYPE_PP(value) != IS_ARRAY) { + throw_tprotocolexception("Attempt to send an incompatible type as an array (T_MAP)", INVALID_DATA); + } HashTable* ht = Z_ARRVAL_PP(value); zval** val_ptr; zend_hash_find(fieldspec, "ktype", 6, (void**)&val_ptr); - convert_to_long_ex(val_ptr); + if (Z_TYPE_PP(val_ptr) != IS_LONG) convert_to_long(*val_ptr); uint8_t keytype = Z_LVAL_PP(val_ptr); transport.writeI8(keytype); zend_hash_find(fieldspec, "vtype", 6, (void**)&val_ptr); - convert_to_long_ex(val_ptr); + if (Z_TYPE_PP(val_ptr) != IS_LONG) convert_to_long(*val_ptr); uint8_t valtype = Z_LVAL_PP(val_ptr); transport.writeI8(valtype); @@ -745,11 +749,15 @@ void binary_serialize(int8_t thrift_typeID, PHPOutputTransport& transport, zval* } } return; case T_LIST: { + if (Z_TYPE_PP(value) != IS_ARRAY) convert_to_array(*value); + if (Z_TYPE_PP(value) != IS_ARRAY) { + throw_tprotocolexception("Attempt to send an incompatible type as an array (T_LIST)", INVALID_DATA); + } HashTable* ht = Z_ARRVAL_PP(value); zval** val_ptr; zend_hash_find(fieldspec, "etype", 6, (void**)&val_ptr); - convert_to_long_ex(val_ptr); + if (Z_TYPE_PP(val_ptr) != IS_LONG) convert_to_long(*val_ptr); uint8_t valtype = Z_LVAL_PP(val_ptr); transport.writeI8(valtype); @@ -763,11 +771,15 @@ void binary_serialize(int8_t thrift_typeID, PHPOutputTransport& transport, zval* } } return; case T_SET: { + if (Z_TYPE_PP(value) != IS_ARRAY) convert_to_array(*value); + if (Z_TYPE_PP(value) != IS_ARRAY) { + throw_tprotocolexception("Attempt to send an incompatible type as an array (T_SET)", INVALID_DATA); + } HashTable* ht = Z_ARRVAL_PP(value); zval** val_ptr; zend_hash_find(fieldspec, "etype", 6, (void**)&val_ptr); - convert_to_long_ex(val_ptr); + if (Z_TYPE_PP(val_ptr) != IS_LONG) convert_to_long(*val_ptr); uint8_t keytype = Z_LVAL_PP(val_ptr); transport.writeI8(keytype); @@ -805,7 +817,7 @@ void binary_serialize_spec(zval* zthis, PHPOutputTransport& transport, HashTable // thrift type zend_hash_find(fieldspec, "type", 5, (void**)&val_ptr); - convert_to_long_ex(val_ptr); + if (Z_TYPE_PP(val_ptr) != IS_LONG) convert_to_long(*val_ptr); int8_t ttype = Z_LVAL_PP(val_ptr); zval* prop = zend_read_property(ce, zthis, varname, strlen(varname), false TSRMLS_CC); @@ -848,12 +860,12 @@ PHP_FUNCTION(thrift_protocol_write_binary) { PHPOutputTransport transport(*args[0]); const char* method_name = Z_STRVAL_PP(args[1]); - convert_to_long_ex(args[2]); + convert_to_long(*args[2]); int32_t msgtype = Z_LVAL_PP(args[2]); zval* request_struct = *args[3]; - convert_to_long_ex(args[4]); + convert_to_long(*args[4]); int32_t seqID = Z_LVAL_PP(args[4]); - convert_to_boolean_ex(args[5]); + convert_to_boolean(*args[5]); bool strictWrite = Z_BVAL_PP(args[5]); efree(args); args = NULL; @@ -903,7 +915,7 @@ PHP_FUNCTION(thrift_protocol_read_binary) { PHPInputTransport transport(*args[0]); char* obj_typename = Z_STRVAL_PP(args[1]); - convert_to_boolean_ex(args[2]); + convert_to_boolean(*args[2]); bool strict_read = Z_BVAL_PP(args[2]); efree(args); args = NULL; -- 2.17.1