From 8fd7f02ea4cd595a792ef37e558d357d97bceefa Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 31 Mar 2020 12:17:32 +0200 Subject: [PATCH] Make cast_object handler required Avoid subtle differences in behavior depending on whether the handler is absent or returns FAILURE. If you previously set cast_object to NULL, create a handler that always returns FAILURE instead. --- UPGRADING.INTERNALS | 4 ++ Zend/zend_API.c | 15 +++---- Zend/zend_builtin_functions.c | 8 ++-- Zend/zend_object_handlers.c | 64 +++++++++++++--------------- Zend/zend_object_handlers.h | 2 +- Zend/zend_operators.c | 40 +++++++---------- ext/ffi/ffi.c | 11 +++-- ext/intl/collator/collator_convert.c | 15 +++---- 8 files changed, 73 insertions(+), 86 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index e7c3f5495b..599a024a4e 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -15,6 +15,7 @@ PHP 8.0 INTERNALS UPGRADE NOTES l. Some VM instructions switched to IS_TMP_VAR result instead of IS_VAR m. All internal functions must have arginfo n. zend_hash_sort compare function signature change + o. cast_object() object handler is now required 2. Build system changes a. Abstract @@ -110,6 +111,9 @@ PHP 8.0 INTERNALS UPGRADE NOTES Previously compare_func_t was used, which accepted void pointers. + o. The cast_object() handler is now required, i.e. must be non-null. You can + indicate that casting is not supported by always returning FAILURE. + ======================== 2. Build system changes ======================== diff --git a/Zend/zend_API.c b/Zend/zend_API.c index acb4366527..00b8076b3d 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -472,15 +472,12 @@ ZEND_API int ZEND_FASTCALL zend_parse_arg_str_weak(zval *arg, zend_string **dest *dest = Z_STR_P(arg); } else if (UNEXPECTED(Z_TYPE_P(arg) == IS_OBJECT)) { zend_object *zobj = Z_OBJ_P(arg); - - if (Z_OBJ_HANDLER_P(arg, cast_object)) { - zval obj; - if (zobj->handlers->cast_object(zobj, &obj, IS_STRING) == SUCCESS) { - OBJ_RELEASE(zobj); - ZVAL_COPY_VALUE(arg, &obj); - *dest = Z_STR_P(arg); - return 1; - } + zval obj; + if (zobj->handlers->cast_object(zobj, &obj, IS_STRING) == SUCCESS) { + OBJ_RELEASE(zobj); + ZVAL_COPY_VALUE(arg, &obj); + *dest = Z_STR_P(arg); + return 1; } return 0; } else { diff --git a/Zend/zend_builtin_functions.c b/Zend/zend_builtin_functions.c index 87c40acf4f..f00996619a 100644 --- a/Zend/zend_builtin_functions.c +++ b/Zend/zend_builtin_functions.c @@ -664,11 +664,9 @@ ZEND_FUNCTION(define) } break; case IS_OBJECT: - if (Z_OBJ_HT_P(val)->cast_object) { - if (Z_OBJ_HT_P(val)->cast_object(Z_OBJ_P(val), &val_free, IS_STRING) == SUCCESS) { - val = &val_free; - break; - } + if (Z_OBJ_HT_P(val)->cast_object(Z_OBJ_P(val), &val_free, IS_STRING) == SUCCESS) { + val = &val_free; + break; } /* no break */ default: diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index fb3b7509df..f678277fd2 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -1572,50 +1572,46 @@ ZEND_API int zend_std_compare_objects(zval *o1, zval *o2) /* {{{ */ zval casted; if (Z_TYPE_P(o1) == IS_OBJECT) { ZEND_ASSERT(Z_TYPE_P(o2) != IS_OBJECT); - if (Z_OBJ_HT_P(o1)->cast_object) { - zend_uchar target_type = (Z_TYPE_P(o2) == IS_FALSE || Z_TYPE_P(o2) == IS_TRUE) - ? _IS_BOOL : Z_TYPE_P(o2); - if (Z_OBJ_HT_P(o1)->cast_object(Z_OBJ_P(o1), &casted, target_type) == FAILURE) { - // TODO: Less crazy. - if (target_type == IS_LONG || target_type == IS_DOUBLE) { - zend_error(E_NOTICE, "Object of class %s could not be converted to %s", - ZSTR_VAL(Z_OBJCE_P(o1)->name), zend_get_type_by_const(target_type)); - if (target_type == IS_LONG) { - ZVAL_LONG(&casted, 1); - } else { - ZVAL_DOUBLE(&casted, 1.0); - } + zend_uchar target_type = (Z_TYPE_P(o2) == IS_FALSE || Z_TYPE_P(o2) == IS_TRUE) + ? _IS_BOOL : Z_TYPE_P(o2); + if (Z_OBJ_HT_P(o1)->cast_object(Z_OBJ_P(o1), &casted, target_type) == FAILURE) { + // TODO: Less crazy. + if (target_type == IS_LONG || target_type == IS_DOUBLE) { + zend_error(E_NOTICE, "Object of class %s could not be converted to %s", + ZSTR_VAL(Z_OBJCE_P(o1)->name), zend_get_type_by_const(target_type)); + if (target_type == IS_LONG) { + ZVAL_LONG(&casted, 1); } else { - return 1; + ZVAL_DOUBLE(&casted, 1.0); } + } else { + return 1; } - int ret = zend_compare(&casted, o2); - zval_ptr_dtor(&casted); - return ret; } + int ret = zend_compare(&casted, o2); + zval_ptr_dtor(&casted); + return ret; } else { ZEND_ASSERT(Z_TYPE_P(o2) == IS_OBJECT); - if (Z_OBJ_HT_P(o2)->cast_object) { - zend_uchar target_type = (Z_TYPE_P(o1) == IS_FALSE || Z_TYPE_P(o1) == IS_TRUE) - ? _IS_BOOL : Z_TYPE_P(o1); - if (Z_OBJ_HT_P(o2)->cast_object(Z_OBJ_P(o2), &casted, target_type) == FAILURE) { - // TODO: Less crazy. - if (target_type == IS_LONG || target_type == IS_DOUBLE) { - zend_error(E_NOTICE, "Object of class %s could not be converted to %s", - ZSTR_VAL(Z_OBJCE_P(o2)->name), zend_get_type_by_const(target_type)); - if (target_type == IS_LONG) { - ZVAL_LONG(&casted, 1); - } else { - ZVAL_DOUBLE(&casted, 1.0); - } + zend_uchar target_type = (Z_TYPE_P(o1) == IS_FALSE || Z_TYPE_P(o1) == IS_TRUE) + ? _IS_BOOL : Z_TYPE_P(o1); + if (Z_OBJ_HT_P(o2)->cast_object(Z_OBJ_P(o2), &casted, target_type) == FAILURE) { + // TODO: Less crazy. + if (target_type == IS_LONG || target_type == IS_DOUBLE) { + zend_error(E_NOTICE, "Object of class %s could not be converted to %s", + ZSTR_VAL(Z_OBJCE_P(o2)->name), zend_get_type_by_const(target_type)); + if (target_type == IS_LONG) { + ZVAL_LONG(&casted, 1); } else { - return -1; + ZVAL_DOUBLE(&casted, 1.0); } + } else { + return -1; } - int ret = zend_compare(o1, &casted); - zval_ptr_dtor(&casted); - return ret; } + int ret = zend_compare(o1, &casted); + zval_ptr_dtor(&casted); + return ret; } return ZEND_UNCOMPARABLE; } diff --git a/Zend/zend_object_handlers.h b/Zend/zend_object_handlers.h index 19b7ff993f..b401d72ca2 100644 --- a/Zend/zend_object_handlers.h +++ b/Zend/zend_object_handlers.h @@ -158,7 +158,7 @@ struct _zend_object_handlers { zend_object_get_method_t get_method; /* required */ zend_object_get_constructor_t get_constructor; /* required */ zend_object_get_class_name_t get_class_name; /* required */ - zend_object_cast_t cast_object; /* optional */ + zend_object_cast_t cast_object; /* required */ zend_object_count_elements_t count_elements; /* optional */ zend_object_get_debug_info_t get_debug_info; /* optional */ zend_object_get_closure_t get_closure; /* optional */ diff --git a/Zend/zend_operators.c b/Zend/zend_operators.c index 600aad7bb7..f90b4ca377 100644 --- a/Zend/zend_operators.c +++ b/Zend/zend_operators.c @@ -137,13 +137,11 @@ ZEND_API zend_long ZEND_FASTCALL zend_atol(const char *str, size_t str_len) /* { /* {{{ convert_object_to_type: dst will be either ctype or UNDEF */ #define convert_object_to_type(op, dst, ctype, conv_func) \ ZVAL_UNDEF(dst); \ - if (Z_OBJ_HT_P(op)->cast_object) { \ - if (Z_OBJ_HT_P(op)->cast_object(Z_OBJ_P(op), dst, ctype) == FAILURE) { \ - zend_error(E_NOTICE, \ - "Object of class %s could not be converted to %s", ZSTR_VAL(Z_OBJCE_P(op)->name),\ - zend_get_type_by_const(ctype)); \ - } \ - } + if (Z_OBJ_HT_P(op)->cast_object(Z_OBJ_P(op), dst, ctype) == FAILURE) { \ + zend_error(E_NOTICE, \ + "Object of class %s could not be converted to %s", ZSTR_VAL(Z_OBJCE_P(op)->name),\ + zend_get_type_by_const(ctype)); \ + } \ /* }}} */ @@ -565,13 +563,10 @@ try_again: break; case IS_OBJECT: { zval tmp; - - if (Z_OBJ_HT_P(op)->cast_object) { - if (Z_OBJ_HT_P(op)->cast_object(Z_OBJ_P(op), &tmp, IS_STRING) == SUCCESS) { - zval_ptr_dtor(op); - ZVAL_COPY_VALUE(op, &tmp); - return; - } + if (Z_OBJ_HT_P(op)->cast_object(Z_OBJ_P(op), &tmp, IS_STRING) == SUCCESS) { + zval_ptr_dtor(op); + ZVAL_COPY_VALUE(op, &tmp); + return; } if (!EG(exception)) { zend_throw_error(NULL, "Object of class %s could not be converted to string", ZSTR_VAL(Z_OBJCE_P(op)->name)); @@ -874,10 +869,8 @@ try_again: NULL : ZSTR_KNOWN(ZEND_STR_ARRAY_CAPITALIZED); case IS_OBJECT: { zval tmp; - if (Z_OBJ_HT_P(op)->cast_object) { - if (Z_OBJ_HT_P(op)->cast_object(Z_OBJ_P(op), &tmp, IS_STRING) == SUCCESS) { - return Z_STR(tmp); - } + if (Z_OBJ_HT_P(op)->cast_object(Z_OBJ_P(op), &tmp, IS_STRING) == SUCCESS) { + return Z_STR(tmp); } if (!EG(exception)) { zend_throw_error(NULL, "Object of class %s could not be converted to string", ZSTR_VAL(Z_OBJCE_P(op)->name)); @@ -2422,14 +2415,11 @@ ZEND_API int ZEND_FASTCALL zend_is_true(zval *op) /* {{{ */ ZEND_API int ZEND_FASTCALL zend_object_is_true(zval *op) /* {{{ */ { zend_object *zobj = Z_OBJ_P(op); - - if (zobj->handlers->cast_object) { - zval tmp; - if (zobj->handlers->cast_object(zobj, &tmp, _IS_BOOL) == SUCCESS) { - return Z_TYPE(tmp) == IS_TRUE; - } - zend_error(E_RECOVERABLE_ERROR, "Object of class %s could not be converted to bool", ZSTR_VAL(zobj->ce->name)); + zval tmp; + if (zobj->handlers->cast_object(zobj, &tmp, _IS_BOOL) == SUCCESS) { + return Z_TYPE(tmp) == IS_TRUE; } + zend_error(E_RECOVERABLE_ERROR, "Object of class %s could not be converted to bool", ZSTR_VAL(zobj->ce->name)); return 1; } /* }}} */ diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c index 7658558ee8..3d1eaaf831 100644 --- a/ext/ffi/ffi.c +++ b/ext/ffi/ffi.c @@ -4659,6 +4659,11 @@ static HashTable *zend_fake_get_gc(zend_object *ob, zval **table, int *n) /* {{{ } /* }}} */ +static int zend_fake_cast_object(zend_object *obj, zval *result, int type) +{ + return FAILURE; +} + static ZEND_COLD zend_never_inline void zend_ffi_use_after_free(void) /* {{{ */ { zend_throw_error(zend_ffi_exception_ce, "Use after free()"); @@ -4911,7 +4916,7 @@ ZEND_MINIT_FUNCTION(ffi) zend_ffi_handlers.unset_dimension = zend_fake_unset_dimension; zend_ffi_handlers.get_method = zend_ffi_get_func; zend_ffi_handlers.compare = NULL; - zend_ffi_handlers.cast_object = NULL; + zend_ffi_handlers.cast_object = zend_fake_cast_object; zend_ffi_handlers.get_debug_info = NULL; zend_ffi_handlers.get_closure = NULL; zend_ffi_handlers.get_properties = zend_fake_get_properties; @@ -4990,7 +4995,7 @@ ZEND_MINIT_FUNCTION(ffi) zend_ffi_cdata_free_handlers.get_method = zend_fake_get_method; zend_ffi_cdata_free_handlers.get_class_name = zend_ffi_cdata_get_class_name; zend_ffi_cdata_free_handlers.compare = zend_ffi_cdata_compare_objects; - zend_ffi_cdata_free_handlers.cast_object = NULL; + zend_ffi_cdata_free_handlers.cast_object = zend_fake_cast_object; zend_ffi_cdata_free_handlers.count_elements = NULL; zend_ffi_cdata_free_handlers.get_debug_info = zend_ffi_free_get_debug_info; zend_ffi_cdata_free_handlers.get_closure = NULL; @@ -5020,7 +5025,7 @@ ZEND_MINIT_FUNCTION(ffi) zend_ffi_ctype_handlers.get_method = zend_fake_get_method; zend_ffi_ctype_handlers.get_class_name = zend_ffi_ctype_get_class_name; zend_ffi_ctype_handlers.compare = zend_ffi_ctype_compare_objects; - zend_ffi_ctype_handlers.cast_object = NULL; + zend_ffi_ctype_handlers.cast_object = zend_fake_cast_object; zend_ffi_ctype_handlers.count_elements = NULL; zend_ffi_ctype_handlers.get_debug_info = zend_ffi_ctype_get_debug_info; zend_ffi_ctype_handlers.get_closure = NULL; diff --git a/ext/intl/collator/collator_convert.c b/ext/intl/collator/collator_convert.c index e19bb719f6..88177a04f6 100644 --- a/ext/intl/collator/collator_convert.c +++ b/ext/intl/collator/collator_convert.c @@ -224,16 +224,13 @@ zval* collator_convert_object_to_string( zval* obj, zval *rv ) } /* Try object's handlers. */ - if( Z_OBJ_HT_P(obj)->cast_object ) - { - zstr = rv; + zstr = rv; - if( Z_OBJ_HT_P(obj)->cast_object( Z_OBJ_P(obj), zstr, IS_STRING ) == FAILURE ) - { - /* cast_object failed => bail out. */ - zval_ptr_dtor( zstr ); - COLLATOR_CONVERT_RETURN_FAILED( obj ); - } + if( Z_OBJ_HT_P(obj)->cast_object( Z_OBJ_P(obj), zstr, IS_STRING ) == FAILURE ) + { + /* cast_object failed => bail out. */ + zval_ptr_dtor( zstr ); + COLLATOR_CONVERT_RETURN_FAILED( obj ); } /* Object wasn't successfully converted => bail out. */ -- 2.50.1