diff --git a/ext/standard/tests/filters/001.phpt b/ext/standard/tests/filters/001.phpt index 3f0d5f5eb44e..0a91b7213445 100644 --- a/ext/standard/tests/filters/001.phpt +++ b/ext/standard/tests/filters/001.phpt @@ -3,30 +3,27 @@ stream_filter_register() and invalid arguments --FILE-- getMessage() . "\n"; + stream_filter_register("", "stdClass"); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; } try { - stream_filter_register("test", ""); -} catch (ValueError $exception) { - echo $exception->getMessage() . "\n"; + stream_filter_register("test", "Throwable"); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; } try { - stream_filter_register("", "test"); -} catch (ValueError $exception) { - echo $exception->getMessage() . "\n"; + var_dump(stream_filter_register("------", "nonexistentclass")); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; } -var_dump(stream_filter_register("------", "nonexistentclass")); - echo "Done\n"; ?> --EXPECT-- -stream_filter_register(): Argument #1 ($filter_name) must be a non-empty string -stream_filter_register(): Argument #2 ($class) must be a non-empty string -stream_filter_register(): Argument #1 ($filter_name) must be a non-empty string -bool(true) +ValueError: stream_filter_register(): Argument #1 ($filter_name) must be a non-empty string +ValueError: stream_filter_register(): Argument #2 ($class) must be a concrete class +TypeError: stream_filter_register(): Argument #2 ($class) must be a valid class name, nonexistentclass given Done diff --git a/ext/standard/tests/filters/gh17037.phpt b/ext/standard/tests/filters/gh17037.phpt index 21319ba26bf9..d36913e1c7ac 100644 --- a/ext/standard/tests/filters/gh17037.phpt +++ b/ext/standard/tests/filters/gh17037.phpt @@ -2,7 +2,11 @@ GH-17037 (UAF in user filter when adding existing filter name due to incorrect error handling) --FILE-- getMessage(), PHP_EOL; +} ?> --EXPECT-- -bool(false) +TypeError: stream_filter_register(): Argument #2 ($class) must be a valid class name, filter_string_toupper given diff --git a/ext/standard/tests/filters/stream_filter_register_non_existing_class.phpt b/ext/standard/tests/filters/stream_filter_register_non_existing_class.phpt index 1c08201cff0c..b404d45524b6 100644 --- a/ext/standard/tests/filters/stream_filter_register_non_existing_class.phpt +++ b/ext/standard/tests/filters/stream_filter_register_non_existing_class.phpt @@ -3,7 +3,11 @@ stream_filter_register() with a class name that does not exist --FILE-- getMessage(), PHP_EOL; +} stream_filter_append(STDOUT, "not_existing_filter"); @@ -12,10 +16,8 @@ var_dump($out); ?> --EXPECTF-- -bool(true) +TypeError: stream_filter_register(): Argument #2 ($class) must be a valid class name, not_existing given -Warning: stream_filter_append(): User-filter "not_existing_filter" requires class "not_existing", but that class is not defined in %s on line %d - -Warning: stream_filter_append(): Unable to create or locate filter "not_existing_filter" in %s on line %d +Warning: stream_filter_append(): Unable to locate filter "not_existing_filter" in %s on line %d Hello int(6) diff --git a/ext/standard/user_filters.c b/ext/standard/user_filters.c index 986bbbd5f4d3..35b4deee31fa 100644 --- a/ext/standard/user_filters.c +++ b/ext/standard/user_filters.c @@ -23,12 +23,6 @@ #define PHP_STREAM_BRIGADE_RES_NAME "userfilter.bucket brigade" #define PHP_STREAM_BUCKET_RES_NAME "userfilter.bucket" -struct php_user_filter_data { - zend_class_entry *ce; - /* variable length; this *must* be last in the structure */ - zend_string *classname; -}; - /* to provide context for calling into the next filter from user-space */ static int le_bucket_brigade; static int le_bucket; @@ -324,7 +318,6 @@ static const php_stream_filter_ops userfilter_ops = { static php_stream_filter *user_filter_factory_create(const char *filtername, zval *filterparams, bool persistent) { - struct php_user_filter_data *fdat = NULL; php_stream_filter *filter; zval obj; zval retval; @@ -343,8 +336,9 @@ static php_stream_filter *user_filter_factory_create(const char *filtername, len = strlen(filtername); - /* determine the classname/class entry */ - if (NULL == (fdat = zend_hash_str_find_ptr(BG(user_filter_map), filtername, len))) { + /* determine the class entry */ + /* const */ zend_class_entry *ce = zend_hash_str_find_ptr(BG(user_filter_map), filtername, len); + if (UNEXPECTED(ce == NULL)) { const char *period; /* Userspace Filters using ambiguous wildcards could cause problems. @@ -362,7 +356,8 @@ static php_stream_filter *user_filter_factory_create(const char *filtername, ZEND_ASSERT(new_period[0] == '.'); new_period[1] = '*'; new_period[2] = '\0'; - if (NULL != (fdat = zend_hash_str_find_ptr(BG(user_filter_map), wildcard, strlen(wildcard)))) { + ce = zend_hash_str_find_ptr(BG(user_filter_map), wildcard, strlen(wildcard)); + if (NULL != ce) { new_period = NULL; } else { *new_period = '\0'; @@ -371,21 +366,11 @@ static php_stream_filter *user_filter_factory_create(const char *filtername, } efree(wildcard); } - ZEND_ASSERT(fdat); - } - - /* bind the classname to the actual class */ - if (fdat->ce == NULL) { - if (NULL == (fdat->ce = zend_lookup_class(fdat->classname))) { - php_error_docref(NULL, E_WARNING, - "User-filter \"%s\" requires class \"%s\", but that class is not defined", - filtername, ZSTR_VAL(fdat->classname)); - return NULL; - } + ZEND_ASSERT(ce); } /* create the object */ - if (object_init_ex(&obj, fdat->ce) == FAILURE) { + if (object_init_ex(&obj, ce) == FAILURE) { return NULL; } @@ -431,13 +416,6 @@ static const php_stream_filter_factory user_filter_factory = { user_filter_factory_create }; -static void filter_item_dtor(zval *zv) -{ - struct php_user_filter_data *fdat = Z_PTR_P(zv); - zend_string_release_ex(fdat->classname, 0); - efree(fdat); -} - /* {{{ Return a bucket object from the brigade for operating on */ PHP_FUNCTION(stream_bucket_make_writeable) { @@ -600,12 +578,12 @@ PHP_FUNCTION(stream_get_filters) /* {{{ Registers a custom filter handler class */ PHP_FUNCTION(stream_filter_register) { - zend_string *filtername, *classname; - struct php_user_filter_data *fdat; + zend_string *filtername; + zend_class_entry *ce = NULL; ZEND_PARSE_PARAMETERS_START(2, 2) Z_PARAM_STR(filtername) - Z_PARAM_STR(classname) + Z_PARAM_CLASS(ce) ZEND_PARSE_PARAMETERS_END(); if (!ZSTR_LEN(filtername)) { @@ -613,28 +591,24 @@ PHP_FUNCTION(stream_filter_register) RETURN_THROWS(); } - if (!ZSTR_LEN(classname)) { - zend_argument_value_error(2, "must be a non-empty string"); + /* TODO: Check class is a child of php_user_filter? */ + if (UNEXPECTED(ce->ce_flags & ZEND_ACC_UNINSTANTIABLE)) { + zend_argument_value_error(2, "must be a concrete class"); RETURN_THROWS(); } if (!BG(user_filter_map)) { BG(user_filter_map) = (HashTable*) emalloc(sizeof(HashTable)); - zend_hash_init(BG(user_filter_map), 8, NULL, (dtor_func_t) filter_item_dtor, 0); + /* We don't need a destructor as we are only storing a CE which should be never modified */ + zend_hash_init(BG(user_filter_map), 8, NULL, NULL, 0); } - fdat = ecalloc(1, sizeof(struct php_user_filter_data)); - fdat->classname = zend_string_copy(classname); - - if (zend_hash_add_ptr(BG(user_filter_map), filtername, fdat) != NULL) { + if (zend_hash_add_ptr(BG(user_filter_map), filtername, ce) != NULL) { if (php_stream_filter_register_factory_volatile(filtername, &user_filter_factory) == SUCCESS) { RETURN_TRUE; } zend_hash_del(BG(user_filter_map), filtername); - } else { - zend_string_release_ex(classname, 0); - efree(fdat); } RETURN_FALSE;