[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
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 ffe081c..7a40201 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 @@
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 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
}
} 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 @@
}
} 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 @@
// 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 @@
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 @@
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;