streams: check class exists when attempting to register stream filter

And do a few other checks such as is the class even instantiable.
It might make sense in the future to raise a deprecation if the class does not extend php_user_filter or create a new StreamFilter interface so that we know the various methods exist

Author: Girgias

ext/standard/tests/filters/001.phpt

@@ -3,30 +3,27 @@ stream_filter_register() and invalid arguments
 --FILE--
 <?php
 try {
-    stream_filter_register("", "");
-} catch (ValueError $exception) {
-    echo $exception->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

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--
 <?php
-var_dump(stream_filter_register('string.toupper', 'filter_string_toupper'));
+try {
+	var_dump(stream_filter_register('string.toupper', 'filter_string_toupper'));
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
 ?>
 --EXPECT--
-bool(false)
+TypeError: stream_filter_register(): Argument #2 ($class) must be a valid class name, filter_string_toupper given

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--
 <?php
 
-var_dump(stream_filter_register("not_existing_filter", "not_existing"));
+try {
+	var_dump(stream_filter_register("not_existing_filter", "not_existing"));
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->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)

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,41 +578,37 @@ 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;
 
 	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)) {
 		zend_argument_value_error(1, "must be a non-empty string");
 		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;