You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "ASF GitHub Bot (JIRA)" <ji...@apache.org> on 2018/04/09 12:48:00 UTC
[jira] [Commented] (THRIFT-4373) Extending Thrift class results in
"Attempt serialize from non-Thrift object"
[ https://issues.apache.org/jira/browse/THRIFT-4373?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16430473#comment-16430473 ]
ASF GitHub Bot commented on THRIFT-4373:
----------------------------------------
jeking3 closed pull request #1401: THRIFT-4373: Derefer PHP zval _TSPEC
URL: https://github.com/apache/thrift/pull/1401
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 75ef2ec8f8..5ac557ffd7 100644
--- a/lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp
+++ b/lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp
@@ -537,6 +537,7 @@ void binary_deserialize(int8_t thrift_typeID, PHPInputTransport& transport, zval
}
zval* spec = zend_read_static_property(Z_OBJCE_P(return_value), "_TSPEC", sizeof("_TSPEC")-1, false);
+ ZVAL_DEREF(spec);
if (EG(exception)) {
zend_object *ex = EG(exception);
EG(exception) = nullptr;
@@ -699,6 +700,9 @@ void binary_serialize_hashtable_key(int8_t keytype, PHPOutputTransport& transpor
static
void binary_serialize(int8_t thrift_typeID, PHPOutputTransport& transport, zval* value, HashTable* fieldspec) {
+ if (value) {
+ ZVAL_DEREF(value);
+ }
// At this point the typeID (and field num, if applicable) should've already been written to the output so all we need to do is write the payload.
switch (thrift_typeID) {
case T_STOP:
@@ -709,6 +713,9 @@ void binary_serialize(int8_t thrift_typeID, PHPOutputTransport& transport, zval*
throw_tprotocolexception("Attempt to send non-object type as a T_STRUCT", INVALID_DATA);
}
zval* spec = zend_read_static_property(Z_OBJCE_P(value), "_TSPEC", sizeof("_TSPEC")-1, true);
+ if (spec && Z_TYPE_P(spec) == IS_REFERENCE) {
+ ZVAL_DEREF(spec);
+ }
if (!spec || Z_TYPE_P(spec) != IS_ARRAY) {
throw_tprotocolexception("Attempt to send non-Thrift object as a T_STRUCT", INVALID_DATA);
}
@@ -893,7 +900,13 @@ static
void validate_thrift_object(zval* object) {
zend_class_entry* object_class_entry = Z_OBJCE_P(object);
zval* is_validate = zend_read_static_property(object_class_entry, "isValidate", sizeof("isValidate")-1, true);
+ if (is_validate) {
+ ZVAL_DEREF(is_validate);
+ }
zval* spec = zend_read_static_property(object_class_entry, "_TSPEC", sizeof("_TSPEC")-1, true);
+ if (spec) {
+ ZVAL_DEREF(spec);
+ }
HashPosition key_ptr;
zval* val_ptr;
@@ -1027,6 +1040,9 @@ PHP_FUNCTION(thrift_protocol_write_binary) {
try {
zval* spec = zend_read_static_property(Z_OBJCE_P(request_struct), "_TSPEC", sizeof("_TSPEC")-1, true);
+ if (spec) {
+ ZVAL_DEREF(spec);
+ }
if (!spec || Z_TYPE_P(spec) != IS_ARRAY) {
throw_tprotocolexception("Attempt serialize from non-Thrift object", INVALID_DATA);
@@ -1091,6 +1107,7 @@ PHP_FUNCTION(thrift_protocol_read_binary) {
zval ex;
createObject("\\Thrift\\Exception\\TApplicationException", &ex);
zval* spec = zend_read_static_property(Z_OBJCE(ex), "_TSPEC", sizeof("_TPSEC")-1, false);
+ ZVAL_DEREF(spec);
if (EG(exception)) {
zend_object *ex = EG(exception);
EG(exception) = nullptr;
@@ -1102,6 +1119,9 @@ PHP_FUNCTION(thrift_protocol_read_binary) {
createObject(ZSTR_VAL(obj_typename), return_value);
zval* spec = zend_read_static_property(Z_OBJCE_P(return_value), "_TSPEC", sizeof("_TSPEC")-1, true);
+ if (spec) {
+ ZVAL_DEREF(spec);
+ }
if (!spec || Z_TYPE_P(spec) != IS_ARRAY) {
throw_tprotocolexception("Attempt deserialize to non-Thrift object", INVALID_DATA);
}
@@ -1135,6 +1155,7 @@ PHP_FUNCTION(thrift_protocol_read_binary_after_message_begin) {
createObject(ZSTR_VAL(obj_typename), return_value);
zval* spec = zend_read_static_property(Z_OBJCE_P(return_value), "_TSPEC", sizeof("_TSPEC")-1, false);
+ ZVAL_DEREF(spec);
binary_deserialize_spec(return_value, transport, Z_ARRVAL_P(spec));
} catch (const PHPExceptionWrapper& ex) {
// ex will be destructed, so copy to a zval that zend_throw_exception_object can take ownership of
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
> Extending Thrift class results in "Attempt serialize from non-Thrift object"
> ----------------------------------------------------------------------------
>
> Key: THRIFT-4373
> URL: https://issues.apache.org/jira/browse/THRIFT-4373
> Project: Thrift
> Issue Type: Bug
> Components: PHP - Library
> Affects Versions: 0.10.0
> Environment: Linux 4.13.3-1-ARCH
> PHP 7.1.10
> Reporter: Josip Sokcevic
> Priority: Major
> Fix For: 0.12.0
>
> Attachments: 0001-THRIFT-4373-Derefer-PHP-zval-_TSPEC.patch
>
>
> This happens when using php extension. thrift_protocol_write_binary will check Z_TYPE_P of spec and expects to be array (IS_ARRAY). However, PHP7 will set it as reference (IS_REFERENCE).
> Example:
> {code}
> $s = new Serializer\TBinarySerializer();
> // Foo is a Thrift type class
> class FooExtended extends Foo {}
> $o = new Foo();
> $o2 = new FooExtended();
> // this works just fine:
> $s->serialize($o); // this uses thrift_protocol_write_binary if available
> // Next line throws \Thrift\Exception\TProtocolException if thrift_protocol_write_binary is used
> // However, it doesn't break if no extension is available.
> $s->serialize($o);
> {code}
> Proposed solution is to dereference using ZVAL_DEREF before checking types (attached). Alternative would be to mark struct type classes as final, but that break compatibility.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)